diff --git a/changelog/@unreleased/pr-2813.v2.yml b/changelog/@unreleased/pr-2813.v2.yml new file mode 100644 index 000000000..95a738366 --- /dev/null +++ b/changelog/@unreleased/pr-2813.v2.yml @@ -0,0 +1,5 @@ +type: fix +fix: + description: cache key for JarClassHasher includes artifact classifier + links: + - https://github.com/palantir/gradle-baseline/pull/2813 diff --git a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/services/ClassUniquenessArtifactIdentifier.java b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/services/ClassUniquenessArtifactIdentifier.java new file mode 100644 index 000000000..775820630 --- /dev/null +++ b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/services/ClassUniquenessArtifactIdentifier.java @@ -0,0 +1,29 @@ +/* + * (c) Copyright 2024 Palantir Technologies Inc. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.palantir.baseline.services; + +import java.util.Optional; +import org.gradle.api.artifacts.ModuleVersionIdentifier; +import org.immutables.value.Value; + +@Value.Immutable +@Value.Style(optionalAcceptNullable = true) +public interface ClassUniquenessArtifactIdentifier { + ModuleVersionIdentifier moduleVersionIdentifier(); + + Optional classifier(); +} diff --git a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/services/JarClassHasher.java b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/services/JarClassHasher.java index fdcdfcac7..c6f6b4346 100644 --- a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/services/JarClassHasher.java +++ b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/services/JarClassHasher.java @@ -18,6 +18,7 @@ import com.github.benmanes.caffeine.cache.Cache; import com.github.benmanes.caffeine.cache.Caffeine; +import com.google.common.base.Strings; import com.google.common.collect.ImmutableListMultimap; import com.google.common.collect.ImmutableSetMultimap; import com.google.common.hash.HashCode; @@ -32,14 +33,14 @@ import java.util.jar.JarEntry; import java.util.jar.JarInputStream; import java.util.stream.Collectors; -import org.gradle.api.artifacts.ModuleVersionIdentifier; import org.gradle.api.artifacts.ResolvedArtifact; import org.gradle.api.services.BuildService; import org.gradle.api.services.BuildServiceParameters; import org.slf4j.Logger; public abstract class JarClassHasher implements BuildService, AutoCloseable { - private final Cache cache = + + private final Cache cache = Caffeine.newBuilder().build(); public static final class Result { @@ -59,7 +60,11 @@ public static Result empty() { } public final Result hashClasses(ResolvedArtifact resolvedArtifact, Logger logger) { - return cache.get(resolvedArtifact.getModuleVersion().getId(), _moduleId -> { + ClassUniquenessArtifactIdentifier key = ImmutableClassUniquenessArtifactIdentifier.builder() + .moduleVersionIdentifier(resolvedArtifact.getModuleVersion().getId()) + .classifier(Strings.nullToEmpty(resolvedArtifact.getClassifier())) + .build(); + return cache.get(key, _moduleId -> { File file = resolvedArtifact.getFile(); if (!file.exists()) { return Result.empty(); diff --git a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckClassUniquenessLockTask.java b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckClassUniquenessLockTask.java index ab6f5669e..446e917a5 100644 --- a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckClassUniquenessLockTask.java +++ b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckClassUniquenessLockTask.java @@ -19,6 +19,7 @@ import com.google.common.base.Splitter; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSortedMap; +import com.palantir.baseline.services.ClassUniquenessArtifactIdentifier; import com.palantir.baseline.services.JarClassHasher; import com.palantir.gradle.failurereports.exceptions.ExceptionWithSuggestion; import difflib.DiffUtils; @@ -37,7 +38,6 @@ import org.gradle.api.GradleException; import org.gradle.api.Task; import org.gradle.api.artifacts.Configuration; -import org.gradle.api.artifacts.ModuleVersionIdentifier; import org.gradle.api.provider.Property; import org.gradle.api.provider.SetProperty; import org.gradle.api.specs.Spec; @@ -111,7 +111,8 @@ public final void doIt() { ClassUniquenessAnalyzer analyzer = new ClassUniquenessAnalyzer( jarClassHasher.get(), getProject().getLogger()); analyzer.analyzeConfiguration(configuration); - Collection> problemJars = analyzer.getDifferingProblemJars(); + Collection> problemJars = + analyzer.getDifferingProblemJars(); if (problemJars.isEmpty()) { return Optional.empty(); @@ -148,16 +149,23 @@ public final void doIt() { } } - private String clashingClasses(ClassUniquenessAnalyzer analyzer, Set clashingJars) { + private String clashingClasses( + ClassUniquenessAnalyzer analyzer, Set clashingJars) { return analyzer.getDifferingSharedClassesInProblemJars(clashingJars).stream() .sorted() .map(className -> String.format(" - %s", className)) .collect(Collectors.joining("\n")); } - private String clashingJarHeader(Set clashingJars) { + private String clashingJarHeader(Set clashingJars) { return clashingJars.stream() - .map(mvi -> mvi.getGroup() + ":" + mvi.getName()) + .map(ident -> { + String mvi = ident.moduleVersionIdentifier().getGroup() + ":" + + ident.moduleVersionIdentifier().getName(); + return ident.classifier().isEmpty() + ? mvi + : mvi + " (classifier=" + ident.classifier().get() + ")"; + }) .sorted() .collect(Collectors.joining(", ", "[", "]")); } diff --git a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/ClassUniquenessAnalyzer.java b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/ClassUniquenessAnalyzer.java index 212dcd4d2..cde71c701 100644 --- a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/ClassUniquenessAnalyzer.java +++ b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/ClassUniquenessAnalyzer.java @@ -23,6 +23,8 @@ import com.google.common.collect.ImmutableSetMultimap; import com.google.common.collect.SetMultimap; import com.google.common.hash.HashCode; +import com.palantir.baseline.services.ClassUniquenessArtifactIdentifier; +import com.palantir.baseline.services.ImmutableClassUniquenessArtifactIdentifier; import com.palantir.baseline.services.JarClassHasher; import java.io.File; import java.time.Duration; @@ -32,13 +34,12 @@ import java.util.Set; import java.util.stream.Collectors; import org.gradle.api.artifacts.Configuration; -import org.gradle.api.artifacts.ModuleVersionIdentifier; import org.gradle.api.artifacts.ResolvedArtifact; import org.slf4j.Logger; public final class ClassUniquenessAnalyzer { private final JarClassHasher jarHasher; - private final SetMultimap, String> jarsToClasses = HashMultimap.create(); + private final SetMultimap, String> jarsToClasses = HashMultimap.create(); private final SetMultimap classToHashCodes = HashMultimap.create(); private final Logger log; @@ -54,7 +55,7 @@ public void analyzeConfiguration(Configuration configuration) { // we use these temporary maps to accumulate information as we process each jar, // so they may include singletons which we filter out later - SetMultimap classToJars = HashMultimap.create(); + SetMultimap classToJars = HashMultimap.create(); SetMultimap tempClassToHashCodes = HashMultimap.create(); for (ResolvedArtifact resolvedArtifact : dependencies) { @@ -70,7 +71,13 @@ public void analyzeConfiguration(Configuration configuration) { for (Map.Entry entry : hashes.entries()) { String className = entry.getKey(); HashCode hashValue = entry.getValue(); - classToJars.put(className, resolvedArtifact.getModuleVersion().getId()); + classToJars.put( + className, + ImmutableClassUniquenessArtifactIdentifier.builder() + .moduleVersionIdentifier( + resolvedArtifact.getModuleVersion().getId()) + .classifier(resolvedArtifact.getClassifier()) + .build()); tempClassToHashCodes.put(className, hashValue); } } @@ -97,24 +104,24 @@ public void analyzeConfiguration(Configuration configuration) { * Any groups jars that all contain some identically named classes. Note: may contain non-scary duplicates - class * files which are 100% identical, so their clashing name doesn't have any effect. */ - private Collection> getProblemJars() { + private Collection> getProblemJars() { return jarsToClasses.keySet(); } /** Class names that appear in all of the given jars. */ - public Set getSharedClassesInProblemJars(Set problemJars) { + public Set getSharedClassesInProblemJars(Set problemJars) { return jarsToClasses.get(problemJars); } /** Jars which contain identically named classes with non-identical implementations. */ - public Collection> getDifferingProblemJars() { + public Collection> getDifferingProblemJars() { return getProblemJars().stream() .filter(jars -> getDifferingSharedClassesInProblemJars(jars).size() > 0) .collect(Collectors.toSet()); } /** Class names which appear in all of the given jars and also have non-identical implementations. */ - public Set getDifferingSharedClassesInProblemJars(Set problemJars) { + public Set getDifferingSharedClassesInProblemJars(Set problemJars) { return getSharedClassesInProblemJars(problemJars).stream() .filter(classToHashCodes::containsKey) .collect(toSet()); diff --git a/gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineClassUniquenessPluginIntegrationTest.groovy b/gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineClassUniquenessPluginIntegrationTest.groovy index f53bcb499..b61fc4dbc 100644 --- a/gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineClassUniquenessPluginIntegrationTest.groovy +++ b/gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineClassUniquenessPluginIntegrationTest.groovy @@ -99,6 +99,36 @@ class BaselineClassUniquenessPluginIntegrationTest extends AbstractPluginTest { lockfile.text == expected.text } + def 'detect duplicates in two external jars with the same ModuleVersionIdentifier but different classifiers'() { + File lockfile = new File(projectDir, 'baseline-class-uniqueness.lock') + + when: + buildFile << standardBuildFile + buildFile << """ + dependencies { + api group: 'com.google.cloud.bigdataoss', name: 'gcs-connector', version: 'hadoop3-2.2.19' + api group: 'com.google.cloud.bigdataoss', name: 'gcs-connector', version: 'hadoop3-2.2.19', classifier: 'shaded' + } + """.stripIndent() + BuildResult result = with('check', '-s').buildAndFail() + + then: + result.getOutput().contains("baseline-class-uniqueness detected multiple jars containing identically named classes.") + result.getOutput().contains("[com.google.cloud.bigdataoss:gcs-connector, com.google.cloud.bigdataoss:gcs-connector (classifier=shaded)]") + result.getOutput().contains("com.google.cloud.hadoop.fs.gcs.FsBenchmark") + + when: + with("checkClassUniqueness", "--write-locks").build() + + then: + lockfile.exists() + File expected = new File("src/test/resources/com/palantir/baseline/baseline-class-uniqueness-with-classifier.expected.lock") + if (Boolean.getBoolean("recreate")) { + GFileUtils.writeFile(lockfile.text, expected) + } + lockfile.text == expected.text + } + def 'detect duplicates in two external jars in non-standard configuration'() { when: buildFile << standardBuildFile diff --git a/gradle-baseline-java/src/test/resources/com/palantir/baseline/baseline-class-uniqueness-with-classifier.expected.lock b/gradle-baseline-java/src/test/resources/com/palantir/baseline/baseline-class-uniqueness-with-classifier.expected.lock new file mode 100644 index 000000000..1b2431138 --- /dev/null +++ b/gradle-baseline-java/src/test/resources/com/palantir/baseline/baseline-class-uniqueness-with-classifier.expected.lock @@ -0,0 +1,54 @@ +# Danger! Multiple jars contain identically named classes. This may cause different behaviour depending on classpath ordering. +# Run ./gradlew checkClassUniqueness --fix to update this file + +## runtimeClasspath +[com.google.cloud.bigdataoss:gcs-connector (classifier=shaded), com.google.cloud.bigdataoss:gcsio] + - com.google.cloud.hadoop.gcsio.authorization.AuthorizationHandler +[com.google.cloud.bigdataoss:gcs-connector (classifier=shaded), com.google.cloud.bigdataoss:util-hadoop] + - com.google.cloud.hadoop.util.AccessTokenProvider + - com.google.cloud.hadoop.util.AccessTokenProvider$AccessToken + - com.google.cloud.hadoop.util.AccessTokenProvider$AccessTokenType +[com.google.cloud.bigdataoss:gcs-connector (classifier=shaded), com.google.cloud.bigdataoss:util] + - com.google.cloud.hadoop.util.AccessBoundary + - com.google.cloud.hadoop.util.AccessBoundary$Action + - com.google.cloud.hadoop.util.AutoValue_AccessBoundary +[com.google.cloud.bigdataoss:gcs-connector, com.google.cloud.bigdataoss:gcs-connector (classifier=shaded)] + - com.google.cloud.hadoop.fs.gcs.AutoValue_SyncableOutputStreamOptions + - com.google.cloud.hadoop.fs.gcs.AutoValue_SyncableOutputStreamOptions$1 + - com.google.cloud.hadoop.fs.gcs.AutoValue_SyncableOutputStreamOptions$Builder + - com.google.cloud.hadoop.fs.gcs.CoopLockFsck + - com.google.cloud.hadoop.fs.gcs.CoopLockFsckRunner + - com.google.cloud.hadoop.fs.gcs.CoopLockFsckRunner$1 + - com.google.cloud.hadoop.fs.gcs.CoopLockFsckRunner$2 + - com.google.cloud.hadoop.fs.gcs.FileSystemDescriptor + - com.google.cloud.hadoop.fs.gcs.FsBenchmark + - com.google.cloud.hadoop.fs.gcs.GhfsStatistic + - com.google.cloud.hadoop.fs.gcs.GhfsStatisticTypeEnum + - com.google.cloud.hadoop.fs.gcs.GhfsStorageStatistics + - com.google.cloud.hadoop.fs.gcs.GhfsStorageStatistics$1 + - com.google.cloud.hadoop.fs.gcs.GhfsStorageStatistics$LongIterator + - com.google.cloud.hadoop.fs.gcs.GhfsStorageStatistics$MeanStatistic + - com.google.cloud.hadoop.fs.gcs.GhfsStreamStats + - com.google.cloud.hadoop.fs.gcs.GoogleHadoopFS + - com.google.cloud.hadoop.fs.gcs.GoogleHadoopFSInputStream + - com.google.cloud.hadoop.fs.gcs.GoogleHadoopFileSystem + - com.google.cloud.hadoop.fs.gcs.GoogleHadoopFileSystemBase + - com.google.cloud.hadoop.fs.gcs.GoogleHadoopFileSystemBase$1 + - com.google.cloud.hadoop.fs.gcs.GoogleHadoopFileSystemBase$GcsFileChecksum + - com.google.cloud.hadoop.fs.gcs.GoogleHadoopFileSystemBase$GcsFileChecksumType + - com.google.cloud.hadoop.fs.gcs.GoogleHadoopFileSystemBase$GlobAlgorithm + - com.google.cloud.hadoop.fs.gcs.GoogleHadoopFileSystemBase$InvocationRaisingIOE + - com.google.cloud.hadoop.fs.gcs.GoogleHadoopFileSystemBase$OutputStreamType + - com.google.cloud.hadoop.fs.gcs.GoogleHadoopFileSystemConfiguration + - com.google.cloud.hadoop.fs.gcs.GoogleHadoopOutputStream + - com.google.cloud.hadoop.fs.gcs.GoogleHadoopSyncableOutputStream + - com.google.cloud.hadoop.fs.gcs.HadoopConfigurationProperty + - com.google.cloud.hadoop.fs.gcs.HadoopCredentialConfiguration + - com.google.cloud.hadoop.fs.gcs.InMemoryGlobberFileSystem + - com.google.cloud.hadoop.fs.gcs.SyncableOutputStreamOptions + - com.google.cloud.hadoop.fs.gcs.SyncableOutputStreamOptions$Builder + - com.google.cloud.hadoop.fs.gcs.auth.AbstractDelegationTokenBinding + - com.google.cloud.hadoop.fs.gcs.auth.AbstractDelegationTokenBinding$TokenSecretManager + - com.google.cloud.hadoop.fs.gcs.auth.DelegationTokenIOException + - com.google.cloud.hadoop.fs.gcs.auth.GcsDelegationTokens + - com.google.cloud.hadoop.fs.gcs.auth.GcsDtFetcher