Repository navigation
Treat go.mod tool directives as import roots - #23670
Open
aegis-cipherowl wants to merge 3 commits into
Open
aegis-cipherowl wants to merge 3 commits into
aegis-cipherowl wants to merge 3 commits into
Conversation
Contributor
|
Please set the PR's base branch to be the branch of the other PR on which it is stacked. That will isolate the diff to just this PR's actual changes. |
Contributor
Author
|
I would like to, but GitHub requires a PR's base branch to exist in the base repository, and I do not have push access to Two ways to get the isolated diff you are after:
In the meantime, |
benjyw
pushed a commit
that referenced
this pull request
Oct 9, 2026
… (#23665) ## 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](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
|
@aegis-cipherowl can you rebase and resolve the merge conflicts, now that the base is merged into pantsbuild/main? |
Contributor
|
Then cc me and I will review. |
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
force-pushed
the
henry-yang/upstream/go-mod-tool-directives
branch
from
October 23, 2026 09:22
bd28b3e to
69cf0bc
Compare
Contributor
|
Thanks for the contribution. We've just branched for 2.34.x, so merging this pull request now will come out in 2.35.x, please move the release notes updates to |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Background
Split out of #23665 at @tdyas's request. Stacked on #23665 — review that one first; this diff is a pure addition on top of it.
A
tooldirective (Go 1.24+) names an executable package a module depends on without any.gofile importing it. It is the official replacement for thetools.gopattern, which[golang].third_party_target_generation = "imported"already handles via the constraint-free scan.Without this, a repository that has migrated to
tooldirectives loses those targets underimportedand fails to build, while one still usingtools.godoes not — so the two idioms behave inconsistently. #23665's option help and release note were amended to stop promisingtoolsupport; this PR restores those sentences.Parses both the single-line and block forms. Comments are stripped first, and matching is on the
toolkeyword rather than a prefix, somodule example.com/toolkitandrequire example.com/toolboxare not mistaken for directives — there is a test for exactly that.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