From 5a2207a76f5869b22724793b2c2aa1aa16fecac7 Mon Sep 17 00:00:00 2001 From: Eric Arellano <14852634+Eric-Arellano@users.noreply.github.com> Date: Fri, 22 Oct 2021 13:38:51 -0700 Subject: [PATCH] [internal] Go `test` and dependency inference no longer crash on compilation failures (#13334) We were erroring during `go list` if the file was invalid. That needs to be fallible. Also `build_pkg.py` needs to handle if any dependencies failed to compile. [ci skip-rust] [ci skip-build-wheels] --- src/python/pants/backend/go/goals/test.py | 60 ++++++-- .../pants/backend/go/goals/test_test.py | 39 +++++ .../pants/backend/go/target_type_rules.py | 14 +- .../backend/go/target_type_rules_test.py | 8 + .../pants/backend/go/util_rules/assembly.py | 29 ++-- .../util_rules/assembly_integration_test.py | 27 ++++ .../pants/backend/go/util_rules/build_pkg.py | 144 +++++++++++------- .../backend/go/util_rules/build_pkg_test.py | 103 ++++++++++++- .../backend/go/util_rules/first_party_pkg.py | 27 +++- .../go/util_rules/first_party_pkg_test.py | 37 ++++- 10 files changed, 397 insertions(+), 91 deletions(-) diff --git a/src/python/pants/backend/go/goals/test.py b/src/python/pants/backend/go/goals/test.py index 49a2308b65e..2e76eb29785 100644 --- a/src/python/pants/backend/go/goals/test.py +++ b/src/python/pants/backend/go/goals/test.py @@ -11,9 +11,13 @@ from pants.backend.go.util_rules.build_pkg import ( BuildGoPackageRequest, BuildGoPackageTargetRequest, - BuiltGoPackage, + FallibleBuildGoPackageRequest, + FallibleBuiltGoPackage, +) +from pants.backend.go.util_rules.first_party_pkg import ( + FallibleFirstPartyPkgInfo, + FirstPartyPkgInfoRequest, ) -from pants.backend.go.util_rules.first_party_pkg import FirstPartyPkgInfo, FirstPartyPkgInfoRequest from pants.backend.go.util_rules.import_analysis import ImportConfig, ImportConfigRequest from pants.backend.go.util_rules.link import LinkedGoBinary, LinkGoBinaryRequest from pants.backend.go.util_rules.tests_analysis import GeneratedTestMain, GenerateTestMainRequest @@ -113,11 +117,24 @@ def transform_test_args(args: Sequence[str]) -> tuple[str, ...]: async def run_go_tests( field_set: GoTestFieldSet, test_subsystem: TestSubsystem, go_test_subsystem: GoTestSubsystem ) -> TestResult: - pkg_info, wrapped_target = await MultiGet( - Get(FirstPartyPkgInfo, FirstPartyPkgInfoRequest(field_set.address)), + maybe_pkg_info, wrapped_target = await MultiGet( + Get(FallibleFirstPartyPkgInfo, FirstPartyPkgInfoRequest(field_set.address)), Get(WrappedTarget, Address, field_set.address), ) + if maybe_pkg_info.info is None: + assert maybe_pkg_info.stderr is not None + return TestResult( + exit_code=maybe_pkg_info.exit_code, + stdout="", + stderr=maybe_pkg_info.stderr, + stdout_digest=EMPTY_FILE_DIGEST, + stderr_digest=EMPTY_FILE_DIGEST, + address=field_set.address, + output_setting=test_subsystem.output, + ) + pkg_info = maybe_pkg_info.info + target = wrapped_target.target import_path = target[GoImportPathField].value @@ -141,17 +158,30 @@ async def run_go_tests( return TestResult( exit_code=0, stdout="", - stdout_digest=EMPTY_FILE_DIGEST, stderr="", + stdout_digest=EMPTY_FILE_DIGEST, stderr_digest=EMPTY_FILE_DIGEST, address=field_set.address, output_setting=test_subsystem.output, ) # Construct the build request for the package under test. - test_pkg_build_request = await Get( - BuildGoPackageRequest, BuildGoPackageTargetRequest(field_set.address, for_tests=True) + maybe_test_pkg_build_request = await Get( + FallibleBuildGoPackageRequest, + BuildGoPackageTargetRequest(field_set.address, for_tests=True), ) + if maybe_test_pkg_build_request.request is None: + assert maybe_test_pkg_build_request.stderr is not None + return TestResult( + exit_code=maybe_test_pkg_build_request.exit_code, + stdout="", + stderr=maybe_test_pkg_build_request.stderr, + stdout_digest=EMPTY_FILE_DIGEST, + stderr_digest=EMPTY_FILE_DIGEST, + address=field_set.address, + output_setting=test_subsystem.output, + ) + test_pkg_build_request = maybe_test_pkg_build_request.request main_direct_deps = [test_pkg_build_request] if testmain.has_xtests: @@ -183,8 +213,8 @@ async def run_go_tests( main_direct_deps.append(xtest_pkg_build_request) # Generate the synthetic main package which imports the test and/or xtest packages. - built_main_pkg = await Get( - BuiltGoPackage, + maybe_built_main_pkg = await Get( + FallibleBuiltGoPackage, BuildGoPackageRequest( import_path="main", digest=testmain.digest, @@ -194,6 +224,18 @@ async def run_go_tests( direct_dependencies=tuple(main_direct_deps), ), ) + if maybe_built_main_pkg.output is None: + assert maybe_built_main_pkg.stderr is not None + return TestResult( + exit_code=maybe_built_main_pkg.exit_code, + stdout="", + stderr=maybe_built_main_pkg.stderr, + stdout_digest=EMPTY_FILE_DIGEST, + stderr_digest=EMPTY_FILE_DIGEST, + address=field_set.address, + output_setting=test_subsystem.output, + ) + built_main_pkg = maybe_built_main_pkg.output main_pkg_a_file_path = built_main_pkg.import_paths_to_pkg_a_files["main"] import_config = await Get( diff --git a/src/python/pants/backend/go/goals/test_test.py b/src/python/pants/backend/go/goals/test_test.py index c079bc7200b..ce04ccdfeb2 100644 --- a/src/python/pants/backend/go/goals/test_test.py +++ b/src/python/pants/backend/go/goals/test_test.py @@ -205,6 +205,45 @@ def test_internal_test_with_test_main(rule_runner: RuleRunner) -> None: assert "FAIL: TestAdd" in result.stdout +def test_internal_test_fails_to_compile(rule_runner: RuleRunner) -> None: + """A compilation failure should not cause Pants to error, only the test to fail.""" + rule_runner.write_files( + { + "foo/BUILD": "go_mod()", + "foo/go.mod": "module foo", + # Test itself is bad. + "foo/bad_test.go": "invalid!!!", + # A dependency of the test is bad. + "foo/dep/f.go": "invalid!!!", + "foo/uses_dep/f_test.go": textwrap.dedent( + """ + package uses_dep + + import ( + "foo/dep" + "testing" + ) + + func TestAdd(t *testing.T) { + if add(2, 3) != 5 { + t.Fail() + } + } + """ + ), + } + ) + tgt = rule_runner.get_target(Address("foo", generated_name="./")) + result = rule_runner.request(TestResult, [GoTestFieldSet.create(tgt)]) + assert result.exit_code == 1 + assert result.stderr == "bad_test.go:1:1: expected 'package', found invalid\n" + + tgt = rule_runner.get_target(Address("foo", generated_name="./uses_dep")) + result = rule_runner.request(TestResult, [GoTestFieldSet.create(tgt)]) + assert result.exit_code == 1 + assert result.stderr == "dep/f.go:1:1: expected 'package', found invalid\n" + + def test_external_test_success(rule_runner: RuleRunner) -> None: rule_runner.write_files( { diff --git a/src/python/pants/backend/go/target_type_rules.py b/src/python/pants/backend/go/target_type_rules.py index e2ae3f961aa..8d698bef3d4 100644 --- a/src/python/pants/backend/go/target_type_rules.py +++ b/src/python/pants/backend/go/target_type_rules.py @@ -25,7 +25,10 @@ GoThirdPartyPackageTarget, ) from pants.backend.go.util_rules import first_party_pkg, import_analysis -from pants.backend.go.util_rules.first_party_pkg import FirstPartyPkgInfo, FirstPartyPkgInfoRequest +from pants.backend.go.util_rules.first_party_pkg import ( + FallibleFirstPartyPkgInfo, + FirstPartyPkgInfoRequest, +) from pants.backend.go.util_rules.go_mod import GoModInfo, GoModInfoRequest from pants.backend.go.util_rules.import_analysis import GoStdLibImports from pants.backend.go.util_rules.third_party_pkg import ( @@ -97,7 +100,14 @@ async def infer_go_dependencies( package_mapping: ImportPathToPackages, ) -> InferredDependencies: addr = request.sources_field.address - pkg_info = await Get(FirstPartyPkgInfo, FirstPartyPkgInfoRequest(addr)) + maybe_pkg_info = await Get(FallibleFirstPartyPkgInfo, FirstPartyPkgInfoRequest(addr)) + if maybe_pkg_info.info is None: + logger.error( + f"Failed to analyze {maybe_pkg_info.import_path} for dependency inference:\n" + f"{maybe_pkg_info.stderr}" + ) + return InferredDependencies([]) + pkg_info = maybe_pkg_info.info inferred_dependencies = [] for import_path in (*pkg_info.imports, *pkg_info.test_imports, *pkg_info.xtest_imports): diff --git a/src/python/pants/backend/go/target_type_rules_test.py b/src/python/pants/backend/go/target_type_rules_test.py index b21e005b0df..e746e17534c 100644 --- a/src/python/pants/backend/go/target_type_rules_test.py +++ b/src/python/pants/backend/go/target_type_rules_test.py @@ -110,6 +110,7 @@ def test_go_package_dependency_inference(rule_runner: RuleRunner) -> None: fmt.Printf("%s\n", pkg.Grok()) }""" ), + "foo/bad/f.go": "invalid!!!", } ) tgt1 = rule_runner.get_target(Address("foo", generated_name="./cmd")) @@ -128,6 +129,13 @@ def test_go_package_dependency_inference(rule_runner: RuleRunner) -> None: [Address("foo", generated_name="github.com/google/go-cmp/cmp")] ) + # Compilation failures should not blow up Pants. + bad_tgt = rule_runner.get_target(Address("foo", generated_name="./bad")) + assert not rule_runner.request( + InferredDependencies, + [InferGoPackageDependenciesRequest(bad_tgt[GoFirstPartyPackageSourcesField])], + ) + # ----------------------------------------------------------------------------------------------- # Generate package targets from `go_mod` diff --git a/src/python/pants/backend/go/util_rules/assembly.py b/src/python/pants/backend/go/util_rules/assembly.py index 383647072e2..69d33dc9fec 100644 --- a/src/python/pants/backend/go/util_rules/assembly.py +++ b/src/python/pants/backend/go/util_rules/assembly.py @@ -16,11 +16,10 @@ @dataclass(frozen=True) class FallibleAssemblyPreCompilation: - results: tuple[FallibleProcessResult, ...] - output: AssemblyPreCompilation | None - - def has_failures(self) -> bool: - return any(result.exit_code != 0 for result in self.results) + result: AssemblyPreCompilation | None + exit_code: int = 0 + stdout: str | None = None + stderr: str | None = None @dataclass(frozen=True) @@ -92,7 +91,10 @@ async def setup_assembly_pre_compilation( ), ) if symabis_result.exit_code != 0: - return FallibleAssemblyPreCompilation((symabis_result,), None) + return FallibleAssemblyPreCompilation( + None, symabis_result.exit_code, symabis_result.stderr.decode("utf-8") + ) + merged = await Get( Digest, MergeDigests([request.compilation_input, symabis_result.output_digest]), @@ -107,7 +109,7 @@ async def setup_assembly_pre_compilation( "tool", "asm", "-I", - str(PurePath(goroot.path, "pkg", "include")), + os.path.join(goroot.path, "pkg", "include"), "-o", f"./{request.source_files_subpath}/{PurePath(s_file).with_suffix('.o')}", f"./{request.source_files_subpath}/{s_file}", @@ -120,9 +122,18 @@ async def setup_assembly_pre_compilation( ) for s_file in request.s_files ) + exit_code = max(result.exit_code for result in assembly_results) + if exit_code != 0: + stdout = "\n\n".join( + result.stdout.decode("utf-8") for result in assembly_results if result.stdout + ) + stderr = "\n\n".join( + result.stderr.decode("utf-8") for result in assembly_results if result.stderr + ) + return FallibleAssemblyPreCompilation(None, exit_code, stdout, stderr) + return FallibleAssemblyPreCompilation( - tuple(assembly_results), - AssemblyPreCompilation(merged, tuple(result.output_digest for result in assembly_results)), + AssemblyPreCompilation(merged, tuple(result.output_digest for result in assembly_results)) ) diff --git a/src/python/pants/backend/go/util_rules/assembly_integration_test.py b/src/python/pants/backend/go/util_rules/assembly_integration_test.py index 0590bd20702..81486c3e764 100644 --- a/src/python/pants/backend/go/util_rules/assembly_integration_test.py +++ b/src/python/pants/backend/go/util_rules/assembly_integration_test.py @@ -23,6 +23,7 @@ sdk, third_party_pkg, ) +from pants.backend.go.util_rules.build_pkg import BuildGoPackageRequest, FallibleBuiltGoPackage from pants.core.goals.package import BuiltPackage from pants.engine.addresses import Address from pants.engine.rules import QueryRule @@ -45,6 +46,7 @@ def rule_runner() -> RuleRunner: *third_party_pkg.rules(), *sdk.rules(), QueryRule(BuiltPackage, (GoBinaryFieldSet,)), + QueryRule(FallibleBuiltGoPackage, (BuildGoPackageRequest,)), ], target_types=[GoBinaryTarget, GoModTarget], ) @@ -123,3 +125,28 @@ def test_build_package_with_assembly(rule_runner: RuleRunner) -> None: result = subprocess.run([os.path.join(rule_runner.build_root, "bin")], stdout=subprocess.PIPE) assert result.returncode == 0 assert result.stdout == b"3\n" + + +def test_build_invalid_package(rule_runner: RuleRunner) -> None: + request = BuildGoPackageRequest( + import_path="example.com/assembly", + subpath="", + go_file_names=("add_amd64.go", "add_arm64.go"), + digest=rule_runner.make_snapshot( + { + "add_amd64.go": "package main\nfunc add(x, y int64) int64", + "add_arm64.go": "package main\nfunc add(x, y int64) int64", + "add_amd64.s": "INVALID!!!", + "add_arm64.s": "INVALID!!!", + } + ).digest, + s_file_names=("add_amd64.s", "add_arm64.s"), + direct_dependencies=(), + ) + result = rule_runner.request(FallibleBuiltGoPackage, [request]) + assert result.output is None + assert result.exit_code == 1 + assert ( + result.stdout + == ".//add_amd64.s:1: unexpected EOF\nasm: assembly of .//add_amd64.s failed\n" + ) 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 566524bc9e1..4e1b19e39ae 100644 --- a/src/python/pants/backend/go/util_rules/build_pkg.py +++ b/src/python/pants/backend/go/util_rules/build_pkg.py @@ -18,7 +18,10 @@ AssemblyPreCompilationRequest, FallibleAssemblyPreCompilation, ) -from pants.backend.go.util_rules.first_party_pkg import FirstPartyPkgInfo, FirstPartyPkgInfoRequest +from pants.backend.go.util_rules.first_party_pkg import ( + FallibleFirstPartyPkgInfo, + FirstPartyPkgInfoRequest, +) from pants.backend.go.util_rules.go_mod import GoModInfo, GoModInfoRequest from pants.backend.go.util_rules.import_analysis import ImportConfig, ImportConfigRequest from pants.backend.go.util_rules.sdk import GoSdkProcess @@ -56,45 +59,30 @@ def debug_hint(self) -> str | None: return self.import_path +@dataclass(frozen=True) +class FallibleBuildGoPackageRequest(EngineAwareParameter): + """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 + for determining the package metadata of user code, which may fail to be analyzed. + """ + + request: BuildGoPackageRequest | None + import_path: str + exit_code: int = 0 + stderr: str | None = None + + @dataclass(frozen=True) class FallibleBuiltGoPackage: - """Fallible version of `BuiltGoPackage with error details.""" + """Fallible version of `BuiltGoPackage` with error details.""" output: BuiltGoPackage | None - exit_code: int + import_path: str + exit_code: int = 0 stdout: str | None = None stderr: str | None = None - @classmethod - def from_fallible_process_result( - cls, - process_result: FallibleProcessResult, - output: BuiltGoPackage | None, - ) -> FallibleBuiltGoPackage: - return cls( - output=output, - exit_code=process_result.exit_code, - stdout=process_result.stdout.decode("utf-8"), - stderr=process_result.stderr.decode("utf-8"), - ) - - @classmethod - def from_fallible_process_results( - cls, - process_results: tuple[FallibleProcessResult, ...], - output: BuiltGoPackage | None, - ) -> FallibleBuiltGoPackage: - return cls( - output=output, - exit_code=max(process_result.exit_code for process_result in process_results), - stdout="\n".join( - process_result.stdout.decode("utf-8") for process_result in process_results - ), - stderr="\n".join( - process_result.stderr.decode("utf-8") for process_result in process_results - ), - ) - @dataclass(frozen=True) class BuiltGoPackage: @@ -109,14 +97,17 @@ class BuiltGoPackage: @rule async def build_go_package(request: BuildGoPackageRequest) -> FallibleBuiltGoPackage: - built_deps = await MultiGet( - Get(BuiltGoPackage, BuildGoPackageRequest, build_request) + maybe_built_deps = await MultiGet( + Get(FallibleBuiltGoPackage, BuildGoPackageRequest, build_request) for build_request in request.direct_dependencies ) import_paths_to_pkg_a_files: dict[str, str] = {} dep_digests = [] - for dep in built_deps: + for maybe_dep in maybe_built_deps: + if maybe_dep.output is None: + return maybe_dep + dep = maybe_dep.output import_paths_to_pkg_a_files.update(dep.import_paths_to_pkg_a_files) dep_digests.append(dep.digest) @@ -136,13 +127,16 @@ async def build_go_package(request: BuildGoPackageRequest) -> FallibleBuiltGoPac FallibleAssemblyPreCompilation, AssemblyPreCompilationRequest(input_digest, request.s_file_names, request.subpath), ) - if assembly_setup.has_failures(): - return FallibleBuiltGoPackage.from_fallible_process_results( - assembly_setup.results, None + if assembly_setup.result is None: + return FallibleBuiltGoPackage( + None, + request.import_path, + assembly_setup.exit_code, + stdout=assembly_setup.stdout, + stderr=assembly_setup.stderr, ) - assert assembly_setup.output - input_digest = assembly_setup.output.merged_compilation_input_digest - assembly_digests = assembly_setup.output.assembly_digests + input_digest = assembly_setup.result.merged_compilation_input_digest + assembly_digests = assembly_setup.result.assembly_digests symabis_path = "./symabis" compile_args = [ @@ -173,7 +167,13 @@ async def build_go_package(request: BuildGoPackageRequest) -> FallibleBuiltGoPac ), ) if compile_result.exit_code != 0: - return FallibleBuiltGoPackage.from_fallible_process_result(compile_result, None) + return FallibleBuiltGoPackage( + None, + request.import_path, + compile_result.exit_code, + stdout=compile_result.stdout.decode("utf-8"), + stderr=compile_result.stderr.decode("utf-8"), + ) compilation_digest = compile_result.output_digest if assembly_digests: @@ -187,7 +187,13 @@ async def build_go_package(request: BuildGoPackageRequest) -> FallibleBuiltGoPac ), ) if assembly_result.result.exit_code != 0: - return FallibleBuiltGoPackage.from_fallible_process_result(assembly_result.result, None) + return FallibleBuiltGoPackage( + None, + request.import_path, + assembly_result.result.exit_code, + stdout=assembly_result.result.stdout.decode("utf-8"), + stderr=assembly_result.result.stderr.decode("utf-8"), + ) assert assembly_result.merged_output_digest compilation_digest = assembly_result.merged_output_digest @@ -197,17 +203,16 @@ async def build_go_package(request: BuildGoPackageRequest) -> FallibleBuiltGoPac merged_result_digest = await Get(Digest, MergeDigests([*dep_digests, output_digest])) output = BuiltGoPackage(merged_result_digest, FrozenDict(import_paths_to_pkg_a_files)) - return FallibleBuiltGoPackage.from_fallible_process_result(compile_result, output) + return FallibleBuiltGoPackage(output, request.import_path) @rule def required_built_go_package(fallible_result: FallibleBuiltGoPackage) -> BuiltGoPackage: - if fallible_result.exit_code == 0: - assert fallible_result.output + if fallible_result.output is not None: return fallible_result.output - # TODO(12927): Wire up to streaming workunit system to log compilation results. raise Exception( - f"Compile failed:\nstdout:\n{fallible_result.stdout}\nstderr:\n{fallible_result.stderr}" + f"Failed to compile {fallible_result.import_path}:\n" + f"{fallible_result.stdout}\n{fallible_result.stderr}" ) @@ -227,15 +232,24 @@ def debug_hint(self) -> str: @rule async def setup_build_go_package_target_request( request: BuildGoPackageTargetRequest, -) -> BuildGoPackageRequest: +) -> FallibleBuildGoPackageRequest: wrapped_target = await Get(WrappedTarget, Address, request.address) target = wrapped_target.target import_path = target[GoImportPathField].value if target.has_field(GoFirstPartyPackageSourcesField): - _first_party_pkg_info = await Get( - FirstPartyPkgInfo, FirstPartyPkgInfoRequest(target.address) + _maybe_first_party_pkg_info = await Get( + FallibleFirstPartyPkgInfo, FirstPartyPkgInfoRequest(target.address) ) + if _maybe_first_party_pkg_info.info is None: + return FallibleBuildGoPackageRequest( + None, + import_path, + exit_code=_maybe_first_party_pkg_info.exit_code, + stderr=_maybe_first_party_pkg_info.stderr, + ) + _first_party_pkg_info = _maybe_first_party_pkg_info.info + digest = _first_party_pkg_info.digest subpath = os.path.join( target.address.spec_path, target[GoFirstPartyPackageSubpathField].value @@ -278,24 +292,42 @@ async def setup_build_go_package_target_request( # TODO: If you use `Targets` here, then we replace the direct dep on the `go_mod` with all # of its generated targets...Figure this out. all_deps = await Get(UnexpandedTargets, DependenciesRequest(target[Dependencies])) - direct_dependencies = await MultiGet( - Get(BuildGoPackageRequest, BuildGoPackageTargetRequest(tgt.address)) + maybe_direct_dependencies = await MultiGet( + Get(FallibleBuildGoPackageRequest, BuildGoPackageTargetRequest(tgt.address)) for tgt in all_deps if ( tgt.has_field(GoFirstPartyPackageSourcesField) or tgt.has_field(GoThirdPartyModulePathField) ) ) + direct_dependencies = [] + for maybe_dep in maybe_direct_dependencies: + if maybe_dep.request is None: + return maybe_dep + direct_dependencies.append(maybe_dep.request) - return BuildGoPackageRequest( + result = BuildGoPackageRequest( digest=digest, import_path="main" if request.is_main else import_path, subpath=subpath, go_file_names=go_file_names, s_file_names=s_file_names, - direct_dependencies=direct_dependencies, + direct_dependencies=tuple(direct_dependencies), for_tests=request.for_tests, ) + return FallibleBuildGoPackageRequest(result, import_path) + + +@rule +def required_build_go_package_request( + fallible_request: FallibleBuildGoPackageRequest, +) -> BuildGoPackageRequest: + if fallible_request.request is not None: + return fallible_request.request + raise Exception( + f"Failed to determine metadata to compile {fallible_request.import_path}:\n" + f"{fallible_request.stderr}" + ) def rules(): diff --git a/src/python/pants/backend/go/util_rules/build_pkg_test.py b/src/python/pants/backend/go/util_rules/build_pkg_test.py index 4a9aa4b3143..935d41bd3ea 100644 --- a/src/python/pants/backend/go/util_rules/build_pkg_test.py +++ b/src/python/pants/backend/go/util_rules/build_pkg_test.py @@ -23,6 +23,8 @@ BuildGoPackageRequest, BuildGoPackageTargetRequest, BuiltGoPackage, + FallibleBuildGoPackageRequest, + FallibleBuiltGoPackage, ) from pants.engine.addresses import Address from pants.engine.fs import Snapshot @@ -44,7 +46,9 @@ def rule_runner() -> RuleRunner: *third_party_pkg.rules(), *target_type_rules.rules(), QueryRule(BuiltGoPackage, [BuildGoPackageRequest]), + QueryRule(FallibleBuiltGoPackage, [BuildGoPackageRequest]), QueryRule(BuildGoPackageRequest, [BuildGoPackageTargetRequest]), + QueryRule(FallibleBuildGoPackageRequest, [BuildGoPackageTargetRequest]), ], target_types=[GoModTarget], ) @@ -153,6 +157,55 @@ def test_build_pkg(rule_runner: RuleRunner) -> None: ) +def test_build_invalid_pkg(rule_runner: RuleRunner) -> None: + invalid_dep = BuildGoPackageRequest( + import_path="example.com/foo/dep", + subpath="dep", + go_file_names=("f.go",), + digest=rule_runner.make_snapshot({"dep/f.go": "invalid!!!"}).digest, + s_file_names=(), + direct_dependencies=(), + ) + main = BuildGoPackageRequest( + import_path="example.com/foo", + subpath="", + go_file_names=("f.go",), + digest=rule_runner.make_snapshot( + { + "f.go": dedent( + """\ + package foo + + import "example.com/foo/dep" + + func main() { + dep.Quote("Hello world!") + } + """ + ) + } + ).digest, + s_file_names=(), + direct_dependencies=(invalid_dep,), + ) + + invalid_direct_result = rule_runner.request(FallibleBuiltGoPackage, [invalid_dep]) + assert invalid_direct_result.output is None + assert invalid_direct_result.exit_code == 1 + assert ( + invalid_direct_result.stdout + == "./dep/f.go:1:1: syntax error: package statement must be first\n" + ) + + invalid_dep_result = rule_runner.request(FallibleBuiltGoPackage, [main]) + assert invalid_dep_result.output is None + assert invalid_dep_result.exit_code == 1 + assert ( + invalid_dep_result.stdout + == "./dep/f.go:1:1: syntax error: package statement must be first\n" + ) + + def assert_pkg_target_built( rule_runner: RuleRunner, addr: Address, @@ -166,7 +219,6 @@ def assert_pkg_target_built( build_request = rule_runner.request(BuildGoPackageRequest, [BuildGoPackageTargetRequest(addr)]) assert build_request.import_path == expected_import_path assert build_request.subpath == expected_subpath - print(build_request.go_file_names) assert build_request.go_file_names == tuple(expected_go_file_names) assert not build_request.s_file_names assert [ @@ -317,11 +369,7 @@ def test_build_target_with_dependencies(rule_runner: RuleRunner) -> None: golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= """ ), - "BUILD": dedent( - """\ - go_mod(name='mod') - """ - ), + "BUILD": "go_mod(name='mod')", } ) @@ -390,3 +438,46 @@ def test_build_target_with_dependencies(rule_runner: RuleRunner) -> None: xerrors_internal_import_path, ], ) + + +def test_build_invalid_target(rule_runner: RuleRunner) -> None: + rule_runner.write_files( + { + "go.mod": dedent( + """\ + module example.com/greeter + go 1.17 + """ + ), + "BUILD": "go_mod(name='mod')", + "direct/f.go": "invalid!!!", + "dep/f.go": "invalid!!!", + "uses_dep/f.go": dedent( + """\ + package uses_dep + + import "example.com/greeter/dep" + + func Hello() { + dep.Foo("Hello world!") + } + """ + ), + } + ) + + direct_build_request = rule_runner.request( + FallibleBuildGoPackageRequest, + [BuildGoPackageTargetRequest(Address("", target_name="mod", generated_name="./direct"))], + ) + assert direct_build_request.request is None + assert direct_build_request.exit_code == 1 + assert direct_build_request.stderr == "direct/f.go:1:1: expected 'package', found invalid\n" + + dep_build_request = rule_runner.request( + FallibleBuildGoPackageRequest, + [BuildGoPackageTargetRequest(Address("", target_name="mod", generated_name="./uses_dep"))], + ) + assert dep_build_request.request is None + assert dep_build_request.exit_code == 1 + assert dep_build_request.stderr == "dep/f.go:1:1: expected 'package', found invalid\n" diff --git a/src/python/pants/backend/go/util_rules/first_party_pkg.py b/src/python/pants/backend/go/util_rules/first_party_pkg.py index f12d9820d20..22b82c20d4f 100644 --- a/src/python/pants/backend/go/util_rules/first_party_pkg.py +++ b/src/python/pants/backend/go/util_rules/first_party_pkg.py @@ -18,7 +18,7 @@ from pants.build_graph.address import Address from pants.engine.engine_aware import EngineAwareParameter from pants.engine.fs import Digest, MergeDigests -from pants.engine.process import ProcessResult +from pants.engine.process import FallibleProcessResult from pants.engine.rules import Get, MultiGet, collect_rules, rule from pants.engine.target import HydratedSources, HydrateSourcesRequest, WrappedTarget @@ -49,6 +49,16 @@ class FirstPartyPkgInfo: s_files: tuple[str, ...] +@dataclass(frozen=True) +class FallibleFirstPartyPkgInfo: + """Info needed to build a first-party Go package, but fallible if `go list` failed.""" + + info: FirstPartyPkgInfo | None + import_path: str + exit_code: int = 0 + stderr: str | None = None + + @dataclass(frozen=True) class FirstPartyPkgInfoRequest(EngineAwareParameter): address: Address @@ -60,7 +70,7 @@ def debug_hint(self) -> str: @rule async def compute_first_party_package_info( request: FirstPartyPkgInfoRequest, -) -> FirstPartyPkgInfo: +) -> FallibleFirstPartyPkgInfo: go_mod_address = request.address.maybe_convert_to_target_generator() wrapped_target, go_mod_info = await MultiGet( Get(WrappedTarget, Address, request.address), @@ -78,7 +88,7 @@ async def compute_first_party_package_info( ) result = await Get( - ProcessResult, + FallibleProcessResult, GoSdkProcess( input_digest=input_digest, command=("list", "-json", f"./{subpath}"), @@ -86,6 +96,14 @@ async def compute_first_party_package_info( working_dir=request.address.spec_path, ), ) + if result.exit_code != 0: + return FallibleFirstPartyPkgInfo( + info=None, + import_path=import_path, + exit_code=result.exit_code, + stderr=result.stderr.decode("utf-8"), + ) + metadata = json.loads(result.stdout) if "CgoFiles" in metadata: @@ -96,7 +114,7 @@ async def compute_first_party_package_info( "prioritize adding support." ) - return FirstPartyPkgInfo( + info = FirstPartyPkgInfo( digest=pkg_sources.snapshot.digest, subpath=os.path.join(target.address.spec_path, subpath), import_path=import_path, @@ -108,6 +126,7 @@ async def compute_first_party_package_info( xtest_files=tuple(metadata.get("XTestGoFiles", [])), s_files=tuple(metadata.get("SFiles", [])), ) + return FallibleFirstPartyPkgInfo(info, import_path) def rules(): diff --git a/src/python/pants/backend/go/util_rules/first_party_pkg_test.py b/src/python/pants/backend/go/util_rules/first_party_pkg_test.py index 360853cbffd..195df14940a 100644 --- a/src/python/pants/backend/go/util_rules/first_party_pkg_test.py +++ b/src/python/pants/backend/go/util_rules/first_party_pkg_test.py @@ -10,7 +10,10 @@ from pants.backend.go import target_type_rules from pants.backend.go.target_types import GoModTarget from pants.backend.go.util_rules import first_party_pkg, go_mod, sdk, third_party_pkg -from pants.backend.go.util_rules.first_party_pkg import FirstPartyPkgInfo, FirstPartyPkgInfoRequest +from pants.backend.go.util_rules.first_party_pkg import ( + FallibleFirstPartyPkgInfo, + FirstPartyPkgInfoRequest, +) from pants.engine.addresses import Address from pants.engine.fs import PathGlobs, Snapshot from pants.engine.rules import QueryRule @@ -26,7 +29,7 @@ def rule_runner() -> RuleRunner: *sdk.rules(), *third_party_pkg.rules(), *target_type_rules.rules(), - QueryRule(FirstPartyPkgInfo, [FirstPartyPkgInfoRequest]), + QueryRule(FallibleFirstPartyPkgInfo, [FirstPartyPkgInfoRequest]), ], target_types=[GoModTarget], ) @@ -84,10 +87,12 @@ def assert_info( test_files: list[str], xtest_files: list[str], ) -> None: - info = rule_runner.request( - FirstPartyPkgInfo, + maybe_info = rule_runner.request( + FallibleFirstPartyPkgInfo, [FirstPartyPkgInfoRequest(Address("foo", generated_name=f"./{subpath}"))], ) + assert maybe_info.info is not None + info = maybe_info.info actual_snapshot = rule_runner.request(Snapshot, [info.digest]) expected_snapshot = rule_runner.request(Snapshot, [PathGlobs([f"foo/{subpath}/*.go"])]) assert actual_snapshot == expected_snapshot @@ -120,6 +125,28 @@ def assert_info( ) +def test_invalid_package(rule_runner) -> None: + rule_runner.write_files( + { + "BUILD": "go_mod(name='mod')\n", + "go.mod": dedent( + """\ + module go.example.com/foo + go 1.17 + """ + ), + "bad.go": "invalid!!!", + } + ) + maybe_info = rule_runner.request( + FallibleFirstPartyPkgInfo, + [FirstPartyPkgInfoRequest(Address("", target_name="mod", generated_name="./"))], + ) + assert maybe_info.info is None + assert maybe_info.exit_code == 1 + assert maybe_info.stderr == "bad.go:1:1: expected 'package', found invalid\n" + + def test_cgo_not_supported(rule_runner: RuleRunner) -> None: rule_runner.write_files( { @@ -151,6 +178,6 @@ def test_cgo_not_supported(rule_runner: RuleRunner) -> None: ) with engine_error(NotImplementedError): rule_runner.request( - FirstPartyPkgInfo, + FallibleFirstPartyPkgInfo, [FirstPartyPkgInfoRequest(Address("", target_name="mod", generated_name="./"))], )