diff --git a/docs/notes/2.34.x.md b/docs/notes/2.34.x.md index 9d50af30c3e..5e50e0d08c3 100644 --- a/docs/notes/2.34.x.md +++ b/docs/notes/2.34.x.md @@ -211,7 +211,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")