From 85cc9db5a8d5a3e89d671b6d4ef72b7928193317 Mon Sep 17 00:00:00 2001 From: Henry Yang Date: Thu, 27 Aug 2026 02:11:31 -0700 Subject: [PATCH 01/10] Key Go module downloads on `cgo_enabled`, not all of `GoBuildOptions` `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) Claude-Session: https://claude.ai/code/session_01GBwx1ZTZJ5oGxSAadmtucS --- .../pants/backend/go/target_type_rules.py | 5 ++ .../backend/go/util_rules/third_party_pkg.py | 20 ++++-- .../go/util_rules/third_party_pkg_test.py | 69 ++++++++++++++++++- 3 files changed, 86 insertions(+), 8 deletions(-) diff --git a/src/python/pants/backend/go/target_type_rules.py b/src/python/pants/backend/go/target_type_rules.py index d2f2a467fa7..04455103b92 100644 --- a/src/python/pants/backend/go/target_type_rules.py +++ b/src/python/pants/backend/go/target_type_rules.py @@ -585,6 +585,11 @@ async def generate_targets_from_go_mod( # TODO: There is a rule graph cycle in this rule if this rule tries to use GoBuildOptionsFromTargetRequest. # For now, just use a default set of options to facilitate analyzing third-party dependencies and # generating targets. + # + # NB: These defaults no longer cost an extra download of every module in the graph. + # `ModuleDownloadRequest` is keyed on `cgo_enabled` alone rather than the whole + # `GoBuildOptions`, so target generation and a compile resolving e.g. `race=True` share + # one memoized download. Only a caller that actually differs in `cgo_enabled` pays twice. build_opts=GoBuildOptions(), ) ) diff --git a/src/python/pants/backend/go/util_rules/third_party_pkg.py b/src/python/pants/backend/go/util_rules/third_party_pkg.py index bc743fc3a2e..565ad98faba 100644 --- a/src/python/pants/backend/go/util_rules/third_party_pkg.py +++ b/src/python/pants/backend/go/util_rules/third_party_pkg.py @@ -190,7 +190,7 @@ class ModuleDescriptors: @dataclass(frozen=True) class ModuleDownloadRequest: """Download and analyze a Go module, keyed by (name, version, minimum_go_version, - build_opts, go_sum_entries). + cgo_enabled, go_sum_entries). This enables cross-go.mod deduplication: if mod-a and mod-b both depend on grpc@v1.60.0 with the same go.sum entries, the download and analysis only @@ -213,7 +213,13 @@ class ModuleDownloadRequest: name: str version: str minimum_go_version: str | None - build_opts: GoBuildOptions + # NB: Only `cgo_enabled` is carried here, rather than the whole `GoBuildOptions`. Downloading + # and analyzing a module is independent of every other build option -- `go mod download` does + # not consult them, and the package analyzer only reads `CGO_ENABLED`. Keying on the full + # options object meant that callers differing in an irrelevant field (e.g. target generation + # using defaults while a compile resolves `race=True`) each paid for their own download of + # every module in the graph. + cgo_enabled: bool go_sum_entries: tuple[str, ...] @@ -526,9 +532,9 @@ async def download_and_analyze_module( ) -> AnalyzedThirdPartyModule: """Download and analyze a single Go module via a synthetic go.mod + go.sum. - Keyed by (name, version, minimum_go_version, build_opts, go_sum_entries), + Keyed by (name, version, minimum_go_version, cgo_enabled, go_sum_entries), which lets the Pants engine deduplicate identical module downloads across - go.mods. + go.mods and across callers using different build options. A synthetic go.mod + go.sum pair is written into the sandbox so that Go's normal checksum verification still runs -- the go.sum entries come straight @@ -623,7 +629,7 @@ async def download_and_analyze_module( }, description=f"Analyze metadata for Go third-party module: {request.name}@{request.version}", level=LogLevel.DEBUG, - env={"CGO_ENABLED": "1" if request.build_opts.cgo_enabled else "0"}, + env={"CGO_ENABLED": "1" if request.cgo_enabled else "0"}, ) ) ) @@ -680,7 +686,7 @@ async def download_and_analyze_third_party_packages( # Parse the go.sum once into a dict for O(1) lookup per module. go_sum_index = _parse_go_sum(go_sum_content) - # The engine memoizes by (name, version, minimum_go_version, build_opts, + # The engine memoizes by (name, version, minimum_go_version, cgo_enabled, # go_sum_entries), so identical modules across go.mods are downloaded # once -- reducing downloads from O(N*M) to O(M). analyzed_modules = await concurrently( @@ -689,7 +695,7 @@ async def download_and_analyze_third_party_packages( name=mod.name, version=mod.version, minimum_go_version=mod.minimum_go_version, - build_opts=request.build_opts, + cgo_enabled=request.build_opts.cgo_enabled, go_sum_entries=go_sum_index.get((mod.name, mod.version), ()), ), **implicitly(), diff --git a/src/python/pants/backend/go/util_rules/third_party_pkg_test.py b/src/python/pants/backend/go/util_rules/third_party_pkg_test.py index 41e41ebdd0f..b442db109ac 100644 --- a/src/python/pants/backend/go/util_rules/third_party_pkg_test.py +++ b/src/python/pants/backend/go/util_rules/third_party_pkg_test.py @@ -27,6 +27,7 @@ AllThirdPartyPackagesRequest, ModuleDescriptors, ModuleDescriptorsRequest, + ModuleDownloadRequest, ThirdPartyPkgAnalysis, ThirdPartyPkgAnalysisRequest, _extract_go_sum_entries_for_module, @@ -620,7 +621,7 @@ def test_local_path_replace_statement_is_not_considered_third_party( def test_cross_go_mod_dedup_produces_identical_results(rule_runner: RuleRunner) -> None: """Two go.mods depending on the same module@version produce byte-identical analyses. - The dedup path is keyed on (name, version, minimum_go_version, build_opts, + The dedup path is keyed on (name, version, minimum_go_version, cgo_enabled, go_sum_entries). go.sum entries are content-addressable, so two well-formed go.mods sharing a dep produce equal ModuleDownloadRequests -- the engine memoizes and both requests share a single download. The observable property @@ -754,3 +755,69 @@ def test_gotoolchain_local_fail_fast(rule_runner: RuleRunner) -> None: ) with engine_error(ProcessExecutionFailure, contains="go.mod requires go >= 1.99.0"): rule_runner.request(ModuleDescriptors, [ModuleDescriptorsRequest(digest, "")]) + + +def test_module_download_key_ignores_build_options_that_cannot_affect_it() -> None: + """Only `cgo_enabled` may split the module download/analysis cache key. + + `go mod download` does not consult build options at all, and the package analyzer reads only + `CGO_ENABLED`. Carrying the whole `GoBuildOptions` meant callers that differed in an + irrelevant field each re-downloaded every module in the graph: target generation passes + `GoBuildOptions()` defaults (see the rule-graph-cycle TODO in `target_type_rules.py`) while a + compile resolves the real options, so a repo building with `race=True` paid exactly twice. + """ + + def key(build_opts: GoBuildOptions) -> ModuleDownloadRequest: + return ModuleDownloadRequest( + name="github.com/google/uuid", + version="v1.3.0", + minimum_go_version="1.16", + cgo_enabled=build_opts.cgo_enabled, + go_sum_entries=(), + ) + + # The case we actually hit: target generation's defaults vs. a compile with the race detector. + assert key(GoBuildOptions()) == key(GoBuildOptions(with_race_detector=True)) + + # Options that genuinely change nothing about the download must not split the key either. + assert key(GoBuildOptions()) == key(GoBuildOptions(with_msan=True)) + assert key(GoBuildOptions()) == key(GoBuildOptions(compiler_flags=("-N",))) + assert key(GoBuildOptions()) == key(GoBuildOptions(linker_flags=("-s",))) + + # `cgo_enabled` reaches the analyzer as CGO_ENABLED, so it must still split the key. + assert key(GoBuildOptions(cgo_enabled=True)) != key(GoBuildOptions(cgo_enabled=False)) + + +def test_race_detector_does_not_duplicate_module_downloads(rule_runner: RuleRunner) -> None: + """The same module requested with and without `race=True` shares one download. + + This is the end-to-end form of the key narrowing: identical analysis digests imply both + requests resolved to the same engine-memoized `ModuleDownloadRequest`. + """ + go_mod = dedent( + """\ + module example.com/mod + go 1.16 + require github.com/google/uuid v1.3.0 + """ + ) + digest = set_up_go_mod(rule_runner, go_mod, UUID_GO_SUM) + + def analyze(build_opts: GoBuildOptions) -> ThirdPartyPkgAnalysis: + result = rule_runner.request( + AllThirdPartyPackages, + [ + AllThirdPartyPackagesRequest( + Address("", target_name="mod"), + digest, + "go.mod", + build_opts=build_opts, + ) + ], + ) + return result.import_paths_to_pkg_info["github.com/google/uuid"] + + default = analyze(GoBuildOptions()) + with_race = analyze(GoBuildOptions(with_race_detector=True)) + + assert default.digest == with_race.digest From 20cf3bb43ab3dcbdf87dfd2bfe0d09b10a320a8b Mon Sep 17 00:00:00 2001 From: Henry Yang Date: Tue, 1 Sep 2026 09:24:47 -0700 Subject: [PATCH 02/10] Add release note for the module download key narrowing Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01GBwx1ZTZJ5oGxSAadmtucS --- docs/notes/2.34.x.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/notes/2.34.x.md b/docs/notes/2.34.x.md index 1e7b00f2b4a..abbca47dacd 100644 --- a/docs/notes/2.34.x.md +++ b/docs/notes/2.34.x.md @@ -99,6 +99,8 @@ Linked Go binaries and test binaries now record a Go toolchain build ID, as `go Dependency inference now resolves imports across local directory `replace` directives. When a `go.mod` replaces a required module with a path to another first-party module in the repo, imports of that module's packages are inferred automatically instead of having to be listed by hand in BUILD metadata. This is a no-op for modules without directory `replace` directives. See [#22097](https://github.com/pantsbuild/pants/issues/22097). +Third-party module downloads are no longer duplicated across callers that differ only in build options which cannot affect them. Downloading and analyzing a module consults just `cgo_enabled` -- `go mod download` ignores build options entirely, and the package analyzer reads only `CGO_ENABLED` -- but the whole `GoBuildOptions` was part of the memoization key. Target generation passes default options while a compile resolves the real ones, so a repository building with e.g. `race=True` downloaded every third-party module twice per run. + ### Plugin API changes `Target`, `TargetAdaptor`, `SourceBlock`, `SourceBlocks`, `TextBlock` and `Hunk` are now backed by native Rust implementations. They are still importable from their previous locations and their public constructors, attributes and methods are unchanged, but they are no longer Python dataclasses and they are built in `__new__` rather than `__init__`, so `dataclasses.is_dataclass`, `dataclasses.fields` and `dataclasses.replace` no longer apply to them. Defining subclasses in Python, and setting attributes on those subclasses, continues to work as before. From a77c32dff120ba54a9c2b3f40ae10d8435e20677 Mon Sep 17 00:00:00 2001 From: Henry Yang Date: Thu, 27 Aug 2026 02:28:16 -0700 Subject: [PATCH 03/10] Report constraint-free imports from the Go package analyzer 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) Claude-Session: https://claude.ai/code/session_01GBwx1ZTZJ5oGxSAadmtucS --- .../go/go_sources/analyze_package/main.go | 57 +++++++++++++++++++ .../backend/go/util_rules/first_party_pkg.py | 27 ++++++++- 2 files changed, 82 insertions(+), 2 deletions(-) diff --git a/src/python/pants/backend/go/go_sources/analyze_package/main.go b/src/python/pants/backend/go/go_sources/analyze_package/main.go index 3417ff55676..d1364ee0937 100644 --- a/src/python/pants/backend/go/go_sources/analyze_package/main.go +++ b/src/python/pants/backend/go/go_sources/analyze_package/main.go @@ -65,6 +65,22 @@ type Package struct { TestImports []string `json:",omitempty"` XTestImports []string `json:",omitempty"` + // Every import appearing in any .go file in this directory, regardless of whether the file + // is selected by the current build constraints and regardless of whether its `package` + // directive agrees with the rest of the directory. + // + // Imports/TestImports/XTestImports above describe what *this* build would compile, so they + // are filtered by GOOS/GOARCH/build tags and are only reported when the directory forms a + // single valid package. Import-scoped third-party target generation needs the opposite: a + // superset that still resolves build-tag-gated imports, imports for other platforms, and the + // `tools.go` pattern (a constraint-excluded file whose package name deliberately differs). + // Missing a root there means failing to generate a target the user asked for. + // + // Files Go ignores outright -- those prefixed with `_` or `.` -- are excluded, as is the + // pseudo-import "C". + AllImports []string `json:",omitempty"` + AllTestImports []string `json:",omitempty"` + // //go:embed patterns found in Go source files // For example, if a source file says // //go:embed a* b.c @@ -232,6 +248,29 @@ func analyzePackage(directory string, buildContext *build.Context) (*Package, er testImportsMap := make(map[string]bool) xtestImportsMap := make(map[string]bool) + allImportsMap := make(map[string]bool) + allTestImportsMap := make(map[string]bool) + + // Record a file's imports into the constraint-free sets. Test files are split out by filename + // rather than by package name, since a constraint-excluded file's package directive is exactly + // what we are declining to trust here. NB: declared before the loop because the loop shadows + // the `fileInfo` type with a variable of the same name. + harvestAllImports := func(name string, analysis *fileInfo) { + if analysis == nil { + return + } + target := allImportsMap + if strings.HasSuffix(name, "_test.go") { + target = allTestImportsMap + } + for _, imp := range analysis.imports { + if imp.path == "C" { + continue + } + target[imp.path] = true + } + } + embedsMap := make(map[string]bool) testEmbedsMap := make(map[string]bool) xtestEmbedsMap := make(map[string]bool) @@ -274,6 +313,13 @@ func analyzePackage(directory string, buildContext *build.Context) (*Package, er // build constraints, do not report it as an ignored file. Fall through. } else if ext == ".go" { pkg.IgnoredGoFiles = append(pkg.IgnoredGoFiles, name) + // This file lost to a build constraint, but its imports are still real: a + // `//go:build` guarded file, an other-platform file, or `tools.go`. Parse it + // solely to harvest roots. Parse failures are ignored -- the file is excluded + // from the build either way, and matchFile already recorded genuine errors. + if excludedAnalysis, err := analyzeFile(fileSet, filepath.Join(directory, name)); err == nil { + harvestAllImports(name, excludedAnalysis) + } } else if fileListForExt(pkg, ext) != nil { pkg.IgnoredOtherFiles = append(pkg.IgnoredOtherFiles, name) } @@ -382,6 +428,11 @@ func analyzePackage(directory string, buildContext *build.Context) (*Package, er } } + // NB: unconditional, and deliberately not gated on importsMapForFile -- a cgo file with + // cgo disabled lands in IgnoredGoFiles with a nil imports map, but its imports are still + // roots for target generation. + harvestAllImports(name, analysis) + if embedsMapForFile != nil && len(analysis.embeds) > 0 { for _, e := range analysis.embeds { embedsMapForFile[e.pattern] = true @@ -395,6 +446,12 @@ func analyzePackage(directory string, buildContext *build.Context) (*Package, er pkg.TestImports = cleanStringSet(testImportsMap) pkg.XTestImports = cleanStringSet(xtestImportsMap) + // NB: assigned here, before the "multiple package names" and "no buildable Go source files" + // errors below can return. Those are precisely the cases -- e.g. a `tools.go`-only directory -- + // where the caller still needs the roots, and `main` marshals the package even when Error is set. + pkg.AllImports = cleanStringSet(allImportsMap) + pkg.AllTestImports = cleanStringSet(allTestImportsMap) + pkg.EmbedPatterns = cleanStringSet(embedsMap) pkg.TestEmbedPatterns = cleanStringSet(testEmbedsMap) pkg.XTestEmbedPatterns = cleanStringSet(xtestEmbedsMap) diff --git a/src/python/pants/backend/go/util_rules/first_party_pkg.py b/src/python/pants/backend/go/util_rules/first_party_pkg.py index 4937cb02566..d856f364981 100644 --- a/src/python/pants/backend/go/util_rules/first_party_pkg.py +++ b/src/python/pants/backend/go/util_rules/first_party_pkg.py @@ -115,6 +115,17 @@ class FallibleFirstPartyPkgAnalysis: exit_code: int = 0 stderr: str | None = None + # Every import seen in the directory, ignoring build constraints -- see `AllImports` in the + # package analyzer. Deliberately carried on the fallible wrapper rather than on + # `FirstPartyPkgAnalysis`, because the cases that most need it are exactly the ones where + # `analysis` is None: a directory holding only a build-tag-gated `tools.go` fails with "no + # buildable Go source files", but its imports are still real target-generation roots. + # + # Empty when the analyzer process itself failed or its output could not be parsed, since + # there is nothing to read in those cases. + all_imports: tuple[str, ...] = () + all_test_imports: tuple[str, ...] = () + @classmethod def from_process_result( cls, @@ -158,7 +169,14 @@ def from_process_result( for filename, error in metadata.get("InvalidGoFiles", {}).items() ) error += "\n" - return cls(analysis=None, import_path=import_path, exit_code=1, stderr=error) + return cls( + analysis=None, + import_path=import_path, + exit_code=1, + stderr=error, + all_imports=tuple(metadata.get("AllImports", [])), + all_test_imports=tuple(metadata.get("AllTestImports", [])), + ) analysis = FirstPartyPkgAnalysis( dir_path=dir_path, @@ -191,7 +209,12 @@ def from_process_result( test_embed_patterns=tuple(metadata.get("TestEmbedPatterns", [])), xtest_embed_patterns=tuple(metadata.get("XTestEmbedPatterns", [])), ) - return cls(analysis, import_path) + return cls( + analysis, + import_path, + all_imports=tuple(metadata.get("AllImports", [])), + all_test_imports=tuple(metadata.get("AllTestImports", [])), + ) @dataclass(frozen=True) From 99c2cf4d13f9e9f3790be8b2fd9285f80fcc859f Mon Sep 17 00:00:00 2001 From: Henry Yang Date: Thu, 27 Aug 2026 02:40:39 -0700 Subject: [PATCH 04/10] Add the first-party import root scan for import-scoped target generation 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) Claude-Session: https://claude.ai/code/session_01GBwx1ZTZJ5oGxSAadmtucS --- .../backend/go/util_rules/import_roots.py | 180 +++++++++++++ .../go/util_rules/import_roots_test.py | 253 ++++++++++++++++++ 2 files changed, 433 insertions(+) create mode 100644 src/python/pants/backend/go/util_rules/import_roots.py create mode 100644 src/python/pants/backend/go/util_rules/import_roots_test.py diff --git a/src/python/pants/backend/go/util_rules/import_roots.py b/src/python/pants/backend/go/util_rules/import_roots.py new file mode 100644 index 00000000000..524c03ca88d --- /dev/null +++ b/src/python/pants/backend/go/util_rules/import_roots.py @@ -0,0 +1,180 @@ +# Copyright 2026 Pants project contributors (see CONTRIBUTORS.md). +# Licensed under the Apache License, Version 2.0 (see LICENSE). + +"""Determine which third-party import paths a `go_mod`'s own code actually references. + +This is the first half of import-scoped third-party target generation. The build list produced by +`go list -m all` over-approximates what a repo uses -- often by a lot, since MVS resolves at module +granularity -- so generating a target for every package of every listed module both explodes the +target count and forces a download of every module in the graph. + +NB: the scan reads `.go` files straight from the filesystem rather than resolving `go_package` +targets. It has to: the only caller is `generate_targets_from_go_mod`, which *is* a target +generator, so touching the target graph here would reintroduce the rule-graph cycle already +documented by the `build_opts` TODO in `target_type_rules.py`. +""" + +from __future__ import annotations + +import logging +import os.path +from dataclasses import dataclass + +import ijson + +from pants.backend.go.util_rules.pkg_analyzer import PackageAnalyzerSetup +from pants.build_graph.address import Address +from pants.engine.engine_aware import EngineAwareParameter +from pants.engine.fs import GlobMatchErrorBehavior, MergeDigests, PathGlobs +from pants.engine.intrinsics import ( + digest_to_snapshot, + execute_process, + get_digest_contents, + merge_digests, +) +from pants.engine.process import Process +from pants.engine.rules import collect_rules, concurrently, implicitly, rule +from pants.util.logging import LogLevel +from pants.util.ordered_set import FrozenOrderedSet + +logger = logging.getLogger(__name__) + + +@dataclass(frozen=True) +class FirstPartyImportRootsRequest(EngineAwareParameter): + """Scan the first-party Go sources owned by a `go_mod` for the imports they reference.""" + + go_mod_address: Address + go_mod_path: str + module_import_path: str + cgo_enabled: bool + + def debug_hint(self) -> str: + return self.go_mod_address.spec + + +@dataclass(frozen=True) +class FirstPartyImportRoots: + """Import paths referenced by first-party code, before stdlib/module filtering.""" + + import_paths: FrozenOrderedSet[str] + + +def _is_nested_module_path(path: str, base_dir: str, nested_module_dirs: frozenset[str]) -> bool: + """Whether `path` belongs to a module nested inside `base_dir` rather than to it. + + A nested `go.mod` carves its subtree out of the parent module entirely -- `go list ./...` in the + parent never descends into it -- so its sources must not contribute roots to the parent. + """ + for nested_dir in nested_module_dirs: + if nested_dir == base_dir: + continue + if path == nested_dir or path.startswith(f"{nested_dir}/"): + return True + return False + + +def _parse_tool_directives(go_mod_content: bytes) -> tuple[str, ...]: + """Extract `tool` directives from a go.mod (Go 1.24+). + + A `tool` directive names an executable package the module depends on without any `.go` file + importing it, which is precisely the modern replacement for the `tools.go` pattern. Missing + these would drop targets the user explicitly asked for. + + Handles both the single-line form (`tool example.com/cmd/foo`) and the block form + (`tool (\\n\\texample.com/cmd/foo\\n)`). + """ + tools: list[str] = [] + in_block = False + for raw_line in go_mod_content.decode(errors="replace").splitlines(): + line = raw_line.split("//", 1)[0].strip() + if not line: + continue + if in_block: + if line == ")": + in_block = False + else: + tools.append(line) + continue + if line == "tool (": + in_block = True + elif line.startswith("tool ") or line.startswith("tool\t"): + tools.append(line[len("tool") :].strip()) + return tuple(t for t in tools if t) + + +@rule(desc="Scan first-party Go sources for third-party imports", level=LogLevel.DEBUG) +async def determine_first_party_import_roots( + request: FirstPartyImportRootsRequest, + analyzer: PackageAnalyzerSetup, +) -> FirstPartyImportRoots: + base_dir = os.path.dirname(request.go_mod_path) + prefix = f"{base_dir}/" if base_dir else "" + + sources_snapshot, nested_mods_snapshot = await concurrently( + digest_to_snapshot( + **implicitly( + # NB: no `description_of_origin` -- these globs may legitimately match nothing + # (a `go_mod` with no first-party sources yet), so match errors stay ignored. + PathGlobs([f"{prefix}**/*.go"]) + ) + ), + digest_to_snapshot(**implicitly(PathGlobs([f"{prefix}**/go.mod"]))), + ) + + nested_module_dirs = frozenset(os.path.dirname(p) for p in nested_mods_snapshot.files) + + candidate_dirs = sorted( + { + os.path.dirname(f) + for f in sources_snapshot.files + if not _is_nested_module_path(os.path.dirname(f), base_dir, nested_module_dirs) + } + ) + + import_paths: set[str] = set() + + if candidate_dirs: + input_digest = await merge_digests(MergeDigests([sources_snapshot.digest, analyzer.digest])) + # NB: the analyzer reports `AllImports` even for directories it considers invalid (a + # `tools.go`-only directory errors with "no buildable Go source files"), so failures here + # are expected and must not abort the scan. `execute_process` rather than + # `execute_process_or_raise` for exactly that reason. + result = await execute_process( + Process( + (analyzer.path, *(d or "." for d in candidate_dirs)), + input_digest=input_digest, + description=( + f"Scan {len(candidate_dirs)} first-party Go package(s) for imports " + f"({request.go_mod_address})" + ), + level=LogLevel.DEBUG, + env={"CGO_ENABLED": "1" if request.cgo_enabled else "0"}, + ), + **implicitly(), + ) + + if result.stdout: + for pkg_json in ijson.items(result.stdout, "", multiple_values=True): + import_paths.update(pkg_json.get("AllImports", ())) + import_paths.update(pkg_json.get("AllTestImports", ())) + + go_mod_contents = await get_digest_contents( + **implicitly( + PathGlobs( + [request.go_mod_path], + glob_match_error_behavior=GlobMatchErrorBehavior.error, + description_of_origin=( + f"the import-scoped target generation scan for {request.go_mod_address}" + ), + ) + ) + ) + for entry in go_mod_contents: + import_paths.update(_parse_tool_directives(entry.content)) + + return FirstPartyImportRoots(FrozenOrderedSet(sorted(import_paths))) + + +def rules(): + return collect_rules() diff --git a/src/python/pants/backend/go/util_rules/import_roots_test.py b/src/python/pants/backend/go/util_rules/import_roots_test.py new file mode 100644 index 00000000000..79290f8e6da --- /dev/null +++ b/src/python/pants/backend/go/util_rules/import_roots_test.py @@ -0,0 +1,253 @@ +# Copyright 2026 Pants project contributors (see CONTRIBUTORS.md). +# Licensed under the Apache License, Version 2.0 (see LICENSE). + +from __future__ import annotations + +from textwrap import dedent + +import pytest + +from pants.backend.go import target_type_rules +from pants.backend.go.go_sources import load_go_binary +from pants.backend.go.target_types import GoModTarget, GoPackageTarget +from pants.backend.go.util_rules import ( + assembly, + build_pkg, + first_party_pkg, + go_mod, + import_analysis, + import_roots, + link, + sdk, + third_party_pkg, +) +from pants.backend.go.util_rules.import_roots import ( + FirstPartyImportRoots, + FirstPartyImportRootsRequest, + _is_nested_module_path, + _parse_tool_directives, +) +from pants.build_graph.address import Address +from pants.engine.rules import QueryRule +from pants.testutil.rule_runner import RuleRunner + + +@pytest.fixture +def rule_runner() -> RuleRunner: + rule_runner = RuleRunner( + rules=[ + *sdk.rules(), + *third_party_pkg.rules(), + *first_party_pkg.rules(), + *import_roots.rules(), + *load_go_binary.rules(), + *build_pkg.rules(), + *import_analysis.rules(), + *link.rules(), + *assembly.rules(), + *target_type_rules.rules(), + *go_mod.rules(), + QueryRule(FirstPartyImportRoots, [FirstPartyImportRootsRequest]), + ], + target_types=[GoModTarget, GoPackageTarget], + ) + rule_runner.set_options(["--golang-cgo-enabled"], env_inherit={"PATH"}) + return rule_runner + + +# ------------------------------------------------------------------------------------------------ +# `tool` directive parsing (Go 1.24+) +# ------------------------------------------------------------------------------------------------ + + +def test_parse_tool_directives_single_line() -> None: + go_mod = b"module example.com/m\n\ngo 1.24\n\ntool example.com/cmd/foo\n" + assert _parse_tool_directives(go_mod) == ("example.com/cmd/foo",) + + +def test_parse_tool_directives_block() -> None: + go_mod = dedent( + """\ + module example.com/m + + go 1.24 + + tool ( + \texample.com/cmd/foo + \texample.com/cmd/bar + ) + """ + ).encode() + assert _parse_tool_directives(go_mod) == ("example.com/cmd/foo", "example.com/cmd/bar") + + +def test_parse_tool_directives_ignores_comments_and_lookalikes() -> None: + go_mod = dedent( + """\ + module example.com/toolkit + + go 1.24 + + // tool example.com/cmd/commented-out + require example.com/toolbox v1.0.0 + + tool example.com/cmd/real // keep this one + """ + ).encode() + # `module example.com/toolkit` and `require example.com/toolbox` both start with "tool" as a + # substring; neither is a tool directive. + assert _parse_tool_directives(go_mod) == ("example.com/cmd/real",) + + +def test_parse_tool_directives_absent() -> None: + assert _parse_tool_directives(b"module example.com/m\n\ngo 1.21\n") == () + + +# ------------------------------------------------------------------------------------------------ +# Nested module exclusion +# ------------------------------------------------------------------------------------------------ + + +def test_nested_module_paths_are_excluded() -> None: + nested = frozenset({"src/go", "src/go/vendored"}) + # The module's own directory never counts as nested against itself. + assert not _is_nested_module_path("src/go", "src/go", nested) + assert not _is_nested_module_path("src/go/pkg/util", "src/go", nested) + # A nested go.mod carves out its whole subtree. + assert _is_nested_module_path("src/go/vendored", "src/go", nested) + assert _is_nested_module_path("src/go/vendored/deep/pkg", "src/go", nested) + # A sibling directory sharing a name prefix is not nested. + assert not _is_nested_module_path("src/go/vendored_helpers", "src/go", nested) + + +# ------------------------------------------------------------------------------------------------ +# The scan itself +# ------------------------------------------------------------------------------------------------ + + +def _roots(rule_runner: RuleRunner, *, go_mod_path: str = "go.mod") -> set[str]: + result = rule_runner.request( + FirstPartyImportRoots, + [ + FirstPartyImportRootsRequest( + go_mod_address=Address("", target_name="mod"), + go_mod_path=go_mod_path, + module_import_path="example.com/m", + cgo_enabled=True, + ) + ], + ) + return set(result.import_paths) + + +def test_scan_finds_imports_the_host_build_would_drop(rule_runner: RuleRunner) -> None: + """Roots must include build-tag-gated, other-platform and `tools.go` imports. + + Each of these is invisible to a normal constraint-applying analysis, and missing any of them + means declining to generate a target the user asked for. + """ + rule_runner.write_files( + { + "BUILD": "go_mod(name='mod')\n", + "go.mod": "module example.com/m\ngo 1.16\n", + "main.go": dedent( + """\ + package main + + import "github.com/normal/dep" + + func main() { dep.Do() } + """ + ), + "main_test.go": dedent( + """\ + package main + + import "github.com/only/intests" + """ + ), + "plan9.go": dedent( + """\ + //go:build plan9 + + package main + + import "github.com/other/platform" + """ + ), + "tools/tools.go": dedent( + """\ + //go:build tools + + package tools + + import _ "github.com/the/tool" + """ + ), + } + ) + roots = _roots(rule_runner) + assert "github.com/normal/dep" in roots + assert "github.com/only/intests" in roots + assert "github.com/other/platform" in roots + # The tools directory has no buildable Go sources at all, so the analyzer reports an error for + # it -- the root must survive that. + assert "github.com/the/tool" in roots + + +def test_scan_skips_nested_modules(rule_runner: RuleRunner) -> None: + """A nested `go.mod` owns its subtree; its imports are not the parent's roots.""" + rule_runner.write_files( + { + "BUILD": "go_mod(name='mod')\n", + "go.mod": "module example.com/m\ngo 1.16\n", + "main.go": dedent( + """\ + package main + + import "github.com/parent/dep" + + func main() { dep.Do() } + """ + ), + "nested/go.mod": "module example.com/nested\ngo 1.16\n", + "nested/main.go": dedent( + """\ + package main + + import "github.com/nested/dep" + + func main() { dep.Do() } + """ + ), + } + ) + roots = _roots(rule_runner) + assert "github.com/parent/dep" in roots + assert "github.com/nested/dep" not in roots + + +def test_scan_includes_tool_directives(rule_runner: RuleRunner) -> None: + """`tool` directives are roots even though no .go file imports them.""" + rule_runner.write_files( + { + "BUILD": "go_mod(name='mod')\n", + "go.mod": dedent( + """\ + module example.com/m + + go 1.24 + + tool example.com/cmd/generator + """ + ), + "main.go": dedent( + """\ + package main + + func main() {} + """ + ), + } + ) + assert "example.com/cmd/generator" in _roots(rule_runner) From 1fc7f479ce15c30e535c45d22f98b42e8b0f3054 Mon Sep 17 00:00:00 2001 From: Henry Yang Date: Thu, 27 Aug 2026 02:48:52 -0700 Subject: [PATCH 05/10] Add `[golang].third_party_target_generation = imported` 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) Claude-Session: https://claude.ai/code/session_01GBwx1ZTZJ5oGxSAadmtucS --- .../pants/backend/go/subsystems/golang.py | 39 +++++ .../pants/backend/go/target_type_rules.py | 3 +- .../backend/go/util_rules/import_roots.py | 1 - .../go/util_rules/import_roots_test.py | 1 - .../backend/go/util_rules/third_party_pkg.py | 139 +++++++++++++++++- .../go/util_rules/third_party_pkg_test.py | 61 ++++++++ 6 files changed, 239 insertions(+), 5 deletions(-) diff --git a/src/python/pants/backend/go/subsystems/golang.py b/src/python/pants/backend/go/subsystems/golang.py index 7b5d179e04e..dfc712651c2 100644 --- a/src/python/pants/backend/go/subsystems/golang.py +++ b/src/python/pants/backend/go/subsystems/golang.py @@ -25,6 +25,13 @@ class ThirdPartyTargetGranularity(Enum): MODULE = "module" +class GoThirdPartyTargetGeneration(Enum): + """Which third-party packages a `go_mod` generates targets for.""" + + BUILD_LIST = "build-list" + IMPORTED = "imported" + + class GolangSubsystem(Subsystem): options_scope = "golang" help = "Options for Golang support." @@ -270,6 +277,38 @@ def extra_tools(self) -> tuple[str, ...]: advanced=True, ) + third_party_target_generation = EnumOption( + default=GoThirdPartyTargetGeneration.BUILD_LIST, + help=softwrap( + """\ + Which `go_third_party_package` targets to generate for each `go_mod` target. + + * `build-list`: generate a target for every package of every module in the module\ + build list, i.e. everything `go list -m all` reports. + * `imported`: generate targets only for third-party packages reachable through the\ + import graph from the `go_mod`'s own first-party code. + + The build list over-approximates what a repo actually uses, often by a lot: minimal + version selection resolves at module granularity, so importing one package from a + module puts that whole module -- and everything it requires -- on the list, and every + package of every listed module becomes a target. Because target generation must + analyze each of those modules, it also downloads them, whether or not anything + imports them. + + Setting `imported` can dramatically reduce target count, memory use and the number of + module downloads, at the cost of two behavior changes: + + * third-party packages nothing imports no longer appear in `pants list ::`; + * hand-written addresses pointing at an unimported third-party package no longer\ + resolve. + + Imports guarded by build tags, imports for other platforms, the `tools.go` pattern and + `tool` directives in `go.mod` are all treated as imports, so they still generate + targets. + """ + ), + ) + cgo_enabled = BoolOption( default=True, help=softwrap( diff --git a/src/python/pants/backend/go/target_type_rules.py b/src/python/pants/backend/go/target_type_rules.py index 04455103b92..640bc5bc348 100644 --- a/src/python/pants/backend/go/target_type_rules.py +++ b/src/python/pants/backend/go/target_type_rules.py @@ -591,7 +591,8 @@ async def generate_targets_from_go_mod( # `GoBuildOptions`, so target generation and a compile resolving e.g. `race=True` share # one memoized download. Only a caller that actually differs in `cgo_enabled` pays twice. build_opts=GoBuildOptions(), - ) + ), + **implicitly(), ) def gen_file_tgt(fp: str) -> TargetGeneratorSourcesHelperTarget: diff --git a/src/python/pants/backend/go/util_rules/import_roots.py b/src/python/pants/backend/go/util_rules/import_roots.py index 524c03ca88d..3f97603f395 100644 --- a/src/python/pants/backend/go/util_rules/import_roots.py +++ b/src/python/pants/backend/go/util_rules/import_roots.py @@ -46,7 +46,6 @@ class FirstPartyImportRootsRequest(EngineAwareParameter): go_mod_address: Address go_mod_path: str - module_import_path: str cgo_enabled: bool def debug_hint(self) -> str: diff --git a/src/python/pants/backend/go/util_rules/import_roots_test.py b/src/python/pants/backend/go/util_rules/import_roots_test.py index 79290f8e6da..5af61721599 100644 --- a/src/python/pants/backend/go/util_rules/import_roots_test.py +++ b/src/python/pants/backend/go/util_rules/import_roots_test.py @@ -132,7 +132,6 @@ def _roots(rule_runner: RuleRunner, *, go_mod_path: str = "go.mod") -> set[str]: FirstPartyImportRootsRequest( go_mod_address=Address("", target_name="mod"), go_mod_path=go_mod_path, - module_import_path="example.com/m", cgo_enabled=True, ) ], diff --git a/src/python/pants/backend/go/util_rules/third_party_pkg.py b/src/python/pants/backend/go/util_rules/third_party_pkg.py index 565ad98faba..c0a0ded8f15 100644 --- a/src/python/pants/backend/go/util_rules/third_party_pkg.py +++ b/src/python/pants/backend/go/util_rules/third_party_pkg.py @@ -13,11 +13,23 @@ import ijson.backends.python as ijson from pants.backend.go.go_sources.load_go_binary import LoadedGoBinaryRequest, setup_go_binary -from pants.backend.go.subsystems.golang import GolangSubsystem, ThirdPartyTargetGranularity +from pants.backend.go.subsystems.golang import ( + GolangSubsystem, + GoThirdPartyTargetGeneration, + ThirdPartyTargetGranularity, +) from pants.backend.go.util_rules import pkg_analyzer from pants.backend.go.util_rules.build_opts import GoBuildOptions from pants.backend.go.util_rules.cgo import CGoCompilerFlags from pants.backend.go.util_rules.embedcfg import EmbedConfig +from pants.backend.go.util_rules.import_analysis import ( + GoStdLibPackagesRequest, + analyze_go_stdlib_packages, +) +from pants.backend.go.util_rules.import_roots import ( + FirstPartyImportRootsRequest, + determine_first_party_import_roots, +) from pants.backend.go.util_rules.pkg_analyzer import PackageAnalyzerSetup from pants.backend.go.util_rules.sdk import GoSdkProcess from pants.build_graph.address import Address @@ -660,9 +672,125 @@ async def download_and_analyze_module( return AnalyzedThirdPartyModule(FrozenOrderedSet(analyzed_packages)) +def _resolve_import_path_to_module( + import_path: str, module_names_longest_first: tuple[str, ...] +) -> str | None: + """Find the module in the build list that would provide `import_path`. + + Matches `cmd/go`: a module provides an import path when the module path is the import path or + a path-segment prefix of it. Where several modules qualify (e.g. `example.com/a` and + `example.com/a/b` both on the build list), the longest match wins, which is the one Go would + select. Passing the names pre-sorted keeps that unambiguous rather than order-dependent. + """ + for name in module_names_longest_first: + if import_path == name or import_path.startswith(f"{name}/"): + return name + return None + + +async def _analyze_imported_packages( + request: AllThirdPartyPackagesRequest, + module_analysis: ModuleDescriptors, + go_sum_index: dict[tuple[str, str], tuple[str, ...]], +) -> dict[str, ThirdPartyPkgAnalysis]: + """Resolve only the third-party packages reachable from first-party code. + + Walks the import graph outward from the `go_mod`'s own imports, downloading a module only when + something actually reaches into it, and iterating until no new import paths appear. Contrast + with the build-list path, which downloads and analyzes every module `go list -m all` reports + regardless of what the repo imports. + """ + modules_by_name = {mod.name: mod for mod in module_analysis.modules} + # Longest first so the most specific module wins; see `_resolve_import_path_to_module`. + module_names_longest_first = tuple(sorted(modules_by_name, key=len, reverse=True)) + + roots, stdlib_packages = await concurrently( + determine_first_party_import_roots( + FirstPartyImportRootsRequest( + go_mod_address=request.go_mod_address, + go_mod_path=request.go_mod_path, + cgo_enabled=request.build_opts.cgo_enabled, + ), + **implicitly(), + ), + analyze_go_stdlib_packages( + GoStdLibPackagesRequest( + with_race_detector=request.build_opts.with_race_detector, + cgo_enabled=request.build_opts.cgo_enabled, + ) + ), + ) + + def is_third_party(import_path: str) -> bool: + # `C` is the cgo pseudo-package, and a relative import can never name a module. + if import_path in stdlib_packages or import_path == "C": + return False + if import_path.startswith(".") or import_path.startswith("/"): + return False + return True + + known_packages: dict[str, ThirdPartyPkgAnalysis] = {} + reachable: dict[str, ThirdPartyPkgAnalysis] = {} + analyzed_module_names: set[str] = set() + seen_import_paths: set[str] = set() + + frontier = {ip for ip in roots.import_paths if is_third_party(ip)} + + while frontier: + seen_import_paths |= frontier + + # Pull in any module a frontier import path lands in that we have not analyzed yet. The + # per-module download is memoized by the engine, so modules shared with another `go_mod` + # cost nothing here. + pending_modules = {} + for import_path in frontier: + module_name = _resolve_import_path_to_module(import_path, module_names_longest_first) + if module_name is not None and module_name not in analyzed_module_names: + pending_modules[module_name] = modules_by_name[module_name] + + if pending_modules: + newly_analyzed = await concurrently( + download_and_analyze_module( + ModuleDownloadRequest( + name=mod.name, + version=mod.version, + minimum_go_version=mod.minimum_go_version, + cgo_enabled=request.build_opts.cgo_enabled, + go_sum_entries=go_sum_index.get((mod.name, mod.version), ()), + ), + **implicitly(), + ) + for mod in pending_modules.values() + ) + for module_name, analyzed_module in zip(pending_modules, newly_analyzed): + analyzed_module_names.add(module_name) + for pkg in analyzed_module.packages: + known_packages[pkg.import_path] = pkg + + next_frontier: set[str] = set() + for import_path in frontier: + reached_pkg = known_packages.get(import_path) + if reached_pkg is None: + # The import path resolved to no package we downloaded. That is normal: an import + # only reachable under a build tag we do not satisfy may name a package that does + # not exist for this platform, or name no module on the build list at all. The + # build-list path would not have produced a target for it either. + continue + reachable[import_path] = reached_pkg + for dep_import_path in reached_pkg.imports: + if dep_import_path in seen_import_paths or not is_third_party(dep_import_path): + continue + next_frontier.add(dep_import_path) + + frontier = next_frontier + + return reachable + + @rule(desc="Download and analyze all third-party Go packages", level=LogLevel.DEBUG) async def download_and_analyze_third_party_packages( request: AllThirdPartyPackagesRequest, + golang_subsystem: GolangSubsystem, ) -> AllThirdPartyPackages: module_analysis = await analyze_module_dependencies( ModuleDescriptorsRequest( @@ -686,6 +814,12 @@ async def download_and_analyze_third_party_packages( # Parse the go.sum once into a dict for O(1) lookup per module. go_sum_index = _parse_go_sum(go_sum_content) + if golang_subsystem.third_party_target_generation is GoThirdPartyTargetGeneration.IMPORTED: + return AllThirdPartyPackages( + EMPTY_DIGEST, + FrozenDict(await _analyze_imported_packages(request, module_analysis, go_sum_index)), + ) + # The engine memoizes by (name, version, minimum_go_version, cgo_enabled, # go_sum_entries), so identical modules across go.mods are downloaded # once -- reducing downloads from O(N*M) to O(M). @@ -722,7 +856,8 @@ async def extract_package_info( request.go_mod_digest, request.go_mod_path, build_opts=request.build_opts, - ) + ), + **implicitly(), ) pkg_info = all_packages.import_paths_to_pkg_info.get(request.import_path) if pkg_info: diff --git a/src/python/pants/backend/go/util_rules/third_party_pkg_test.py b/src/python/pants/backend/go/util_rules/third_party_pkg_test.py index b442db109ac..55c8e41b154 100644 --- a/src/python/pants/backend/go/util_rules/third_party_pkg_test.py +++ b/src/python/pants/backend/go/util_rules/third_party_pkg_test.py @@ -821,3 +821,64 @@ def analyze(build_opts: GoBuildOptions) -> ThirdPartyPkgAnalysis: with_race = analyze(GoBuildOptions(with_race_detector=True)) assert default.digest == with_race.digest + + +def test_imported_mode_generates_only_reachable_packages(rule_runner: RuleRunner) -> None: + """`imported` resolves the import closure; `build-list` resolves everything on the build list. + + The go.mod requires both `rsc.io/quote` and `github.com/google/uuid`, but the first-party code + imports only `quote`. Under `imported`, `uuid` must not appear -- while `quote`'s own + transitive dependency `rsc.io/sampler` must, which is what proves the walk iterates rather + than stopping at the direct imports. + """ + files = { + "BUILD": "go_mod(name='mod')\n", + "go.mod": GO_MOD, + "go.sum": GO_SUM, + "main.go": dedent( + """\ + package main + + import "rsc.io/quote" + + func main() { println(quote.Hello()) } + """ + ), + } + rule_runner.write_files(files) + digest = rule_runner.make_snapshot({"go.mod": GO_MOD, "go.sum": GO_SUM}).digest + + def resolve() -> set[str]: + result = rule_runner.request( + AllThirdPartyPackages, + [ + AllThirdPartyPackagesRequest( + Address("", target_name="mod"), + digest, + "go.mod", + build_opts=GoBuildOptions(), + ) + ], + ) + return set(result.import_paths_to_pkg_info) + + rule_runner.set_options( + ["--golang-cgo-enabled", "--golang-third-party-target-generation=build-list"], + env_inherit={"PATH"}, + ) + build_list_packages = resolve() + assert "rsc.io/quote" in build_list_packages + assert "github.com/google/uuid" in build_list_packages + + rule_runner.set_options( + ["--golang-cgo-enabled", "--golang-third-party-target-generation=imported"], + env_inherit={"PATH"}, + ) + imported_packages = resolve() + + assert "rsc.io/quote" in imported_packages + # Reached only through `rsc.io/quote`, so its presence shows the walk followed edges. + assert "rsc.io/sampler" in imported_packages + # On the build list, but nothing imports it. + assert "github.com/google/uuid" not in imported_packages + assert imported_packages < build_list_packages From 6fdc8ea672d4856f6b00eb996a6471af33057684 Mon Sep 17 00:00:00 2001 From: Henry Yang Date: Thu, 27 Aug 2026 08:48:23 -0700 Subject: [PATCH 06/10] Contribute codegen import roots so `imported` mode supports Protobuf 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) Claude-Session: https://claude.ai/code/session_01GBwx1ZTZJ5oGxSAadmtucS --- .../backend/codegen/protobuf/go/rules.py | 53 ++++++++++++++ .../protobuf/go/rules_integration_test.py | 66 +++++++++++++++++ .../pants/backend/experimental/go/register.py | 2 + .../backend/go/util_rules/import_roots.py | 35 ++++++++++ .../backend/go/util_rules/third_party_pkg.py | 70 +++++++++++++++++-- 5 files changed, 222 insertions(+), 4 deletions(-) diff --git a/src/python/pants/backend/codegen/protobuf/go/rules.py b/src/python/pants/backend/codegen/protobuf/go/rules.py index eafecc2b3b9..339b6962fad 100644 --- a/src/python/pants/backend/codegen/protobuf/go/rules.py +++ b/src/python/pants/backend/codegen/protobuf/go/rules.py @@ -71,6 +71,10 @@ GoStdLibPackagesRequest, analyze_go_stdlib_packages, ) +from pants.backend.go.util_rules.import_roots import ( + GoCodegenImportRoots, + GoCodegenImportRootsRequest, +) from pants.backend.go.util_rules.pkg_analyzer import PackageAnalyzerSetup from pants.backend.go.util_rules.sdk import GoSdkProcess from pants.backend.go.util_rules.third_party_pkg import ( @@ -89,6 +93,7 @@ Directory, FileContent, MergeDigests, + PathGlobs, RemovePrefix, ) from pants.engine.internals.graph import hydrate_sources, resolve_source_paths, transitive_targets @@ -158,6 +163,53 @@ class ProtobufGoModuleImportPathsMappingsHook(GoModuleImportPathsMappingsHook): pass +class ProtobufGoCodegenImportRootsRequest(GoCodegenImportRootsRequest): + pass + + +# The packages `protoc-gen-go` and `protoc-gen-go-grpc` emit into generated files. Named as whole +# modules rather than individual import paths -- a module is downloaded and analyzed as a unit, so +# naming every package of it costs no extra downloads. +_PROTOBUF_RUNTIME_MODULE = "google.golang.org/protobuf" +_GRPC_RUNTIME_MODULE = "google.golang.org/grpc" + + +@rule(desc="Determine Go import roots contributed by Protobuf codegen", level=LogLevel.DEBUG) +async def protobuf_go_codegen_import_roots( + request: ProtobufGoCodegenImportRootsRequest, +) -> GoCodegenImportRoots: + """Report the runtime modules that generated `.pb.go` files import. + + Generated sources do not exist when the import scan runs, so their imports are invisible to it. + Without this, `[golang].third_party_target_generation = "imported"` would drop the protobuf + runtime targets and generated code would fail to build. + + NB: this deliberately does not consult the target graph. The only consumer is + `generate_targets_from_go_mod`, which is itself a target generator, so requesting targets here + deadlocks the engine with "The dependency graph contained a cycle". That also rules out reading + each target's `grpc` toggle, so the gRPC runtime is reported whenever any proto generates Go. + Over-reporting costs one extra module's targets; under-reporting breaks the build. + """ + base_dir = os.path.dirname(request.go_mod_path) + prefix = f"{base_dir}/" if base_dir else "" + + # NB: a `.proto` under a *nested* go.mod is counted for the parent too. That is a harmless + # over-approximation of an already over-approximating hook. + protos = await digest_to_snapshot(**implicitly(PathGlobs([f"{prefix}**/*.proto"]))) + if not protos.files: + return GoCodegenImportRoots(()) + + proto_contents = await get_digest_contents(protos.digest) + # `option go_package` is what makes a proto generate Go at all. + generates_go = any( + parse_go_package_option(entry.content) is not None for entry in proto_contents + ) + if not generates_go: + return GoCodegenImportRoots(()) + + return GoCodegenImportRoots((_PROTOBUF_RUNTIME_MODULE, _GRPC_RUNTIME_MODULE)) + + @rule(desc="Map import paths for all Go Protobuf targets.", level=LogLevel.DEBUG) async def map_import_paths_of_all_go_protobuf_targets( _request: ProtobufGoModuleImportPathsMappingsHook, @@ -777,6 +829,7 @@ def rules(): UnionRule(GenerateSourcesRequest, GenerateGoFromProtobufRequest), UnionRule(GoCodegenBuildRequest, GoCodegenBuildProtobufRequest), UnionRule(GoModuleImportPathsMappingsHook, ProtobufGoModuleImportPathsMappingsHook), + UnionRule(GoCodegenImportRootsRequest, ProtobufGoCodegenImportRootsRequest), ProtobufSourcesGeneratorTarget.register_plugin_field(GoOwningGoModAddressField), ProtobufSourceTarget.register_plugin_field(GoOwningGoModAddressField), *protoc.rules(), diff --git a/src/python/pants/backend/codegen/protobuf/go/rules_integration_test.py b/src/python/pants/backend/codegen/protobuf/go/rules_integration_test.py index 323f7985893..daf8a6bf42d 100644 --- a/src/python/pants/backend/codegen/protobuf/go/rules_integration_test.py +++ b/src/python/pants/backend/codegen/protobuf/go/rules_integration_test.py @@ -10,6 +10,7 @@ from pants.backend.codegen.protobuf import protobuf_dependency_inference, target_types from pants.backend.codegen.protobuf.go.rules import ( GenerateGoFromProtobufRequest, + ProtobufGoCodegenImportRootsRequest, parse_go_package_option, ) from pants.backend.codegen.protobuf.go.rules import rules as go_protobuf_rules @@ -35,6 +36,7 @@ tests_analysis, third_party_pkg, ) +from pants.backend.go.util_rules.import_roots import GoCodegenImportRoots from pants.build_graph.address import Address from pants.core.goals.test import TestResult, get_filtered_environment from pants.core.util_rules import config_files, source_files, stripped_source_files @@ -77,6 +79,7 @@ def rule_runner() -> RuleRunner: QueryRule(GeneratedSources, [GenerateGoFromProtobufRequest]), QueryRule(DigestContents, (Digest,)), QueryRule(TestResult, (GoTestRequest.Batch,)), + QueryRule(GoCodegenImportRoots, [ProtobufGoCodegenImportRootsRequest]), ], target_types=[ GoModTarget, @@ -408,3 +411,66 @@ def test_generates_go_grpc(rule_runner: RuleRunner) -> None: "protos/service_grpc.pb.go", ], ) + + +def test_codegen_import_roots_reported_for_go_generating_protos(rule_runner: RuleRunner) -> None: + """Protobuf contributes the runtime modules that generated `.pb.go` files import. + + Those files do not exist when the import scan runs, so without this hook + `[golang].third_party_target_generation = "imported"` would generate no targets for the + protobuf runtime and the generated code would fail to build. + """ + rule_runner.write_files( + { + "BUILD": "go_mod(name='mod')\n", + "go.mod": "module example.com/m\ngo 1.16\n", + "f.proto": dedent( + """\ + syntax = "proto3"; + package example; + option go_package = "example.com/m/gen"; + message Foo { string bar = 1; } + """ + ), + } + ) + result = rule_runner.request( + GoCodegenImportRoots, + [ + ProtobufGoCodegenImportRootsRequest( + go_mod_address=Address("", target_name="mod"), + go_mod_path="go.mod", + ) + ], + ) + assert "google.golang.org/protobuf" in result.module_paths + # The per-target grpc toggle lives in the target graph, which this hook must not touch, so the + # grpc runtime is reported unconditionally. + assert "google.golang.org/grpc" in result.module_paths + + +def test_no_codegen_import_roots_without_go_generating_protos(rule_runner: RuleRunner) -> None: + """A proto with no `option go_package` generates no Go, so it contributes no roots.""" + rule_runner.write_files( + { + "BUILD": "go_mod(name='mod')\n", + "go.mod": "module example.com/m\ngo 1.16\n", + "f.proto": dedent( + """\ + syntax = "proto3"; + package example; + message Foo { string bar = 1; } + """ + ), + } + ) + result = rule_runner.request( + GoCodegenImportRoots, + [ + ProtobufGoCodegenImportRootsRequest( + go_mod_address=Address("", target_name="mod"), + go_mod_path="go.mod", + ) + ], + ) + assert result.module_paths == () diff --git a/src/python/pants/backend/experimental/go/register.py b/src/python/pants/backend/experimental/go/register.py index ae8da72de17..9e0d12189a3 100644 --- a/src/python/pants/backend/experimental/go/register.py +++ b/src/python/pants/backend/experimental/go/register.py @@ -31,6 +31,7 @@ implicit_linker_deps, import_analysis, import_config, + import_roots, link, pkg_analyzer, sdk, @@ -65,6 +66,7 @@ def rules(): *coverage_output.rules(), *cgo.rules(), *dep_inference_rules(), + *import_roots.rules(), *third_party_pkg.rules(), *generate.rules(), *go_bootstrap.rules(), diff --git a/src/python/pants/backend/go/util_rules/import_roots.py b/src/python/pants/backend/go/util_rules/import_roots.py index 3f97603f395..673638aa69a 100644 --- a/src/python/pants/backend/go/util_rules/import_roots.py +++ b/src/python/pants/backend/go/util_rules/import_roots.py @@ -25,6 +25,7 @@ from pants.backend.go.util_rules.pkg_analyzer import PackageAnalyzerSetup from pants.build_graph.address import Address from pants.engine.engine_aware import EngineAwareParameter +from pants.engine.environment import EnvironmentName from pants.engine.fs import GlobMatchErrorBehavior, MergeDigests, PathGlobs from pants.engine.intrinsics import ( digest_to_snapshot, @@ -34,6 +35,7 @@ ) from pants.engine.process import Process from pants.engine.rules import collect_rules, concurrently, implicitly, rule +from pants.engine.unions import union from pants.util.logging import LogLevel from pants.util.ordered_set import FrozenOrderedSet @@ -59,6 +61,39 @@ class FirstPartyImportRoots: import_paths: FrozenOrderedSet[str] +@union(in_scope_types=[EnvironmentName]) +@dataclass(frozen=True) +class GoCodegenImportRootsRequest: + """Entry point for contributing roots from Go sources that do not exist yet. + + Generated sources -- `.pb.go` and friends -- are not on disk when the scan runs, so nothing + reports the third-party packages they import. Without this, `imported` mode would decline to + generate targets for those packages and the generated code would fail to build. + + Implementations name whole *modules* rather than 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. + """ + + go_mod_address: Address + go_mod_path: str + + +@dataclass(frozen=True) +class GoCodegenImportRoots: + """Modules whose packages are all reachable because generated code may import them.""" + + module_paths: tuple[str, ...] + + +@rule(polymorphic=True) +async def determine_codegen_import_roots( + req: GoCodegenImportRootsRequest, env_name: EnvironmentName +) -> GoCodegenImportRoots: + raise NotImplementedError() + + def _is_nested_module_path(path: str, base_dir: str, nested_module_dirs: frozenset[str]) -> bool: """Whether `path` belongs to a module nested inside `base_dir` rather than to it. diff --git a/src/python/pants/backend/go/util_rules/third_party_pkg.py b/src/python/pants/backend/go/util_rules/third_party_pkg.py index c0a0ded8f15..1adfa2aa193 100644 --- a/src/python/pants/backend/go/util_rules/third_party_pkg.py +++ b/src/python/pants/backend/go/util_rules/third_party_pkg.py @@ -28,6 +28,8 @@ ) from pants.backend.go.util_rules.import_roots import ( FirstPartyImportRootsRequest, + GoCodegenImportRootsRequest, + determine_codegen_import_roots, determine_first_party_import_roots, ) from pants.backend.go.util_rules.pkg_analyzer import PackageAnalyzerSetup @@ -55,6 +57,7 @@ ) from pants.engine.process import Process, fallible_to_exec_result_or_raise from pants.engine.rules import collect_rules, concurrently, implicitly, rule +from pants.engine.unions import UnionMembership from pants.util.dirutil import group_by_dir from pants.util.frozendict import FrozenDict from pants.util.logging import LogLevel @@ -692,6 +695,7 @@ async def _analyze_imported_packages( request: AllThirdPartyPackagesRequest, module_analysis: ModuleDescriptors, go_sum_index: dict[tuple[str, str], tuple[str, ...]], + union_membership: UnionMembership, ) -> dict[str, ThirdPartyPkgAnalysis]: """Resolve only the third-party packages reachable from first-party code. @@ -729,12 +733,65 @@ def is_third_party(import_path: str) -> bool: return False return True + # Sources that do not exist yet -- generated `.pb.go` and friends -- cannot be scanned, so + # codegen backends name the runtime modules their output imports. These are pulled in whole: + # see `GoCodegenImportRootsRequest` for why package-level precision would buy nothing. + codegen_roots = await concurrently( + determine_codegen_import_roots( + **implicitly( + { + hook( + go_mod_address=request.go_mod_address, + go_mod_path=request.go_mod_path, + ): GoCodegenImportRootsRequest + } + ) + ) + for hook in union_membership.get(GoCodegenImportRootsRequest) + ) + codegen_module_names = { + module_path + for codegen_root in codegen_roots + for module_path in codegen_root.module_paths + if module_path in modules_by_name + } + known_packages: dict[str, ThirdPartyPkgAnalysis] = {} reachable: dict[str, ThirdPartyPkgAnalysis] = {} analyzed_module_names: set[str] = set() seen_import_paths: set[str] = set() + if codegen_module_names: + codegen_analyzed = await concurrently( + download_and_analyze_module( + ModuleDownloadRequest( + name=modules_by_name[module_name].name, + version=modules_by_name[module_name].version, + minimum_go_version=modules_by_name[module_name].minimum_go_version, + cgo_enabled=request.build_opts.cgo_enabled, + go_sum_entries=go_sum_index.get( + ( + modules_by_name[module_name].name, + modules_by_name[module_name].version, + ), + (), + ), + ), + **implicitly(), + ) + for module_name in sorted(codegen_module_names) + ) + for module_name, analyzed_module in zip(sorted(codegen_module_names), codegen_analyzed): + analyzed_module_names.add(module_name) + for pkg in analyzed_module.packages: + known_packages[pkg.import_path] = pkg + reachable[pkg.import_path] = pkg + frontier = {ip for ip in roots.import_paths if is_third_party(ip)} + # Whatever the codegen modules themselves import has to resolve too. + for pkg in reachable.values(): + frontier.update(ip for ip in pkg.imports if is_third_party(ip)) + seen_import_paths |= set(reachable) while frontier: seen_import_paths |= frontier @@ -744,9 +801,9 @@ def is_third_party(import_path: str) -> bool: # cost nothing here. pending_modules = {} for import_path in frontier: - module_name = _resolve_import_path_to_module(import_path, module_names_longest_first) - if module_name is not None and module_name not in analyzed_module_names: - pending_modules[module_name] = modules_by_name[module_name] + owning_module = _resolve_import_path_to_module(import_path, module_names_longest_first) + if owning_module is not None and owning_module not in analyzed_module_names: + pending_modules[owning_module] = modules_by_name[owning_module] if pending_modules: newly_analyzed = await concurrently( @@ -791,6 +848,7 @@ def is_third_party(import_path: str) -> bool: async def download_and_analyze_third_party_packages( request: AllThirdPartyPackagesRequest, golang_subsystem: GolangSubsystem, + union_membership: UnionMembership, ) -> AllThirdPartyPackages: module_analysis = await analyze_module_dependencies( ModuleDescriptorsRequest( @@ -817,7 +875,11 @@ async def download_and_analyze_third_party_packages( if golang_subsystem.third_party_target_generation is GoThirdPartyTargetGeneration.IMPORTED: return AllThirdPartyPackages( EMPTY_DIGEST, - FrozenDict(await _analyze_imported_packages(request, module_analysis, go_sum_index)), + FrozenDict( + await _analyze_imported_packages( + request, module_analysis, go_sum_index, union_membership + ) + ), ) # The engine memoizes by (name, version, minimum_go_version, cgo_enabled, From 5adb776a23405654c2c2b58729104c1da1bde323 Mon Sep 17 00:00:00 2001 From: Henry Yang Date: Tue, 1 Sep 2026 09:35:36 -0700 Subject: [PATCH 07/10] Adopt the rule-call convention at the new call sites, and add a release note `download_and_analyze_third_party_packages` now takes a subsystem and union membership, so the two call sites introduced on main by #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) Claude-Session: https://claude.ai/code/session_01GBwx1ZTZJ5oGxSAadmtucS --- docs/notes/2.34.x.md | 6 ++++++ src/python/pants/backend/codegen/protobuf/go/rules.py | 3 ++- src/python/pants/backend/go/target_type_rules.py | 3 ++- 3 files changed, 10 insertions(+), 2 deletions(-) diff --git a/docs/notes/2.34.x.md b/docs/notes/2.34.x.md index abbca47dacd..552b96903c2 100644 --- a/docs/notes/2.34.x.md +++ b/docs/notes/2.34.x.md @@ -99,6 +99,12 @@ Linked Go binaries and test binaries now record a Go toolchain build ID, as `go Dependency inference now resolves imports across local directory `replace` directives. When a `go.mod` replaces a required module with a path to another first-party module in the repo, imports of that module's packages are inferred automatically instead of having to be listed by hand in BUILD metadata. This is a no-op for modules without directory `replace` directives. See [#22097](https://github.com/pantsbuild/pants/issues/22097). +The new option `[golang].third_party_target_generation` controls which third-party packages a `go_mod` generates targets for. The default `build-list` keeps today's behavior, generating targets for every package of every module in the module build list. `imported` instead generates targets only for third-party packages reachable through the import graph from the `go_mod`'s own first-party code, and stops target generation from downloading and analyzing modules that nothing imports. + +The build list over-approximates what a repository actually uses: minimal version selection resolves at module granularity, so importing one package from a module puts that whole module, and everything it requires, on the list. Because target generation must analyze each listed module, it also downloads each one, whether or not anything imports it -- and the cost is independent of what you asked to build. + +Opting in changes two behaviors: third-party packages nothing imports no longer appear in `pants list ::`, and hand-written addresses pointing at such a package no longer resolve. Imports guarded by build tags, imports for other platforms, the `tools.go` pattern, and `tool` directives in `go.mod` are all treated as imports, so they still generate targets. See [#20340](https://github.com/pantsbuild/pants/issues/20340). + Third-party module downloads are no longer duplicated across callers that differ only in build options which cannot affect them. Downloading and analyzing a module consults just `cgo_enabled` -- `go mod download` ignores build options entirely, and the package analyzer reads only `CGO_ENABLED` -- but the whole `GoBuildOptions` was part of the memoization key. Target generation passes default options while a compile resolves the real ones, so a repository building with e.g. `race=True` downloaded every third-party module twice per run. ### Plugin API changes diff --git a/src/python/pants/backend/codegen/protobuf/go/rules.py b/src/python/pants/backend/codegen/protobuf/go/rules.py index 339b6962fad..77d9e89a96c 100644 --- a/src/python/pants/backend/codegen/protobuf/go/rules.py +++ b/src/python/pants/backend/codegen/protobuf/go/rules.py @@ -465,7 +465,8 @@ async def setup_full_package_build_request( go_mod_info.digest, go_mod_info.mod_path, build_opts=request.build_opts, - ) + ), + **implicitly(), ) third_party_index = all_third_party_packages.import_paths_to_pkg_info diff --git a/src/python/pants/backend/go/target_type_rules.py b/src/python/pants/backend/go/target_type_rules.py index 640bc5bc348..8457d39fcaa 100644 --- a/src/python/pants/backend/go/target_type_rules.py +++ b/src/python/pants/backend/go/target_type_rules.py @@ -122,7 +122,8 @@ async def expand_module_import_paths( go_mod_info.mod_path, # Matches target generation (see `generate_targets_from_go_mod`). build_opts=GoBuildOptions(), - ) + ), + **implicitly(), ) return ModuleImportPathsExpansion( { From e7d403e1c2c5ea3ad6f74678a673bb04f3ee7ab5 Mon Sep 17 00:00:00 2001 From: Henry Yang Date: Tue, 8 Sep 2026 23:01:51 -0700 Subject: [PATCH 08/10] Address review feedback 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 Claude-Session: https://claude.ai/code/session_01GBwx1ZTZJ5oGxSAadmtucS --- .../go/go_sources/analyze_package/main.go | 7 +-- .../pants/backend/go/subsystems/golang.py | 8 +-- .../backend/go/util_rules/first_party_pkg.py | 27 +-------- .../backend/go/util_rules/import_roots.py | 25 +++++++- .../go/util_rules/import_roots_test.py | 54 ++++++++++++++++- .../backend/go/util_rules/third_party_pkg.py | 60 +++++++++++++------ 6 files changed, 125 insertions(+), 56 deletions(-) diff --git a/src/python/pants/backend/go/go_sources/analyze_package/main.go b/src/python/pants/backend/go/go_sources/analyze_package/main.go index d1364ee0937..b320f27b098 100644 --- a/src/python/pants/backend/go/go_sources/analyze_package/main.go +++ b/src/python/pants/backend/go/go_sources/analyze_package/main.go @@ -253,8 +253,7 @@ func analyzePackage(directory string, buildContext *build.Context) (*Package, er // Record a file's imports into the constraint-free sets. Test files are split out by filename // rather than by package name, since a constraint-excluded file's package directive is exactly - // what we are declining to trust here. NB: declared before the loop because the loop shadows - // the `fileInfo` type with a variable of the same name. + // what we are declining to trust here. harvestAllImports := func(name string, analysis *fileInfo) { if analysis == nil { return @@ -302,12 +301,12 @@ func analyzePackage(directory string, buildContext *build.Context) (*Package, er // TODO: `MatchFile` will actually parse the imports but does not return the AST. Consider vendoring // the MatchFile logic to avoid double parsing. binaryOnly := false - fileInfo, err := matchFile(buildContext, directory, name, allTags, &binaryOnly, fileSet) + matchedFile, err := matchFile(buildContext, directory, name, allTags, &binaryOnly, fileSet) if err != nil { pkg.InvalidGoFiles[name] = err.Error() continue } - if fileInfo == nil { + if matchedFile == nil { if strings.HasPrefix(name, "_") || strings.HasPrefix(name, ".") { // `go` ignores files prefixed with underscore or period. Since this is not due to // build constraints, do not report it as an ignored file. Fall through. diff --git a/src/python/pants/backend/go/subsystems/golang.py b/src/python/pants/backend/go/subsystems/golang.py index dfc712651c2..fe2d931fd61 100644 --- a/src/python/pants/backend/go/subsystems/golang.py +++ b/src/python/pants/backend/go/subsystems/golang.py @@ -280,12 +280,12 @@ def extra_tools(self) -> tuple[str, ...]: third_party_target_generation = EnumOption( default=GoThirdPartyTargetGeneration.BUILD_LIST, help=softwrap( - """\ + """ Which `go_third_party_package` targets to generate for each `go_mod` target. - * `build-list`: generate a target for every package of every module in the module\ + * `build-list`: generate a target for every package of every module in the module build list, i.e. everything `go list -m all` reports. - * `imported`: generate targets only for third-party packages reachable through the\ + * `imported`: generate targets only for third-party packages reachable through the import graph from the `go_mod`'s own first-party code. The build list over-approximates what a repo actually uses, often by a lot: minimal @@ -299,7 +299,7 @@ def extra_tools(self) -> tuple[str, ...]: module downloads, at the cost of two behavior changes: * third-party packages nothing imports no longer appear in `pants list ::`; - * hand-written addresses pointing at an unimported third-party package no longer\ + * hand-written addresses pointing at an unimported third-party package no longer resolve. Imports guarded by build tags, imports for other platforms, the `tools.go` pattern and diff --git a/src/python/pants/backend/go/util_rules/first_party_pkg.py b/src/python/pants/backend/go/util_rules/first_party_pkg.py index d856f364981..4937cb02566 100644 --- a/src/python/pants/backend/go/util_rules/first_party_pkg.py +++ b/src/python/pants/backend/go/util_rules/first_party_pkg.py @@ -115,17 +115,6 @@ class FallibleFirstPartyPkgAnalysis: exit_code: int = 0 stderr: str | None = None - # Every import seen in the directory, ignoring build constraints -- see `AllImports` in the - # package analyzer. Deliberately carried on the fallible wrapper rather than on - # `FirstPartyPkgAnalysis`, because the cases that most need it are exactly the ones where - # `analysis` is None: a directory holding only a build-tag-gated `tools.go` fails with "no - # buildable Go source files", but its imports are still real target-generation roots. - # - # Empty when the analyzer process itself failed or its output could not be parsed, since - # there is nothing to read in those cases. - all_imports: tuple[str, ...] = () - all_test_imports: tuple[str, ...] = () - @classmethod def from_process_result( cls, @@ -169,14 +158,7 @@ def from_process_result( for filename, error in metadata.get("InvalidGoFiles", {}).items() ) error += "\n" - return cls( - analysis=None, - import_path=import_path, - exit_code=1, - stderr=error, - all_imports=tuple(metadata.get("AllImports", [])), - all_test_imports=tuple(metadata.get("AllTestImports", [])), - ) + return cls(analysis=None, import_path=import_path, exit_code=1, stderr=error) analysis = FirstPartyPkgAnalysis( dir_path=dir_path, @@ -209,12 +191,7 @@ def from_process_result( test_embed_patterns=tuple(metadata.get("TestEmbedPatterns", [])), xtest_embed_patterns=tuple(metadata.get("XTestEmbedPatterns", [])), ) - return cls( - analysis, - import_path, - all_imports=tuple(metadata.get("AllImports", [])), - all_test_imports=tuple(metadata.get("AllTestImports", [])), - ) + return cls(analysis, import_path) @dataclass(frozen=True) diff --git a/src/python/pants/backend/go/util_rules/import_roots.py b/src/python/pants/backend/go/util_rules/import_roots.py index 673638aa69a..feb3c03d2a7 100644 --- a/src/python/pants/backend/go/util_rules/import_roots.py +++ b/src/python/pants/backend/go/util_rules/import_roots.py @@ -20,7 +20,7 @@ import os.path from dataclasses import dataclass -import ijson +import ijson.backends.python as ijson from pants.backend.go.util_rules.pkg_analyzer import PackageAnalyzerSetup from pants.build_graph.address import Address @@ -48,7 +48,6 @@ class FirstPartyImportRootsRequest(EngineAwareParameter): go_mod_address: Address go_mod_path: str - cgo_enabled: bool def debug_hint(self) -> str: return self.go_mod_address.spec @@ -94,6 +93,22 @@ async def determine_codegen_import_roots( raise NotImplementedError() +def _is_ignored_by_go_walk(path: str, base_dir: str) -> bool: + """Whether `go list ./...` would skip this directory. + + `cmd/go` ignores `testdata` and `vendor` directories, and any path component beginning with + `.` or `_`. Without this a `testdata` fixture importing a module on the build list would make + it a root, downloading the module and generating targets for code nothing builds. + """ + relative = path[len(base_dir) :].lstrip("/") if base_dir else path + if not relative: + return False + return any( + component in ("testdata", "vendor") or component.startswith((".", "_")) + for component in relative.split("/") + ) + + def _is_nested_module_path(path: str, base_dir: str, nested_module_dirs: frozenset[str]) -> bool: """Whether `path` belongs to a module nested inside `base_dir` rather than to it. @@ -163,6 +178,7 @@ async def determine_first_party_import_roots( os.path.dirname(f) for f in sources_snapshot.files if not _is_nested_module_path(os.path.dirname(f), base_dir, nested_module_dirs) + and not _is_ignored_by_go_walk(os.path.dirname(f), base_dir) } ) @@ -183,7 +199,10 @@ async def determine_first_party_import_roots( f"({request.go_mod_address})" ), level=LogLevel.DEBUG, - env={"CGO_ENABLED": "1" if request.cgo_enabled else "0"}, + # NB: pinned rather than taken from the caller's build options. `AllImports` is + # harvested from every file regardless of cgo, so both settings yield the same + # roots and threading it through would only split this scan's memo key. + env={"CGO_ENABLED": "1"}, ), **implicitly(), ) diff --git a/src/python/pants/backend/go/util_rules/import_roots_test.py b/src/python/pants/backend/go/util_rules/import_roots_test.py index 5af61721599..ae557a58798 100644 --- a/src/python/pants/backend/go/util_rules/import_roots_test.py +++ b/src/python/pants/backend/go/util_rules/import_roots_test.py @@ -24,6 +24,7 @@ from pants.backend.go.util_rules.import_roots import ( FirstPartyImportRoots, FirstPartyImportRootsRequest, + _is_ignored_by_go_walk, _is_nested_module_path, _parse_tool_directives, ) @@ -132,7 +133,6 @@ def _roots(rule_runner: RuleRunner, *, go_mod_path: str = "go.mod") -> set[str]: FirstPartyImportRootsRequest( go_mod_address=Address("", target_name="mod"), go_mod_path=go_mod_path, - cgo_enabled=True, ) ], ) @@ -250,3 +250,55 @@ def test_scan_includes_tool_directives(rule_runner: RuleRunner) -> None: } ) assert "example.com/cmd/generator" in _roots(rule_runner) + + +def test_directories_go_list_ignores_are_skipped() -> None: + """`go list ./...` skips `testdata`, `vendor`, and `.`/`_` prefixed components.""" + assert not _is_ignored_by_go_walk("src/go/pkg/util", "src/go") + assert _is_ignored_by_go_walk("src/go/pkg/testdata", "src/go") + assert _is_ignored_by_go_walk("src/go/testdata/deep/pkg", "src/go") + assert _is_ignored_by_go_walk("src/go/vendor/github.com/x/y", "src/go") + assert _is_ignored_by_go_walk("src/go/_scratch", "src/go") + assert _is_ignored_by_go_walk("src/go/.hidden/pkg", "src/go") + # Only whole components count -- these merely share a prefix. + assert not _is_ignored_by_go_walk("src/go/testdata_helpers", "src/go") + assert not _is_ignored_by_go_walk("src/go/vendoring", "src/go") + # The module root itself is never ignored, even under a `_`-prefixed path. + assert not _is_ignored_by_go_walk("_vendored/mod", "_vendored/mod") + + +def test_scan_skips_testdata_and_vendor(rule_runner: RuleRunner) -> None: + """Imports that `go list ./...` never sees must not become roots.""" + rule_runner.write_files( + { + "BUILD": "go_mod(name='mod')\n", + "go.mod": "module example.com/m\ngo 1.16\n", + "main.go": dedent( + """\ + package main + + import "github.com/real/dep" + + func main() { dep.Do() } + """ + ), + "testdata/fixture.go": dedent( + """\ + package fixture + + import "github.com/testdata/only" + """ + ), + "vendor/github.com/x/y/y.go": dedent( + """\ + package y + + import "github.com/vendored/only" + """ + ), + } + ) + roots = _roots(rule_runner) + assert "github.com/real/dep" in roots + assert "github.com/testdata/only" not in roots + assert "github.com/vendored/only" not in roots diff --git a/src/python/pants/backend/go/util_rules/third_party_pkg.py b/src/python/pants/backend/go/util_rules/third_party_pkg.py index 1adfa2aa193..a8252a125ec 100644 --- a/src/python/pants/backend/go/util_rules/third_party_pkg.py +++ b/src/python/pants/backend/go/util_rules/third_party_pkg.py @@ -89,7 +89,9 @@ class ThirdPartyPkgAnalysis: # Note that we don't care about test-related metadata like `TestImports`, as we'll never run # tests directly on a third-party package. + # NB: filtered by the host's build constraints. See `all_imports` below. imports: tuple[str, ...] + go_files: tuple[str, ...] cgo_files: tuple[str, ...] cgo_flags: CGoCompilerFlags @@ -109,6 +111,13 @@ class ThirdPartyPkgAnalysis: test_embed_patterns: tuple[str, ...] xtest_embed_patterns: tuple[str, ...] + # `imports` without the host's build constraints applied. Import-scoped target generation must + # walk this set, or the closure it computes would depend on the machine running Pants: a + # first-party `//go:build windows` file contributes its direct import as a root, but that + # module's own Windows-only dependencies would not be followed. Defaulted because a failed + # analysis has no imports to report at all. + all_imports: tuple[str, ...] = () + embed_config: EmbedConfig | None = None test_embed_config: EmbedConfig | None = None xtest_embed_config: EmbedConfig | None = None @@ -464,6 +473,7 @@ async def analyze_go_third_party_package( dir_path=request.package_path, module_import_path=request.module_import_path, imports=tuple(request.pkg_json.get("Imports", ())), + all_imports=tuple(request.pkg_json.get("AllImports", ())), go_files=tuple(request.pkg_json.get("GoFiles", ())), c_files=tuple(request.pkg_json.get("CFiles", ())), cxx_files=tuple(request.pkg_json.get("CXXFiles", ())), @@ -675,20 +685,32 @@ async def download_and_analyze_module( return AnalyzedThirdPartyModule(FrozenOrderedSet(analyzed_packages)) -def _resolve_import_path_to_module( - import_path: str, module_names_longest_first: tuple[str, ...] -) -> str | None: - """Find the module in the build list that would provide `import_path`. +class ModuleImportPathResolver: + """Resolves an import path to the build-list module that provides it. Matches `cmd/go`: a module provides an import path when the module path is the import path or - a path-segment prefix of it. Where several modules qualify (e.g. `example.com/a` and - `example.com/a/b` both on the build list), the longest match wins, which is the one Go would - select. Passing the names pre-sorted keeps that unambiguous rather than order-dependent. + a path-segment prefix of it, and where several qualify (e.g. `example.com/a` and + `example.com/a/b` are both on the build list) the longest match wins. The longest-first + ordering is established in the constructor so `resolve` can rely on it rather than the caller + having to uphold it. + + Modules are indexed under both their name and their `import_path`: a versioned `replace` that + changes the path (`replace example.com/orig => example.com/fork v1.1.0`) has a `name` of + `example.com/fork` while first-party code still imports `example.com/orig/...`. """ - for name in module_names_longest_first: - if import_path == name or import_path.startswith(f"{name}/"): - return name - return None + + def __init__(self, module_analysis: ModuleDescriptors) -> None: + self._modules_by_path: dict[str, ModuleDescriptor] = {} + for mod in module_analysis.modules: + self._modules_by_path.setdefault(mod.name, mod) + self._modules_by_path.setdefault(mod.import_path, mod) + self._paths_longest_first = tuple(sorted(self._modules_by_path, key=len, reverse=True)) + + def resolve(self, import_path: str) -> ModuleDescriptor | None: + for path in self._paths_longest_first: + if import_path == path or import_path.startswith(f"{path}/"): + return self._modules_by_path[path] + return None async def _analyze_imported_packages( @@ -704,16 +726,14 @@ async def _analyze_imported_packages( with the build-list path, which downloads and analyzes every module `go list -m all` reports regardless of what the repo imports. """ + resolver = ModuleImportPathResolver(module_analysis) modules_by_name = {mod.name: mod for mod in module_analysis.modules} - # Longest first so the most specific module wins; see `_resolve_import_path_to_module`. - module_names_longest_first = tuple(sorted(modules_by_name, key=len, reverse=True)) roots, stdlib_packages = await concurrently( determine_first_party_import_roots( FirstPartyImportRootsRequest( go_mod_address=request.go_mod_address, go_mod_path=request.go_mod_path, - cgo_enabled=request.build_opts.cgo_enabled, ), **implicitly(), ), @@ -790,7 +810,7 @@ def is_third_party(import_path: str) -> bool: frontier = {ip for ip in roots.import_paths if is_third_party(ip)} # Whatever the codegen modules themselves import has to resolve too. for pkg in reachable.values(): - frontier.update(ip for ip in pkg.imports if is_third_party(ip)) + frontier.update(ip for ip in (pkg.all_imports or pkg.imports) if is_third_party(ip)) seen_import_paths |= set(reachable) while frontier: @@ -801,9 +821,9 @@ def is_third_party(import_path: str) -> bool: # cost nothing here. pending_modules = {} for import_path in frontier: - owning_module = _resolve_import_path_to_module(import_path, module_names_longest_first) - if owning_module is not None and owning_module not in analyzed_module_names: - pending_modules[owning_module] = modules_by_name[owning_module] + owning_module = resolver.resolve(import_path) + if owning_module is not None and owning_module.name not in analyzed_module_names: + pending_modules[owning_module.name] = owning_module if pending_modules: newly_analyzed = await concurrently( @@ -834,7 +854,9 @@ def is_third_party(import_path: str) -> bool: # build-list path would not have produced a target for it either. continue reachable[import_path] = reached_pkg - for dep_import_path in reached_pkg.imports: + # NB: `all_imports`, not `imports` -- see the field's comment. Falls back to the + # filtered set for analyses produced before the field existed. + for dep_import_path in reached_pkg.all_imports or reached_pkg.imports: if dep_import_path in seen_import_paths or not is_third_party(dep_import_path): continue next_frontier.add(dep_import_path) From 87874a1c3ff0c871d1cc7c479876eb4ce0fe1343 Mon Sep 17 00:00:00 2001 From: Henry Yang Date: Wed, 9 Sep 2026 00:40:53 -0700 Subject: [PATCH 09/10] Split `tool` directive support out of this PR 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 Claude-Session: https://claude.ai/code/session_01GBwx1ZTZJ5oGxSAadmtucS --- docs/notes/2.34.x.md | 2 +- .../pants/backend/go/subsystems/golang.py | 5 +- .../backend/go/util_rules/import_roots.py | 52 +------------ .../go/util_rules/import_roots_test.py | 75 ------------------- 4 files changed, 5 insertions(+), 129 deletions(-) diff --git a/docs/notes/2.34.x.md b/docs/notes/2.34.x.md index 72a3d151ce7..134452b9c08 100644 --- a/docs/notes/2.34.x.md +++ b/docs/notes/2.34.x.md @@ -104,7 +104,7 @@ The new option `[golang].third_party_target_generation` controls which third-par The build list over-approximates what a repository actually uses: minimal version selection resolves at module granularity, so importing one package from a module puts that whole module, and everything it requires, on the list. Because target generation must analyze each listed module, it also downloads each one, whether or not anything imports it -- and the cost is independent of what you asked to build. -Opting in changes two behaviors: third-party packages nothing imports no longer appear in `pants list ::`, and hand-written addresses pointing at such a package no longer resolve. Imports guarded by build tags, imports for other platforms, the `tools.go` pattern, and `tool` directives in `go.mod` are all treated as imports, so they still generate targets. See [#20340](https://github.com/pantsbuild/pants/issues/20340). +Opting in changes two behaviors: third-party packages nothing imports no longer appear in `pants list ::`, and hand-written addresses pointing at such a package no longer resolve. Imports guarded by build tags, imports for other platforms, and the `tools.go` pattern are all treated as imports, so they still generate targets. See [#20340](https://github.com/pantsbuild/pants/issues/20340). Third-party module downloads are no longer duplicated across callers that differ only in build options which cannot affect them. Downloading and analyzing a module consults just `cgo_enabled` -- `go mod download` ignores build options entirely, and the package analyzer reads only `CGO_ENABLED` -- but the whole `GoBuildOptions` was part of the memoization key. Target generation passes default options while a compile resolves the real ones, so a repository building with e.g. `race=True` downloaded every third-party module twice per run. diff --git a/src/python/pants/backend/go/subsystems/golang.py b/src/python/pants/backend/go/subsystems/golang.py index fe2d931fd61..27b57577f20 100644 --- a/src/python/pants/backend/go/subsystems/golang.py +++ b/src/python/pants/backend/go/subsystems/golang.py @@ -302,9 +302,8 @@ def extra_tools(self) -> tuple[str, ...]: * hand-written addresses pointing at an unimported third-party package no longer resolve. - Imports guarded by build tags, imports for other platforms, the `tools.go` pattern and - `tool` directives in `go.mod` are all treated as imports, so they still generate - targets. + Imports guarded by build tags, imports for other platforms and the `tools.go` pattern + are all treated as imports, so they still generate targets. """ ), ) diff --git a/src/python/pants/backend/go/util_rules/import_roots.py b/src/python/pants/backend/go/util_rules/import_roots.py index feb3c03d2a7..e9dac0b3312 100644 --- a/src/python/pants/backend/go/util_rules/import_roots.py +++ b/src/python/pants/backend/go/util_rules/import_roots.py @@ -26,13 +26,8 @@ from pants.build_graph.address import Address from pants.engine.engine_aware import EngineAwareParameter from pants.engine.environment import EnvironmentName -from pants.engine.fs import GlobMatchErrorBehavior, MergeDigests, PathGlobs -from pants.engine.intrinsics import ( - digest_to_snapshot, - execute_process, - get_digest_contents, - merge_digests, -) +from pants.engine.fs import MergeDigests, PathGlobs +from pants.engine.intrinsics import digest_to_snapshot, execute_process, merge_digests from pants.engine.process import Process from pants.engine.rules import collect_rules, concurrently, implicitly, rule from pants.engine.unions import union @@ -123,35 +118,6 @@ def _is_nested_module_path(path: str, base_dir: str, nested_module_dirs: frozens return False -def _parse_tool_directives(go_mod_content: bytes) -> tuple[str, ...]: - """Extract `tool` directives from a go.mod (Go 1.24+). - - A `tool` directive names an executable package the module depends on without any `.go` file - importing it, which is precisely the modern replacement for the `tools.go` pattern. Missing - these would drop targets the user explicitly asked for. - - Handles both the single-line form (`tool example.com/cmd/foo`) and the block form - (`tool (\\n\\texample.com/cmd/foo\\n)`). - """ - tools: list[str] = [] - in_block = False - for raw_line in go_mod_content.decode(errors="replace").splitlines(): - line = raw_line.split("//", 1)[0].strip() - if not line: - continue - if in_block: - if line == ")": - in_block = False - else: - tools.append(line) - continue - if line == "tool (": - in_block = True - elif line.startswith("tool ") or line.startswith("tool\t"): - tools.append(line[len("tool") :].strip()) - return tuple(t for t in tools if t) - - @rule(desc="Scan first-party Go sources for third-party imports", level=LogLevel.DEBUG) async def determine_first_party_import_roots( request: FirstPartyImportRootsRequest, @@ -212,20 +178,6 @@ async def determine_first_party_import_roots( import_paths.update(pkg_json.get("AllImports", ())) import_paths.update(pkg_json.get("AllTestImports", ())) - go_mod_contents = await get_digest_contents( - **implicitly( - PathGlobs( - [request.go_mod_path], - glob_match_error_behavior=GlobMatchErrorBehavior.error, - description_of_origin=( - f"the import-scoped target generation scan for {request.go_mod_address}" - ), - ) - ) - ) - for entry in go_mod_contents: - import_paths.update(_parse_tool_directives(entry.content)) - return FirstPartyImportRoots(FrozenOrderedSet(sorted(import_paths))) diff --git a/src/python/pants/backend/go/util_rules/import_roots_test.py b/src/python/pants/backend/go/util_rules/import_roots_test.py index ae557a58798..0e49c7021f1 100644 --- a/src/python/pants/backend/go/util_rules/import_roots_test.py +++ b/src/python/pants/backend/go/util_rules/import_roots_test.py @@ -26,7 +26,6 @@ FirstPartyImportRootsRequest, _is_ignored_by_go_walk, _is_nested_module_path, - _parse_tool_directives, ) from pants.build_graph.address import Address from pants.engine.rules import QueryRule @@ -56,54 +55,6 @@ def rule_runner() -> RuleRunner: return rule_runner -# ------------------------------------------------------------------------------------------------ -# `tool` directive parsing (Go 1.24+) -# ------------------------------------------------------------------------------------------------ - - -def test_parse_tool_directives_single_line() -> None: - go_mod = b"module example.com/m\n\ngo 1.24\n\ntool example.com/cmd/foo\n" - assert _parse_tool_directives(go_mod) == ("example.com/cmd/foo",) - - -def test_parse_tool_directives_block() -> None: - go_mod = dedent( - """\ - module example.com/m - - go 1.24 - - tool ( - \texample.com/cmd/foo - \texample.com/cmd/bar - ) - """ - ).encode() - assert _parse_tool_directives(go_mod) == ("example.com/cmd/foo", "example.com/cmd/bar") - - -def test_parse_tool_directives_ignores_comments_and_lookalikes() -> None: - go_mod = dedent( - """\ - module example.com/toolkit - - go 1.24 - - // tool example.com/cmd/commented-out - require example.com/toolbox v1.0.0 - - tool example.com/cmd/real // keep this one - """ - ).encode() - # `module example.com/toolkit` and `require example.com/toolbox` both start with "tool" as a - # substring; neither is a tool directive. - assert _parse_tool_directives(go_mod) == ("example.com/cmd/real",) - - -def test_parse_tool_directives_absent() -> None: - assert _parse_tool_directives(b"module example.com/m\n\ngo 1.21\n") == () - - # ------------------------------------------------------------------------------------------------ # Nested module exclusion # ------------------------------------------------------------------------------------------------ @@ -226,32 +177,6 @@ def test_scan_skips_nested_modules(rule_runner: RuleRunner) -> None: assert "github.com/nested/dep" not in roots -def test_scan_includes_tool_directives(rule_runner: RuleRunner) -> None: - """`tool` directives are roots even though no .go file imports them.""" - rule_runner.write_files( - { - "BUILD": "go_mod(name='mod')\n", - "go.mod": dedent( - """\ - module example.com/m - - go 1.24 - - tool example.com/cmd/generator - """ - ), - "main.go": dedent( - """\ - package main - - func main() {} - """ - ), - } - ) - assert "example.com/cmd/generator" in _roots(rule_runner) - - def test_directories_go_list_ignores_are_skipped() -> None: """`go list ./...` skips `testdata`, `vendor`, and `.`/`_` prefixed components.""" assert not _is_ignored_by_go_walk("src/go/pkg/util", "src/go") From 30292e927c29191942f8f00d8646a630ae0351f2 Mon Sep 17 00:00:00 2001 From: Henry Yang Date: Wed, 9 Sep 2026 09:37:24 -0700 Subject: [PATCH 10/10] Adopt the rule-call convention at the remaining call sites 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 Claude-Session: https://claude.ai/code/session_01GBwx1ZTZJ5oGxSAadmtucS --- src/python/pants/backend/go/goals/debug_goals.py | 3 ++- src/python/pants/backend/go/util_rules/build_pkg_target.py | 3 ++- .../pants/backend/go/util_rules/build_pkg_third_party.py | 3 ++- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/python/pants/backend/go/goals/debug_goals.py b/src/python/pants/backend/go/goals/debug_goals.py index 4cfbe2cb851..f9e3ed989c4 100644 --- a/src/python/pants/backend/go/goals/debug_goals.py +++ b/src/python/pants/backend/go/goals/debug_goals.py @@ -149,7 +149,8 @@ async def go_show_package_analysis( request.go_mod_digest, request.go_mod_path, build_opts=request.build_opts, - ) + ), + **implicitly(), ) if request.import_path not in all_packages.import_paths_to_pkg_info: console.write_stdout( 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 0f26183c701..b9680b081a1 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 @@ -477,7 +477,8 @@ async def setup_build_go_package_target_request( owning_go_mod_info.digest, owning_go_mod_info.mod_path, build_opts=request.build_opts, - ) + ), + **implicitly(), ) resolvable = [ imp diff --git a/src/python/pants/backend/go/util_rules/build_pkg_third_party.py b/src/python/pants/backend/go/util_rules/build_pkg_third_party.py index dd6dfa8a3a1..fe810383a76 100644 --- a/src/python/pants/backend/go/util_rules/build_pkg_third_party.py +++ b/src/python/pants/backend/go/util_rules/build_pkg_third_party.py @@ -64,7 +64,8 @@ async def setup_build_go_package_target_request_for_third_party( go_mod_info.digest, go_mod_info.mod_path, build_opts=request.build_opts, - ) + ), + **implicitly(), ) pkg_info = all_packages.import_paths_to_pkg_info.get(request.import_path) if pkg_info is None: