diff --git a/src/python/pants/backend/go/goals/check.py b/src/python/pants/backend/go/goals/check.py index d70f30282b5..b90d779809e 100644 --- a/src/python/pants/backend/go/goals/check.py +++ b/src/python/pants/backend/go/goals/check.py @@ -2,7 +2,6 @@ # Licensed under the Apache License, Version 2.0 (see LICENSE). from dataclasses import dataclass -from typing import cast from pants.backend.go.target_types import GoFirstPartyPackageSourcesField from pants.backend.go.util_rules.build_pkg import ( @@ -15,6 +14,7 @@ from pants.engine.rules import Get, MultiGet, collect_rules, rule from pants.engine.target import FieldSet from pants.engine.unions import UnionRule +from pants.util.logging import LogLevel @dataclass(frozen=True) @@ -28,7 +28,7 @@ class GoCheckRequest(CheckRequest): field_set_type = GoCheckFieldSet -@rule +@rule(desc="Check Go compilation", level=LogLevel.DEBUG) async def check_go(request: GoCheckRequest) -> CheckResults: build_requests = await MultiGet( Get(FallibleBuildGoPackageRequest, BuildGoPackageTargetRequest(field_set.address)) @@ -46,30 +46,16 @@ async def check_go(request: GoCheckRequest) -> CheckResults: Get(FallibleBuiltGoPackage, BuildGoPackageRequest, request) for request in valid_requests ) - # TODO: Update `build_pkg.py` to use streaming workunits to log compilation results, which has - # the benefit of other contexts like `test.py` using it. Switch this to only preserve the - # exit code. - check_results = [ - *( - CheckResult( - result.exit_code, - "", - cast(str, result.stderr), - partition_description=result.import_path, - ) - for result in invalid_requests + # NB: We don't pass stdout/stderr as it will have already been rendered as streaming. + exit_code = next( + ( + result.exit_code # type: ignore[attr-defined] + for result in (*build_results, *invalid_requests) + if result.exit_code != 0 # type: ignore[attr-defined] ), - *( - CheckResult( - result.exit_code, - result.stdout or "", - result.stderr or "", - partition_description=result.import_path, - ) - for result in build_results - ), - ] - return CheckResults(check_results, checker_name="go") + 0, + ) + return CheckResults([CheckResult(exit_code, "", "")], checker_name="go compile") def rules(): diff --git a/src/python/pants/backend/go/goals/check_test.py b/src/python/pants/backend/go/goals/check_test.py index 672b1927158..785e5e7db98 100644 --- a/src/python/pants/backend/go/goals/check_test.py +++ b/src/python/pants/backend/go/goals/check_test.py @@ -77,9 +77,4 @@ def test_check(rule_runner: RuleRunner) -> None: results = rule_runner.request( CheckResults, [GoCheckRequest(GoCheckFieldSet.create(tgt) for tgt in targets)] ).results - assert set(results) == { - CheckResult(0, "", "", "example.com/greeter/good"), - CheckResult( - 1, "", "bad/f.go:1:1: expected 'package', found invalid\n", "example.com/greeter/bad" - ), - } + assert set(results) == {CheckResult(1, "", "")} diff --git a/src/python/pants/backend/go/util_rules/build_pkg.py b/src/python/pants/backend/go/util_rules/build_pkg.py index 9374cdb2a14..b9847467d9e 100644 --- a/src/python/pants/backend/go/util_rules/build_pkg.py +++ b/src/python/pants/backend/go/util_rules/build_pkg.py @@ -26,12 +26,13 @@ from pants.backend.go.util_rules.sdk import GoSdkProcess from pants.backend.go.util_rules.third_party_pkg import ThirdPartyPkgInfo, ThirdPartyPkgInfoRequest from pants.build_graph.address import Address -from pants.engine.engine_aware import EngineAwareParameter +from pants.engine.engine_aware import EngineAwareParameter, EngineAwareReturnType from pants.engine.fs import AddPrefix, Digest, MergeDigests from pants.engine.process import FallibleProcessResult from pants.engine.rules import Get, MultiGet, collect_rules, rule from pants.engine.target import Dependencies, DependenciesRequest, UnexpandedTargets, WrappedTarget from pants.util.frozendict import FrozenDict +from pants.util.logging import LogLevel from pants.util.strutil import path_safe @@ -58,7 +59,7 @@ def debug_hint(self) -> str | None: @dataclass(frozen=True) -class FallibleBuildGoPackageRequest(EngineAwareParameter): +class FallibleBuildGoPackageRequest(EngineAwareParameter, EngineAwareReturnType): """Request to build a package, but fallible if determining the request metadata failed. When creating "synthetic" packages, use `GoPackageRequest` directly. This type is only intended @@ -70,9 +71,25 @@ class FallibleBuildGoPackageRequest(EngineAwareParameter): exit_code: int = 0 stderr: str | None = None + def level(self) -> LogLevel: + return LogLevel.ERROR if self.exit_code != 0 else LogLevel.DEBUG + + def message(self) -> str: + message = self.import_path + message += ( + " succeeded." if self.exit_code == 0 else f" failed (exit code {self.exit_code})." + ) + 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 + @dataclass(frozen=True) -class FallibleBuiltGoPackage: +class FallibleBuiltGoPackage(EngineAwareReturnType): """Fallible version of `BuiltGoPackage` with error details.""" output: BuiltGoPackage | None @@ -81,6 +98,24 @@ class FallibleBuiltGoPackage: stdout: str | None = None stderr: str | None = None + def level(self) -> LogLevel: + return LogLevel.ERROR if self.exit_code != 0 else LogLevel.DEBUG + + def message(self) -> str: + message = self.import_path + 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 + @dataclass(frozen=True) class BuiltGoPackage: @@ -93,7 +128,9 @@ class BuiltGoPackage: import_paths_to_pkg_a_files: FrozenDict[str, str] -@rule +# NB: We must have a description for the streaming of this rule to work properly +# (triggered by `FallibleBuiltGoPackage` subclassing `EngineAwareReturnType`). +@rule(desc="Compile with Go") async def build_go_package(request: BuildGoPackageRequest) -> FallibleBuiltGoPackage: maybe_built_deps = await MultiGet( Get(FallibleBuiltGoPackage, BuildGoPackageRequest, build_request) @@ -227,7 +264,9 @@ def debug_hint(self) -> str: return str(self.address) -@rule +# NB: We must have a description for the streaming of this rule to work properly +# (triggered by `FallibleBuildGoPackageRequest` subclassing `EngineAwareReturnType`). +@rule(desc="Set up Go compilation request") async def setup_build_go_package_target_request( request: BuildGoPackageTargetRequest, ) -> FallibleBuildGoPackageRequest: diff --git a/src/python/pants/backend/java/compile/javac.py b/src/python/pants/backend/java/compile/javac.py index a8c1c636afe..2bd11b29f8e 100644 --- a/src/python/pants/backend/java/compile/javac.py +++ b/src/python/pants/backend/java/compile/javac.py @@ -231,7 +231,7 @@ async def compile_java_source( ) -@rule(desc="Check compilation for javac", level=LogLevel.DEBUG) +@rule(desc="Check javac compilation", 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) @@ -242,20 +242,9 @@ async def javac_check(request: JavacCheckRequest) -> CheckResults: 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", - ) + # NB: We don't pass stdout/stderr as it will have already been rendered as streaming. + exit_code = next((result.exit_code for result in results if result.exit_code != 0), 0) + return CheckResults([CheckResult(exit_code, "", "")], checker_name="javac") def rules(): diff --git a/src/python/pants/backend/java/compile/javac_test.py b/src/python/pants/backend/java/compile/javac_test.py index 8a4278695d9..250b9e35979 100644 --- a/src/python/pants/backend/java/compile/javac_test.py +++ b/src/python/pants/backend/java/compile/javac_test.py @@ -15,7 +15,7 @@ from pants.backend.java.target_types import JavaSourcesGeneratorTarget from pants.backend.java.target_types import rules as target_types_rules from pants.build_graph.address import Address -from pants.core.goals.check import CheckResults +from pants.core.goals.check import CheckResult, CheckResults from pants.core.util_rules import archive, config_files, source_files from pants.core.util_rules.archive import UnzipBinary from pants.core.util_rules.external_tool import rules as external_tool_rules @@ -196,11 +196,8 @@ def test_compile_no_deps(rule_runner: RuleRunner) -> None: [JavacCheckRequest.field_set_type.create(coarsened_target.representative)] ) ], - ) - - assert len(check_results.results) == 1 - check_result = check_results.results[0] - assert check_result.partition_description == str(coarsened_target) + ).results + assert set(check_results) == {CheckResult(0, "", "")} @maybe_skip_jdk_test diff --git a/src/python/pants/jvm/compile.py b/src/python/pants/jvm/compile.py index fb9148d69b1..48c1f0609d2 100644 --- a/src/python/pants/jvm/compile.py +++ b/src/python/pants/jvm/compile.py @@ -71,7 +71,7 @@ def prep_output(s: bytes) -> str: ) def level(self) -> LogLevel: - return LogLevel.ERROR if self.exit_code != 0 else LogLevel.INFO + return LogLevel.ERROR if self.exit_code != 0 else LogLevel.DEBUG def message(self) -> str: message = self.description