diff --git a/private/lib/coordinates.bzl b/private/lib/coordinates.bzl index cb86755f..5f6eae00 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,22 @@ 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 + else: + if 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 8a3e7dc1..7c94acf0 100644 --- a/private/rules/coursier.bzl +++ b/private/rules/coursier.bzl @@ -530,7 +530,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..9728e732 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 == 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..eb4d4c5e 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 @@ -14,6 +14,7 @@ package com.github.bazelbuild.rules_jvm_external; +import java.util.Arrays; import java.util.Objects; /** @@ -36,29 +37,63 @@ 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/resolver/cmd/Main.java b/private/tools/java/com/github/bazelbuild/rules_jvm_external/resolver/cmd/Main.java index c6313f97..4f6788f2 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 @@ -51,6 +51,7 @@ import java.util.Map; import java.util.Objects; import java.util.Set; +import java.util.StringJoiner; import java.util.TreeMap; import java.util.TreeSet; import java.util.concurrent.CompletableFuture; @@ -292,8 +293,7 @@ private static void writeLockFile( 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()); + toReturn.put("__RESOLVED_ARTIFACTS_HASH", calculateArtifactsHash(infos)); if (config.getInputHash() != null) { toReturn.put("__INPUT_ARTIFACTS_HASH", Long.parseLong(config.getInputHash())); @@ -307,4 +307,25 @@ private static void writeLockFile( bos.write(converted.getBytes(UTF_8)); } } + + private static int calculateArtifactsHash(Set 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/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); + } }