Skip to content

Commit

Permalink
Add special casing for '--javac-jdk=system' as a temporary hack for p…
Browse files Browse the repository at this point in the history
…antsbuild#12293.

This causes JavacBinary to pass the `--system-jvm` option to Coursier,
which selects whichever JDK Coursier happens to find already installed
on the system.  Long term, we shouldn't use this behavior in tests and
probably shouldn't even allow it at all since the inherent non-hermeticity
isn't properly captured in cache keys, but for now it allows testing
of Coursier-dependent backends in CI until pantsbuild#12293 is resolved properly.
  • Loading branch information
patricklaw committed Jul 25, 2021
1 parent c553b79 commit 0a25aad
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 12 deletions.
26 changes: 18 additions & 8 deletions src/python/pants/backend/java/compile/javac_binary.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,24 @@ class JavacBinary:

@rule
async def setup_javac_binary(coursier: Coursier, javac: JavacSubsystem) -> JavacBinary:
javac_wrapper_script = textwrap.dedent(
f"""\
set -eux
javac_path="$({coursier.coursier.exe} java-home --jvm {javac.options.jdk})/bin/javac"
/bin/mkdir -p {JavacBinary.classfiles_relpath}
exec "${{javac_path}}" "$@"
"""
)
if javac.options.jdk == "system":
javac_wrapper_script = textwrap.dedent(
f"""\
set -eux
javac_path="$({coursier.coursier.exe} java-home --system-jvm)/bin/javac"
/bin/mkdir -p {JavacBinary.classfiles_relpath}
exec "${{javac_path}}" "$@"
"""
)
else:
javac_wrapper_script = textwrap.dedent(
f"""\
set -eux
javac_path="$({coursier.coursier.exe} java-home --jvm {javac.options.jdk})/bin/javac"
/bin/mkdir -p {JavacBinary.classfiles_relpath}
exec "${{javac_path}}" "$@"
"""
)
javac_wrapper_script_digest = await Get(
Digest,
CreateDigest(
Expand Down
5 changes: 4 additions & 1 deletion src/python/pants/backend/java/compile/javac_subsystem.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,5 +23,8 @@ def register_options(cls, register):
advanced=True,
help="The JDK to use for invoking javac."
" This string will be passed directly to Coursier's `--jvm` parameter."
" Run `cs java --available` to see a list of available JVM versions on your platform.",
" Run `cs java --available` to see a list of available JVM versions on your platform."
" If the string 'system' is passed, Coursier's `--system-jvm` option will be used"
" instead, but note that this can lead to inconsistent behavior since the JVM version"
" will be whatever happens to be found first on the system's PATH.",
)
6 changes: 3 additions & 3 deletions src/python/pants/backend/java/compile/javac_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,6 @@
from pants.jvm.util_rules import rules as util_rules
from pants.testutil.rule_runner import QueryRule, RuleRunner

# TODO(#12293): Stabilize network flakiness.
pytestmark = pytest.mark.skip


@pytest.fixture
def rule_runner() -> RuleRunner:
Expand All @@ -47,6 +44,9 @@ def rule_runner() -> RuleRunner:
QueryRule(CompiledClassfiles, (CompileJavaSourceRequest,)),
],
target_types=[JvmDependencyLockfile, JavaLibrary],
# TODO(#12293): Remove this nonhermetic hack once Coursier JVM boostrapping flakiness
# is properly fixed in CI.
bootstrap_args=["--javac-jdk=system"],
)


Expand Down

0 comments on commit 0a25aad

Please sign in to comment.