Implement the new artifacts hashing algorithm

Author: shs96c

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 = [

private/lib/coordinates.bzl

@@ -57,17 +57,30 @@ 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.
 
     The returned format matches Gradle's "external dependency" short-form
     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/",

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):

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,

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,

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>:<artifactId>[:<extension>[:<classifier>][:<version>]");
+              + " <groupId>:<artifactId>[:<version>][:<classifier>][@<extension>");
     }
 
     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

private/tools/java/com/github/bazelbuild/rules_jvm_external/coursier/LockFileConverter.java

@@ -100,7 +100,7 @@ public static void main(String[] args) {
     Set<DependencyInfo> infos = converter.getDependencies();
     Set<Conflict> conflicts = converter.getConflicts();
 
-    Map<String, Object> rendered = new V2LockFile(repositories, infos, conflicts).render();
+    Map<String, Object> rendered = new V2LockFile(-1, repositories, infos, conflicts).render();
 
     String converted =
         new GsonBuilder().setPrettyPrinting().serializeNulls().create().toJson(rendered);

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<DependencyInfo> infos) {
+    Set<String> 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();
+  }
+}