Add `[golang].third_party_target_generation = imported` (closes #20340) by aegis-cipherowl · Pull Request #23665 · pantsbuild/pants

aegis-cipherowl · 2026-09-01T16:36:23Z

Background

Closes #20340. Implements the design @rdeknijf proposed and @benjyw approved there; the design is theirs, this is our implementation of it, opened with their go-ahead.

Adds [golang].third_party_target_generation = build-list (default, today's behavior) | imported. Under imported, go_third_party_package targets are generated only for third-party packages reachable through the import graph from first-party code, so target generation stops downloading and analyzing modules nothing imports.

#23499's third_party_target_granularity = module does not cover this: generate_targets_from_go_mod calls download_and_analyze_third_party_packages unconditionally, and the granularity branch selects from the already-resolved result. The two compose — import scoping decides which modules are resolved, granularity decides how many targets each produces.

Four parts: constraint-free AllImports/AllTestImports on the package analyzer (so build-tag-gated, other-platform and tools.go imports still resolve); a first-party root scan; a fixpoint walk downloading only reached modules; and a codegen roots union with Protobuf registered, so generated .pb.go imports still get targets.

The root scan reads .go files from the filesystem rather than resolving targets. That is forced: its only caller is a target generator, so touching the target graph deadlocks the engine with The dependency graph contained a cycle. It is easy to miss, because injecting AllTargets there leaves pants list green in this repo — which has no go_mod targets — and only fails once a real one exists.

Measured

32-go_mod production repo, check src/go/config:: (an 8-module leaf), fresh local store:

scenario	build-list	imported
remote cache off	420s	165s
remote cache on, warm	446s	96s

	build-list	imported
distinct modules resolved	1102	266
GOSUMDB fallbacks	1060	0
targets, repo-wide	107,973	6,610
packages compiled	104	104

The identical compile count is the correctness check. GOSUMDB fallbacks go to zero structurally: Go 1.17+ prunes go.sum to exactly the imported closure, which is what the walk resolves, so the fallback branch is unreachable.

Notes

tool directive support is split out to #23670, stacked on this.

LLM assistance notice

This PR was written primarily by Claude Code (Claude Opus 5). I directed the scope and reviewed the result.

🤖 Generated with Claude Code

`ModuleDownloadRequest` carried the whole `GoBuildOptions` in its memoization key, but downloading and analyzing a module barely depends on it: `go mod download` never consults build options, and the only consumer anywhere in `download_and_analyze_module` is `CGO_ENABLED` for the package analyzer. So callers differing only in a field that cannot affect the result each paid for their own download of every module in the graph. That is not hypothetical: target generation passes `GoBuildOptions()` defaults to dodge a rule-graph cycle (see the TODO in `target_type_rules.py`) while a compile resolves the real options, so a repo building with `race=True` downloaded every third-party module exactly twice per run. Measured against a real monorepo: ~1,979 remote-cache AC lookups for 1,059 distinct modules. Narrow the key to `cgo_enabled`. Both it and `[golang].cgo_enabled` default to `True`, so the two keys collapse into one. This also fixes the general N-way case -- any two callers differing only in irrelevant options now share one download -- which resolving the rule-graph cycle would not have done, since targets in a module can legitimately differ in build options. The TODO is annotated rather than removed: the cycle still exists, it just no longer costs a duplicate download. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GBwx1ZTZJ5oGxSAadmtucS

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GBwx1ZTZJ5oGxSAadmtucS

Import-scoped third-party target generation needs to know every third-party import a `go_mod`'s first-party code could reference. `Imports`/`TestImports` cannot answer that: they describe what *this* build compiles, so they are filtered by the host's GOOS/GOARCH and build tags, and they are only reported when the directory forms a single valid package. Three cases would silently lose roots, and a missing root means failing to generate a target the user asked for: * a `//go:build` guarded file, or a file for another platform; * a cgo file when cgo is disabled, which lands in IgnoredGoFiles with its imports dropped entirely; * the `tools.go` pattern, where a constraint-excluded file deliberately declares a different package name. Add `AllImports`/`AllTestImports`, collected from every .go file regardless of build constraints or package-name agreement. Files Go ignores outright (`_` or `.` prefixed) and the pseudo-import "C" are excluded. Constraint-excluded files are parsed only to harvest imports, and parse failures there are ignored -- the file is out of the build either way and matchFile already recorded real errors. The fields are assigned before the "multiple package names encountered" and "no buildable Go source files" errors can return, because those are exactly the cases the caller still needs roots for -- a directory holding only a build-tag-gated tools.go hits the latter. `main` marshals the package even when Error is set. Carry them on `FallibleFirstPartyPkgAnalysis` rather than `FirstPartyPkgAnalysis` for the same reason: in those cases `analysis` is None. Verified against a directory combining all three cases: Imports stays ["github.com/normal/dep"] while AllImports also reports the plan9-only and tools.go imports; a tools-only directory still reports its root alongside its "no buildable Go source files" error. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GBwx1ZTZJ5oGxSAadmtucS

Stage one of import-scoped `go_third_party_package` generation: find the third-party import paths a `go_mod`'s own code actually references, so the walk that follows can resolve only those modules instead of the whole build list. The scan reads `.go` files from the filesystem rather than resolving `go_package` targets. That is not a shortcut -- the only caller is `generate_targets_from_go_mod`, which *is* a target generator, so consulting the target graph here would reintroduce the same rule-graph cycle already documented by the `build_opts` TODO in `target_type_rules.py`. Roots come from `AllImports`/`AllTestImports` (constraint-free, so build-tag gated files, other-platform files and the `tools.go` pattern all resolve) plus `tool` directives in go.mod, which name executable packages no `.go` file imports -- the Go 1.24+ replacement for `tools.go`. Two details worth keeping: * `execute_process`, not `execute_process_or_raise`. A `tools.go`-only directory legitimately fails analysis with "no buildable Go source files" while still reporting the roots we need, so a non-zero exit must not abort the scan. * a nested `go.mod` carves its whole subtree out of the parent module, so its sources contribute nothing to the parent's roots. Prefix matching is done on path segments, so a sibling like `vendored_helpers` is not mistaken for a child of `vendored`. Filtering roots down to third-party packages and walking them to a fixpoint comes next; this commit only produces the root set. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GBwx1ZTZJ5oGxSAadmtucS

Second half of import-scoped generation: walk the import graph outward from the first-party roots, downloading a module only when something actually reaches into it, and iterating until no new import paths appear. The default stays `build-list`, so this is a no-op unless opted into. Under `imported`, `go_third_party_package` targets are generated only for packages reachable from the `go_mod`'s own code, which also means target generation stops downloading the entire module graph -- the reason it was expensive in the first place. Two behavior changes come with opting in, both called out in the option help: unimported third-party packages no longer appear in `pants list ::`, and hand-written addresses pointing at one no longer resolve. Import paths are resolved to modules the way `cmd/go` does: a module provides an import path when its path is that path or a path-segment prefix of it, longest match winning. Names are sorted longest-first so that is deterministic rather than dict-order dependent. An import path that resolves to no downloaded package is skipped rather than raising. That is routine -- an import reachable only under an unsatisfied build tag may name a package that does not exist for this platform, or name no module on the build list at all -- and the build-list path would not have produced a target for it either. Tested against a go.mod requiring both `rsc.io/quote` and `github.com/google/uuid` where first-party code imports only `quote`: `imported` yields `quote` and its transitive `rsc.io/sampler` (so the walk demonstrably iterates rather than stopping at direct imports) while excluding `uuid`, and the result is a strict subset of what `build-list` produces. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GBwx1ZTZJ5oGxSAadmtucS

Generated `.pb.go` files do not exist when the import scan runs, so nothing reports the runtime packages they import. Under `[golang].third_party_target_generation = "imported"` that meant no targets for the protobuf runtime and generated code that could not build. Since we enable the Go protobuf backend, this made `imported` unusable rather than merely imprecise. Add a `GoCodegenImportRootsRequest` union so codegen backends can name the modules their output imports, and implement it for Protobuf. Roots are named as whole *modules*, not individual import paths. Precision would buy nothing: a module is downloaded and analyzed as a unit, so treating all of its packages as reachable costs no extra downloads -- only a handful of extra targets in the one or two runtime modules involved. The expensive behavior this option exists to fix is unaffected either way. The hook must not consult the target graph. Its only consumer is `generate_targets_from_go_mod`, which is itself a target generator; requesting targets from there deadlocks the engine with "The dependency graph contained a cycle" (confirmed by probe: injecting `AllTargets` into that rule fails 3 of 5 tests in `target_type_rules_test.py` with exactly that error, while leaving the Pants repo's own `pants list` green, since this repo has no `go_mod` targets). That constraint has one visible consequence: `ProtobufGrpcToggleField` is per-target BUILD metadata with no existence outside the target graph, so the hook cannot know whether the grpc plugin runs and reports the grpc runtime whenever any proto generates Go. Over-reporting costs one module's targets; under-reporting breaks the build. Whether a proto generates Go at all is decided by `option go_package`, which is readable straight from the file. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GBwx1ZTZJ5oGxSAadmtucS

…se note `download_and_analyze_third_party_packages` now takes a subsystem and union membership, so the two call sites introduced on main by pantsbuild#23499 -- `expand_module_import_paths` and the module-granularity branch of `setup_full_package_build_request` -- need `**implicitly()`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GBwx1ZTZJ5oGxSAadmtucS

rdeknijf

Read the walk, the scan, the analyzer change and the codegen hook. Nothing blocking; details inline.

rdeknijf · 2026-09-02T06:47:53Z

@aegis-cipherowl thanks for opening this, and for asking on #20340 first. I only saw that comment today. Go ahead with this PR. I will not open my branch.

Yours is the better design anyway; You scope inside download_and_analyze_third_party_packages, so check and test get the reduced set through extract_package_info. My branch scoped generate_targets_from_go_mod only, and execution goals would still have pulled the whole build list. And your codegen hook reports the gRPC runtime. Mine reported only the protobuf runtime, so a gRPC proto would have failed under imported unless some first-party file already imported grpc.

CI has not run on either PR yet, so here are the touched tests on this branch against current main (free-threaded Python 3.14, Go 1.27):

✓ src/python/pants/backend/codegen/protobuf/go/rules_integration_test.py:tests succeeded in 63.24s.
✓ src/python/pants/backend/go/target_type_rules_test.py:tests succeeded in 72.82s.
✓ src/python/pants/backend/go/util_rules/first_party_pkg_test.py:tests succeeded in 51.09s.
✓ src/python/pants/backend/go/util_rules/import_roots_test.py:tests succeeded in 40.80s.
✓ src/python/pants/backend/go/util_rules/third_party_pkg_test.py:tests succeeded in 66.33s.

That is head 5adb776, which already has the #23664 key change, so it covers both PRs. I also ran it end to end on the 3-go.mod reproducer from #20340 with a fresh local store per mode. list :: drops from 9183 to 498 third-party targets. check passes on the gRPC module and on the sparse module in both modes; uncached, the gRPC module needs 230 processes for 109 transitive third-party deps.

Review comments are inline, nothing blocking.

tdyas · 2026-09-04T04:43:07Z


+    third_party_target_generation = EnumOption(
+        default=GoThirdPartyTargetGeneration.BUILD_LIST,
+        help=softwrap(


Using the Pants-internalhelp_text function instead of softwrap should allow you to avoid the backslash-continued lines.

Leaving this one open for you, since I did not do quite what you suggested.

help_text does not typecheck for EnumOption: it returns str | Callable[[], str], EnumOption expects Callable[[Any], Any] | str, and mypy rejects it. Its docstring says it exists because "mypy does not respect the type hints defined on base Field and Target classes" — and no EnumOption in the tree uses it.

The backslash continuations are gone regardless: softwrap preserves indented bullet blocks on its own (same shape as [go-test].coverage_mode), so they were never needed. Verified the rendered pants help golang still shows the bullets correctly.

Happy to go another way if you had a different form in mind.

…/import-scoped-target-generation # Conflicts: # docs/notes/2.34.x.md # src/python/pants/backend/go/util_rules/third_party_pkg.py # src/python/pants/backend/go/util_rules/third_party_pkg_test.py

@rdeknijf

Correctness: * The walk followed `ThirdPartyPkgAnalysis.imports`, which the analyzer had already filtered by the host's build constraints, while the roots feeding it were constraint-free. Past the first hop the computed closure therefore depended on the machine running Pants. Carry `all_imports` on the analysis and walk that instead (@rdeknijf). * The source glob took every `.go` file under the module, but `go list ./...` skips `testdata`, `vendor`, and any component starting with `.` or `_`. A `testdata` fixture importing a build-list module made it a root, downloading the module and generating targets for code nothing builds (@rdeknijf). * `ModuleImportPathResolver` now indexes modules under both `name` and `import_path`, so a versioned `replace` that changes the path resolves (@rdeknijf). Cleanups: * Drop the unused `all_imports`/`all_test_imports` from `FallibleFirstPartyPkgAnalysis`. Nothing read them: the scan parses the analyzer's JSON directly, and it cannot go through that type without resolving targets, which is the rule-graph cycle this design avoids (@rdeknijf). * `FirstPartyImportRootsRequest` no longer carries `cgo_enabled`. `AllImports` is harvested regardless of cgo, so it only split the scan's memo key; `CGO_ENABLED=1` is pinned in the process env instead (@rdeknijf). * Use `ijson.backends.python`, matching the other Go rules (@rdeknijf). * Replace the sorted-tuple-plus-free-function with `ModuleImportPathResolver`, so the longest-first invariant is established in the constructor rather than upheld by callers (@tdyas). * Rename the `fileInfo` variable in the analyzer's file loop, which shadowed the `fileInfo` type (@tdyas). * Drop the backslash continuations from the option's help text. `help_text` was suggested, but it returns `str | Callable[[], str]` and exists for `Field`/`Target`, so it does not typecheck for `EnumOption`; `softwrap` preserves the indented bullets on its own (@tdyas). New tests cover the `go list` walk exclusions, at both the helper and scan level. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GBwx1ZTZJ5oGxSAadmtucS

Per review, `tool` directive support is a distinct change and goes up as its own PR stacked on this one. The option help and the release note no longer claim `tool` directives are treated as imports, since after this commit they are not. `imported` still handles the older `tools.go` pattern, which the constraint-free scan covers. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GBwx1ZTZJ5oGxSAadmtucS

aegis-cipherowl · 2026-09-09T14:49:57Z

Merged latest main (#23664 landed, so the stacked commits collapse out) and pushed the review fixes.

@rdeknijf — thanks for the go-ahead, and for running the tests on main. That closed a gap I had flagged: my machine SIGSEGVs on every pytest invocation on main, including tests with no Go involvement.

Correctness, all three yours:

The walk followed imports, which the analyzer had already filtered by host constraints, while its roots were constraint-free — so the closure depended on the machine past the first hop. ThirdPartyPkgAnalysis now carries all_imports and the walk uses it.
Added a go list ./... component filter (testdata, vendor, ./_ prefixes), with tests at helper and scan level.
ModuleImportPathResolver indexes by both name and import_path, so a versioned replace resolves.

Cleanups: dropped the redundant cgo_enabled from the scan request and pinned CGO_ENABLED=1; switched to ijson.backends.python; @tdyas's ModuleImportPathResolver replaces the sorted-tuple-plus-free-function; renamed the shadowing fileInfo variable.

Two places I did not do what was suggested:

The unused all_imports/all_test_imports on FallibleFirstPartyPkgAnalysis are dropped rather than routed through. Routing the scan through that type means analyze_first_party_package, which resolves targets — and the scan runs under a target generator, so that deadlocks the engine. Direct JSON parsing is forced here.
help_text does not typecheck for EnumOption: it returns str | Callable[[], str] and exists for Field/Target (per its own docstring), and no EnumOption in the tree uses it. Kept softwrap but removed the backslash continuations, which was the actual complaint — softwrap preserves indented bullets on its own. Verified the rendered pants help golang.

tool directive support is split out to #23670, stacked on this. The option help and release note here no longer claim tool support, since after the split they would have been wrong; #23670 restores those sentences.

CI caught three more callers of `download_and_analyze_third_party_packages` that need `**implicitly()` now that the rule takes a subsystem and union membership: `build_pkg_third_party`, `build_pkg_target` and `debug_goals`. Missed locally because mypy was run over an explicit file list rather than the whole repo, which is what CI checks. Verified with `pants check ::` this time: clean across all 1831 source files. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GBwx1ZTZJ5oGxSAadmtucS

benjyw · 2026-09-15T15:16:15Z

@tdyas can you PTAL? Thanks

tdyas · 2026-09-16T03:23:13Z

How does this PR interact with the prior work to build all of the packages in a third-party module together instead of separately?

benjyw · 2026-10-01T05:20:45Z

How does this PR interact with the prior work to build all of the packages in a third-party module together instead of separately?

@aegis-cipherowl can you respond? I'd like to get this merged once @tdyas approves.

aegis-cipherowl · 2026-10-07T06:17:10Z

How does this PR interact with the prior work to build all of the packages in a third-party module together instead of separately?

I'm not 100% sure about what the "prior work" refers to, but imported mode never changes what a build packages sees. pkg_info — including digest, the slicing tier, and the native-code decision — comes from download_and_analyze_module, which analyses a whole module as a unit. imported changes only
which modules get analysed at all and which packages get targets. So for any package that is actually built, inputs
are byte-identical to build-list. That's precisely why we measured 104 compiles either way.

benjyw

LGTM, thanks!

A `tool` directive (Go 1.24+) names an executable package a module depends on without any `.go` file importing it. It is the official replacement for the `tools.go` pattern, which `[golang].third_party_target_generation = "imported"` already handles via the constraint-free scan. Without this, a repository that has migrated to `tool` directives loses those targets under `imported` and fails to build, while one still using `tools.go` does not -- an inconsistency between the old idiom and its successor. Parses both the single-line form (`tool example.com/cmd/foo`) and the block form. Comments are stripped first, and matching is on the `tool` keyword rather than a prefix, so `module example.com/toolkit` and `require example.com/toolbox` are not mistaken for directives. Split out of pantsbuild#23665 at review request. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GBwx1ZTZJ5oGxSAadmtucS

aegis-cipherowl and others added 7 commits August 31, 2026 09:18

Add release note for the module download key narrowing

20cf3bb

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GBwx1ZTZJ5oGxSAadmtucS

rdeknijf reviewed Sep 2, 2026

View reviewed changes

benjyw requested a review from jgranstrom September 2, 2026 21:56

tdyas reviewed Sep 4, 2026

View reviewed changes

aegis-cipherowl and others added 3 commits September 8, 2026 18:11

Merge remote-tracking branch 'upstream/main' into henry-yang/upstream…

ed1c8f9

…/import-scoped-target-generation # Conflicts: # docs/notes/2.34.x.md # src/python/pants/backend/go/util_rules/third_party_pkg.py # src/python/pants/backend/go/util_rules/third_party_pkg_test.py

aegis-cipherowl mentioned this pull request Sep 9, 2026

Treat go.mod tool directives as import roots #23670

Open

benjyw approved these changes Oct 9, 2026

View reviewed changes

benjyw merged commit 5ec37d4 into pantsbuild:main Oct 9, 2026
28 checks passed

Conversation

aegis-cipherowl commented Sep 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Background

Measured

Notes

LLM assistance notice

Uh oh!

rdeknijf left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rdeknijf commented Sep 2, 2026

Uh oh!

Uh oh!

tdyas Sep 4, 2026

Choose a reason for hiding this comment

Uh oh!

aegis-cipherowl Sep 10, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

aegis-cipherowl commented Sep 9, 2026

Uh oh!

benjyw commented Sep 15, 2026

Uh oh!

tdyas commented Sep 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

benjyw commented Oct 1, 2026

Uh oh!

aegis-cipherowl commented Oct 7, 2026

Uh oh!

benjyw left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

aegis-cipherowl commented Sep 1, 2026 •

edited

Loading

tdyas commented Sep 16, 2026 •

edited

Loading