use_coverage causes "Can only merge Directories with no duplicates" error for Go packages with _test packaged files

[nickbruun]

Describe the bug When use_coverage is enabled, Go packages that have <package>_test packaged test files (ie. for the package foo, the tests are in the same folder but are in the foo_test package and import foo, see reproduction repository for an example) causes an IntrinsicError of the following shape when the test goal is run:

IntrinsicError: Can only merge Directories with no duplicates, but found 2 duplicate entries in __pkgs__/foo_pkg_repro:

`__pkg__.a`: 1.) file digest=58ba11fe1afd8ddcccdd2a69c32f391ab4314b10430c13dfc8e00501f2368b5f size=1960:

<snip>

`__pkg__.a`: 2.) file digest=085a4521ca0c1cca011a41fc59dd11487677270df92568eead5cd16548eeb5da size=8798:

<snip>

A simple reproduction case can be found at nickbruun/pants-use-coverage-repro. The repository has two commits:

1. https://github.com/nickbruun/pants-use-coverage-repro/commit/c3ca0a0f715725cbe143a42451cf58b8248b7031: With use_coverage enabled. Failure can be seen in GitHub Actions log
2. https://github.com/nickbruun/pants-use-coverage-repro/commit/0b7a49d4a1f99668ebe881eb5bcd263a155ae647: With use_coverage disabled. The successful test run can be seen in GitHub Actions log.

Pants version 2.21.1, but confirmed at least as far back as 2.17 (I just hadn't gotten around to reporting it until now -- sorry!)

OS Reproduced on Linux and MacOS.

Additional info See https://github.com/nickbruun/pants-use-coverage-repro/pull/1 for evidence that if the test has the non-_test package, the test goal succeeds.

The following addition to src/python/pants/backend/go/goals/test_test.py will reproduce the issue in tests:

def test_external_test_with_use_coverage(rule_runner: RuleRunner) -> None:
    rule_runner.write_files(
        {
            "foo/BUILD": "go_mod(name='mod')\ngo_package()",
            "foo/go.mod": "module foo",
            "foo/add.go": textwrap.dedent(
                """
                package foo
                func Add(x, y int) int {
                  return x + y
                }
                """
            ),
            "foo/add_test.go": textwrap.dedent(
                """
                package foo_test
                import (
                  "foo"
                  "testing"
                )
                func TestAdd(t *testing.T) {
                  if foo.Add(2, 3) != 5 {
                    t.Fail()
                  }
                }
                """
            ),
        }
    )
    tgt = rule_runner.get_target(Address("foo", generated_name="./"))
    rule_runner.set_options(
        [
            "--test-use-coverage",
        ],
        env_inherit={"PATH"},
    )
    result = rule_runner.request(
        TestResult, [GoTestRequest.Batch("", (GoTestFieldSet.create(tgt),), None)]
    )
    assert result.exit_code == 0

[huonw]

Sorry for the trouble and thanks for the reproducer and test case!

Is this something you're interested in trying to fix?

[nickbruun]

I'll likely give it a swing some evening when I have a minute, but if someone has any quick pointers as to why duplicate libraries are being built, I'm all ears!

[huonw]

Great. Maybe @tdyas or @tgolsson might have a hint or two? (I'm not personally familiar with the Go backend or Go in general.)

[tdyas]

What version of Go are you using?

[nickbruun]

@tdyas I reproduced this in 1.21, 1.22 and now 1.23.

[tdyas]

Given the test is an external test (xtest), I wonder if Pants is generating the package archives for the package under test and the external test package at the same location with each digest.

[tdyas]

I added some debugging to https://github.com/pantsbuild/pants/blob/1d1e93edcdf617c651c3eb1d1cbadd29d99172b2/src/python/pants/backend/go/util_rules/build_pkg.py#L518 and found that the foo package is showing up multiple times in the dependencies. The difference is that one version is built with coverage and the other version is built without coverage.

This change fixes this bug (although I still need to test if it has consequences for other parts of the Go backend):

diff --git a/src/python/pants/backend/go/util_rules/build_pkg_target.py b/src/python/pants/backend/go/util_rules/build_pkg_target.py
index fccc4290b7..6f7aa29cae 100644
--- a/src/python/pants/backend/go/util_rules/build_pkg_target.py
+++ b/src/python/pants/backend/go/util_rules/build_pkg_target.py
@@ -458,7 +458,7 @@ async def setup_build_go_package_target_request(
         maybe_base_pkg_dep = await Get(
             FallibleBuildGoPackageRequest,
             BuildGoPackageTargetRequest(
-                request.address, for_tests=True, build_opts=request.build_opts
+                request.address, for_tests=True, with_coverage=request.with_coverage, build_opts=request.build_opts
             ),
         )
         if maybe_base_pkg_dep.request is None:

[tdyas]

https://github.com/pantsbuild/pants/pull/21452 has the fix. I will also back-port the fix back to v2.2[1-3].x.

[nickbruun]

@tdyas I lost track of this one for a second. Thanks a million for debugging and fixing this :)