diff --git a/docs/notes/2.34.x.md b/docs/notes/2.34.x.md index 040c8d45391..a132e2092ec 100644 --- a/docs/notes/2.34.x.md +++ b/docs/notes/2.34.x.md @@ -64,6 +64,8 @@ Optimized third-party Go package compilation by materializing only the package's The new advanced option `[golang].third_party_target_granularity` controls how third-party targets are generated. The default `package` keeps one `go_third_party_package` target per third-party package; `module` instead generates one `go_third_party_module` target per module, matching how other backends model third-party dependencies, with transitive resolution owned by `go.mod`/`go.sum`. This dramatically reduces target count and dependency-resolution time on projects with large third-party modules. +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). + ### Plugin API changes ## Full Changelog diff --git a/src/python/pants/backend/go/target_type_rules.py b/src/python/pants/backend/go/target_type_rules.py index dd662b31853..d2f2a467fa7 100644 --- a/src/python/pants/backend/go/target_type_rules.py +++ b/src/python/pants/backend/go/target_type_rules.py @@ -48,6 +48,7 @@ GoModInfoRequest, OwningGoModRequest, determine_go_mod_info, + find_all_go_mod_targets, find_owning_go_mod, ) from pants.backend.go.util_rules.import_analysis import ( @@ -273,7 +274,52 @@ async def map_import_paths_to_packages( request: GoImportPathMappingRequest, module_import_path_mappings: AllGoModuleImportPathsMappings, ) -> GoModuleImportPathsMapping: - return module_import_path_mappings.modules[request.go_mod_address] + base = module_import_path_mappings.modules[request.go_mod_address] + + # #22097: a `go.mod` may `replace` a `require`d module with a local directory that holds + # another first-party `go_mod` target. Imports of that module's packages are not in this + # module's own import-path map, so fold in the replaced modules' maps. Go's `replace` + # directives only apply within the module that declares them (they are not transitive), so a + # single level of merging matches the build semantics. + go_mod_info = await determine_go_mod_info(GoModInfoRequest(request.go_mod_address)) + if not go_mod_info.local_replace_module_dirs: + return base + + all_go_mod_targets = await find_all_go_mod_targets(**implicitly()) + address_by_spec_path = {tgt.address.spec_path: tgt.address for tgt in all_go_mod_targets} + + merged_mapping: dict[str, GoImportPathsMappingAddressSet] = dict(base.mapping) + merged_address_to_import_path: dict[Address, str] = dict(base.address_to_import_path) + for replaced_module_path, replaced_dir in go_mod_info.local_replace_module_dirs.items(): + replaced_address = address_by_spec_path.get(replaced_dir) + if replaced_address is None or replaced_address == request.go_mod_address: + continue + replaced_mapping = module_import_path_mappings.modules.get(replaced_address) + if replaced_mapping is None: + continue + # Only fold in the replaced module's OWN packages -- import paths under its module path. + # Its third-party dependencies must NOT be merged: the referencing module resolves those + # through its own `go.mod`, and merging them would make every shared third-party import + # path ambiguous (two addresses), which suppresses the inferred dependency. + prefix = f"{replaced_module_path}/" + for import_path, address_set in replaced_mapping.mapping.items(): + if import_path != replaced_module_path and not import_path.startswith(prefix): + continue + existing = merged_mapping.get(import_path) + if existing is None: + merged_mapping[import_path] = address_set + else: + merged_mapping[import_path] = GoImportPathsMappingAddressSet( + addresses=tuple(sorted({*existing.addresses, *address_set.addresses})), + infer_all=existing.infer_all or address_set.infer_all, + ) + for address in address_set.addresses: + merged_address_to_import_path.setdefault(address, import_path) + + return GoModuleImportPathsMapping( + mapping=FrozenDict(merged_mapping), + address_to_import_path=FrozenDict(merged_address_to_import_path), + ) @dataclass(frozen=True) diff --git a/src/python/pants/backend/go/target_type_rules_test.py b/src/python/pants/backend/go/target_type_rules_test.py index a3bd9655862..5be5e4763ce 100644 --- a/src/python/pants/backend/go/target_type_rules_test.py +++ b/src/python/pants/backend/go/target_type_rules_test.py @@ -225,6 +225,147 @@ def get_deps(addr: Address) -> set[Address]: assert get_deps(Address("foo", generated_name="golang.org/x/text")) == go_mod_file_tgts +def test_cross_module_local_replace_dependency_inference(rule_runner: RuleRunner) -> None: + # A package in one module that imports a package from a sibling first-party module, wired via a + # local-directory `replace` directive, should have that dependency inferred (#22097). + rule_runner.write_files( + { + "app/BUILD": "go_mod()", + "app/go.mod": dedent( + """\ + module go.example.com/app + go 1.17 + + require go.example.com/lib v0.0.0 + + replace go.example.com/lib => ../lib + """ + ), + "app/go.sum": "", + "app/pkg/app.go": dedent( + """\ + package pkg + + import "go.example.com/lib/greet" + + var _ = greet.Hello + """ + ), + "app/pkg/BUILD": "go_package()", + "lib/BUILD": "go_mod()", + "lib/go.mod": dedent( + """\ + module go.example.com/lib + go 1.17 + """ + ), + "lib/greet/greet.go": dedent( + """\ + package greet + + const Hello = "hello" + """ + ), + "lib/greet/BUILD": "go_package()", + } + ) + + def get_deps(addr: Address) -> set[Address]: + tgt = rule_runner.get_target(addr) + return set( + rule_runner.request( + Addresses, + [DependenciesRequest(tgt[Dependencies])], + ) + ) + + assert get_deps(Address("app/pkg")) == {Address("lib/greet")} + + +def test_cross_module_local_replace_does_not_shadow_third_party(rule_runner: RuleRunner) -> None: + # A locally-replaced module's OWN third-party dependencies must not be folded into the + # referencing module's import map. If they were, a third-party import shared by both modules + # would map to two addresses (one generated per `go_mod`), become ambiguous, and have its + # inferred dependency silently dropped (#22097 regression guard). + go_sum = dedent( + """\ + golang.org/x/text v0.0.0-20170915032832-14c0d48ead0c h1:qgOY6WgZOaTkIIMiVjBQcw93ERBE4m30iBm00nkL0i8= + golang.org/x/text v0.0.0-20170915032832-14c0d48ead0c/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= + rsc.io/quote v1.5.2 h1:w5fcysjrx7yqtD/aO+QwRjYZOKnaM9Uh2b40tElTs3Y= + rsc.io/quote v1.5.2/go.mod h1:LzX7hefJvL54yjefDEDHNONDjII0t9xZLPXsUe+TKr0= + rsc.io/sampler v1.3.0 h1:7uVkIFmeBqHfdjD+gZwtXXI+RODJ2Wc4O7MPEh/QiW4= + rsc.io/sampler v1.3.0/go.mod h1:T1hPZKmBbMNahiBKFy5HrXp6adAjACjK9JXDnKaTXpA= + """ + ) + rule_runner.write_files( + { + "app/BUILD": "go_mod()", + "app/go.mod": dedent( + """\ + module go.example.com/app + go 1.17 + + require ( + go.example.com/lib v0.0.0 + rsc.io/quote v1.5.2 + ) + + replace go.example.com/lib => ../lib + """ + ), + "app/go.sum": go_sum, + "app/pkg/app.go": dedent( + """\ + package pkg + + import ( + "go.example.com/lib/greet" + "rsc.io/quote" + ) + + var _ = greet.Hello + var _ = quote.Hello + """ + ), + "app/pkg/BUILD": "go_package()", + "lib/BUILD": "go_mod()", + "lib/go.mod": dedent( + """\ + module go.example.com/lib + go 1.17 + + require rsc.io/quote v1.5.2 + """ + ), + "lib/go.sum": go_sum, + "lib/greet/greet.go": dedent( + """\ + package greet + + import "rsc.io/quote" + + var Hello = quote.Hello + """ + ), + "lib/greet/BUILD": "go_package()", + } + ) + + def get_deps(addr: Address) -> set[Address]: + tgt = rule_runner.get_target(addr) + return set( + rule_runner.request( + Addresses, + [DependenciesRequest(tgt[Dependencies])], + ) + ) + + deps = get_deps(Address("app/pkg")) + # The first-party cross-module import resolves, AND the shared third-party import resolves to + # `app`'s own generated target -- unambiguously, not dropped. + assert deps == {Address("lib/greet"), Address("app", generated_name="rsc.io/quote")} + + # ----------------------------------------------------------------------------------------------- # `go_package` validation # ----------------------------------------------------------------------------------------------- diff --git a/src/python/pants/backend/go/util_rules/go_mod.py b/src/python/pants/backend/go/util_rules/go_mod.py index d7b58a7f4a2..ef88e9ada8d 100644 --- a/src/python/pants/backend/go/util_rules/go_mod.py +++ b/src/python/pants/backend/go/util_rules/go_mod.py @@ -34,6 +34,7 @@ WrappedTargetRequest, ) from pants.util.docutil import bin_name +from pants.util.frozendict import FrozenDict from pants.util.logging import LogLevel logger = logging.getLogger(__name__) @@ -218,6 +219,18 @@ async def find_owning_go_mod( ) +def _is_directory_replacement(new_path: str) -> bool: + # Mirrors `modfile.IsDirectoryPath`: a `replace` directive whose target is a local + # directory (as opposed to a different module path) starts with `./`, `../`, `/`, or + # is `.`/`..` (or an absolute Windows path). Such replacements wire a `require`d module + # to a sibling first-party module on disk; module-path replacements do not. + return ( + new_path in (".", "..") + or new_path.startswith(("./", "../", "/")) + or os.path.isabs(new_path) + ) + + @dataclass(frozen=True) class GoModInfo: # Import path of the Go module, based on the `module` in `go.mod`. @@ -225,6 +238,10 @@ class GoModInfo: digest: Digest mod_path: str minimum_go_version: str | None + # Maps the module path of each `replace`d module whose target is a local directory to + # that directory (relative to the build root). These are first-party modules referenced + # across `go.mod` boundaries; see `map_import_paths_to_packages` and issue #22097. + local_replace_module_dirs: FrozenDict[str, str] = FrozenDict() @dataclass(frozen=True) @@ -268,11 +285,29 @@ async def determine_go_mod_info( ) ) module_metadata = json.loads(mod_json.stdout) + + # Record `replace` directives that point a `require`d module at a local directory, so that + # dependency inference can resolve imports satisfied by sibling first-party modules. See #22097. + local_replace_module_dirs: dict[str, str] = {} + for replace in module_metadata.get("Replace") or []: + new = replace.get("New") or {} + new_path = new.get("Path") + # A directory replacement targets a filesystem path and carries no module version; a + # module-path replacement (path + version) is left to normal third-party resolution. + if not new_path or new.get("Version") or not _is_directory_replacement(new_path): + continue + old_path = (replace.get("Old") or {}).get("Path") + # Absolute on-disk paths cannot be mapped to in-repo `go_mod` targets, so skip them. + if not old_path or os.path.isabs(new_path): + continue + local_replace_module_dirs[old_path] = os.path.normpath(os.path.join(go_mod_dir, new_path)) + return GoModInfo( import_path=module_metadata["Module"]["Path"], digest=sources_digest, mod_path=go_mod_path, minimum_go_version=module_metadata.get("Go"), + local_replace_module_dirs=FrozenDict(local_replace_module_dirs), ) 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 2ce10577432..bc743fc3a2e 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 @@ -284,8 +284,9 @@ async def analyze_module_dependencies(request: ModuleDescriptorsRequest) -> Modu if "Main" in mod_json and mod_json["Main"]: continue - # Skip first-party modules referenced from other first-party modules. - # TODO Issue #22097: These cross-module references could be used for dependency inference + # Skip first-party modules referenced from other first-party modules via a local + # directory `replace`. These are not third-party packages; `map_import_paths_to_packages` + # folds their import paths into the referencing module's inference map (#22097). if "Replace" in mod_json and "Version" not in mod_json["Replace"]: continue