[internal] Go test and dependency inference no longer crash on comp…

…ilation 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

@@ -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(

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(
         {

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):

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`

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))
     )
 
 

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"
+    )