From 69cf0bc9c70eb6db0c2a2dc18c18baf70eaf6928 Mon Sep 17 00:00:00 2001 From: Henry Yang Date: Wed, 9 Sep 2026 00:41:22 -0700 Subject: [PATCH] Treat `go.mod` `tool` directives as import roots A `tool` directive (Go 1.24+) names an executable package a module depends on without any `.go` file importing it. It is the official replacement for the `tools.go` pattern, which `[golang].third_party_target_generation = "imported"` already handles via the constraint-free scan. Without this, a repository that has migrated to `tool` directives loses those targets under `imported` and fails to build, while one still using `tools.go` does not -- an inconsistency between the old idiom and its successor. Parses both the single-line form (`tool example.com/cmd/foo`) and the block form. Comments are stripped first, and matching is on the `tool` keyword rather than a prefix, so `module example.com/toolkit` and `require example.com/toolbox` are not mistaken for directives. Split out of #23665 at review request. 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, 129 insertions(+), 5 deletions(-) diff --git a/docs/notes/2.34.x.md b/docs/notes/2.34.x.md index 1b7b6729b42..eba8e8c561a 100644 --- a/docs/notes/2.34.x.md +++ b/docs/notes/2.34.x.md @@ -160,7 +160,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, and the `tools.go` pattern 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, 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. diff --git a/src/python/pants/backend/go/subsystems/golang.py b/src/python/pants/backend/go/subsystems/golang.py index 27b57577f20..fe2d931fd61 100644 --- a/src/python/pants/backend/go/subsystems/golang.py +++ b/src/python/pants/backend/go/subsystems/golang.py @@ -302,8 +302,9 @@ 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 and the `tools.go` pattern - are all treated as imports, so they still generate targets. + 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. """ ), ) 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 e9dac0b3312..feb3c03d2a7 100644 --- a/src/python/pants/backend/go/util_rules/import_roots.py +++ b/src/python/pants/backend/go/util_rules/import_roots.py @@ -26,8 +26,13 @@ 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 MergeDigests, PathGlobs -from pants.engine.intrinsics import digest_to_snapshot, execute_process, merge_digests +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.engine.unions import union @@ -118,6 +123,35 @@ 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, @@ -178,6 +212,20 @@ 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 0e49c7021f1..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 @@ -26,6 +26,7 @@ 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 @@ -55,6 +56,54 @@ 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 # ------------------------------------------------------------------------------------------------ @@ -177,6 +226,32 @@ 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")