[internal] jvm: fix default paths for tool lockfiles (pantsbuild#13822)

Fix how the `--TOOL-lockfile` option's default is set. It was set to a default of its location in the Pants repository and not the special string `<default>`. The special string should trigger using the resource for the default lockfile embedded in the Pants distribution. Pants will error if the user explicitly requests resolving the lockfile of a tool using the default lockfile.

In the Pants repository itself, the `--TOOL-lockfile` options are set appropriately for their locations in the Pants repository (which are then consumed by `resource` targets).

This PR only fixes the current broken behavior, preserving the status quo.

[ci skip-rust]

pants.toml

@@ -173,6 +173,12 @@ java_parser = "src/python/pants/backend/java/dependency_inference/java_parser.lo
 # Has the same isolation requirements as `java_parser`.
 scala_parser = "src/python/pants/backend/scala/dependency_inference/scala_parser.lockfile"
 
+[junit]
+lockfile = "src/python/pants/backend/java/test/junit.default.lockfile.txt"
+
+[google-java-format]
+lockfile = "src/python/pants/backend/java/lint/google_java_format/google_java_format.default.lockfile.txt"
+
 [toolchain-setup]
 repo = "pants"
 

src/python/pants/backend/java/lint/google_java_format/BUILD

@@ -1,8 +1,8 @@
 # Copyright 2021 Pants project contributors (see CONTRIBUTORS.md).
 # Licensed under the Apache License, Version 2.0 (see LICENSE).
 
-python_sources(dependencies=[":resources"])
+python_sources(dependencies=[":lockfile"])
 
 python_tests(name="tests")
 
-resources(name="resources", sources=["*.lockfile.txt"])
+resource(name="lockfile", source="google_java_format.default.lockfile.txt")

src/python/pants/backend/java/lint/google_java_format/subsystem.py

@@ -16,8 +16,9 @@ class GoogleJavaFormatSubsystem(JvmToolBase):
         "pants.backend.java.lint.google_java_format",
         "google_java_format.default.lockfile.txt",
     )
-    default_lockfile_path = "src/python/pants/backend/java/lint/google_java_format/google_java_format.default.lockfile.txt"
-    default_lockfile_url = git_url(default_lockfile_path)
+    default_lockfile_url = git_url(
+        "src/python/pants/backend/java/lint/google_java_format/google_java_format.default.lockfile.txt"
+    )
 
     @classmethod
     def register_options(cls, register):

src/python/pants/backend/java/subsystems/junit.py

@@ -16,8 +16,7 @@ class JUnit(JvmToolBase):
         "org.junit.vintage:junit-vintage-engine:{version}",
     ]
     default_lockfile_resource = ("pants.backend.java.test", "junit.default.lockfile.txt")
-    default_lockfile_path = "src/python/pants/backend/java/test/junit.default.lockfile.txt"
-    default_lockfile_url = git_url(default_lockfile_path)
+    default_lockfile_url = git_url("src/python/pants/backend/java/test/junit.default.lockfile.txt")
 
     @classmethod
     def register_options(cls, register):

src/python/pants/backend/java/test/BUILD

@@ -1,7 +1,7 @@
 # Copyright 2021 Pants project contributors (see CONTRIBUTORS.md).
 # Licensed under the Apache License, Version 2.0 (see LICENSE).
 
-python_sources(dependencies=[":lockfiles"])
+python_sources(dependencies=[":lockfile"])
 python_tests(name="tests", timeout=240)
 
-resources(name="lockfiles", sources=["*.lockfile.txt"])
+resource(name="lockfile", source="junit.default.lockfile.txt")

src/python/pants/jvm/resolve/jvm_tool.py

@@ -21,6 +21,7 @@
 from pants.jvm.resolve.coursier_fetch import (
     ArtifactRequirements,
     Coordinate,
+    Coordinates,
     CoursierResolvedLockfile,
 )
 from pants.jvm.target_types import JvmArtifactFieldSet
@@ -79,7 +80,7 @@ def register_options(cls, register):
         register(
             "--lockfile",
             type=str,
-            default=cls.default_lockfile_path,
+            default=DEFAULT_TOOL_LOCKFILE,
             advanced=True,
             help=(
                 "Path to a lockfile used for installing the tool.\n\n"
@@ -104,10 +105,7 @@ def artifact_inputs(self) -> tuple[str, ...]:
     def lockfile(self) -> str:
         f"""The path to a lockfile or special string '{DEFAULT_TOOL_LOCKFILE}'."""
         lockfile = cast(str, self.options.lockfile)
-        if lockfile != DEFAULT_TOOL_LOCKFILE:
-            return lockfile
-        pkg, path = self.default_lockfile_resource
-        return f"src/python/{pkg.replace('.', '/')}/{path}"
+        return lockfile
 
     def lockfile_content(self) -> bytes:
         lockfile_path = self.lockfile
@@ -135,6 +133,12 @@ class JvmToolLockfileRequest:
 
     @classmethod
     def from_tool(cls, tool: JvmToolBase) -> JvmToolLockfileRequest:
+        lockfile_dest = tool.lockfile
+        if lockfile_dest == DEFAULT_TOOL_LOCKFILE:
+            raise ValueError(
+                f"Internal error: Request to write tool lockfile but `[{tool.options_scope}.lockfile]` "
+                f'is set to the default ("{DEFAULT_TOOL_LOCKFILE}").'
+            )
         return cls(
             artifact_inputs=FrozenOrderedSet(tool.artifact_inputs),
             resolve_name=tool.options_scope,
@@ -149,6 +153,12 @@ class JvmToolLockfile:
     path: str
 
 
+@dataclass(frozen=True)
+class GatherJvmCoordinatesRequest:
+    artifact_inputs: FrozenOrderedSet[str]
+    option_name: str
+
+
 class GenerateJvmLockfilesSubsystem(GoalSubsystem):
     name = "jvm-generate-lockfiles"
     help = "Generate lockfiles for JVM tools third-party dependencies."
@@ -166,7 +176,7 @@ def register_options(cls, register) -> None:
                 "Only generate lockfiles for the specified resolve(s).\n\n"
                 "Resolves are the logical names for tool lockfiles which are "
                 "the options scope for that tool such as `junit`.\n\n"
-                "For example, you can run `./pants generate-lockfiles --resolve=junit "
+                "For example, you can run `./pants jvm-generate-lockfiles --resolve=junit "
                 "to only generate lockfiles for the `junit` tool.\n\n"
                 "If you specify an invalid resolve name, like 'fake', Pants will output all "
                 "possible values.\n\n"
@@ -216,10 +226,8 @@ async def generate_lockfiles_goal(
     return GenerateJvmLockfilesGoal(exit_code=0)
 
 
-@rule(desc="Generate JVM lockfile", level=LogLevel.DEBUG)
-async def generate_jvm_lockfile(
-    request: JvmToolLockfileRequest,
-) -> JvmToolLockfile:
+@rule
+async def gather_coordinates_for_jvm_lockfile(request: GatherJvmCoordinatesRequest) -> Coordinates:
     # Separate `artifact_inputs` by whether the strings parse as an `Address` or not.
     coordinates: set[Coordinate] = set()
     candidate_address_inputs: set[AddressInput] = set()
@@ -244,7 +252,7 @@ async def generate_jvm_lockfile(
     if bad_artifact_inputs:
         raise ValueError(
             "The following values could not be parsed as an address nor as a JVM coordinate string. "
-            f"The problematic inputs supplied to the `--{request.resolve_name}-artifacts option were: "
+            f"The problematic inputs supplied to the `{request.option_name}` option were: "
             f"{', '.join(bad_artifact_inputs)}."
         )
 
@@ -261,10 +269,21 @@ async def generate_jvm_lockfile(
     if other_targets:
         raise ValueError(
             "The following addresses reference targets that are not `jvm_artifact` targets. "
-            f"Please only supply the addresses of `jvm_artifact` for the `--{request.resolve_name}-artifacts "
+            f"Please only supply the addresses of `jvm_artifact` for the `{request.option_name}` "
             f"option. The problematic addresses are: {', '.join(str(tgt.address) for tgt in other_targets)}."
         )
 
+    return Coordinates(coordinates)
+
+
+@rule(desc="Generate JVM lockfile", level=LogLevel.DEBUG)
+async def generate_jvm_lockfile(
+    request: JvmToolLockfileRequest,
+) -> JvmToolLockfile:
+    coordinates = await Get(
+        Coordinates,
+        GatherJvmCoordinatesRequest(request.artifact_inputs, f"[{request.resolve_name}].artifacts"),
+    )
     resolved_lockfile = await Get(CoursierResolvedLockfile, ArtifactRequirements(coordinates))
     lockfile_content = resolved_lockfile.to_json()
     lockfile_digest = await Get(

src/python/pants/jvm/resolve/jvm_tool_test.py

@@ -3,7 +3,6 @@
 
 from __future__ import annotations
 
-import json
 import textwrap
 
 import pytest
@@ -12,20 +11,15 @@
 from pants.backend.python.target_types import UnrecognizedResolveNamesError
 from pants.core.util_rules import config_files, source_files
 from pants.core.util_rules.external_tool import rules as external_tool_rules
-from pants.engine.fs import Digest, DigestContents, FileDigest
+from pants.engine.fs import Digest, DigestContents
 from pants.engine.rules import SubsystemRule, rule
 from pants.jvm.resolve import jvm_tool
-from pants.jvm.resolve.coursier_fetch import (
-    Coordinate,
-    Coordinates,
-    CoursierLockfileEntry,
-    CoursierResolvedLockfile,
-)
+from pants.jvm.resolve.coursier_fetch import Coordinate, Coordinates
 from pants.jvm.resolve.coursier_fetch import rules as coursier_fetch_rules
 from pants.jvm.resolve.coursier_setup import rules as coursier_setup_rules
 from pants.jvm.resolve.jvm_tool import (
+    GatherJvmCoordinatesRequest,
     JvmToolBase,
-    JvmToolLockfile,
     JvmToolLockfileRequest,
     JvmToolLockfileSentinel,
     determine_resolves_to_generate,
@@ -34,15 +28,8 @@
 from pants.jvm.target_types import JvmArtifact, JvmDependencyLockfile
 from pants.jvm.util_rules import rules as util_rules
 from pants.testutil.rule_runner import PYTHON_BOOTSTRAP_ENV, QueryRule, RuleRunner
-from pants.util.docutil import git_url
 from pants.util.ordered_set import FrozenOrderedSet
 
-HAMCREST_COORD = Coordinate(
-    group="org.hamcrest",
-    artifact="hamcrest-core",
-    version="1.3",
-)
-
 
 class MockJvmTool(JvmToolBase):
     options_scope = "mock-tool"
@@ -52,8 +39,7 @@ class MockJvmTool(JvmToolBase):
         "org.hamcrest:hamcrest-core:{version}",
     ]
     default_lockfile_resource = ("pants.backend.jvm.resolve", "mock-tool.default.lockfile.txt")
-    default_lockfile_path = "src/python/pants/backend/jvm/resolve/mock-tool.default.lockfile.txt"
-    default_lockfile_url = git_url(default_lockfile_path)
+    default_lockfile_url = ""
 
 
 class MockJvmToolLockfileSentinel(JvmToolLockfileSentinel):
@@ -80,14 +66,15 @@ def test_jvm_tool_base_extracts_correct_coordinates() -> None:
             generate_test_tool_lockfile_request,
             SubsystemRule(MockJvmTool),
             QueryRule(JvmToolLockfileRequest, (MockJvmToolLockfileSentinel,)),
-            QueryRule(JvmToolLockfile, (JvmToolLockfileRequest,)),
+            QueryRule(Coordinates, (GatherJvmCoordinatesRequest,)),
             QueryRule(DigestContents, (Digest,)),
         ],
         target_types=[JvmDependencyLockfile, JvmArtifact],
     )
     rule_runner.set_options(
         args=[
             "--mock-tool-artifacts=//:junit_junit",
+            "--mock-tool-lockfile=/dev/null",
         ],
         env_inherit=PYTHON_BOOTSTRAP_ENV,
     )
@@ -112,42 +99,13 @@ def test_jvm_tool_base_extracts_correct_coordinates() -> None:
         "org.hamcrest:hamcrest-core:1.3",
     ]
 
-    tool_lockfile = rule_runner.request(JvmToolLockfile, [lockfile_request])
-    assert tool_lockfile.resolve_name == "mock-tool"
-
-    lockfile_digest_contents = rule_runner.request(DigestContents, [tool_lockfile.digest])
-    assert len(lockfile_digest_contents) == 1
-    lockfile_content = lockfile_digest_contents[0]
-    assert (
-        lockfile_content.path
-        == "src/python/pants/backend/jvm/resolve/mock-tool.default.lockfile.txt"
-    )
-    lockfile_json = json.loads(lockfile_content.content)
-    actual_lockfile = CoursierResolvedLockfile.from_json_dict(lockfile_json)
-    assert actual_lockfile == CoursierResolvedLockfile(
-        entries=(
-            CoursierLockfileEntry(
-                coord=Coordinate(group="junit", artifact="junit", version="4.13.2"),
-                file_name="junit_junit_4.13.2.jar",
-                direct_dependencies=Coordinates([HAMCREST_COORD]),
-                dependencies=Coordinates([HAMCREST_COORD]),
-                file_digest=FileDigest(
-                    fingerprint="8e495b634469d64fb8acfa3495a065cbacc8a0fff55ce1e31007be4c16dc57d3",
-                    serialized_bytes_length=384581,
-                ),
-            ),
-            CoursierLockfileEntry(
-                coord=HAMCREST_COORD,
-                file_name="org.hamcrest_hamcrest-core_1.3.jar",
-                direct_dependencies=Coordinates([]),
-                dependencies=Coordinates([]),
-                file_digest=FileDigest(
-                    fingerprint="66fdef91e9739348df7a096aa384a5685f4e875584cce89386a7a47251c4d8e9",
-                    serialized_bytes_length=45024,
-                ),
-            ),
-        )
+    coordinates = rule_runner.request(
+        Coordinates, [GatherJvmCoordinatesRequest(lockfile_request.artifact_inputs, "")]
     )
+    assert sorted(coordinates, key=lambda c: (c.group, c.artifact, c.version)) == [
+        Coordinate(group="junit", artifact="junit", version="4.13.2"),
+        Coordinate(group="org.hamcrest", artifact="hamcrest-core", version="1.3"),
+    ]
 
 
 def test_determine_tool_sentinels_to_generate() -> None: