bazel-contrib / bazel-contrib/rules_go
gopackagesdriver: test-vendored deps resolve to production targets, violating `go/packages` PkgPath uniqueness
- Dominant language
- Go
- Stars
- 1.5k
- Forks
- 760
- Avg merge
- 1d 11h
- Merged PRs (30d)
- 12
Description
### What version of rules_go are you using?
v0.60.0 (also reproducible at HEAD).
### What version of gazelle are you using?
v0.50.0.
### What version of Bazel are you using?
8.5.0.
### Does this issue reproduce with the latest releases of all the above?
Yes.
### What operating system and processor architecture are you using?
linux amd64.
### Any other potentially useful information about your toolchain?
The bug is in `go_pkg_info_aspect`'s output, so it reproduces with any toolchain.
### What did you do?
This is distinct from #3981 (missing imports). Both bugs share the same trigger — a `go_test` with an external test that imports a package that transitively reaches the embedded library — but #3981 is a *missing* edge in the driver response, and this is a *wrong* edge that survives even after #4606 restores the missing one.
This is the source of over two years of confusion in golang/go#63822.
CC @jayconrod @adonovan
**Repro:**
Set up a `go_test` whose external test (`package foo_test`) imports a package `helper` that itself imports `foo`:
```
go_library(name = "foo", importpath = "example.com/foo", srcs = ["foo.go"])
go_library(name = "helper", importpath = "example.com/helper", srcs = ["helper.go"], deps = [":foo"])
go_test(
name = "foo_test",
srcs = ["foo_internal_test.go", "foo_external_test.go"], # package foo + package foo_test
embed = [":foo"],
deps = [":helper"],
)
```
Then load `foo_external_test.go` through the gopackagesdriver (e.g. open it in gopls, or run `go vet` with `GOPACKAGESDRIVER` set) and inspect the `DriverResponse`.
`go_test` has to recompile `helper` here, because `helper` was compiled against the production `foo` but the test embeds a *different* `foo` — the production sources plus `foo_internal_test.go`. If the external test got `helper`'s view of `foo` from one compilation and its own view from another, type identity would break: `helper` would return a `*foo.T` and the test would hold a different `*foo.T`. So `_recompile_external_deps` recompiles the chain between the external test and the embedded library against the test variant — the same thing `cmd/go` does, and what `go list -compiled -test` reports as `helper [example.com/foo.test]`: a separate package record with the same import path but a distinct identity.
The build does this correctly. The driver's *description* of the build does not.
`go_pkg_info_aspect` emits one `pkg.json` per Bazel target. The recompiled `helper [foo.test]` is not a target — it is an archive `_recompile_external_deps` constructs internally. So the aspect never emits a record for it. When the aspect builds the `Imports` map for the test record, the only `helper` it can point at is the production `:helper` target. Production `helper` imports production `:foo`. And the external test also reaches the test variant of `foo` directly, by embedding it.
Walking the transitive `Imports` graph from the external test then reaches **two** records with `PkgPath = "example.com/foo"`: the test variant (the merged `:foo_test` record) and the production `:foo` (through production `:helper`).
### What did you expect to see?
The `go/packages` driver protocol requires that, from any package, the transitive `Imports` graph reaches at most one record per import path. That is the invariant `go/types` consumers rely on — `go/types` keys its package universe on `PkgPath`, and two `*types.Package` reachable from the same root with the same `Path` means every type in `foo` exists twice.
`go list -test` upholds the invariant by giving the recompiled chain distinct IDs (`helper [example.com/foo.test]`) with `Imports` that point at the test variant, not the production package. The gopackagesdriver should do the same.
### What did you see instead?
A driver response with two records for `example.com/foo` reachable from the external test. Downstream `go/types` consumers see two `*types.Package` with the same `Path` and produce errors like:
```
cannot use h.Get() (value of type *foo.T) as *foo.T value in assignment
```
— two types that print identically and are defined in the same source file but are not the same `*types.Named`.
In gopls this is intermittent and hard to attribute: gopls deduplicates packages by `Metadata.ID` per query and reuses results across queries via the filecache, so whether the duplicate is observed depends on load order, what else is in the cache, and which file the user opens first. The visible symptoms are flaky type-identity diagnostics that go away on a cache rebuild, and duplicate `PkgPath` entries in the shallow export-data manifest. The latter is the long-standing telemetry signal under golang/go#63822, which has lacked a known cause; this is one.
Interestingly, an alternative fix I considered for #3981 was to have the aspect query by (label, name) rather than just name, and that is probably the shape of the solution here: the aspect *cannot* fix this by querying for more targets; the recompiled archives are not targets. It has to walk `_recompile_external_deps`'s output through the `GoArchive` provider on the `go_test` target, which carries the recompiled archives as `direct` deps of the internal/external test archives.
I will send a fix for this issue.
Contributor guide
Research direction
Reproduce the case with a go_test containing internal and external test packages, then inspect the DriverResponse from go_pkg_info_aspect. Read the GoArchive provider data on the go_test target and follow _recompile_external_deps outputs, focusing on how Imports are assembled. Done means the external test’s transitive graph reaches only one record per PkgPath and points recompiled dependencies at the test variant.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- build-system, tooling
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 25/100