From 0ac1075789370a6e5275ca8e7de14dc85586e116 Mon Sep 17 00:00:00 2001 From: jgranstrom Date: Thu, 15 Oct 2026 14:42:18 +0200 Subject: [PATCH 1/2] perf(jvm): stop merging a jvm_artifact's transitive jars into its entry A `jvm_artifact`'s classpath entry was one digest merging its own jar with all of its transitive jars, so a jar that many artifacts depend on, such as scala-library, was on a compile's classpath once per artifact that pulled it in, each copy under a different path. Every copy is materialized on disk, and scalac indexes each path separately and keeps that index for the life of a nailgun JVM. This keeps those jars apart: an artifact's entry holds only its own jar and lists its transitive jars' entries, which were already fetched one per jar, in a new `exports` field. Targets without sources, `resources` and Java sources with inferred exports likewise export the entries they used to merge. `with_exports`, `closure`, `args` and `immutable_inputs(_args)` include each export right after its exporter and list each path once; `args` and `immutable_inputs(_args)` take `transitive=True` to cover the closure in one walk. Because entries are now shared by many classpaths, equality visits each pair of shared entries once, like `CoarsenedTarget`'s, and the hash is cached. On a large Scala codebase, 57% of compile classpath paths were duplicates; now none are. The scalac nailgun JVMs held about 60% less heap after compiling it, and across repeated compiles their jar index grew about 40% less. Immutable inputs took 0.6 GB of disk instead of 1.9 GB. Cold compiles were slightly faster, and warm runs, compile results and test results are unchanged. `deploy_jar` no longer receives a jar twice, which changes some `duplicate_policy` outcomes; the release notes cover them. --- docs/notes/2.34.x.md | 6 + .../pants/backend/java/compile/javac.py | 19 +- .../pants/backend/kotlin/compile/kotlinc.py | 5 +- .../backend/kotlin/compile/kotlinc_plugins.py | 2 +- .../backend/kotlin/compile/kotlinc_test.py | 54 ++- src/python/pants/backend/scala/bsp/rules.py | 5 +- .../pants/backend/scala/compile/scalac.py | 5 +- .../backend/scala/compile/scalac_plugins.py | 5 +- .../backend/scala/compile/scalac_test.py | 60 ++- src/python/pants/bsp/protocol_test.py | 21 +- src/python/pants/jvm/bsp/dependencies.py | 6 +- src/python/pants/jvm/classpath.py | 19 +- src/python/pants/jvm/compile.py | 145 ++++-- src/python/pants/jvm/compile_test.py | 425 +++++++++++++++++- .../pants/jvm/resolve/coursier_fetch.py | 9 +- src/python/pants/jvm/resources.py | 14 +- src/python/pants/jvm/resources_test.py | 20 + src/python/pants/jvm/testutil.py | 14 +- 18 files changed, 738 insertions(+), 96 deletions(-) diff --git a/docs/notes/2.34.x.md b/docs/notes/2.34.x.md index 255244b2324..67433a21954 100644 --- a/docs/notes/2.34.x.md +++ b/docs/notes/2.34.x.md @@ -80,6 +80,10 @@ Fixed `java` processes run against a Java 9+ JDK without nailgun (e.g. JUnit/Sca Fixed a bug where a single, unparametrized `protobuf_sources` target could not be depended on by both a `java_sources` and a `scala_sources` target at the same time: Pants would raise an ambiguity error when resolving the classpath entry and when hydrating the generated sources, since it couldn't tell which language's codegen a given dependent wanted. Both are now resolved per dependency edge, based on which language is actually asking, with no BUILD file changes required. +A `jvm_artifact`'s classpath entry no longer merges in its transitive jars, so a jar that several `jvm_artifact`s depend on, such as `scala-library`, appears once on a compile's classpath instead of once per artifact that pulled it in. This lowers `scalac`'s heap use on large classpaths. Some JVM compiles, `scalafix` runs, tests and `deploy_jar` builds miss the cache once after upgrading. + +`deploy_jar` no longer receives the same jar twice, which changes some `duplicate_policy` outcomes: `concat` and `concat_text` no longer repeat a jar's content, `replace` keeps the copy from the last distinct jar, and `throw` no longer fails on a jar duplicated only against itself. + #### Python With `[mypy].config_discovery` enabled (the default), each Python source file now uses the mypy config in its nearest ancestor directory, rather than only one at the build root, following [mypy's filename precedence](https://mypy.readthedocs.io/en/stable/config_file.html). Files with different configs are type-checked in separate mypy runs, and previously ignored nested configs may now change results. Behavior is unchanged when `[mypy].config` is set or discovery is disabled. @@ -161,6 +165,8 @@ Fixed a sandbox leak where Go module downloads left their extracted module cache `Field` subclasses now resolve `default`, `required` and `none_is_valid_value`, and whether `removal_version` is set, once per field type and cache the result. A field type that computes any of these dynamically, for example through a descriptor returning a different value on each access, will now only ever see the first value. +`ClasspathEntry` has a new `exports` field: a `jvm_artifact`'s entry now holds only its own jar and exports one entry per transitive jar, and entries that used to merge other entries into their `digest` export them instead. Code that reads `digest` or `filenames` directly misses exported jars, and `merge` lists the merged entries' exports after their own jars. + ## Full Changelog For the full changelog, see the individual GitHub Releases for this series: diff --git a/src/python/pants/backend/java/compile/javac.py b/src/python/pants/backend/java/compile/javac.py index 951de73fcd0..d11af2891d8 100644 --- a/src/python/pants/backend/java/compile/javac.py +++ b/src/python/pants/backend/java/compile/javac.py @@ -127,10 +127,7 @@ async def compile_java_source( ] if not component_members_and_java_source_files: # Is a generator, and so exports all of its direct deps. - exported_digest = await merge_digests( - MergeDigests(cpe.digest for cpe in direct_dependency_classpath_entries) - ) - classpath_entry = ClasspathEntry.merge(exported_digest, direct_dependency_classpath_entries) + classpath_entry = ClasspathEntry(EMPTY_DIGEST, exports=direct_dependency_classpath_entries) return FallibleClasspathEntry( description=str(request.component), result=CompileResult.SUCCEEDED, @@ -233,18 +230,12 @@ async def compile_java_source( **implicitly(StripJarRequest(digest=jar_output_digest, filenames=output_files)) ) output_classpath = ClasspathEntry( - jar_output_digest, output_files, direct_dependency_classpath_entries + jar_output_digest, + output_files, + direct_dependency_classpath_entries, + exports=export_classpath_entries, ) - if export_classpath_entries: - merged_export_digest = await merge_digests( - MergeDigests((output_classpath.digest, *(i.digest for i in export_classpath_entries))) - ) - merged_classpath = ClasspathEntry.merge( - merged_export_digest, (output_classpath, *export_classpath_entries) - ) - output_classpath = merged_classpath - return FallibleClasspathEntry.from_fallible_process_result( str(request.component), compile_result, diff --git a/src/python/pants/backend/kotlin/compile/kotlinc.py b/src/python/pants/backend/kotlin/compile/kotlinc.py index 93303393ceb..1f05c11eafd 100644 --- a/src/python/pants/backend/kotlin/compile/kotlinc.py +++ b/src/python/pants/backend/kotlin/compile/kotlinc.py @@ -110,10 +110,7 @@ async def compile_kotlin_source( if not component_members_and_kotlin_source_files: # Is a generator, and so exports all of its direct deps. - exported_digest = await merge_digests( - MergeDigests(cpe.digest for cpe in direct_dependency_classpath_entries) - ) - classpath_entry = ClasspathEntry.merge(exported_digest, direct_dependency_classpath_entries) + classpath_entry = ClasspathEntry(EMPTY_DIGEST, exports=direct_dependency_classpath_entries) return FallibleClasspathEntry( description=str(request.component), result=CompileResult.SUCCEEDED, diff --git a/src/python/pants/backend/kotlin/compile/kotlinc_plugins.py b/src/python/pants/backend/kotlin/compile/kotlinc_plugins.py index 387f2b7f964..3bafc87b99d 100644 --- a/src/python/pants/backend/kotlin/compile/kotlinc_plugins.py +++ b/src/python/pants/backend/kotlin/compile/kotlinc_plugins.py @@ -184,7 +184,7 @@ async def fetch_kotlinc_plugins(request: KotlincPluginsRequest) -> KotlincPlugin entries = list(ClasspathEntry.closure(artifacts)) merged_classpath_digest = await merge_digests(MergeDigests(entry.digest for entry in entries)) - merged = ClasspathEntry.merge(merged_classpath_digest, entries) + merged = ClasspathEntry(merged_classpath_digest, ClasspathEntry.args(entries)) ids = tuple(_plugin_id(target) for target in request.plugins) diff --git a/src/python/pants/backend/kotlin/compile/kotlinc_test.py b/src/python/pants/backend/kotlin/compile/kotlinc_test.py index 12842266f1e..4c6c72e5100 100644 --- a/src/python/pants/backend/kotlin/compile/kotlinc_test.py +++ b/src/python/pants/backend/kotlin/compile/kotlinc_test.py @@ -13,6 +13,7 @@ from pants.backend.kotlin.compile import kotlinc_plugins from pants.backend.kotlin.compile.kotlinc import CompileKotlinSourceRequest from pants.backend.kotlin.compile.kotlinc import rules as kotlinc_rules +from pants.backend.kotlin.compile.kotlinc_plugins import KotlincPlugins, KotlincPluginsRequest from pants.backend.kotlin.compile.testutil import _KOTLIN_VERSION, KOTLIN_STDLIB_REQUIREMENTS from pants.backend.kotlin.dependency_inference.rules import rules as kotlin_dep_inf_rules from pants.backend.kotlin.goals.check import KotlincCheckRequest @@ -23,7 +24,8 @@ from pants.core.goals.check import CheckResults from pants.core.util_rules import config_files, source_files, system_binaries from pants.engine.addresses import Addresses -from pants.engine.target import CoarsenedTargets +from pants.engine.fs import Digest, Snapshot +from pants.engine.target import CoarsenedTargets, Targets from pants.jvm import jdk_rules, testutil from pants.jvm.compile import ClasspathEntry, CompileResult, FallibleClasspathEntry from pants.jvm.resolve import jvm_tool @@ -63,6 +65,8 @@ def rule_runner() -> RuleRunner: QueryRule(FallibleClasspathEntry, (CompileKotlinSourceRequest,)), QueryRule(RenderedClasspath, (CompileKotlinSourceRequest,)), QueryRule(ClasspathEntry, (CompileKotlinSourceRequest,)), + QueryRule(KotlincPlugins, (KotlincPluginsRequest,)), + QueryRule(Snapshot, (Digest,)), ], target_types=[JvmArtifactTarget, KotlinSourcesGeneratorTarget, KotlincPluginTarget], ) @@ -455,3 +459,51 @@ class B: A() { fallible_result = rule_runner.request(FallibleClasspathEntry, [request]) print(f"stdout:\n{fallible_result.stdout}\nstderr:\n{fallible_result.stderr}") assert fallible_result.result == CompileResult.SUCCEEDED + + +@maybe_skip_jdk_test +def test_plugin_classpath_lists_a_jar_shared_by_two_plugins_once( + rule_runner: RuleRunner, kotlinc_allopen_lockfile: JVMLockfileFixture +) -> None: + rule_runner.write_files( + { + "lib/BUILD": dedent( + """\ + kotlinc_plugin( + name="reflect", + plugin_id="reflect", + artifact="3rdparty/jvm:org.jetbrains.kotlin_kotlin-reflect", + ) + + kotlinc_plugin( + name="stdlib", + plugin_id="stdlib", + artifact="3rdparty/jvm:org.jetbrains.kotlin_kotlin-stdlib", + ) + """ + ), + "3rdparty/jvm/BUILD": kotlinc_allopen_lockfile.requirements_as_jvm_artifact_targets(), + "3rdparty/jvm/default.lock": kotlinc_allopen_lockfile.serialized_lockfile, + } + ) + plugins = [ + rule_runner.get_target(Address("lib", target_name=name)) for name in ("reflect", "stdlib") + ] + artifacts = [ + rule_runner.get_target(Address("3rdparty/jvm", target_name=name)) + for name in ("org.jetbrains.kotlin_kotlin-reflect", "org.jetbrains.kotlin_kotlin-stdlib") + ] + + plugins_classpath = rule_runner.request( + KotlincPlugins, + [KotlincPluginsRequest(Targets(plugins), Targets(artifacts), make_resolve(rule_runner))], + ).classpath + + assert sorted(plugins_classpath.filenames) == [ + "org.jetbrains.kotlin_kotlin-reflect_1.6.20.jar", + "org.jetbrains.kotlin_kotlin-stdlib-common_1.6.20.jar", + "org.jetbrains.kotlin_kotlin-stdlib_1.6.20.jar", + "org.jetbrains_annotations_13.0.jar", + ] + snapshot = rule_runner.request(Snapshot, [plugins_classpath.digest]) + assert set(snapshot.files) == set(plugins_classpath.filenames) diff --git a/src/python/pants/backend/scala/bsp/rules.py b/src/python/pants/backend/scala/bsp/rules.py index 35ca9ccf807..09a75df76a6 100644 --- a/src/python/pants/backend/scala/bsp/rules.py +++ b/src/python/pants/backend/scala/bsp/rules.py @@ -78,7 +78,7 @@ from pants.jvm.bsp.resources import _jvm_bsp_resources from pants.jvm.bsp.resources import rules as jvm_resources_rules from pants.jvm.bsp.spec import JvmBuildTarget -from pants.jvm.compile import ClasspathEntryRequestFactory +from pants.jvm.compile import ClasspathEntry, ClasspathEntryRequestFactory from pants.jvm.jdk_rules import DefaultJdk, JdkRequest, prepare_jdk_environment from pants.jvm.resolve.common import ArtifactRequirements from pants.jvm.resolve.coursier_fetch import ( @@ -338,8 +338,7 @@ async def handle_bsp_scalac_options_request( build_root.pathlib_path.joinpath( f".pants.d/bsp/{thirdparty_modules_prefix}/{filename}" ).as_uri() - for cp_entry in thirdparty_modules.entries.values() - for filename in cp_entry.filenames + for filename in ClasspathEntry.args(thirdparty_modules.entries.values()) ) return HandleScalacOptionsResult( diff --git a/src/python/pants/backend/scala/compile/scalac.py b/src/python/pants/backend/scala/compile/scalac.py index ed630863201..c49690fe844 100644 --- a/src/python/pants/backend/scala/compile/scalac.py +++ b/src/python/pants/backend/scala/compile/scalac.py @@ -138,10 +138,7 @@ async def compile_scala_source( if not component_members_and_scala_source_files: # Is a generator, and so exports all of its direct deps. - exported_digest = await merge_digests( - MergeDigests(cpe.digest for cpe in direct_dependency_classpath_entries) - ) - classpath_entry = ClasspathEntry.merge(exported_digest, direct_dependency_classpath_entries) + classpath_entry = ClasspathEntry(EMPTY_DIGEST, exports=direct_dependency_classpath_entries) return FallibleClasspathEntry( description=str(request.component), result=CompileResult.SUCCEEDED, diff --git a/src/python/pants/backend/scala/compile/scalac_plugins.py b/src/python/pants/backend/scala/compile/scalac_plugins.py index 220cb5df6ba..2c3406f3e2f 100644 --- a/src/python/pants/backend/scala/compile/scalac_plugins.py +++ b/src/python/pants/backend/scala/compile/scalac_plugins.py @@ -239,8 +239,9 @@ async def fetch_plugins(request: ScalaPluginsRequest) -> ScalaPlugins: failed = [i for i in fallible_artifacts if i.exit_code != 0] raise Exception(f"Fetching local scala plugins failed: {failed}") - merged_classpath_digest = await merge_digests(MergeDigests(i.digest for i in artifacts)) - merged = ClasspathEntry.merge(merged_classpath_digest, artifacts) + entries = list(ClasspathEntry.closure(artifacts)) + merged_classpath_digest = await merge_digests(MergeDigests(entry.digest for entry in entries)) + merged = ClasspathEntry(merged_classpath_digest, ClasspathEntry.args(entries)) names = tuple(_plugin_name(target) for target in request.plugins) diff --git a/src/python/pants/backend/scala/compile/scalac_test.py b/src/python/pants/backend/scala/compile/scalac_test.py index f930f169f0f..7d6c050d516 100644 --- a/src/python/pants/backend/scala/compile/scalac_test.py +++ b/src/python/pants/backend/scala/compile/scalac_test.py @@ -14,6 +14,7 @@ ) from pants.backend.scala.compile.scalac import CompileScalaSourceRequest from pants.backend.scala.compile.scalac import rules as scalac_rules +from pants.backend.scala.compile.scalac_plugins import ScalaPlugins, ScalaPluginsRequest from pants.backend.scala.dependency_inference.rules import rules as scala_dep_inf_rules from pants.backend.scala.goals.check import ScalacCheckRequest from pants.backend.scala.goals.check import rules as scalac_check_rules @@ -28,9 +29,10 @@ from pants.core.goals.check import CheckResults from pants.core.util_rules import source_files, stripped_source_files, system_binaries from pants.engine.addresses import Addresses +from pants.engine.fs import Digest, Snapshot from pants.engine.internals.parametrize import Parametrize from pants.engine.internals.scheduler import ExecutionError -from pants.engine.target import CoarsenedTargets +from pants.engine.target import CoarsenedTargets, Targets from pants.jvm import jdk_rules, testutil from pants.jvm.compile import ClasspathEntry, CompileResult, FallibleClasspathEntry from pants.jvm.resolve.coursier_fetch import rules as coursier_fetch_rules @@ -68,6 +70,8 @@ def rule_runner() -> RuleRunner: QueryRule(FallibleClasspathEntry, (CompileScalaSourceRequest,)), QueryRule(RenderedClasspath, (CompileScalaSourceRequest,)), QueryRule(ClasspathEntry, (CompileScalaSourceRequest,)), + QueryRule(ScalaPlugins, (ScalaPluginsRequest,)), + QueryRule(Snapshot, (Digest,)), ], target_types=[ JvmArtifactTarget, @@ -730,6 +734,60 @@ def example: Option[String] = { rule_runner.request(RenderedClasspath, [request]) +@maybe_skip_jdk_test +def test_plugin_classpath_includes_each_transitive_jar_once( + rule_runner: RuleRunner, multiple_scala_plugins_jvm_lockfile: JVMLockfileFixture +) -> None: + rule_runner.write_files( + { + "lib/BUILD": dedent( + """\ + scalac_plugin( + name="kind-projector", + plugin_name="kind-projector", + artifact="3rdparty/jvm:org.typelevel_kind-projector_2.13.8", + ) + + scalac_plugin( + name="better-monadic-for", + plugin_name="bm4", + artifact="3rdparty/jvm:com.olegpy_better-monadic-for_2.13", + ) + """ + ), + "3rdparty/jvm/BUILD": multiple_scala_plugins_jvm_lockfile.requirements_as_jvm_artifact_targets(), + "3rdparty/jvm/default.lock": multiple_scala_plugins_jvm_lockfile.serialized_lockfile, + } + ) + plugins = [ + rule_runner.get_target(Address("lib", target_name=name)) + for name in ("kind-projector", "better-monadic-for") + ] + artifacts = [ + rule_runner.get_target(Address("3rdparty/jvm", target_name=name)) + for name in ("org.typelevel_kind-projector_2.13.8", "com.olegpy_better-monadic-for_2.13") + ] + + scala_plugins = rule_runner.request( + ScalaPlugins, + [ScalaPluginsRequest(Targets(plugins), Targets(artifacts), make_resolve(rule_runner))], + ) + + filenames = scala_plugins.classpath.filenames + assert len(filenames) == len(set(filenames)) + assert set(filenames) == { + "org.typelevel_kind-projector_2.13.8_0.13.2.jar", + "com.olegpy_better-monadic-for_2.13_0.3.1.jar", + "net.java.dev.jna_jna_5.9.0.jar", + "org.jline_jline_3.21.0.jar", + "org.scala-lang_scala-compiler_2.13.8.jar", + "org.scala-lang_scala-library_2.13.8.jar", + "org.scala-lang_scala-reflect_2.13.8.jar", + } + snapshot = rule_runner.request(Snapshot, [scala_plugins.classpath.digest]) + assert set(snapshot.files) == set(filenames) + + @pytest.fixture def scala_2_12_lockfile_def() -> JVMLockfileFixtureDefinition: return JVMLockfileFixtureDefinition( diff --git a/src/python/pants/bsp/protocol_test.py b/src/python/pants/bsp/protocol_test.py index 363be45ee1a..0fdf443d835 100644 --- a/src/python/pants/bsp/protocol_test.py +++ b/src/python/pants/bsp/protocol_test.py @@ -33,6 +33,7 @@ ) from pants.bsp.spec.resources import ResourcesParams from pants.bsp.spec.targets import ( + DependencyModulesParams, DependencySourcesParams, SourcesParams, SourcesResult, @@ -262,10 +263,28 @@ class ExampleLibSpec extends AnyFunSpec { "buildTarget/scalacOptions", ScalacOptionsParams(target_ids).to_json_dict() ).result(timeout=timeout) ) - assert scalac_options.items[0].classpath + classpath = [Path(urlparse(uri).path) for uri in scalac_options.items[0].classpath] + assert len(classpath) == len(set(classpath)) + assert all(jar.is_file() for jar in classpath) + assert any(jar.name.startswith("org.scalactic_scalactic_2.13") for jar in classpath) class_directory = Path(urlparse(scalac_options.items[0].class_directory).path) assert not class_directory.exists() + # buildTarget/dependencyModules + dependency_modules = endpoint.request( + "buildTarget/dependencyModules", + DependencyModulesParams(target_ids).to_json_dict(), + ).result(timeout=timeout) + (dependency_modules_item,) = dependency_modules["items"] + modules = {module["name"]: module for module in dependency_modules_item["modules"]} + scalatest_jars = [ + Path(urlparse(artifact["uri"]).path) + for artifact in modules["org.scalatest:scalatest_2.13"]["data"]["artifacts"] + ] + assert scalatest_jars[0].name == "org.scalatest_scalatest_2.13_3.2.10.jar" + assert len(scalatest_jars) == len(set(scalatest_jars)) + assert set(scalatest_jars) <= set(classpath) + # buildTarget/compile compile_result = CompileResult.from_json_dict( endpoint.request( diff --git a/src/python/pants/jvm/bsp/dependencies.py b/src/python/pants/jvm/bsp/dependencies.py index d77332ee27c..6206c0b3e67 100644 --- a/src/python/pants/jvm/bsp/dependencies.py +++ b/src/python/pants/jvm/bsp/dependencies.py @@ -152,7 +152,9 @@ async def collect_thirdparty_modules( required_classfiles(fce) for fce in fallible_classpath_entries ) - resolve_digest = await merge_digests(MergeDigests(cpe.digest for cpe in classpath_entries)) + resolve_digest = await merge_digests( + MergeDigests(cpe.digest for cpe in ClasspathEntry.with_exports(classpath_entries)) + ) return ThirdpartyModules( resolve, @@ -407,7 +409,7 @@ async def _jvm_bsp_dependency_modules( f".pants.d/bsp/jvm/resolves/{resolve.name}/lib/{filename}" ).as_uri() ) - for filename in cp_entry.filenames + for filename in ClasspathEntry.args([cp_entry]) ), ), ) diff --git a/src/python/pants/jvm/classpath.py b/src/python/pants/jvm/classpath.py index 57fd4b75e8a..a76d35c9374 100644 --- a/src/python/pants/jvm/classpath.py +++ b/src/python/pants/jvm/classpath.py @@ -4,6 +4,7 @@ from __future__ import annotations import logging +import os from collections.abc import Iterator from dataclasses import dataclass @@ -49,7 +50,7 @@ class Classpath: def args(self, *, prefix: str = "") -> Iterator[str]: """All transitive filenames for this Classpath.""" - return ClasspathEntry.args(ClasspathEntry.closure(self.entries), prefix=prefix) + return ClasspathEntry.args(self.entries, prefix=prefix, transitive=True) def root_args(self, *, prefix: str = "") -> Iterator[str]: """The root filenames for this Classpath.""" @@ -61,13 +62,11 @@ def digests(self) -> Iterator[Digest]: def immutable_inputs(self, *, prefix: str = "") -> Iterator[tuple[str, Digest]]: """Returns (relpath, Digest) tuples for use with `Process.immutable_input_digests`.""" - return ClasspathEntry.immutable_inputs(ClasspathEntry.closure(self.entries), prefix=prefix) + return ClasspathEntry.immutable_inputs(self.entries, prefix=prefix, transitive=True) def immutable_inputs_args(self, *, prefix: str = "") -> Iterator[str]: """Returns relative filenames for the given entries to be used as immutable_inputs.""" - return ClasspathEntry.immutable_inputs_args( - ClasspathEntry.closure(self.entries), prefix=prefix - ) + return ClasspathEntry.immutable_inputs_args(self.entries, prefix=prefix, transitive=True) def root_immutable_inputs(self, *, prefix: str = "") -> Iterator[tuple[str, Digest]]: """Returns root (relpath, Digest) tuples for use with `Process.immutable_input_digests`.""" @@ -123,6 +122,7 @@ async def loose_classfiles( classpath_entry: ClasspathEntry, unzip_binary: UnzipBinary ) -> LooseClassfiles: dest_dir = "dest" + jar_dir = "jar" process_results = await concurrently( execute_process_or_raise( **implicitly( @@ -131,18 +131,17 @@ async def loose_classfiles( unzip_binary.path, "-d", dest_dir, - filename, + os.path.join(jar_dir, filename), ], output_directories=(dest_dir,), description=f"Extract {filename}", - immutable_input_digests=dict( - ClasspathEntry.immutable_inputs([classpath_entry]) - ), + immutable_input_digests={jar_dir: cpe.digest}, level=LogLevel.TRACE, ) ) ) - for filename in ClasspathEntry.immutable_inputs_args([classpath_entry]) + for cpe in ClasspathEntry.with_exports([classpath_entry]) + for filename in cpe.filenames ) merged_digest = await merge_digests(MergeDigests(pr.output_digest for pr in process_results)) diff --git a/src/python/pants/jvm/compile.py b/src/python/pants/jvm/compile.py index c47f6ffe8e5..0029796f8d0 100644 --- a/src/python/pants/jvm/compile.py +++ b/src/python/pants/jvm/compile.py @@ -10,7 +10,8 @@ from collections.abc import Iterable, Iterator, Sequence from dataclasses import dataclass from enum import Enum, auto -from typing import ClassVar +from functools import cached_property +from typing import Any, ClassVar from pants.core.target_types import ( FilesGeneratingSourcesField, @@ -260,9 +261,10 @@ async def calculate_jvm_request_types( class ClasspathEntry: """A JVM classpath entry represented as a series of JAR files, and their dependencies. - This is a series of JAR files in order to account for "exported" dependencies, when a node - and some of its dependencies are indistinguishable (such as for aliases, or potentially - explicitly declared or inferred `exported=` lists in the future). + `exports` are entries that are part of this one, when a node and some of its dependencies are + indistinguishable (such as a target without sources, or a `jvm_artifact` and its transitive + jars). `digest` and `filenames` exclude them; `with_exports`, `args`, `immutable_inputs(_args)` + and `closure` include them. This class additionally keeps filenames in order to preserve classpath ordering for the `classpath_arg` method: although Digests encode filenames, they are stored sorted. @@ -281,76 +283,163 @@ class ClasspathEntry: digest: Digest filenames: tuple[str, ...] dependencies: FrozenOrderedSet[ClasspathEntry] + exports: FrozenOrderedSet[ClasspathEntry] def __init__( self, digest: Digest, filenames: Iterable[str] = (), dependencies: Iterable[ClasspathEntry] = (), + exports: Iterable[ClasspathEntry] = (), ): object.__setattr__(self, "digest", digest) object.__setattr__(self, "filenames", tuple(filenames)) object.__setattr__(self, "dependencies", FrozenOrderedSet(dependencies)) + object.__setattr__(self, "exports", FrozenOrderedSet(exports)) @classmethod def merge(cls, digest: Digest, entries: Iterable[ClasspathEntry]) -> ClasspathEntry: - """After merging the Digests for entries, merge their filenames and dependencies.""" + """After merging the Digests for entries, merge their filenames, dependencies and exports.""" + entries = tuple(entries) return cls( digest, (f for cpe in entries for f in cpe.filenames), (d for cpe in entries for d in cpe.dependencies), + (e for cpe in entries for e in cpe.exports), ) @classmethod - def args(cls, entries: Iterable[ClasspathEntry], *, prefix: str = "") -> Iterator[str]: - """Returns the filenames for the given entries. + def args( + cls, entries: Iterable[ClasspathEntry], *, prefix: str = "", transitive: bool = False + ) -> Iterator[str]: + """Returns the filenames for the given entries and their exports. TODO: See whether this method can be completely eliminated in favor of `immutable_inputs(_args)`. - To compute transitive filenames, first expand the entries with `cls.closure()`. + To compute transitive filenames, pass `transitive=True`. """ - return (os.path.join(prefix, f) for cpe in entries for f in cpe.filenames) + return iter( + FrozenOrderedSet( + os.path.join(prefix, f) + for cpe in cls._walk(entries, follow_dependencies=transitive) + for f in cpe.filenames + ) + ) @classmethod def immutable_inputs( - cls, entries: Iterable[ClasspathEntry], *, prefix: str = "" + cls, entries: Iterable[ClasspathEntry], *, prefix: str = "", transitive: bool = False ) -> Iterator[tuple[str, Digest]]: """Returns (relpath, Digest) tuples for use with `Process.immutable_input_digests`. - To compute transitive input tuples, first expand the entries with `cls.closure()`. + Includes the entries' exports. To compute transitive input tuples, pass `transitive=True`. """ - return ((os.path.join(prefix, cpe.digest.fingerprint[:12]), cpe.digest) for cpe in entries) + return iter( + FrozenOrderedSet( + (os.path.join(prefix, cpe._immutable_input_dir), cpe.digest) + for cpe in cls._walk(entries, follow_dependencies=transitive) + ) + ) @classmethod def immutable_inputs_args( - cls, entries: Iterable[ClasspathEntry], *, prefix: str = "" + cls, entries: Iterable[ClasspathEntry], *, prefix: str = "", transitive: bool = False ) -> Iterator[str]: """Returns the relative filenames for the given entries to be used as immutable_inputs. - To compute transitive input tuples, first expand the entries with `cls.closure()`. + Includes the entries' exports. To compute transitive filenames, pass `transitive=True`. """ - for cpe in entries: - fingerprint_prefix = cpe.digest.fingerprint[:12] - for filename in cpe.filenames: - yield os.path.join(prefix, fingerprint_prefix, filename) + return iter( + FrozenOrderedSet( + os.path.join(prefix, cpe._immutable_input_dir, filename) + for cpe in cls._walk(entries, follow_dependencies=transitive) + for filename in cpe.filenames + ) + ) @classmethod - def closure(cls, roots: Iterable[ClasspathEntry]) -> Iterator[ClasspathEntry]: - """All ClasspathEntries reachable from the given roots.""" + def with_exports(cls, entries: Iterable[ClasspathEntry]) -> Iterator[ClasspathEntry]: + """The given entries, each followed by its transitive exports.""" + return cls._walk(entries, follow_dependencies=False) - visited = set() + @classmethod + def closure(cls, roots: Iterable[ClasspathEntry]) -> Iterator[ClasspathEntry]: + """All ClasspathEntries reachable from the given roots, including exports.""" + return cls._walk(roots, follow_dependencies=True) + + @staticmethod + def _walk( + roots: Iterable[ClasspathEntry], *, follow_dependencies: bool + ) -> Iterator[ClasspathEntry]: + visited: set[ClasspathEntry] = set() queue = deque(roots) while queue: - ct = queue.popleft() - if ct in visited: - continue - visited.add(ct) - yield ct - queue.extend(ct.dependencies) + stack = [queue.popleft()] + while stack: + cpe = stack.pop() + if cpe in visited: + continue + visited.add(cpe) + yield cpe + stack.extend(reversed(cpe.exports)) + if follow_dependencies: + queue.extend(cpe.dependencies) + + @cached_property + def _immutable_input_dir(self) -> str: + return self.digest.fingerprint[:12] + + @cached_property + def _hashcode(self) -> int: + return hash((self.digest, self.filenames, self.dependencies, self.exports)) + + def _eq_helper(self, other: ClasspathEntry, equal_items: set[tuple[int, int]]) -> bool: + key = (id(self), id(other)) + if key[0] == key[1] or key in equal_items: + return True + + is_eq = ( + self.__class__ is other.__class__ + and self._hashcode == other._hashcode + and self.digest == other.digest + and self.filenames == other.filenames + and len(self.dependencies) == len(other.dependencies) + and len(self.exports) == len(other.exports) + and all( + self._child_eq(l, r, equal_items) + for l, r in zip(self.dependencies, other.dependencies) + ) + and all(self._child_eq(l, r, equal_items) for l, r in zip(self.exports, other.exports)) + ) + + # NB: We only track equal items because any non-equal item will cause the entire + # operation to shortcircuit. + if is_eq: + equal_items.add(key) + return is_eq + + @staticmethod + def _child_eq( + left: ClasspathEntry, right: ClasspathEntry, equal_items: set[tuple[int, int]] + ) -> bool: + if type(left) is type(right) and type(left).__eq__ is ClasspathEntry.__eq__: + return left._eq_helper(right, equal_items) + return left == right + + def __eq__(self, other: Any) -> bool: + if other.__class__ is not self.__class__: + return NotImplemented + return self._eq_helper(other, set()) + + def __hash__(self) -> int: + return self._hashcode def __repr__(self): - return f"ClasspathEntry({self.filenames}, dependencies={len(self.dependencies)})" + return ( + f"ClasspathEntry({self.filenames}, dependencies={len(self.dependencies)}, " + f"exports={len(self.exports)})" + ) def __str__(self) -> str: return repr(self) diff --git a/src/python/pants/jvm/compile_test.py b/src/python/pants/jvm/compile_test.py index 3e4ca22ee1a..5ef650cda30 100644 --- a/src/python/pants/jvm/compile_test.py +++ b/src/python/pants/jvm/compile_test.py @@ -11,10 +11,14 @@ from __future__ import annotations +import hashlib +import os from collections import defaultdict -from collections.abc import Sequence +from collections.abc import Iterable, Sequence +from dataclasses import dataclass from textwrap import dedent -from typing import cast +from typing import Any, cast +from unittest.mock import Mock import chevron import pytest @@ -50,8 +54,10 @@ from pants.core.target_types import FilesGeneratorTarget, RelocatedFiles from pants.core.util_rules import config_files, source_files, stripped_source_files from pants.core.util_rules.external_tool import rules as external_tool_rules +from pants.core.util_rules.system_binaries import UnzipBinary from pants.engine.addresses import Addresses -from pants.engine.fs import EMPTY_DIGEST +from pants.engine.fs import EMPTY_DIGEST, Digest +from pants.engine.process import Process, ProcessResult from pants.engine.target import ( CoarsenedTarget, GeneratedSources, @@ -64,6 +70,7 @@ from pants.jvm import classpath, jdk_rules, testutil from pants.jvm.classpath import Classpath from pants.jvm.compile import ( + ClasspathEntry, ClasspathEntryRequest, ClasspathEntryRequestFactory, ClasspathSourceAmbiguity, @@ -79,11 +86,18 @@ from pants.jvm.testutil import ( RenderedClasspath, expect_single_expanded_coarsened_target, + make_resolve, maybe_skip_jdk_test, ) from pants.jvm.util_rules import rules as util_rules -from pants.testutil.rule_runner import PYTHON_BOOTSTRAP_ENV, QueryRule, RuleRunner +from pants.testutil.rule_runner import ( + PYTHON_BOOTSTRAP_ENV, + QueryRule, + RuleRunner, + run_rule_with_mocks, +) from pants.util.frozendict import FrozenDict +from pants.util.ordered_set import FrozenOrderedSet @pytest.fixture @@ -101,6 +115,18 @@ def scala_stdlib_jvm_lockfile( return scala_stdlib_jvm_lockfile_def.load(request) +@pytest.fixture +def antlr_jvm_lockfile_def() -> JVMLockfileFixtureDefinition: + return JVMLockfileFixtureDefinition("antlr.test.lock", ["org.antlr:antlr4:4.11.1"]) + + +@pytest.fixture +def antlr_jvm_lockfile( + antlr_jvm_lockfile_def: JVMLockfileFixtureDefinition, request +) -> JVMLockfileFixture: + return antlr_jvm_lockfile_def.load(request) + + @pytest.fixture def rule_runner() -> RuleRunner: rule_runner = RuleRunner( @@ -521,3 +547,394 @@ def main(args: Array[String]): Unit = { main_address = Address(spec_path="", target_name="main") rule_runner.request(Classpath, [Addresses([main_address])]) + + +def _jar( + name: str, + *, + dependencies: Iterable[ClasspathEntry] = (), + exports: Iterable[ClasspathEntry] = (), +) -> ClasspathEntry: + return ClasspathEntry( + Digest(hashlib.sha256(name.encode()).hexdigest(), len(name)), + (f"{name}.jar",), + dependencies=dependencies, + exports=exports, + ) + + +def test_exports_follow_their_exporter_once() -> None: + scala_library = _jar("scala-library") + zio = _jar("zio", exports=[scala_library]) + zio_json = _jar("zio-json", exports=[scala_library]) + app = _jar("app", dependencies=[zio, zio_json]) + + assert list(ClasspathEntry.closure([app])) == [app, zio, scala_library, zio_json] + assert list(ClasspathEntry.args([zio, zio_json])) == [ + "zio.jar", + "scala-library.jar", + "zio-json.jar", + ] + assert list(ClasspathEntry.immutable_inputs_args([zio, zio_json], prefix="__cp")) == [ + f"__cp/{cpe.digest.fingerprint[:12]}/{cpe.filenames[0]}" + for cpe in (zio, scala_library, zio_json) + ] + assert list(ClasspathEntry.immutable_inputs([zio, zio_json], prefix="__cp")) == [ + (f"__cp/{cpe.digest.fingerprint[:12]}", cpe.digest) + for cpe in (zio, scala_library, zio_json) + ] + + +def test_exports_are_transitive_and_their_dependencies_are_not_root() -> None: + runtime_dep = _jar("runtime-dep") + core = _jar("core", dependencies=[runtime_dep]) + api = _jar("api", exports=[core]) + bundle = _jar("bundle", exports=[api]) + user = _jar("user", dependencies=[bundle]) + + assert list(ClasspathEntry.with_exports([bundle])) == [bundle, api, core] + assert list(ClasspathEntry.closure([user])) == [user, bundle, api, core, runtime_dep] + + +def test_a_jar_that_is_both_an_artifact_and_an_export_is_listed_once() -> None: + scala_library = _jar("scala-library") + zio = _jar("zio", exports=[scala_library]) + zio_json = _jar("zio-json", exports=[_jar("zio"), scala_library]) + + assert list(ClasspathEntry.args([zio, zio_json])) == [ + "zio.jar", + "scala-library.jar", + "zio-json.jar", + ] + assert list(ClasspathEntry.immutable_inputs_args([zio, zio_json])) == [ + f"{cpe.digest.fingerprint[:12]}/{cpe.filenames[0]}" + for cpe in (zio, scala_library, zio_json) + ] + assert list(ClasspathEntry.immutable_inputs([zio, zio_json])) == [ + (cpe.digest.fingerprint[:12], cpe.digest) for cpe in (zio, scala_library, zio_json) + ] + + +def test_transitive_covers_the_closure() -> None: + scala_library = _jar("scala-library") + zio = _jar("zio", exports=[scala_library]) + lib = _jar("lib", dependencies=[zio], exports=[_jar("lib-api")]) + app = _jar("app", dependencies=[lib, zio]) + closure = list(ClasspathEntry.closure([app])) + + assert list(ClasspathEntry.args([app], transitive=True)) == list(ClasspathEntry.args(closure)) + assert list(ClasspathEntry.immutable_inputs([app], prefix="__cp", transitive=True)) == list( + ClasspathEntry.immutable_inputs(closure, prefix="__cp") + ) + assert list( + ClasspathEntry.immutable_inputs_args([app], prefix="__cp", transitive=True) + ) == list(ClasspathEntry.immutable_inputs_args(closure, prefix="__cp")) + assert list(ClasspathEntry.args([app])) == ["app.jar"] + + +def test_merge_keeps_exports() -> None: + scala_library = _jar("scala-library") + zio = _jar("zio", exports=[scala_library]) + app = _jar("app", dependencies=[zio]) + + merged = ClasspathEntry.merge(EMPTY_DIGEST, [app, zio]) + + assert merged.filenames == ("app.jar", "zio.jar") + assert merged.dependencies == FrozenOrderedSet([zio]) + assert merged.exports == FrozenOrderedSet([scala_library]) + + +def test_classpath_entry_equality() -> None: + assert _jar("a") == _jar("a") + assert _jar("a") != _jar("b") + + assert _jar("app", dependencies=[_jar("a")]) == _jar("app", dependencies=[_jar("a")]) + assert _jar("app", dependencies=[_jar("a")]) != _jar("app", dependencies=[_jar("b")]) + assert _jar("app", exports=[_jar("a")]) != _jar("app", dependencies=[_jar("a")]) + assert _jar("app", dependencies=[_jar("a"), _jar("b")]) != _jar( + "app", dependencies=[_jar("b"), _jar("a")] + ) + + class Subclass(ClasspathEntry): + pass + + assert Subclass(EMPTY_DIGEST) != ClasspathEntry(EMPTY_DIGEST) + assert ClasspathEntry(EMPTY_DIGEST) != Subclass(EMPTY_DIGEST) + assert _jar("app", dependencies=[Subclass(EMPTY_DIGEST)]) == _jar( + "app", dependencies=[Subclass(EMPTY_DIGEST)] + ) + assert _jar("app", dependencies=[ClasspathEntry(EMPTY_DIGEST)]) != _jar( + "app", dependencies=[Subclass(EMPTY_DIGEST)] + ) + + @dataclass(frozen=True) + class WithExtra(ClasspathEntry): + extra: int + + def with_extra(extra: int) -> WithExtra: + return WithExtra(EMPTY_DIGEST, (), FrozenOrderedSet(), FrozenOrderedSet(), extra) + + assert hash(with_extra(-1)) == hash(with_extra(-2)) + assert with_extra(-1) == with_extra(-1) + assert with_extra(-1) != with_extra(-2) + assert _jar("app", dependencies=[with_extra(-1)]) == _jar("app", dependencies=[with_extra(-1)]) + assert _jar("app", dependencies=[with_extra(-1)]) != _jar("app", dependencies=[with_extra(-2)]) + + @dataclass(frozen=True, eq=False) + class Validates(ClasspathEntry): + extra: int + + def __post_init__(self) -> None: + if self.extra < 0: + raise ValueError(self.extra) + + def validates(extra: int) -> Validates: + return Validates(EMPTY_DIGEST, (), FrozenOrderedSet(), FrozenOrderedSet(), extra) + + assert validates(1) in {validates(1)} + assert _jar("app", dependencies=[validates(1)]) == _jar("app", dependencies=[validates(1)]) + + class SameDigest(ClasspathEntry): + def __eq__(self, other: Any) -> bool: + return isinstance(other, SameDigest) and self.digest == other.digest + + def __hash__(self) -> int: + return hash(self.digest) + + assert _jar("app", dependencies=[SameDigest(EMPTY_DIGEST, ["a.jar"])]) == _jar( + "app", dependencies=[SameDigest(EMPTY_DIGEST, ["b.jar"])] + ) + + class EqualsBase(ClasspathEntry): + def __eq__(self, other: Any) -> bool: + if not isinstance(other, ClasspathEntry): + return NotImplemented + fields = ("digest", "filenames", "dependencies", "exports") + return all(getattr(self, f) == getattr(other, f) for f in fields) + + __hash__ = ClasspathEntry.__hash__ + + base_child, sub_child = _jar("a"), EqualsBase(_jar("a").digest, ["a.jar"]) + assert _jar("app", dependencies=[base_child]) == _jar("app", dependencies=[sub_child]) + assert _jar("app", dependencies=[sub_child]) == _jar("app", dependencies=[base_child]) + + def nested(): + shared = _jar("shared") + lib = _jar("lib", dependencies=[shared], exports=[shared]) + return _jar("app", dependencies=[lib, shared], exports=[lib]) + + a, b = nested(), nested() + assert a is not b + assert a == b + assert hash(a) == hash(b) + + +def test_classpath_entry_eq_is_linear_in_nodes_not_paths() -> None: + def fan_out(width: int, depth: int, first_leaf: str = "leaf0") -> ClasspathEntry: + level = tuple(_jar(first_leaf if i == 0 else f"leaf{i}") for i in range(width)) + for d in range(depth): + level = tuple( + _jar(f"n{d}_{i}", dependencies=level, exports=level[:1]) for i in range(width) + ) + return _jar("root", dependencies=level) + + def all_nodes(root: ClasspathEntry) -> dict[int, ClasspathEntry]: + nodes: dict[int, ClasspathEntry] = {} + stack = [root] + while stack: + node = stack.pop() + if id(node) not in nodes: + nodes[id(node)] = node + stack.extend(node.dependencies) + stack.extend(node.exports) + return nodes + + calls = 0 + original_helper = ClasspathEntry._eq_helper + + def counting( + self: ClasspathEntry, other: ClasspathEntry, equal_items: set[tuple[int, int]] + ) -> bool: + nonlocal calls + calls += 1 + return original_helper(self, other, equal_items) + + ClasspathEntry._eq_helper = counting # type: ignore[method-assign] + try: + a, b = fan_out(6, 6), fan_out(6, 6) + assert a == b + nodes = all_nodes(a).values() + nodes_and_edges = len(nodes) + sum(len(n.dependencies) + len(n.exports) for n in nodes) + assert calls <= nodes_and_edges + + c = fan_out(6, 6, first_leaf="different") + for node in [*all_nodes(a).values(), *all_nodes(c).values()]: + object.__setattr__(node, "_hashcode", 0) + assert a != c + finally: + ClasspathEntry._eq_helper = original_helper # type: ignore[method-assign] + + +def test_loose_classfiles_extracts_each_jar_from_its_own_digest() -> None: + scala_library = _jar("scala-library") + zio = _jar("zio", exports=[scala_library]) + zio_json = _jar("zio-json", exports=[scala_library]) + lib = _jar("lib", exports=[zio, zio_json]) + extracted: list[tuple[Digest, str]] = [] + + def extract(process: Process) -> ProcessResult: + ((jar_dir, digest),) = process.immutable_input_digests.items() + extracted.append((digest, os.path.relpath(process.argv[-1], jar_dir))) + return Mock(output_digest=EMPTY_DIGEST) + + run_rule_with_mocks( + classpath.loose_classfiles, + rule_args=[lib, UnzipBinary("/usr/bin/unzip")], + mock_calls={ + "pants.engine.process.fallible_to_exec_result_or_raise": extract, + "pants.engine.intrinsics.merge_digests": lambda _: EMPTY_DIGEST, + "pants.engine.intrinsics.remove_prefix": lambda _: EMPTY_DIGEST, + }, + ) + + assert extracted == [ + (cpe.digest, cpe.filenames[0]) for cpe in (lib, zio, scala_library, zio_json) + ] + + +@maybe_skip_jdk_test +def test_jvm_artifact_exports_each_transitive_jar_as_its_own_entry( + rule_runner: RuleRunner, antlr_jvm_lockfile: JVMLockfileFixture +) -> None: + rule_runner.write_files( + { + "3rdparty/jvm/BUILD": antlr_jvm_lockfile.requirements_as_jvm_artifact_targets(), + "3rdparty/jvm/default.lock": antlr_jvm_lockfile.serialized_lockfile, + } + ) + classpath = rule_runner.request( + Classpath, + [Addresses([Address(spec_path="3rdparty/jvm", target_name="org.antlr_antlr4")])], + ) + + (antlr4,) = classpath.entries + assert antlr4.filenames == ("org.antlr_antlr4_4.11.1.jar",) + (antlr4_requirement,) = antlr_jvm_lockfile.requirements + _, antlr4_dependencies = antlr_jvm_lockfile.lockfile.dependencies( + make_resolve(rule_runner), antlr4_requirement.coordinate + ) + assert [cpe.filenames for cpe in antlr4.exports] == [ + (dependency.file_name,) for dependency in antlr4_dependencies + ] + assert list(classpath.immutable_inputs_args(prefix="__cp")) == [ + f"__cp/{cpe.digest.fingerprint[:12]}/{cpe.filenames[0]}" + for cpe in (antlr4, *antlr4.exports) + ] + + +@maybe_skip_jdk_test +def test_javac_sees_jars_exported_by_a_direct_dependency( + rule_runner: RuleRunner, antlr_jvm_lockfile: JVMLockfileFixture +) -> None: + rule_runner.write_files( + { + "3rdparty/jvm/BUILD": antlr_jvm_lockfile.requirements_as_jvm_artifact_targets(), + "3rdparty/jvm/default.lock": antlr_jvm_lockfile.serialized_lockfile, + "BUILD": "java_sources(name='lib', dependencies=['3rdparty/jvm:org.antlr_antlr4'])", + "Token.java": dedent( + """\ + package org.pantsbuild.example; + + import org.antlr.runtime.CommonToken; + + public class Token { + public static Object make() { + return new CommonToken(0); + } + } + """ + ), + } + ) + + rendered_classpath = rule_runner.request( + RenderedClasspath, [Addresses([Address(spec_path="", target_name="lib")])] + ) + + assert rendered_classpath.content[".Token.java.lib.javac.jar"] == { + "org/pantsbuild/example/Token.class" + } + + +@maybe_skip_jdk_test +def test_javac_exports_keep_each_exported_artifacts_jars_after_it( + rule_runner: RuleRunner, antlr_jvm_lockfile: JVMLockfileFixture +) -> None: + rule_runner.write_files( + { + "3rdparty/jvm/BUILD": antlr_jvm_lockfile.requirements_as_jvm_artifact_targets(), + "3rdparty/jvm/default.lock": antlr_jvm_lockfile.serialized_lockfile, + "other/BUILD": "java_sources()", + "other/Other.java": dedent( + """\ + package org.pantsbuild.other; + + public class Other {} + """ + ), + "lib/BUILD": "java_sources(dependencies=['3rdparty/jvm:org.antlr_antlr4'])", + "lib/Lib.java": dedent( + """\ + package org.pantsbuild.lib; + + import org.antlr.runtime.CommonToken; + import org.pantsbuild.other.Other; + + public class Lib { + public static CommonToken token() { + return new CommonToken(0); + } + + public static Other other() { + return new Other(); + } + } + """ + ), + "app/BUILD": "java_sources()", + "app/App.java": dedent( + """\ + package org.pantsbuild.app; + + import org.pantsbuild.lib.Lib; + + public class App { + public static String describe() { + return Lib.token().getText() + Lib.other(); + } + } + """ + ), + } + ) + + lib_classpath = rule_runner.request( + Classpath, [Addresses([Address("lib", relative_file_path="Lib.java")])] + ) + lib_jar, antlr4_jar, *antlr4_exports, other_jar = lib_classpath.root_args() + assert lib_jar.endswith(".javac.jar") and "Lib.java" in lib_jar + assert antlr4_jar == "org.antlr_antlr4_4.11.1.jar" + assert sorted(antlr4_exports) == [ + "com.ibm.icu_icu4j_71.1.jar", + "org.abego.treelayout_org.abego.treelayout.core_1.0.3.jar", + "org.antlr_ST4_4.3.4.jar", + "org.antlr_antlr-runtime_3.5.3.jar", + "org.antlr_antlr4-runtime_4.11.1.jar", + "org.glassfish_javax.json_1.1.4.jar", + ] + assert other_jar.endswith(".javac.jar") and "Other.java" in other_jar + + rendered_classpath = rule_runner.request( + RenderedClasspath, [Addresses([Address("app", relative_file_path="App.java")])] + ) + assert {"org/pantsbuild/app/App.class"} in rendered_classpath.content.values() diff --git a/src/python/pants/jvm/resolve/coursier_fetch.py b/src/python/pants/jvm/resolve/coursier_fetch.py index 5a7b954b4b1..30998f870a7 100644 --- a/src/python/pants/jvm/resolve/coursier_fetch.py +++ b/src/python/pants/jvm/resolve/coursier_fetch.py @@ -672,15 +672,18 @@ async def fetch_with_coursier(request: CoursierFetchRequest) -> FallibleClasspat requirement.coordinate, ) - classpath_entries = await concurrently( + root_classpath_entry, *transitive_classpath_entries = await concurrently( coursier_fetch_one_coord(entry) for entry in (root_entry, *transitive_entries) ) - exported_digest = await merge_digests(MergeDigests(cpe.digest for cpe in classpath_entries)) return FallibleClasspathEntry( description=str(request.component), result=CompileResult.SUCCEEDED, - output=ClasspathEntry.merge(exported_digest, classpath_entries), + output=ClasspathEntry( + root_classpath_entry.digest, + root_classpath_entry.filenames, + exports=transitive_classpath_entries, + ), exit_code=0, ) diff --git a/src/python/pants/jvm/resources.py b/src/python/pants/jvm/resources.py index f671f849cba..a4496dec916 100644 --- a/src/python/pants/jvm/resources.py +++ b/src/python/pants/jvm/resources.py @@ -12,9 +12,7 @@ from pants.core.util_rules.source_files import SourceFilesRequest from pants.core.util_rules.stripped_source_files import strip_source_roots from pants.core.util_rules.system_binaries import BashBinary, TouchBinary, ZipBinary -from pants.engine.fs import MergeDigests from pants.engine.internals.selectors import concurrently -from pants.engine.intrinsics import merge_digests from pants.engine.process import Process, execute_process_or_raise from pants.engine.rules import collect_rules, implicitly, rule from pants.engine.target import SourcesField @@ -131,17 +129,9 @@ async def assemble_resources_jar( **implicitly(StripJarRequest(output_digest, tuple(output_files))) ) - cpe = ClasspathEntry(output_digest, output_files, []) + cpe = ClasspathEntry(output_digest, output_files, exports=direct_dependency_classpath_entries) - merged_cpe_digest = await merge_digests( - MergeDigests(chain((cpe.digest,), (i.digest for i in direct_dependency_classpath_entries))) - ) - - merged_cpe = ClasspathEntry.merge( - digest=merged_cpe_digest, entries=[cpe, *direct_dependency_classpath_entries] - ) - - return FallibleClasspathEntry(output_filename, CompileResult.SUCCEEDED, merged_cpe, 0) + return FallibleClasspathEntry(output_filename, CompileResult.SUCCEEDED, cpe, 0) def rules(): diff --git a/src/python/pants/jvm/resources_test.py b/src/python/pants/jvm/resources_test.py index 7f379558bbc..ea545646837 100644 --- a/src/python/pants/jvm/resources_test.py +++ b/src/python/pants/jvm/resources_test.py @@ -106,6 +106,26 @@ def test_resources(rule_runner: RuleRunner) -> None: assert filenames_from_zip(contents[0]) == ["one.txt"] +@maybe_skip_jdk_test +def test_resources_jar_exports_its_dependencies(rule_runner: RuleRunner) -> None: + rule_runner.write_files( + { + "BUILD": "resources(name='root', sources=['root.txt'], dependencies=['dep'])", + "root.txt": "", + "dep/BUILD": "resources(sources=['dep.txt'])", + "dep/dep.txt": "", + "3rdparty/jvm/default.lock": EMPTY_JVM_LOCKFILE, + } + ) + + classpath = rule_runner.request( + Classpath, + [Addresses([Address(spec_path="", target_name="root", relative_file_path="root.txt")])], + ) + + assert list(classpath.root_args()) == [".root.txt.root.resources.jar", "dep.resources.jar"] + + @maybe_skip_jdk_test def test_resources_jar_is_deterministic(rule_runner: RuleRunner) -> None: rule_runner.write_files( diff --git a/src/python/pants/jvm/testutil.py b/src/python/pants/jvm/testutil.py index d687b5bd8fb..27632159a12 100644 --- a/src/python/pants/jvm/testutil.py +++ b/src/python/pants/jvm/testutil.py @@ -64,6 +64,11 @@ async def render_classpath_entry( classpath_entry: ClasspathEntry, unzip_binary: UnzipBinary ) -> RenderedClasspath: dest_dir = "dest" + jars = [ + (cpe.digest, filename) + for cpe in ClasspathEntry.with_exports([classpath_entry]) + for filename in cpe.filenames + ] process_results = await concurrently( execute_process_or_raise( **implicitly( @@ -74,13 +79,13 @@ async def render_classpath_entry( dest_dir, filename, ], - input_digest=classpath_entry.digest, + input_digest=digest, output_directories=(dest_dir,), description=f"Extract {filename}", ), ) ) - for filename in classpath_entry.filenames + for digest, filename in jars ) listing_snapshots = await concurrently( @@ -89,10 +94,7 @@ async def render_classpath_entry( ) return RenderedClasspath( - { - path: set(listing.files) - for path, listing in zip(classpath_entry.filenames, listing_snapshots) - } + {filename: set(listing.files) for (_, filename), listing in zip(jars, listing_snapshots)} ) From 54dc46a6c175476dfc3d9a2c478a58cdfa24012a Mon Sep 17 00:00:00 2001 From: jgranstrom Date: Tue, 27 Oct 2026 08:25:46 +0100 Subject: [PATCH 2/2] refactor(jvm): rename ClasspathEntry.with_exports to export_closure --- src/python/pants/jvm/bsp/dependencies.py | 2 +- src/python/pants/jvm/classpath.py | 2 +- src/python/pants/jvm/compile.py | 6 +++--- src/python/pants/jvm/compile_test.py | 2 +- src/python/pants/jvm/testutil.py | 2 +- 5 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/python/pants/jvm/bsp/dependencies.py b/src/python/pants/jvm/bsp/dependencies.py index 6206c0b3e67..6b7aafc4ea2 100644 --- a/src/python/pants/jvm/bsp/dependencies.py +++ b/src/python/pants/jvm/bsp/dependencies.py @@ -153,7 +153,7 @@ async def collect_thirdparty_modules( ) resolve_digest = await merge_digests( - MergeDigests(cpe.digest for cpe in ClasspathEntry.with_exports(classpath_entries)) + MergeDigests(cpe.digest for cpe in ClasspathEntry.export_closure(classpath_entries)) ) return ThirdpartyModules( diff --git a/src/python/pants/jvm/classpath.py b/src/python/pants/jvm/classpath.py index a76d35c9374..04a98058602 100644 --- a/src/python/pants/jvm/classpath.py +++ b/src/python/pants/jvm/classpath.py @@ -140,7 +140,7 @@ async def loose_classfiles( ) ) ) - for cpe in ClasspathEntry.with_exports([classpath_entry]) + for cpe in ClasspathEntry.export_closure([classpath_entry]) for filename in cpe.filenames ) diff --git a/src/python/pants/jvm/compile.py b/src/python/pants/jvm/compile.py index 0029796f8d0..ee5f2459e74 100644 --- a/src/python/pants/jvm/compile.py +++ b/src/python/pants/jvm/compile.py @@ -263,8 +263,8 @@ class ClasspathEntry: `exports` are entries that are part of this one, when a node and some of its dependencies are indistinguishable (such as a target without sources, or a `jvm_artifact` and its transitive - jars). `digest` and `filenames` exclude them; `with_exports`, `args`, `immutable_inputs(_args)` - and `closure` include them. + jars). `digest` and `filenames` exclude them; `export_closure`, `args`, + `immutable_inputs(_args)` and `closure` include them. This class additionally keeps filenames in order to preserve classpath ordering for the `classpath_arg` method: although Digests encode filenames, they are stored sorted. @@ -359,7 +359,7 @@ def immutable_inputs_args( ) @classmethod - def with_exports(cls, entries: Iterable[ClasspathEntry]) -> Iterator[ClasspathEntry]: + def export_closure(cls, entries: Iterable[ClasspathEntry]) -> Iterator[ClasspathEntry]: """The given entries, each followed by its transitive exports.""" return cls._walk(entries, follow_dependencies=False) diff --git a/src/python/pants/jvm/compile_test.py b/src/python/pants/jvm/compile_test.py index 5ef650cda30..f988cbb1908 100644 --- a/src/python/pants/jvm/compile_test.py +++ b/src/python/pants/jvm/compile_test.py @@ -592,7 +592,7 @@ def test_exports_are_transitive_and_their_dependencies_are_not_root() -> None: bundle = _jar("bundle", exports=[api]) user = _jar("user", dependencies=[bundle]) - assert list(ClasspathEntry.with_exports([bundle])) == [bundle, api, core] + assert list(ClasspathEntry.export_closure([bundle])) == [bundle, api, core] assert list(ClasspathEntry.closure([user])) == [user, bundle, api, core, runtime_dep] diff --git a/src/python/pants/jvm/testutil.py b/src/python/pants/jvm/testutil.py index 27632159a12..17797ac34b0 100644 --- a/src/python/pants/jvm/testutil.py +++ b/src/python/pants/jvm/testutil.py @@ -66,7 +66,7 @@ async def render_classpath_entry( dest_dir = "dest" jars = [ (cpe.digest, filename) - for cpe in ClasspathEntry.with_exports([classpath_entry]) + for cpe in ClasspathEntry.export_closure([classpath_entry]) for filename in cpe.filenames ] process_results = await concurrently(