diff --git a/MODULE.bazel b/MODULE.bazel index 119230bc..fe44fb10 100644 --- a/MODULE.bazel +++ b/MODULE.bazel @@ -738,6 +738,35 @@ dev_maven.install( ], ) +# These repos are used for testing the artifact hash functions. See +# `ArtifactsHashTest.java` +dev_maven.install( + name = "artifacts_hash_no_deps", + artifacts = [ + # Has no dependencies + "org.hamcrest:hamcrest-core:3.0", + ], + fail_if_repin_required = True, + lock_file = "//tests/custom_maven_install:artifacts_hash_no_deps_install.json", +) +dev_maven.install( + name = "artifacts_hash_with_deps", + artifacts = [ + "com.google.code.gson:gson:2.11.0", + ], + fail_if_repin_required = True, + lock_file = "//tests/custom_maven_install:artifacts_hash_with_deps_install.json", +) +dev_maven.install( + name = "artifacts_hash_with_deps_from_maven", + artifacts = [ + "com.google.guava:guava:33.3.1-jre", + ], + fail_if_repin_required = True, + lock_file = "//tests/custom_maven_install:artifacts_hash_with_deps_from_maven_install.json", + resolver = "maven", +) + # Where there are file locks, the pinned and unpinned repos are listed # next to each other. Where compat repositories are created, they are # listed next to the repo that created them. The list is otherwise kept @@ -746,6 +775,9 @@ dev_maven.install( # want it to use_repo( dev_maven, + "artifacts_hash_no_deps", + "artifacts_hash_with_deps", + "artifacts_hash_with_deps_from_maven", "duplicate_version_warning", "duplicate_version_warning_same_version", "exclusion_testing", @@ -837,6 +869,7 @@ use_repo( http_file( name = "com.google.ar.sceneform_rendering", + dev_dependency = True, downloaded_file_path = "rendering-1.10.0.aar", sha256 = "d2f6cd1d54eee0d5557518d1edcf77a3ba37494ae94f9bb862e570ee426a3431", urls = [ @@ -846,6 +879,7 @@ http_file( http_file( name = "hamcrest_core_for_test", + dev_dependency = True, downloaded_file_path = "hamcrest-core-1.3.jar", sha256 = "66fdef91e9739348df7a096aa384a5685f4e875584cce89386a7a47251c4d8e9", urls = [ @@ -855,6 +889,7 @@ http_file( http_file( name = "hamcrest_core_srcs_for_test", + dev_dependency = True, downloaded_file_path = "hamcrest-core-1.3-sources.jar", sha256 = "e223d2d8fbafd66057a8848cc94222d63c3cedd652cc48eddc0ab5c39c0f84df", urls = [ @@ -864,6 +899,7 @@ http_file( http_file( name = "gson_for_test", + dev_dependency = True, downloaded_file_path = "gson-2.9.0.jar", sha256 = "c96d60551331a196dac54b745aa642cd078ef89b6f267146b705f2c2cbef052d", urls = [ @@ -873,6 +909,7 @@ http_file( http_file( name = "junit_platform_commons_for_test", + dev_dependency = True, downloaded_file_path = "junit-platform-commons-1.8.2.jar", sha256 = "d2e015fca7130e79af2f4608dc54415e4b10b592d77333decb4b1a274c185050", urls = [ @@ -883,6 +920,7 @@ http_file( # https://github.com/bazelbuild/rules_jvm_external/issues/865 http_file( name = "google_api_services_compute_javadoc_for_test", + dev_dependency = True, downloaded_file_path = "google-api-services-compute-v1-rev235-1.25.0-javadoc.jar", sha256 = "b03be5ee8effba3bfbaae53891a9c01d70e2e3bd82ad8889d78e641b22bd76c2", urls = [ @@ -892,6 +930,7 @@ http_file( http_file( name = "lombok_for_test", + dev_dependency = True, downloaded_file_path = "lombok-1.18.22.jar", sha256 = "ecef1581411d7a82cc04281667ee0bac5d7c0a5aae74cfc38430396c91c31831", urls = [ diff --git a/private/lib/coordinates.bzl b/private/lib/coordinates.bzl index cb86755f..56615265 100644 --- a/private/lib/coordinates.bzl +++ b/private/lib/coordinates.bzl @@ -57,6 +57,22 @@ def unpack_coordinates(coords): def _is_version_number(part): return part[0].isdigit() +def _unpack_if_necssary(coords): + if type(coords) == "string": + unpacked = unpack_coordinates(coords) + elif type(coords) == "dict": + unpacked = struct( + group = coords.get("group"), + artifact = coords.get("artifact"), + version = coords.get("version", None), + classifier = coords.get("classifier", None), + extension = coords.get("extension", None), + ) + else: + unpacked = coords + + return unpacked + def to_external_form(coords): """Formats `coords` as a string suitable for use by tools such as Gradle. @@ -64,10 +80,7 @@ def to_external_form(coords): syntax: `group:name:version:classifier@packaging` """ - if type(coords) == "string": - unpacked = unpack_coordinates(coords) - else: - unpacked = coords + unpacked = _unpack_if_necssary(coords) to_return = "%s:%s:%s" % (unpacked.group, unpacked.artifact, unpacked.version) @@ -81,6 +94,21 @@ def to_external_form(coords): return to_return +# This matches the `Coordinates#asKey` method in the Java tree, and the +# implementations must be kept in sync. +def to_key(coords): + unpacked = _unpack_if_necssary(coords) + + key = unpacked.group + ":" + unpacked.artifact + + if unpacked.classifier and "jar" != unpacked.classifier: + extension = unpacked.packaging if unpacked.packaging else "jar" + key += ":" + unpacked.packaging + ":" + unpacked.classifier + elif unpacked.packaging and "jar" != unpacked.packaging: + key += ":" + unpacked.packaging + + return key + _DEFAULT_PURL_REPOS = [ "https://repo.maven.apache.org/maven2", "https://repo.maven.apache.org/maven2/", diff --git a/private/rules/coursier.bzl b/private/rules/coursier.bzl index 69b4f4fb..70d1d945 100644 --- a/private/rules/coursier.bzl +++ b/private/rules/coursier.bzl @@ -531,7 +531,7 @@ def _pinned_coursier_fetch_impl(repository_ctx): "This feature ensures that the file is not modified manually. To generate this " + "signature, run 'bazel run %s'." % pin_target, ) - elif importer.compute_lock_file_hash(maven_install_json_content) != dep_tree_signature: + elif not importer.validate_lock_file_hash(maven_install_json_content, dep_tree_signature): # Then, validate that the signature provided matches the contents of the dependency_tree. # This is to stop users from manually modifying maven_install.json. if _get_fail_if_repin_required(repository_ctx): diff --git a/private/rules/v1_lock_file.bzl b/private/rules/v1_lock_file.bzl index 4fa52618..546c7677 100644 --- a/private/rules/v1_lock_file.bzl +++ b/private/rules/v1_lock_file.bzl @@ -69,6 +69,9 @@ def _compute_lock_file_hash(lock_file_contents): signature_inputs.append(":".join(artifact_group)) return hash(repr(sorted(signature_inputs))) +def _validate_lock_file_hash(lock_file_contents, expected_hash): + return _compute_lock_file_hash(lock_file_contents) == expected_hash + def create_dependency(dep): url = dep.get("url") if url: @@ -139,6 +142,7 @@ v1_lock_file = struct( get_input_artifacts_hash = _get_input_artifacts_hash, get_lock_file_hash = _get_lock_file_hash, compute_lock_file_hash = _compute_lock_file_hash, + validate_lock_file_hash = _validate_lock_file_hash, get_artifacts = _get_artifacts, get_netrc_entries = _get_netrc_entries, has_m2local = _has_m2local, diff --git a/private/rules/v2_lock_file.bzl b/private/rules/v2_lock_file.bzl index 8259873b..b0b6c5b9 100644 --- a/private/rules/v2_lock_file.bzl +++ b/private/rules/v2_lock_file.bzl @@ -11,6 +11,8 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and +load("//private/lib:coordinates.bzl", "to_external_form", "to_key") + _REQUIRED_KEYS = ["artifacts", "dependencies", "repositories"] def _is_valid_lock_file(lock_file_contents): @@ -35,7 +37,7 @@ def _get_input_artifacts_hash(lock_file_contents): def _get_lock_file_hash(lock_file_contents): return lock_file_contents.get("__RESOLVED_ARTIFACTS_HASH") -def _compute_lock_file_hash(lock_file_contents): +def _original_compute_lock_file_hash(lock_file_contents): to_hash = {} for key in sorted(_REQUIRED_KEYS): value = lock_file_contents.get(key) @@ -45,6 +47,27 @@ def _compute_lock_file_hash(lock_file_contents): to_hash.update({key: json.decode(json.encode(value))}) return hash(repr(to_hash)) +def _compute_lock_file_hash(lock_file_contents): + lines = [] + artifacts = _get_artifacts(lock_file_contents) + + for artifact in artifacts: + line = "%s | %s | " % (to_external_form(artifact["coordinates"]), artifact["sha256"] if artifact["sha256"] else "") + deps = [] + for dep in artifact["deps"]: + deps.append(to_key(dep)) + line += ",".join(sorted(deps)) + + lines.append(line) + + lines = sorted(lines) + to_hash = "\n".join(lines) + + return hash(to_hash) + +def _validate_lock_file_hash(lock_file_contents, expected_hash): + return _compute_lock_file_hash(lock_file_contents) == expected_hash or _original_compute_lock_file_hash(lock_file_contents) == expected_hash + def _to_m2_path(unpacked): path = "{group}/{artifact}/{version}/{artifact}-{version}".format( artifact = unpacked["artifact"], @@ -192,6 +215,7 @@ v2_lock_file = struct( get_input_artifacts_hash = _get_input_artifacts_hash, get_lock_file_hash = _get_lock_file_hash, compute_lock_file_hash = _compute_lock_file_hash, + validate_lock_file_hash = _validate_lock_file_hash, get_artifacts = _get_artifacts, get_netrc_entries = _get_netrc_entries, render_lock_file = _render_lock_file, diff --git a/private/tools/java/com/github/bazelbuild/rules_jvm_external/Coordinates.java b/private/tools/java/com/github/bazelbuild/rules_jvm_external/Coordinates.java index c1f6ff5c..acefd869 100644 --- a/private/tools/java/com/github/bazelbuild/rules_jvm_external/Coordinates.java +++ b/private/tools/java/com/github/bazelbuild/rules_jvm_external/Coordinates.java @@ -36,29 +36,64 @@ public Coordinates(String coordinates) { "Bad artifact coordinates " + coordinates + ", expected format is" - + " :[:[:][:]"); + + " :[:][:][@"); } groupId = Objects.requireNonNull(parts[0]); artifactId = Objects.requireNonNull(parts[1]); + boolean isGradle = + coordinates.contains("@") + || (parts.length > 2 && !parts[2].isEmpty() && Character.isDigit(parts[2].charAt(0))); + + String version = null; + String extension = "jar"; + String classifier = "jar"; + if (parts.length == 2) { extension = "jar"; classifier = ""; version = ""; - } else if (parts.length == 3) { - extension = "jar"; - classifier = ""; - version = parts[2]; - } else if (parts.length == 4) { - extension = parts[2]; - classifier = ""; - version = parts[3]; - } else { + } else if (parts.length == 5) { // Unambiguously the original format extension = "".equals(parts[2]) ? "jar" : parts[2]; classifier = "jar".equals(parts[3]) ? "" : parts[3]; version = parts[4]; + } else if (parts.length == 3) { + // Could either be g:a:e or g:a:v or g:a:v@e + if (isGradle) { + classifier = ""; + + if (parts[2].contains("@")) { + String[] subparts = parts[2].split("@", 2); + version = subparts[0]; + extension = subparts[1]; + } else { + extension = "jar"; + version = parts[2]; + } + } + } else { + // Could be either g:a:e:c or g:a:v:c or g:a:v:c@e + if (isGradle) { + version = parts[2]; + if (parts[3].contains("@")) { + String[] subparts = parts[3].split("@", 2); + classifier = subparts[0]; + extension = subparts[1]; + } else { + classifier = parts[3]; + extension = "jar"; + } + } else { + extension = parts[2]; + classifier = ""; + version = parts[3]; + } } + + this.version = version; + this.classifier = classifier; + this.extension = extension; } public Coordinates( @@ -103,6 +138,7 @@ public String getExtension() { return extension; } + // This method matches `coordinates.bzl#to_key`. Any changes here must be matched there. public String asKey() { StringBuilder coords = new StringBuilder(); coords.append(groupId).append(":").append(artifactId); @@ -155,13 +191,23 @@ public int compareTo(Coordinates o) { } public String toString() { - String versionless = asKey(); + StringBuilder builder = new StringBuilder(); + + builder.append(getGroupId()).append(":").append(getArtifactId()); + + if (getVersion() != null && !getVersion().isEmpty()) { + builder.append(":").append(getVersion()); + } + + if (getClassifier() != null && !getClassifier().isEmpty() && !"jar".equals(getClassifier())) { + builder.append(":").append(getClassifier()); + } - if (version != null && !version.isEmpty()) { - return versionless + ":" + version; + if (getExtension() != null && !getExtension().isEmpty() && !"jar".equals(getExtension())) { + builder.append("@").append(getExtension()); } - return versionless; + return builder.toString(); } @Override diff --git a/private/tools/java/com/github/bazelbuild/rules_jvm_external/coursier/LockFileConverter.java b/private/tools/java/com/github/bazelbuild/rules_jvm_external/coursier/LockFileConverter.java index 5899e054..f4b84b0b 100644 --- a/private/tools/java/com/github/bazelbuild/rules_jvm_external/coursier/LockFileConverter.java +++ b/private/tools/java/com/github/bazelbuild/rules_jvm_external/coursier/LockFileConverter.java @@ -100,7 +100,7 @@ public static void main(String[] args) { Set infos = converter.getDependencies(); Set conflicts = converter.getConflicts(); - Map rendered = new V2LockFile(repositories, infos, conflicts).render(); + Map rendered = new V2LockFile(-1, repositories, infos, conflicts).render(); String converted = new GsonBuilder().setPrettyPrinting().serializeNulls().create().toJson(rendered); diff --git a/private/tools/java/com/github/bazelbuild/rules_jvm_external/resolver/ArtifactsHash.java b/private/tools/java/com/github/bazelbuild/rules_jvm_external/resolver/ArtifactsHash.java new file mode 100644 index 00000000..08d90dc9 --- /dev/null +++ b/private/tools/java/com/github/bazelbuild/rules_jvm_external/resolver/ArtifactsHash.java @@ -0,0 +1,36 @@ +package com.github.bazelbuild.rules_jvm_external.resolver; + +import com.github.bazelbuild.rules_jvm_external.Coordinates; +import java.util.Collection; +import java.util.Set; +import java.util.TreeSet; +import java.util.stream.Collectors; + +public class ArtifactsHash { + + private ArtifactsHash() { + // utility class + } + + public static int calculateArtifactsHash(Collection infos) { + Set lines = new TreeSet<>(); + + for (DependencyInfo info : infos) { + StringBuilder line = new StringBuilder(); + line.append(info.getCoordinates().toString()) + .append(" | ") + .append(info.getSha256().orElseGet(() -> "")) + .append(" | "); + + line.append( + info.getDependencies().stream() + .map(Coordinates::asKey) + .sorted() + .collect(Collectors.joining(","))); + + lines.add(line.toString()); + } + + return String.join("\n", lines).hashCode(); + } +} diff --git a/private/tools/java/com/github/bazelbuild/rules_jvm_external/resolver/cmd/Main.java b/private/tools/java/com/github/bazelbuild/rules_jvm_external/resolver/cmd/Main.java index be9010ae..072f2fe5 100644 --- a/private/tools/java/com/github/bazelbuild/rules_jvm_external/resolver/cmd/Main.java +++ b/private/tools/java/com/github/bazelbuild/rules_jvm_external/resolver/cmd/Main.java @@ -44,7 +44,6 @@ import java.nio.file.Files; import java.nio.file.Path; import java.util.Collections; -import java.util.LinkedHashMap; import java.util.LinkedList; import java.util.List; import java.util.Map; @@ -267,24 +266,14 @@ private static void writeLockFile( listener.close(); Map rendered = - new V2LockFile(request.getRepositories(), infos, conflicts).render(); + new V2LockFile(config.getInputHash(), request.getRepositories(), infos, conflicts).render(); Map toReturn = new TreeMap<>(rendered); // We don't need this, and having it will cause problems toReturn.remove("files"); - toReturn.put( - "__AUTOGENERATED_FILE_DO_NOT_MODIFY_THIS_FILE_MANUALLY", "THERE_IS_NO_DATA_ONLY_ZUUL"); - - LinkedHashMap toHash = new LinkedHashMap<>(); - toHash.put("artifacts", rendered.get("artifacts")); - toHash.put("dependencies", rendered.get("dependencies")); - toHash.put("repositories", rendered.get("repositories")); - String reprString = new StarlarkRepr().repr(toHash); - toReturn.put("__RESOLVED_ARTIFACTS_HASH", reprString.hashCode()); - - if (config.getInputHash() != null) { - toReturn.put("__INPUT_ARTIFACTS_HASH", Long.parseLong(config.getInputHash())); + if (config.getInputHash() != -1) { + toReturn.put("__INPUT_ARTIFACTS_HASH", config.getInputHash()); } String converted = diff --git a/private/tools/java/com/github/bazelbuild/rules_jvm_external/resolver/cmd/ResolverConfig.java b/private/tools/java/com/github/bazelbuild/rules_jvm_external/resolver/cmd/ResolverConfig.java index 38f0538b..20856c81 100644 --- a/private/tools/java/com/github/bazelbuild/rules_jvm_external/resolver/cmd/ResolverConfig.java +++ b/private/tools/java/com/github/bazelbuild/rules_jvm_external/resolver/cmd/ResolverConfig.java @@ -21,6 +21,7 @@ import com.github.bazelbuild.rules_jvm_external.resolver.events.PhaseEvent; import com.github.bazelbuild.rules_jvm_external.resolver.maven.MavenResolver; import com.github.bazelbuild.rules_jvm_external.resolver.netrc.Netrc; +import com.google.common.base.Strings; import com.google.gson.Gson; import com.google.gson.reflect.TypeToken; import java.io.IOException; @@ -235,8 +236,11 @@ public int getMaxThreads() { return maxThreads; } - public String getInputHash() { - return inputHash; + public int getInputHash() { + if (Strings.isNullOrEmpty(inputHash)) { + return -1; + } + return Integer.parseInt(inputHash); } public Path getOutput() { diff --git a/private/tools/java/com/github/bazelbuild/rules_jvm_external/resolver/lockfile/BUILD b/private/tools/java/com/github/bazelbuild/rules_jvm_external/resolver/lockfile/BUILD index d58948e6..a7435865 100644 --- a/private/tools/java/com/github/bazelbuild/rules_jvm_external/resolver/lockfile/BUILD +++ b/private/tools/java/com/github/bazelbuild/rules_jvm_external/resolver/lockfile/BUILD @@ -5,7 +5,7 @@ java_library( srcs = glob(["*.java"]), visibility = [ "//private/tools/java/com/github/bazelbuild/rules_jvm_external:__subpackages__", - "//tests/com/github/bazelbuild/rules_jvm_external/resolver/lockfile:__pkg__", + "//tests/com/github/bazelbuild/rules_jvm_external/resolver:__subpackages__", ], deps = [ "//private/tools/java/com/github/bazelbuild/rules_jvm_external", diff --git a/private/tools/java/com/github/bazelbuild/rules_jvm_external/resolver/lockfile/V2LockFile.java b/private/tools/java/com/github/bazelbuild/rules_jvm_external/resolver/lockfile/V2LockFile.java index 0345790d..449ee212 100644 --- a/private/tools/java/com/github/bazelbuild/rules_jvm_external/resolver/lockfile/V2LockFile.java +++ b/private/tools/java/com/github/bazelbuild/rules_jvm_external/resolver/lockfile/V2LockFile.java @@ -14,6 +14,7 @@ package com.github.bazelbuild.rules_jvm_external.resolver.lockfile; +import static com.github.bazelbuild.rules_jvm_external.resolver.ArtifactsHash.calculateArtifactsHash; import static com.google.common.base.StandardSystemProperty.USER_HOME; import com.github.bazelbuild.rules_jvm_external.Coordinates; @@ -38,12 +39,17 @@ /** Format resolution results into the v2 lock file format. */ public class V2LockFile { + private final int inputHash; private final Collection allRepos; private final Set infos; private final Set conflicts; public V2LockFile( - Collection repositories, Set infos, Set conflicts) { + int inputHash, + Collection repositories, + Set infos, + Set conflicts) { + this.inputHash = inputHash; this.allRepos = repositories; this.infos = infos; this.conflicts = conflicts; @@ -171,7 +177,10 @@ public static V2LockFile create(String from) { conflicts.add(new Conflict(resolved, requested)); } - return new V2LockFile(repos, infos, conflicts); + Number rawInputHash = (Number) raw.get("__INPUT_ARTIFACTS_HASH"); + int inputHash = rawInputHash == null ? -1 : rawInputHash.intValue(); + + return new V2LockFile(inputHash, repos, infos, conflicts); } /** "Render" the resolution result to a `Map` suitable for printing as JSON. */ @@ -252,6 +261,11 @@ public Map render() { }); Map lock = new LinkedHashMap<>(); + lock.put("__AUTOGENERATED_FILE_DO_NOT_MODIFY_THIS_FILE_MANUALLY", "THERE_IS_NO_DATA_ONLY_ZUUL"); + if (inputHash != -1) { + lock.put("__INPUT_ARTIFACTS_HASH", inputHash); + } + lock.put("__RESOLVED_ARTIFACTS_HASH", calculateArtifactsHash(infos)); lock.put("artifacts", ensureArtifactsAllHaveAtLeastOneShaSum(artifacts)); lock.put("dependencies", removeEmptyItems(deps)); lock.put("packages", removeEmptyItems(packages)); diff --git a/private/tools/java/com/github/bazelbuild/rules_jvm_external/resolver/maven/MavenCoordinates.java b/private/tools/java/com/github/bazelbuild/rules_jvm_external/resolver/maven/MavenCoordinates.java index 93a860aa..7376e5c1 100644 --- a/private/tools/java/com/github/bazelbuild/rules_jvm_external/resolver/maven/MavenCoordinates.java +++ b/private/tools/java/com/github/bazelbuild/rules_jvm_external/resolver/maven/MavenCoordinates.java @@ -40,24 +40,4 @@ public static Coordinates asCoordinates(Artifact artifact) { classifier, artifact.getVersion()); } - - private static String construct( - String groupId, String artifactId, String extension, String classifier, String version) { - StringBuilder coords = new StringBuilder(); - coords.append(groupId).append(":").append(artifactId); - - if (extension != null && !extension.isEmpty()) { - coords.append(":").append(extension); - } - - if (classifier != null && !classifier.isEmpty() && !"jar".equals(classifier)) { - coords.append(":").append(classifier); - } - - if (version != null && !version.isEmpty()) { - coords.append(":").append(version); - } - - return coords.toString(); - } } diff --git a/private/tools/java/com/github/bazelbuild/rules_jvm_external/resolver/maven/MavenResolver.java b/private/tools/java/com/github/bazelbuild/rules_jvm_external/resolver/maven/MavenResolver.java index b79b2b02..83a3bfcf 100644 --- a/private/tools/java/com/github/bazelbuild/rules_jvm_external/resolver/maven/MavenResolver.java +++ b/private/tools/java/com/github/bazelbuild/rules_jvm_external/resolver/maven/MavenResolver.java @@ -14,6 +14,8 @@ package com.github.bazelbuild.rules_jvm_external.resolver.maven; +import static com.github.bazelbuild.rules_jvm_external.resolver.maven.MavenCoordinates.asCoordinates; + import com.github.bazelbuild.rules_jvm_external.Coordinates; import com.github.bazelbuild.rules_jvm_external.resolver.Conflict; import com.github.bazelbuild.rules_jvm_external.resolver.ResolutionRequest; @@ -239,7 +241,7 @@ public ResolutionResult resolve(ResolutionRequest request) { && artifact.getVersion().equals(anfe.getArtifact().getVersion()))) { String message = "The POM for " - + anfe.getArtifact() + + asCoordinates(anfe.getArtifact()).setClassifier("pom") + " is missing, no dependency information available."; String detail = "[WARNING]: " + anfe.getMessage(); listener.onEvent(new LogEvent("maven", message, detail)); @@ -346,9 +348,9 @@ private Set getConflicts( } Artifact winningArtifact = ((DependencyNode) winner).getArtifact(); - Coordinates winningCoords = MavenCoordinates.asCoordinates(winningArtifact); + Coordinates winningCoords = asCoordinates(winningArtifact); Artifact artifact = node.getArtifact(); - Coordinates nodeCoords = MavenCoordinates.asCoordinates(artifact); + Coordinates nodeCoords = asCoordinates(artifact); if (!winningCoords.equals(nodeCoords)) { if (!artifactsCoordinates.contains(winningCoords)) { @@ -516,7 +518,7 @@ private Graph buildDependencyGraph( final DependencyNode actualNode = getDependencyNode(node); Artifact artifact = amendArtifact(actualNode.getArtifact()); - Coordinates from = MavenCoordinates.asCoordinates(artifact); + Coordinates from = asCoordinates(artifact); Coordinates remapped = remappings.getOrDefault(from, from); dependencyGraph.addNode(remapped); diff --git a/rules_jvm_external_deps_install.json b/rules_jvm_external_deps_install.json index c5aaaf9e..9d799e44 100644 --- a/rules_jvm_external_deps_install.json +++ b/rules_jvm_external_deps_install.json @@ -1,7 +1,7 @@ { "__AUTOGENERATED_FILE_DO_NOT_MODIFY_THIS_FILE_MANUALLY": "THERE_IS_NO_DATA_ONLY_ZUUL", "__INPUT_ARTIFACTS_HASH": -951616327, - "__RESOLVED_ARTIFACTS_HASH": -84218881, + "__RESOLVED_ARTIFACTS_HASH": -325633796, "artifacts": { "aopalliance:aopalliance": { "shasums": { diff --git a/tests/com/github/bazelbuild/rules_jvm_external/CoordinatesTest.java b/tests/com/github/bazelbuild/rules_jvm_external/CoordinatesTest.java index 7faadc4e..65fdcc34 100644 --- a/tests/com/github/bazelbuild/rules_jvm_external/CoordinatesTest.java +++ b/tests/com/github/bazelbuild/rules_jvm_external/CoordinatesTest.java @@ -30,11 +30,24 @@ public void coordinatesAreConsistentWhenDefaultValuesAreUsed() { } @Test - public void coordinatesAreConsistentWhenEllidedValuesAreUsed() { + public void coordinatesAreConsistentWhenElidedValuesAreUsed() { Coordinates plain = new Coordinates("com.example:foo:1.2.3"); Coordinates fancy = new Coordinates("com.example:foo:::1.2.3"); assertEquals(plain, fancy); assertEquals(plain.hashCode(), fancy.hashCode()); } + + @Test + public void canConvertBetweenFormatsSeamlessly() { + Coordinates gradle = new Coordinates("com.example:foo:1.2.3:classifier@extension"); + Coordinates original = new Coordinates("com.example:foo:extension:classifier:1.2.3"); + assertEquals(gradle, original); + + Coordinates refreshedGradle = new Coordinates(gradle.toString()); + assertEquals(gradle, refreshedGradle); + + Coordinates refreshedOriginal = new Coordinates(original.toString()); + assertEquals(gradle, refreshedGradle); + } } diff --git a/tests/com/github/bazelbuild/rules_jvm_external/resolver/ArtifactsHashTest.java b/tests/com/github/bazelbuild/rules_jvm_external/resolver/ArtifactsHashTest.java new file mode 100644 index 00000000..f4d715bf --- /dev/null +++ b/tests/com/github/bazelbuild/rules_jvm_external/resolver/ArtifactsHashTest.java @@ -0,0 +1,46 @@ +package com.github.bazelbuild.rules_jvm_external.resolver; + +import static com.github.bazelbuild.rules_jvm_external.resolver.ArtifactsHash.calculateArtifactsHash; +import static org.junit.Assert.assertEquals; + +import com.github.bazelbuild.rules_jvm_external.resolver.lockfile.V2LockFile; +import com.google.devtools.build.runfiles.Runfiles; +import com.google.gson.Gson; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Paths; +import java.util.Map; +import org.junit.Test; + +// The tests in this file must match the tests in `artifacts_hash_test.bzl` +public class ArtifactsHashTest { + @Test + public void emptyInfos() throws IOException { + assertHashCodesMatch( + "rules_jvm_external/tests/custom_maven_install/artifacts_hash_no_deps_install.json"); + } + + @Test + public void withDeps() throws IOException { + assertHashCodesMatch( + "rules_jvm_external/tests/custom_maven_install/artifacts_hash_with_deps_install.json"); + } + + @Test + public void withDepsFromManve() throws IOException { + assertHashCodesMatch( + "rules_jvm_external/tests/custom_maven_install/artifacts_hash_with_deps_from_maven_install.json"); + } + + private void assertHashCodesMatch(String path) throws IOException { + String file = Runfiles.create().rlocation(path); + String contents = Files.readString(Paths.get(file)); + + V2LockFile lockFile = V2LockFile.create(contents); + + Map read = new Gson().fromJson(contents, Map.class); + Number expected = (Number) read.get("__RESOLVED_ARTIFACTS_HASH"); + + assertEquals(expected.intValue(), calculateArtifactsHash(lockFile.getDependencyInfos())); + } +} diff --git a/tests/com/github/bazelbuild/rules_jvm_external/resolver/BUILD b/tests/com/github/bazelbuild/rules_jvm_external/resolver/BUILD index 65a11ff5..5b40206d 100644 --- a/tests/com/github/bazelbuild/rules_jvm_external/resolver/BUILD +++ b/tests/com/github/bazelbuild/rules_jvm_external/resolver/BUILD @@ -4,7 +4,10 @@ load("//:defs.bzl", "artifact") java_library( name = "resolver", testonly = True, - srcs = glob(["*.java"]), + srcs = glob( + ["*.java"], + exclude = ["*Test.java"], + ), visibility = [ "//tests/com/github/bazelbuild/rules_jvm_external:__subpackages__", ], @@ -34,3 +37,28 @@ java_library( ), ], ) + +java_test( + name = "ArtifactsHashTest", + srcs = ["ArtifactsHashTest.java"], + data = [ + "//tests/custom_maven_install:artifacts_hash_no_deps_install.json", + "//tests/custom_maven_install:artifacts_hash_with_deps_from_maven_install.json", + "//tests/custom_maven_install:artifacts_hash_with_deps_install.json", + ], + test_class = "com.github.bazelbuild.rules_jvm_external.resolver.ArtifactsHashTest", + deps = [ + "//private/tools/java/com/github/bazelbuild/rules_jvm_external", + "//private/tools/java/com/github/bazelbuild/rules_jvm_external/resolver", + "//private/tools/java/com/github/bazelbuild/rules_jvm_external/resolver/lockfile", + "@bazel_tools//tools/java/runfiles", + artifact( + "com.google.code.gson:gson", + repository_name = "rules_jvm_external_deps", + ), + artifact( + "junit:junit", + repository_name = "regression_testing_coursier", + ), + ], +) diff --git a/tests/com/github/bazelbuild/rules_jvm_external/resolver/ResolverTestBase.java b/tests/com/github/bazelbuild/rules_jvm_external/resolver/ResolverTestBase.java index f41f43c4..9e202bde 100644 --- a/tests/com/github/bazelbuild/rules_jvm_external/resolver/ResolverTestBase.java +++ b/tests/com/github/bazelbuild/rules_jvm_external/resolver/ResolverTestBase.java @@ -575,7 +575,7 @@ public void shouldWarnOnMissingTransitiveDependencies() { assertEquals(Set.of(present, missing), resolution.nodes()); logEvents.stream() - .filter(e -> e.toString().contains("The POM for " + missing.setExtension("pom"))) + .filter(e -> e.toString().contains("The POM for " + missing.setClassifier("pom"))) .findFirst() .orElseThrow(() -> new AssertionError("Cannot find expected log message")); } diff --git a/tests/com/github/bazelbuild/rules_jvm_external/resolver/lockfile/V2LockFileTest.java b/tests/com/github/bazelbuild/rules_jvm_external/resolver/lockfile/V2LockFileTest.java index 51b1f1bb..de228fe1 100644 --- a/tests/com/github/bazelbuild/rules_jvm_external/resolver/lockfile/V2LockFileTest.java +++ b/tests/com/github/bazelbuild/rules_jvm_external/resolver/lockfile/V2LockFileTest.java @@ -47,7 +47,7 @@ public void shouldRenderAggregatingJarsAsJarWithNullShasum() { Set.of(), new TreeMap<>()); - Map rendered = new V2LockFile(repos, Set.of(aggregator), Set.of()).render(); + Map rendered = new V2LockFile(-1, repos, Set.of(aggregator), Set.of()).render(); Map artifacts = (Map) rendered.get("artifacts"); Map data = (Map) artifacts.get("com.example:aggregator"); @@ -60,7 +60,7 @@ public void shouldRenderAggregatingJarsAsJarWithNullShasum() { @Test public void shouldRoundTripASimpleSetOfDependencies() { - V2LockFile roundTripped = roundTrip(new V2LockFile(repos, Set.of(), Set.of())); + V2LockFile roundTripped = roundTrip(new V2LockFile(-1, repos, Set.of(), Set.of())); assertEquals(repos, roundTripped.getRepositories()); assertEquals(Set.of(), roundTripped.getDependencyInfos()); @@ -69,7 +69,7 @@ public void shouldRoundTripASimpleSetOfDependencies() { @Test public void shouldRoundTripM2Local() { - V2LockFile lockFile = new V2LockFile(repos, Set.of(), Set.of()); + V2LockFile lockFile = new V2LockFile(-1, repos, Set.of(), Set.of()); Map rendered = lockFile.render(); rendered.put("m2local", true); @@ -94,7 +94,7 @@ public void shouldRoundTripASingleArtifact() { Set.of(), new TreeMap<>()); - V2LockFile lockFile = roundTrip(new V2LockFile(repos, Set.of(info), Set.of())); + V2LockFile lockFile = roundTrip(new V2LockFile(-1, repos, Set.of(info), Set.of())); assertEquals(Set.of(info), lockFile.getDependencyInfos()); } @@ -123,7 +123,7 @@ public void shouldRoundTripASingleArtifactWithADependency() { Set.of(), new TreeMap<>()); - V2LockFile lockFile = roundTrip(new V2LockFile(repos, Set.of(info, dep), Set.of())); + V2LockFile lockFile = roundTrip(new V2LockFile(-1, repos, Set.of(info, dep), Set.of())); assertEquals(Set.of(info, dep), lockFile.getDependencyInfos()); } @@ -137,7 +137,7 @@ public void shouldRoundTripConflicts() { new Conflict( new Coordinates("com.foo:bar:1.2.3"), new Coordinates("com.foo:bar:1.2.1"))); - V2LockFile lockFile = roundTrip(new V2LockFile(repos, Set.of(), conflicts)); + V2LockFile lockFile = roundTrip(new V2LockFile(-1, repos, Set.of(), conflicts)); assertEquals(conflicts, lockFile.getConflicts()); } diff --git a/tests/custom_maven_install/BUILD b/tests/custom_maven_install/BUILD index deaaeb11..44421ffe 100644 --- a/tests/custom_maven_install/BUILD +++ b/tests/custom_maven_install/BUILD @@ -1,5 +1,8 @@ exports_files([ "artifact_with_plus_repin_install.json", + "artifacts_hash_no_deps_install.json", + "artifacts_hash_with_deps_from_maven_install.json", + "artifacts_hash_with_deps_install.json", "m2local_testing_with_pinned_file_install.json", "manifest_stamp_testing_install.json", "maven_install.json", diff --git a/tests/custom_maven_install/artifacts_hash_no_deps_install.json b/tests/custom_maven_install/artifacts_hash_no_deps_install.json new file mode 100644 index 00000000..b23d694b --- /dev/null +++ b/tests/custom_maven_install/artifacts_hash_no_deps_install.json @@ -0,0 +1,50 @@ +{ + "__AUTOGENERATED_FILE_DO_NOT_MODIFY_THIS_FILE_MANUALLY": "THERE_IS_NO_DATA_ONLY_ZUUL", + "__INPUT_ARTIFACTS_HASH": -1684779452, + "__RESOLVED_ARTIFACTS_HASH": 562963644, + "artifacts": { + "org.hamcrest:hamcrest": { + "shasums": { + "jar": "5d66b6a4a680755cb6ed7cb104fa7835ef644667586ff0737adeb977c39ecdbc" + }, + "version": "3.0" + }, + "org.hamcrest:hamcrest-core": { + "shasums": { + "jar": "b78a3a81692f421cc01fc17ded9a45e9fb6f3949c712f8ec4d01da6b8c06bc6e" + }, + "version": "3.0" + } + }, + "dependencies": { + "org.hamcrest:hamcrest-core": [ + "org.hamcrest:hamcrest" + ] + }, + "packages": { + "org.hamcrest:hamcrest": [ + "org.hamcrest", + "org.hamcrest.beans", + "org.hamcrest.collection", + "org.hamcrest.comparator", + "org.hamcrest.core", + "org.hamcrest.internal", + "org.hamcrest.io", + "org.hamcrest.number", + "org.hamcrest.object", + "org.hamcrest.text", + "org.hamcrest.xml" + ], + "org.hamcrest:hamcrest-core": [ + "org.hamcrest.core.deprecated" + ] + }, + "repositories": { + "https://repo1.maven.org/maven2/": [ + "org.hamcrest:hamcrest", + "org.hamcrest:hamcrest-core" + ] + }, + "services": {}, + "version": "2" +} diff --git a/tests/custom_maven_install/artifacts_hash_with_deps_from_maven_install.json b/tests/custom_maven_install/artifacts_hash_with_deps_from_maven_install.json new file mode 100644 index 00000000..44f37ca8 --- /dev/null +++ b/tests/custom_maven_install/artifacts_hash_with_deps_from_maven_install.json @@ -0,0 +1,142 @@ +{ + "__AUTOGENERATED_FILE_DO_NOT_MODIFY_THIS_FILE_MANUALLY": "THERE_IS_NO_DATA_ONLY_ZUUL", + "__INPUT_ARTIFACTS_HASH": -1269964981, + "__RESOLVED_ARTIFACTS_HASH": 144119858, + "artifacts": { + "com.google.code.findbugs:jsr305": { + "shasums": { + "jar": "766ad2a0783f2687962c8ad74ceecc38a28b9f72a2d085ee438b7813e928d0c7" + }, + "version": "3.0.2" + }, + "com.google.errorprone:error_prone_annotations": { + "shasums": { + "jar": "f3fc8a3a0a4020706a373b00e7f57c2512dd26d1f83d28c7d38768f8682b231e" + }, + "version": "2.28.0" + }, + "com.google.guava:failureaccess": { + "shasums": { + "jar": "8a8f81cf9b359e3f6dfa691a1e776985c061ef2f223c9b2c80753e1b458e8064" + }, + "version": "1.0.2" + }, + "com.google.guava:guava": { + "shasums": { + "jar": "4bf0e2c5af8e4525c96e8fde17a4f7307f97f8478f11c4c8e35a0e3298ae4e90" + }, + "version": "33.3.1-jre" + }, + "com.google.guava:listenablefuture": { + "shasums": { + "jar": "b372a037d4230aa57fbeffdef30fd6123f9c0c2db85d0aced00c91b974f33f99" + }, + "version": "9999.0-empty-to-avoid-conflict-with-guava" + }, + "com.google.j2objc:j2objc-annotations": { + "shasums": { + "jar": "88241573467ddca44ffd4d74aa04c2bbfd11bf7c17e0c342c94c9de7a70a7c64" + }, + "version": "3.0.0" + }, + "org.checkerframework:checker-qual": { + "shasums": { + "jar": "3fbc2e98f05854c3df16df9abaa955b91b15b3ecac33623208ed6424640ef0f6" + }, + "version": "3.43.0" + } + }, + "dependencies": { + "com.google.guava:guava": [ + "com.google.code.findbugs:jsr305", + "com.google.errorprone:error_prone_annotations", + "com.google.guava:failureaccess", + "com.google.guava:listenablefuture", + "com.google.j2objc:j2objc-annotations", + "org.checkerframework:checker-qual" + ] + }, + "packages": { + "com.google.code.findbugs:jsr305": [ + "javax.annotation", + "javax.annotation.concurrent", + "javax.annotation.meta" + ], + "com.google.errorprone:error_prone_annotations": [ + "com.google.errorprone.annotations", + "com.google.errorprone.annotations.concurrent" + ], + "com.google.guava:failureaccess": [ + "com.google.common.util.concurrent.internal" + ], + "com.google.guava:guava": [ + "com.google.common.annotations", + "com.google.common.base", + "com.google.common.base.internal", + "com.google.common.cache", + "com.google.common.collect", + "com.google.common.escape", + "com.google.common.eventbus", + "com.google.common.graph", + "com.google.common.hash", + "com.google.common.html", + "com.google.common.io", + "com.google.common.math", + "com.google.common.net", + "com.google.common.primitives", + "com.google.common.reflect", + "com.google.common.util.concurrent", + "com.google.common.xml", + "com.google.thirdparty.publicsuffix" + ], + "com.google.j2objc:j2objc-annotations": [ + "com.google.j2objc.annotations" + ], + "org.checkerframework:checker-qual": [ + "org.checkerframework.checker.builder.qual", + "org.checkerframework.checker.calledmethods.qual", + "org.checkerframework.checker.compilermsgs.qual", + "org.checkerframework.checker.fenum.qual", + "org.checkerframework.checker.formatter.qual", + "org.checkerframework.checker.guieffect.qual", + "org.checkerframework.checker.i18n.qual", + "org.checkerframework.checker.i18nformatter.qual", + "org.checkerframework.checker.index.qual", + "org.checkerframework.checker.initialization.qual", + "org.checkerframework.checker.interning.qual", + "org.checkerframework.checker.lock.qual", + "org.checkerframework.checker.mustcall.qual", + "org.checkerframework.checker.nullness.qual", + "org.checkerframework.checker.optional.qual", + "org.checkerframework.checker.propkey.qual", + "org.checkerframework.checker.regex.qual", + "org.checkerframework.checker.signature.qual", + "org.checkerframework.checker.signedness.qual", + "org.checkerframework.checker.tainting.qual", + "org.checkerframework.checker.units.qual", + "org.checkerframework.common.aliasing.qual", + "org.checkerframework.common.initializedfields.qual", + "org.checkerframework.common.reflection.qual", + "org.checkerframework.common.returnsreceiver.qual", + "org.checkerframework.common.subtyping.qual", + "org.checkerframework.common.util.count.report.qual", + "org.checkerframework.common.value.qual", + "org.checkerframework.dataflow.qual", + "org.checkerframework.framework.qual" + ] + }, + "repositories": { + "https://repo1.maven.org/maven2/": [ + "com.google.code.findbugs:jsr305", + "com.google.errorprone:error_prone_annotations", + "com.google.guava:failureaccess", + "com.google.guava:guava", + "com.google.guava:listenablefuture", + "com.google.j2objc:j2objc-annotations", + "org.checkerframework:checker-qual" + ] + }, + "services": {}, + "skipped": [], + "version": "2" +} diff --git a/tests/custom_maven_install/artifacts_hash_with_deps_install.json b/tests/custom_maven_install/artifacts_hash_with_deps_install.json new file mode 100644 index 00000000..686b18b5 --- /dev/null +++ b/tests/custom_maven_install/artifacts_hash_with_deps_install.json @@ -0,0 +1,49 @@ +{ + "__AUTOGENERATED_FILE_DO_NOT_MODIFY_THIS_FILE_MANUALLY": "THERE_IS_NO_DATA_ONLY_ZUUL", + "__INPUT_ARTIFACTS_HASH": -976415106, + "__RESOLVED_ARTIFACTS_HASH": 107581547, + "artifacts": { + "com.google.code.gson:gson": { + "shasums": { + "jar": "57928d6e5a6edeb2abd3770a8f95ba44dce45f3b23b7a9dc2b309c581552a78b" + }, + "version": "2.11.0" + }, + "com.google.errorprone:error_prone_annotations": { + "shasums": { + "jar": "24c923372c58e35d0b9f16a028929bb9aedc77521867c274f2bd0735df5ba1f5" + }, + "version": "2.27.0" + } + }, + "dependencies": { + "com.google.code.gson:gson": [ + "com.google.errorprone:error_prone_annotations" + ] + }, + "packages": { + "com.google.code.gson:gson": [ + "com.google.gson", + "com.google.gson.annotations", + "com.google.gson.internal", + "com.google.gson.internal.bind", + "com.google.gson.internal.bind.util", + "com.google.gson.internal.reflect", + "com.google.gson.internal.sql", + "com.google.gson.reflect", + "com.google.gson.stream" + ], + "com.google.errorprone:error_prone_annotations": [ + "com.google.errorprone.annotations", + "com.google.errorprone.annotations.concurrent" + ] + }, + "repositories": { + "https://repo1.maven.org/maven2/": [ + "com.google.code.gson:gson", + "com.google.errorprone:error_prone_annotations" + ] + }, + "services": {}, + "version": "2" +} diff --git a/tests/custom_maven_install/regression_testing_maven_install.json b/tests/custom_maven_install/regression_testing_maven_install.json index 24efa97b..6f4c97cb 100644 --- a/tests/custom_maven_install/regression_testing_maven_install.json +++ b/tests/custom_maven_install/regression_testing_maven_install.json @@ -1,7 +1,7 @@ { "__AUTOGENERATED_FILE_DO_NOT_MODIFY_THIS_FILE_MANUALLY": "THERE_IS_NO_DATA_ONLY_ZUUL", "__INPUT_ARTIFACTS_HASH": -1308247853, - "__RESOLVED_ARTIFACTS_HASH": -1668077746, + "__RESOLVED_ARTIFACTS_HASH": 1818396785, "artifacts": { "io.opentelemetry:opentelemetry-api": { "shasums": { diff --git a/tests/integration/BUILD b/tests/integration/BUILD index 33cee8b0..b76b739d 100644 --- a/tests/integration/BUILD +++ b/tests/integration/BUILD @@ -126,3 +126,38 @@ genquery( scope = ["@maven_resolved_with_boms//:org_seleniumhq_selenium_selenium_java"], deps = [], ) + +# This target will fail to build if the artifacts hashes are not valid, which +# acts as a check that `ArtifactsHash.java` is working as expected. +genrule( + name = "assert-artifact-hashes", + srcs = [ + artifact( + "org.hamcrest:hamcrest-core", + repository_name = "artifacts_hash_no_deps", + ), + artifact( + "com.google.guava:guava", + repository_name = "artifacts_hash_with_deps_from_maven", + ), + artifact( + "com.google.code.gson:gson", + repository_name = "artifacts_hash_with_deps", + ), + ], + outs = ["ignored-output.txt"], + cmd = "echo $(location %s) $(location %s) $(location %s) >$@" % ( + artifact( + "org.hamcrest:hamcrest-core", + repository_name = "artifacts_hash_no_deps", + ), + artifact( + "com.google.guava:guava", + repository_name = "artifacts_hash_with_deps_from_maven", + ), + artifact( + "com.google.code.gson:gson", + repository_name = "artifacts_hash_with_deps", + ), + ), +)