diff --git a/docs/notes/2.34.x.md b/docs/notes/2.34.x.md index f1cc0ca82f1..8302e1abceb 100644 --- a/docs/notes/2.34.x.md +++ b/docs/notes/2.34.x.md @@ -53,6 +53,8 @@ Fixed nailgun servers being left running after a `--no-pantsd` run exits. The Scala protobuf codegen backend (`pants.backend.codegen.protobuf.scala`) now honors `protobuf_sources`/`protobuf_source`'s `grpc=True` field: ScalaPB now generates `*Grpc.scala` service stubs, and the `scalapb-runtime-grpc` runtime is automatically inferred as a dependency, matching the existing behavior of the Java protobuf codegen backend. This is a behavior change for anyone using the Scala protobuf backend with `grpc=True` set on a `protobuf_sources` target: a `jvm_artifact` providing `com.thesamet.scalapb:scalapb-runtime-grpc_` must now be resolvable, or dependency inference will fail. Callers remain responsible for declaring their own gRPC transport dependency (e.g. `grpc-netty-shaded`, `grpc-netty`, or `grpc-okhttp`), since that is a deployment choice not dictated by the generated code. +Fixed `javac`/`scalac`/`kotlinc` invocations, and `java` processes run against a Java 9+ JDK, failing with an OS-level "Argument list too long" error for components with a large number of dependencies or a long classpath: compiler arguments and (for Java 9+) the `java` classpath and program arguments are now passed via an `@argfile` instead of directly on the command line. + #### Python Support for creating multiplatform/foreign platform pexes when using the `uv` resolver. This includes support for FAAS (AWS Lambda/Google Cloud Functions). diff --git a/src/python/pants/backend/java/compile/javac.py b/src/python/pants/backend/java/compile/javac.py index 951de73fcd0..5513f042810 100644 --- a/src/python/pants/backend/java/compile/javac.py +++ b/src/python/pants/backend/java/compile/javac.py @@ -16,7 +16,7 @@ from pants.backend.java.target_types import JavaFieldSet, JavaGeneratorFieldSet, JavaSourceField from pants.core.util_rules.source_files import SourceFilesRequest, determine_source_files from pants.core.util_rules.system_binaries import BashBinary, ZipBinary -from pants.engine.fs import EMPTY_DIGEST, CreateDigest, Directory, MergeDigests +from pants.engine.fs import EMPTY_DIGEST, CreateDigest, Directory, FileContent, MergeDigests from pants.engine.intrinsics import ( create_digest, digest_to_snapshot, @@ -39,13 +39,15 @@ compile_classpath_entries, ) from pants.jvm.compile import rules as jvm_compile_rules -from pants.jvm.jdk_rules import JdkRequest, JvmProcess, prepare_jdk_environment +from pants.jvm.jdk_rules import JdkRequest, JvmProcess, jvm_argfile_content, prepare_jdk_environment from pants.jvm.strip_jar.strip_jar import StripJarRequest, strip_jar from pants.jvm.subsystems import JvmSubsystem from pants.util.logging import LogLevel logger = logging.getLogger(__name__) +_JAVAC_ARGUMENT_FILE = "__javac_args.txt" + class CompileJavaSourceRequest(ClasspathEntryRequest): field_sets = (JavaFieldSet, JavaGeneratorFieldSet) @@ -139,13 +141,33 @@ async def compile_java_source( ) dest_dir = "classfiles" - dest_dir_digest, jdk = await concurrently( + usercp = "__cp" + user_classpath = Classpath(direct_dependency_classpath_entries, request.resolve) + classpath_arg = ":".join(user_classpath.root_immutable_inputs_args(prefix=usercp)) + immutable_input_digests = dict(user_classpath.root_immutable_inputs(prefix=usercp)) + + compiler_args = [ + *(("-cp", classpath_arg) if classpath_arg else ()), + *javac.args, + "-d", + dest_dir, + *sorted( + chain.from_iterable( + sources.snapshot.files for _, sources in component_members_and_java_source_files + ) + ), + ] + compiler_args_digest, dest_dir_digest, jdk = await concurrently( + create_digest( + CreateDigest([FileContent(_JAVAC_ARGUMENT_FILE, jvm_argfile_content(compiler_args))]) + ), create_digest(CreateDigest([Directory(dest_dir)])), prepare_jdk_environment(**implicitly(JdkRequest.from_target(request.component))), ) merged_digest = await merge_digests( MergeDigests( ( + compiler_args_digest, dest_dir_digest, *( sources.snapshot.digest @@ -155,11 +177,6 @@ async def compile_java_source( ) ) - usercp = "__cp" - user_classpath = Classpath(direct_dependency_classpath_entries, request.resolve) - classpath_arg = ":".join(user_classpath.root_immutable_inputs_args(prefix=usercp)) - immutable_input_digests = dict(user_classpath.root_immutable_inputs(prefix=usercp)) - # Compile. compile_result = await execute_process( **implicitly( @@ -168,16 +185,7 @@ async def compile_java_source( classpath_entries=[f"{jdk.java_home}/lib/tools.jar"], argv=[ "com.sun.tools.javac.Main", - *(("-cp", classpath_arg) if classpath_arg else ()), - *javac.args, - "-d", - dest_dir, - *sorted( - chain.from_iterable( - sources.snapshot.files - for _, sources in component_members_and_java_source_files - ) - ), + f"@{_JAVAC_ARGUMENT_FILE}", ], input_digest=merged_digest, extra_immutable_input_digests=immutable_input_digests, diff --git a/src/python/pants/backend/kotlin/compile/kotlinc.py b/src/python/pants/backend/kotlin/compile/kotlinc.py index 93303393ceb..43b37355365 100644 --- a/src/python/pants/backend/kotlin/compile/kotlinc.py +++ b/src/python/pants/backend/kotlin/compile/kotlinc.py @@ -20,9 +20,10 @@ KotlinSourceField, ) from pants.core.util_rules.source_files import SourceFilesRequest, determine_source_files +from pants.engine.fs import CreateDigest, FileContent from pants.engine.internals.native_engine import EMPTY_DIGEST, MergeDigests from pants.engine.internals.selectors import concurrently -from pants.engine.intrinsics import execute_process, merge_digests +from pants.engine.intrinsics import create_digest, execute_process, merge_digests from pants.engine.rules import collect_rules, implicitly, rule from pants.engine.target import CoarsenedTarget, SourcesField from pants.engine.unions import UnionRule @@ -36,7 +37,7 @@ compile_classpath_entries, ) from pants.jvm.compile import rules as jvm_compile_rules -from pants.jvm.jdk_rules import JdkRequest, JvmProcess, prepare_jdk_environment +from pants.jvm.jdk_rules import JdkRequest, JvmProcess, jvm_argfile_content, prepare_jdk_environment from pants.jvm.resolve.common import ArtifactRequirements from pants.jvm.resolve.coordinate import Coordinate from pants.jvm.resolve.coursier_fetch import ToolClasspathRequest, materialize_classpath_for_tool @@ -44,6 +45,8 @@ logger = logging.getLogger(__name__) +_KOTLINC_ARGUMENT_FILE = "__kotlinc_args.txt" + class CompileKotlinSourceRequest(ClasspathEntryRequest): field_sets = (KotlinFieldSet, KotlinGeneratorFieldSet) @@ -167,6 +170,22 @@ async def compile_kotlin_source( classpath_arg = ":".join(user_classpath.immutable_inputs_args(prefix=usercp)) output_file = compute_output_jar_filename(request.component) + compiler_args = [ + *(("-classpath", classpath_arg) if classpath_arg else ()), + "-d", + output_file, + *local_plugins.args(local_kotlinc_plugins_relpath), + *kotlinc.args, + *sorted( + itertools.chain.from_iterable( + sources.snapshot.files for _, sources in component_members_and_kotlin_source_files + ) + ), + ] + compiler_args_digest = await create_digest( + CreateDigest([FileContent(_KOTLINC_ARGUMENT_FILE, jvm_argfile_content(compiler_args))]) + ) + process_input_digest = await merge_digests(MergeDigests([sources_digest, compiler_args_digest])) process_result = await execute_process( **implicitly( JvmProcess( @@ -174,19 +193,9 @@ async def compile_kotlin_source( classpath_entries=tool_classpath.classpath_entries(toolcp_relpath), argv=[ "org.jetbrains.kotlin.cli.jvm.K2JVMCompiler", - *(("-classpath", classpath_arg) if classpath_arg else ()), - "-d", - output_file, - *(local_plugins.args(local_kotlinc_plugins_relpath)), - *kotlinc.args, - *sorted( - itertools.chain.from_iterable( - sources.snapshot.files - for _, sources in component_members_and_kotlin_source_files - ) - ), + f"@{_KOTLINC_ARGUMENT_FILE}", ], - input_digest=sources_digest, + input_digest=process_input_digest, extra_immutable_input_digests=extra_immutable_input_digests, extra_nailgun_keys=extra_nailgun_keys, output_files=(output_file,), diff --git a/src/python/pants/backend/scala/compile/scalac.py b/src/python/pants/backend/scala/compile/scalac.py index ed630863201..fd0b11db6b5 100644 --- a/src/python/pants/backend/scala/compile/scalac.py +++ b/src/python/pants/backend/scala/compile/scalac.py @@ -27,7 +27,7 @@ ) from pants.core.util_rules.source_files import SourceFilesRequest, determine_source_files from pants.core.util_rules.system_binaries import BashBinary, ZipBinary -from pants.engine.fs import EMPTY_DIGEST, CreateDigest, Directory, MergeDigests +from pants.engine.fs import EMPTY_DIGEST, CreateDigest, Directory, FileContent, MergeDigests from pants.engine.intrinsics import create_digest, execute_process, merge_digests from pants.engine.process import Process, execute_process_or_raise from pants.engine.rules import collect_rules, concurrently, implicitly, rule @@ -43,7 +43,7 @@ compile_classpath_entries, ) from pants.jvm.compile import rules as jvm_compile_rules -from pants.jvm.jdk_rules import JdkRequest, JvmProcess, prepare_jdk_environment +from pants.jvm.jdk_rules import JdkRequest, JvmProcess, jvm_argfile_content, prepare_jdk_environment from pants.jvm.resolve.common import ArtifactRequirements from pants.jvm.resolve.coursier_fetch import ToolClasspathRequest, materialize_classpath_for_tool from pants.jvm.strip_jar import strip_jar @@ -54,6 +54,8 @@ logger = logging.getLogger(__name__) +_SCALAC_ARGUMENT_FILE = "__scalac_args.txt" + class CompileScalaSourceRequest(ClasspathEntryRequest): field_sets = (ScalaFieldSet, ScalaGeneratorFieldSet) @@ -185,8 +187,29 @@ async def compile_scala_source( output_file = compute_output_jar_filename(request.component) compilation_output_dir = "__out" - compilation_empty_dir = await create_digest(CreateDigest([Directory(compilation_output_dir)])) - merged_digest = await merge_digests(MergeDigests([sources_digest, compilation_empty_dir])) + compiler_args = [ + "-bootclasspath", + ":".join(tool_classpath.classpath_entries(toolcp_relpath)), + *local_plugins.args(local_scalac_plugins_relpath), + *(("-classpath", classpath_arg) if classpath_arg else ()), + *scalac.parsed_args_for_resolve(request.resolve.name), + "-d", + compilation_output_dir, + *sorted( + chain.from_iterable( + sources.snapshot.files for _, sources in component_members_and_scala_source_files + ) + ), + ] + compiler_args_digest, compilation_empty_dir = await concurrently( + create_digest( + CreateDigest([FileContent(_SCALAC_ARGUMENT_FILE, jvm_argfile_content(compiler_args))]) + ), + create_digest(CreateDigest([Directory(compilation_output_dir)])), + ) + merged_digest = await merge_digests( + MergeDigests([sources_digest, compilation_empty_dir, compiler_args_digest]) + ) compile_result = await execute_process( **implicitly( JvmProcess( @@ -194,19 +217,7 @@ async def compile_scala_source( classpath_entries=tool_classpath.classpath_entries(toolcp_relpath), argv=[ scala_artifacts.compiler_main, - "-bootclasspath", - ":".join(tool_classpath.classpath_entries(toolcp_relpath)), - *local_plugins.args(local_scalac_plugins_relpath), - *(("-classpath", classpath_arg) if classpath_arg else ()), - *scalac.parsed_args_for_resolve(request.resolve.name), - "-d", - compilation_output_dir, - *sorted( - chain.from_iterable( - sources.snapshot.files - for _, sources in component_members_and_scala_source_files - ) - ), + f"@{_SCALAC_ARGUMENT_FILE}", ], input_digest=merged_digest, extra_immutable_input_digests=extra_immutable_input_digests, diff --git a/src/python/pants/jvm/jdk_rules.py b/src/python/pants/jvm/jdk_rules.py index cd6567994e8..3bdd960fe6e 100644 --- a/src/python/pants/jvm/jdk_rules.py +++ b/src/python/pants/jvm/jdk_rules.py @@ -37,6 +37,9 @@ logger = logging.getLogger(__name__) +_JVM_ARGUMENT_FILE = "__jvm_args.txt" + + @dataclass(frozen=True) class Nailgun: classpath_entry: ClasspathEntry @@ -102,9 +105,7 @@ class JdkEnvironment: jdk_preparation_script: ClassVar[str] = f"{bin_dir}/jdk.sh" java_home: ClassVar[str] = "__java_home" - def args( - self, bash: BashBinary, classpath_entries: Iterable[str], chroot: str | None = None - ) -> tuple[str, ...]: + def java_binary_args(self, bash: BashBinary, chroot: str | None = None) -> tuple[str, ...]: def in_chroot(path: str) -> str: if not chroot: return path @@ -114,6 +115,18 @@ def in_chroot(path: str) -> str: bash.path, in_chroot(self.jdk_preparation_script), f"{self.java_home}/bin/java", + ) + + def args( + self, bash: BashBinary, classpath_entries: Iterable[str], chroot: str | None = None + ) -> tuple[str, ...]: + def in_chroot(path: str) -> str: + if not chroot: + return path + return os.path.join(chroot, path) + + return ( + *self.java_binary_args(bash, chroot), "-cp", ":".join([in_chroot(self.nailgun_jar), *classpath_entries]), ) @@ -390,6 +403,14 @@ def __post_init__(self): _JVM_HEAP_SIZE_UNITS = ["", "k", "m", "g"] +def _java_argfile_escape(arg: str) -> str: + return '"' + arg.replace("\\", "\\\\").replace('"', '\\"') + '"' + + +def jvm_argfile_content(args: Iterable[str]) -> bytes: + return "\n".join(_java_argfile_escape(arg) for arg in args).encode("utf-8") + b"\n" + + @rule async def jvm_process( bash: BashBinary, request: JvmProcess, jvm: JvmSubsystem, global_options: GlobalOptions @@ -431,8 +452,23 @@ def valid_jvm_opt(opt: str) -> str: *[valid_jvm_opt(opt) for opt in jvm_user_options], ] + # The `{chroot}` placeholder (see `RunRequest`) is only substituted by the engine within a + # `Process`'s `argv`, not within file contents. `JvmProcess`es whose classpath or argv rely + # on that substitution (as `run.py` and `run_deploy_jar.py` do, to build a `RunRequest` that + # runs in the workspace) can't have those arguments moved into an argfile, since the + # placeholder would never get resolved. + contains_chroot_placeholder = any( + "{chroot}" in value for value in (*request.classpath_entries, *request.argv) + ) + # Nailgun sends the classpath and program arguments to the already-running server over a + # socket rather than via argv, so it isn't subject to the OS argument-length limit the argfile + # works around, and doesn't benefit from one. Route through the argfile only when nailgun + # won't be used, rather than disabling nailgun to make room for it. + use_argfile = ( + jdk.jre_major_version >= 9 and not request.use_nailgun and not contains_chroot_placeholder + ) use_nailgun = [] - if request.use_nailgun: + if request.use_nailgun and not use_argfile: use_nailgun = [*jdk.immutable_input_digests, *request.extra_nailgun_keys] if jvm.nailgun_enable_agent: jvm_options.append(f"-javaagent:{request.jdk.nailgun_jar}") @@ -441,12 +477,36 @@ def valid_jvm_opt(opt: str) -> str: remote_cache_speculation_delay_millis = 0 if request.remote_cache_speculation_delay is not None: remote_cache_speculation_delay_millis = request.remote_cache_speculation_delay - elif request.use_nailgun: + elif use_nailgun: remote_cache_speculation_delay_millis = jvm.nailgun_remote_cache_speculation_delay + java_args = [ + "-cp", + ":".join([jdk.nailgun_jar, *request.classpath_entries]), + *jvm_options, + *request.argv, + ] + + if use_argfile: + jvm_args_digest = await create_digest( + CreateDigest( + [ + FileContent( + _JVM_ARGUMENT_FILE, + jvm_argfile_content(java_args), + ) + ] + ) + ) + input_digest = await merge_digests(MergeDigests([request.input_digest, jvm_args_digest])) + process_argv = [*jdk.java_binary_args(bash), f"@{_JVM_ARGUMENT_FILE}"] + else: + input_digest = request.input_digest + process_argv = [*jdk.java_binary_args(bash), *java_args] + return Process( - [*jdk.args(bash, request.classpath_entries), *jvm_options, *request.argv], - input_digest=request.input_digest, + process_argv, + input_digest=input_digest, immutable_input_digests=immutable_input_digests, use_nailgun=use_nailgun, description=request.description, diff --git a/src/python/pants/jvm/jdk_rules_test.py b/src/python/pants/jvm/jdk_rules_test.py index a3c9168fb3d..b281a000822 100644 --- a/src/python/pants/jvm/jdk_rules_test.py +++ b/src/python/pants/jvm/jdk_rules_test.py @@ -10,11 +10,16 @@ from pants.core.util_rules import config_files, source_files, system_binaries from pants.core.util_rules.external_tool import rules as external_tool_rules from pants.core.util_rules.system_binaries import BashBinary -from pants.engine.fs import CreateDigest, Digest, FileContent +from pants.engine.fs import CreateDigest, Digest, DigestContents, FileContent from pants.engine.internals.native_engine import EMPTY_DIGEST from pants.engine.internals.scheduler import ExecutionError from pants.engine.process import Process, ProcessResult -from pants.jvm.jdk_rules import InternalJdk, JvmProcess, parse_jre_major_version +from pants.jvm.jdk_rules import ( + InternalJdk, + JvmProcess, + jvm_argfile_content, + parse_jre_major_version, +) from pants.jvm.jdk_rules import rules as jdk_rules from pants.jvm.resolve.coursier_fetch import rules as coursier_fetch_rules from pants.jvm.resolve.coursier_setup import rules as coursier_setup_rules @@ -65,6 +70,24 @@ def javac_version_proc(rule_runner: RuleRunner) -> Process: ) +def javac_version_proc_with_nailgun(rule_runner: RuleRunner) -> Process: + jdk = rule_runner.request(InternalJdk, []) + return rule_runner.request( + Process, + [ + JvmProcess( + jdk=jdk, + classpath_entries=(), + argv=[ + "-version", + ], + input_digest=EMPTY_DIGEST, + description="", + ) + ], + ) + + def run_javac_version(rule_runner: RuleRunner) -> str: process_result = rule_runner.request( ProcessResult, @@ -75,6 +98,18 @@ def run_javac_version(rule_runner: RuleRunner) -> str: ) +def get_jvm_argfile(rule_runner: RuleRunner, proc: Process) -> str: + digest_contents = rule_runner.request(DigestContents, [proc.input_digest]) + argfile = next(fc for fc in digest_contents if fc.path == "__jvm_args.txt") + return argfile.content.decode("utf-8") + + +def test_jvm_argfile_content() -> None: + assert jvm_argfile_content(["hello world", "a#b", r"a\b", 'a"b', "@literal"]) == ( + b'"hello world"\n"a#b"\n"a\\\\b"\n"a\\"b"\n"@literal"\n' + ) + + @maybe_skip_jdk_test def test_java_binary_system_version(rule_runner: RuleRunner) -> None: rule_runner.set_options(["--jvm-jdk=system"], env_inherit=PYTHON_BOOTSTRAP_ENV) @@ -129,7 +164,8 @@ def test_parse_java_version() -> None: @maybe_skip_jdk_test def test_include_default_heap_size_in_jvm_options(rule_runner: RuleRunner) -> None: proc = javac_version_proc(rule_runner) - assert "-Xmx512m" in proc.argv + assert proc.argv[-1] == "@__jvm_args.txt" + assert '"-Xmx512m"' in get_jvm_argfile(rule_runner, proc) @maybe_skip_jdk_test @@ -139,7 +175,53 @@ def test_include_child_mem_constraint_in_jvm_options(rule_runner: RuleRunner) -> env_inherit=PYTHON_BOOTSTRAP_ENV, ) proc = javac_version_proc(rule_runner) - assert "-Xmx1g" in proc.argv + assert '"-Xmx1g"' in get_jvm_argfile(rule_runner, proc) + + +@maybe_skip_jdk_test +def test_uses_nailgun_instead_of_jvm_argfile_for_java_9_plus(rule_runner: RuleRunner) -> None: + # Nailgun sends the classpath and args to the running server over a socket rather than via + # argv, so it isn't subject to the argument-length limit the argfile works around: prefer it + # over the argfile whenever it's requested (the default), even on a Java 9+ JDK. + proc = javac_version_proc_with_nailgun(rule_runner) + assert proc.argv[-1] == "-version" + assert proc.use_nailgun + + +@maybe_skip_jdk_test +def test_uses_jvm_argfile_for_java_arguments(rule_runner: RuleRunner) -> None: + jdk = rule_runner.request(InternalJdk, []) + proc = rule_runner.request( + Process, + [ + JvmProcess( + jdk=jdk, + classpath_entries=("tool.jar", "another tool.jar"), + argv=["com.example.Main", "hello world", "a#b", r"a\b", 'a"b', "@literal"], + input_digest=EMPTY_DIGEST, + description="", + use_nailgun=False, + ) + ], + ) + + assert proc.argv == ( + rule_runner.request(BashBinary, []).path, + "__jdk/jdk.sh", + "__java_home/bin/java", + "@__jvm_args.txt", + ) + assert get_jvm_argfile(rule_runner, proc).splitlines() == [ + '"-cp"', + f'"{jdk.nailgun_jar}:tool.jar:another tool.jar"', + '"-Xmx512m"', + '"com.example.Main"', + '"hello world"', + '"a#b"', + r'"a\\b"', + r'"a\"b"', + '"@literal"', + ] @maybe_skip_jdk_test