Add support for the check goal for javac (#12859)

Add support for the `check` goal by making the result of `javac` compilation a `FallibleCompiledClassfiles` type so that results are streamed, and the compilation graph can (optionally) complete without raising an exception.

Successful check:
<img width="869" alt="Screen Shot 2021-09-13 at 18 18 07" src="https://user-images.githubusercontent.com/46740/133178683-dbd7a77a-54ec-4fe7-b56b-9c0a8709760a.png">

Failed check:
<img width="953" alt="Screen Shot 2021-09-13 at 18 17 27" src="https://user-images.githubusercontent.com/46740/133178693-f8fa2aa3-d891-4496-a458-44e1085d655d.png">

[ci skip-rust]
[ci skip-build-wheels]

pants.toml

@@ -14,8 +14,9 @@ backend_packages.add = [
   "pants.backend.shell",
   "pants.backend.shell.lint.shellcheck",
   "pants.backend.shell.lint.shfmt",
-  "pants.backend.experimental.python",
   "pants.backend.experimental.docker",
+  "pants.backend.experimental.java",
+  "pants.backend.experimental.python",
   "internal_plugins.releases",
 ]
 plugins = [

src/python/pants/backend/java/compile/javac.py

@@ -5,16 +5,20 @@
 
 import logging
 from dataclasses import dataclass
+from enum import Enum
 from itertools import chain
 
 from pants.backend.java.compile.javac_binary import JavacBinary
 from pants.backend.java.target_types import JavaSources
+from pants.core.goals.check import CheckRequest, CheckResult, CheckResults
 from pants.core.util_rules.source_files import SourceFiles, SourceFilesRequest
 from pants.engine.addresses import Addresses
+from pants.engine.engine_aware import EngineAwareReturnType
 from pants.engine.fs import EMPTY_DIGEST, AddPrefix, Digest, MergeDigests, RemovePrefix
-from pants.engine.process import BashBinary, Process, ProcessResult
+from pants.engine.process import BashBinary, FallibleProcessResult, Process
 from pants.engine.rules import Get, MultiGet, collect_rules, rule
-from pants.engine.target import CoarsenedTarget, CoarsenedTargets, Sources, Targets
+from pants.engine.target import CoarsenedTarget, CoarsenedTargets, FieldSet, Sources, Targets
+from pants.engine.unions import UnionRule
 from pants.jvm.resolve.coursier_fetch import (
     CoursierLockfileForTargetRequest,
     CoursierResolvedLockfile,
@@ -23,10 +27,22 @@
 )
 from pants.jvm.resolve.coursier_setup import Coursier
 from pants.util.logging import LogLevel
+from pants.util.strutil import strip_v2_chroot_path
 
 logger = logging.getLogger(__name__)
 
 
+@dataclass(frozen=True)
+class JavacFieldSet(FieldSet):
+    required_fields = (JavaSources,)
+
+    sources: JavaSources
+
+
+class JavacCheckRequest(CheckRequest):
+    field_set_type = JavacFieldSet
+
+
 @dataclass(frozen=True)
 class CompileJavaSourceRequest:
     component: CoarsenedTarget
@@ -37,13 +53,75 @@ class CompiledClassfiles:
     digest: Digest
 
 
-@rule(level=LogLevel.DEBUG)
+class CompileResult(Enum):
+    SUCCEEDED = "succeeded"
+    FAILED = "failed"
+    DEPENDENCY_FAILED = "dependency failed"
+
+
+@dataclass(frozen=True)
+class FallibleCompiledClassfiles(EngineAwareReturnType):
+    description: str
+    result: CompileResult
+    output: CompiledClassfiles | None
+    exit_code: int
+    stdout: str | None = None
+    stderr: str | None = None
+
+    @classmethod
+    def from_fallible_process_result(
+        cls,
+        description: str,
+        process_result: FallibleProcessResult,
+        output: CompiledClassfiles | None,
+        *,
+        strip_chroot_path: bool = False,
+    ) -> FallibleCompiledClassfiles:
+        def prep_output(s: bytes) -> str:
+            return strip_v2_chroot_path(s) if strip_chroot_path else s.decode()
+
+        exit_code = process_result.exit_code
+        # TODO: Coursier renders this line on macOS.
+        stderr = "\n".join(
+            line
+            for line in prep_output(process_result.stderr).splitlines()
+            if line != "setrlimit to increase file descriptor limit failed, errno 22"
+        )
+        return cls(
+            description=description,
+            result=(CompileResult.SUCCEEDED if exit_code == 0 else CompileResult.FAILED),
+            output=output,
+            exit_code=exit_code,
+            stdout=prep_output(process_result.stdout),
+            stderr=stderr,
+        )
+
+    def level(self) -> LogLevel:
+        return LogLevel.ERROR if self.exit_code != 0 else LogLevel.INFO
+
+    def message(self) -> str:
+        message = self.description
+        message += (
+            " succeeded." if self.exit_code == 0 else f" failed (exit code {self.exit_code})."
+        )
+        if self.stdout:
+            message += f"\n{self.stdout}"
+        if self.stderr:
+            message += f"\n{self.stderr}"
+        return message
+
+    def cacheable(self) -> bool:
+        # Failed compile outputs should be re-rendered in every run.
+        return self.exit_code == 0
+
+
+@rule(desc="Compile with javac")
 async def compile_java_source(
     bash: BashBinary,
     coursier: Coursier,
     javac_binary: JavacBinary,
     request: CompileJavaSourceRequest,
-) -> CompiledClassfiles:
+) -> FallibleCompiledClassfiles:
     component_members_with_sources = tuple(
         t for t in request.component.members if t.has_field(Sources)
     )
@@ -69,7 +147,12 @@ async def compile_java_source(
     ]
 
     if not component_members_and_java_source_files:
-        return CompiledClassfiles(digest=EMPTY_DIGEST)
+        return FallibleCompiledClassfiles(
+            description=str(request.component),
+            result=CompileResult.SUCCEEDED,
+            output=CompiledClassfiles(digest=EMPTY_DIGEST),
+            exit_code=0,
+        )
 
     # Target coarsening currently doesn't perform dep expansion, which matters for targets
     # with multiple sources that expand to individual source subtargets.
@@ -87,10 +170,21 @@ async def compile_java_source(
         CoursierResolvedLockfile,
         CoursierLockfileForTargetRequest(Targets(request.component.members)),
     )
-    direct_dependency_classfiles = await MultiGet(
-        Get(CompiledClassfiles, CompileJavaSourceRequest(component=coarsened_dep))
+    direct_dependency_classfiles_fallible = await MultiGet(
+        Get(FallibleCompiledClassfiles, CompileJavaSourceRequest(component=coarsened_dep))
         for coarsened_dep in coarsened_direct_deps
     )
+    direct_dependency_classfiles = [
+        fcc.output for fcc in direct_dependency_classfiles_fallible if fcc.output
+    ]
+    if len(direct_dependency_classfiles) != len(direct_dependency_classfiles_fallible):
+        return FallibleCompiledClassfiles(
+            description=str(request.component),
+            result=CompileResult.DEPENDENCY_FAILED,
+            output=None,
+            exit_code=1,
+        )
+
     materialized_classpath, merged_direct_dependency_classfiles_digest = await MultiGet(
         Get(
             MaterializedClasspath,
@@ -128,7 +222,7 @@ async def compile_java_source(
     )
 
     process_result = await Get(
-        ProcessResult,
+        FallibleProcessResult,
         Process(
             argv=[
                 bash.path,
@@ -150,13 +244,59 @@ async def compile_java_source(
             level=LogLevel.DEBUG,
         ),
     )
-    stripped_classfiles_digest = await Get(
-        Digest, RemovePrefix(process_result.output_digest, "classfiles")
+    output: CompiledClassfiles | None = None
+    if process_result.exit_code == 0:
+        stripped_classfiles_digest = await Get(
+            Digest, RemovePrefix(process_result.output_digest, "classfiles")
+        )
+        output = CompiledClassfiles(stripped_classfiles_digest)
+
+    return FallibleCompiledClassfiles.from_fallible_process_result(
+        str(request.component),
+        process_result,
+        output,
+    )
+
+
+@rule
+def required_classfiles(fallible_result: FallibleCompiledClassfiles) -> CompiledClassfiles:
+    if fallible_result.result == CompileResult.SUCCEEDED:
+        assert fallible_result.output
+        return fallible_result.output
+    # NB: The compile outputs will already have been streamed as FallibleCompiledClassfiles finish.
+    raise Exception("Compile failed.")
+
+
+@rule(desc="Check compilation for javac", level=LogLevel.DEBUG)
+async def javac_check(request: JavacCheckRequest) -> CheckResults:
+    coarsened_targets = await Get(
+        CoarsenedTargets, Addresses(field_set.address for field_set in request.field_sets)
+    )
+
+    # TODO: This should be fallible so that we exit cleanly.
+    results = await MultiGet(
+        Get(FallibleCompiledClassfiles, CompileJavaSourceRequest(component=t))
+        for t in coarsened_targets
+    )
+
+    # NB: We return CheckResults with exit codes for the root targets, but we do not pass
+    # stdout/stderr because it will already have been rendered as streaming.
+    return CheckResults(
+        [
+            CheckResult(
+                result.exit_code,
+                stdout="",
+                stderr="",
+                partition_description=str(coarsened_target),
+            )
+            for result, coarsened_target in zip(results, coarsened_targets)
+        ],
+        checker_name="javac",
     )
-    return CompiledClassfiles(digest=stripped_classfiles_digest)
 
 
 def rules():
     return [
         *collect_rules(),
+        UnionRule(CheckRequest, JavacCheckRequest),
     ]

src/python/pants/backend/java/compile/javac_binary.py

@@ -67,7 +67,7 @@ async def setup_javac_binary(coursier: Coursier, javac: JavacSubsystem) -> Javac
             fingerprint_comment,
             textwrap.dedent(
                 f"""\
-                set -eux
+                set -eu
                 {javac_path_line}
                 /bin/mkdir -p {JavacBinary.classfiles_relpath}
                 exec "${{javac_path}}" "$@"

src/python/pants/backend/java/compile/javac_test.py

@@ -7,11 +7,18 @@
 
 import pytest
 
-from pants.backend.java.compile.javac import CompiledClassfiles, CompileJavaSourceRequest
+from pants.backend.java.compile.javac import (
+    CompiledClassfiles,
+    CompileJavaSourceRequest,
+    CompileResult,
+    FallibleCompiledClassfiles,
+    JavacCheckRequest,
+)
 from pants.backend.java.compile.javac import rules as javac_rules
 from pants.backend.java.compile.javac_binary import rules as javac_binary_rules
 from pants.backend.java.target_types import JavaLibrary
 from pants.build_graph.address import Address
+from pants.core.goals.check import CheckResults
 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.addresses import Addresses
@@ -43,6 +50,8 @@ def rule_runner() -> RuleRunner:
             *javac_rules(),
             *util_rules(),
             *javac_binary_rules(),
+            QueryRule(CheckResults, (JavacCheckRequest,)),
+            QueryRule(FallibleCompiledClassfiles, (CompileJavaSourceRequest,)),
             QueryRule(CompiledClassfiles, (CompileJavaSourceRequest,)),
             QueryRule(CoarsenedTargets, (Addresses,)),
         ],
@@ -118,23 +127,30 @@ def test_compile_no_deps(rule_runner: RuleRunner) -> None:
             "ExampleLib.java": JAVA_LIB_SOURCE,
         }
     )
+    coarsened_target = expect_single_expanded_coarsened_target(
+        rule_runner, Address(spec_path="", target_name="lib")
+    )
 
     compiled_classfiles = rule_runner.request(
         CompiledClassfiles,
-        [
-            CompileJavaSourceRequest(
-                component=expect_single_expanded_coarsened_target(
-                    rule_runner, Address(spec_path="", target_name="lib")
-                )
-            )
-        ],
+        [CompileJavaSourceRequest(component=coarsened_target)],
     )
 
     classfile_digest_contents = rule_runner.request(DigestContents, [compiled_classfiles.digest])
     assert frozenset(content.path for content in classfile_digest_contents) == frozenset(
         ["org/pantsbuild/example/lib/ExampleLib.class"]
     )
 
+    # Additionally validate that `check` works.
+    check_results = rule_runner.request(
+        CheckResults,
+        [JavacCheckRequest([JavacCheckRequest.field_set_type.create(coarsened_target.members[0])])],
+    )
+
+    assert len(check_results.results) == 1
+    check_result = check_results.results[0]
+    assert check_result.partition_description == str(coarsened_target)
+
 
 @pytest.mark.skip(reason="#12293 Coursier JDK bootstrapping is currently flaky in CI")
 def test_compile_jdk_versions(rule_runner: RuleRunner) -> None:
@@ -596,9 +612,9 @@ def test_compile_with_missing_dep_fails(rule_runner: RuleRunner) -> None:
             rule_runner, Address(spec_path="", target_name="main")
         )
     )
-    expected_exception_msg = r".*?package org.pantsbuild.example.lib does not exist.*?"
-    with pytest.raises(ExecutionError, match=expected_exception_msg):
-        rule_runner.request(CompiledClassfiles, [request])
+    fallible_result = rule_runner.request(FallibleCompiledClassfiles, [request])
+    assert fallible_result.result == CompileResult.FAILED and fallible_result.stderr
+    assert "package org.pantsbuild.example.lib does not exist" in fallible_result.stderr
 
 
 def test_compile_with_maven_deps(rule_runner: RuleRunner) -> None:
@@ -710,6 +726,6 @@ def test_compile_with_missing_maven_dep_fails(rule_runner: RuleRunner) -> None:
             rule_runner, Address(spec_path="", target_name="main")
         )
     )
-    expected_exception_msg = r".*?package org.joda.time does not exist.*?"
-    with pytest.raises(ExecutionError, match=expected_exception_msg):
-        rule_runner.request(CompiledClassfiles, [request])
+    fallible_result = rule_runner.request(FallibleCompiledClassfiles, [request])
+    assert fallible_result.result == CompileResult.FAILED and fallible_result.stderr
+    assert "package org.joda.time does not exist" in fallible_result.stderr

src/python/pants/core/goals/check.py

@@ -32,7 +32,7 @@
 
 
 @dataclass(frozen=True)
-class CheckResult(EngineAwareReturnType):
+class CheckResult:
     exit_code: int
     stdout: str
     stderr: str

src/python/pants/engine/target.py

@@ -612,7 +612,7 @@ def expect_single(self) -> Target:
 
 
 @dataclass(frozen=True)
-class CoarsenedTarget:
+class CoarsenedTarget(EngineAwareParameter):
     """A set of Targets which cyclicly reach one another, and are thus indivisable."""
 
     # The members of the cycle.
@@ -623,6 +623,15 @@ class CoarsenedTarget:
     # To expand these dependencies, request `CoarsenedTargets` for them.
     dependencies: FrozenOrderedSet[Address]
 
+    def debug_hint(self) -> str:
+        return str(self)
+
+    def __str__(self) -> str:
+        if len(self.members) > 1:
+            others = len(self.members) - 1
+            return f"{self.members[0].address.spec} (and {others} more)"
+        return self.members[0].address.spec
+
 
 class CoarsenedTargets(Collection[CoarsenedTarget]):
     """A set of direct (not transitive) disjoint CoarsenedTarget instances."""

testprojects/src/java/org/pantsbuild/example/BUILD

@@ -0,0 +1,16 @@
+# Copyright 2021 Pants project contributors (see CONTRIBUTORS.md).
+# Licensed under the Apache License, Version 2.0 (see LICENSE).
+
+java_library(
+    dependencies=[
+      ":lockfile",
+    ],
+)
+
+coursier_lockfile(
+    name = "lockfile",
+    maven_requirements = [],
+    sources = [
+        "coursier_resolve.lockfile",
+    ],
+)