From 7b5b86f515d0f9eb8032d3fa309de99d35f857ca Mon Sep 17 00:00:00 2001 From: Eric Arellano <14852634+Eric-Arellano@users.noreply.github.com> Date: Thu, 7 Oct 2021 20:15:40 -0700 Subject: [PATCH] [internal] Fix `go_binary`'s `main` field inference to work with generated targets (#13163) Restores support for https://github.com/pantsbuild/pants/pull/13117 post-https://github.com/pantsbuild/pants/pull/13139. [ci skip-rust] --- .../goals/package_binary_integration_test.py | 3 +- .../pants/backend/go/target_type_rules.py | 40 ++++++++++-------- .../backend/go/target_type_rules_test.py | 42 +++++++++++-------- src/python/pants/backend/go/target_types.py | 21 ++++++++-- 4 files changed, 65 insertions(+), 41 deletions(-) diff --git a/src/python/pants/backend/go/goals/package_binary_integration_test.py b/src/python/pants/backend/go/goals/package_binary_integration_test.py index e429644944b..c704b303e77 100644 --- a/src/python/pants/backend/go/goals/package_binary_integration_test.py +++ b/src/python/pants/backend/go/goals/package_binary_integration_test.py @@ -161,8 +161,7 @@ def test_package_with_dependencies(rule_runner: RuleRunner) -> None: "BUILD": dedent( """\ go_mod(name='mod') - # TODO: Remove `main` once it's fixed for target gen. - go_binary(name='bin', main='//:mod#./') + go_binary(name='bin') """ ), } diff --git a/src/python/pants/backend/go/target_type_rules.py b/src/python/pants/backend/go/target_type_rules.py index bacf1142214..da5ace8258e 100644 --- a/src/python/pants/backend/go/target_type_rules.py +++ b/src/python/pants/backend/go/target_type_rules.py @@ -41,7 +41,7 @@ ) from pants.backend.go.util_rules.import_analysis import GoStdLibImports from pants.base.exceptions import ResolveError -from pants.base.specs import AddressSpecs, DescendantAddresses, SiblingAddresses +from pants.base.specs import AddressSpecs, AscendantAddresses, DescendantAddresses from pants.core.goals.tailor import group_by_dir from pants.engine.addresses import Address, AddressInput from pants.engine.fs import PathGlobs, Paths @@ -290,34 +290,40 @@ async def determine_main_pkg_for_go_binary( if not wrapped_specified_tgt.target.has_field(GoInternalPackageSourcesField): raise InvalidFieldException( f"The {repr(GoBinaryMainPackageField.alias)} field in target {addr} must point to " - "a `go_package` target, but was the address for a " + "a `_go_internal_package` target, but was the address for a " f"`{wrapped_specified_tgt.target.alias}` target.\n\n" - "Hint: consider leaving off this field so that Pants will find the `go_package` " - "target for you." + "Hint: consider leaving off this field so that Pants will find the " + "`_go_internal_package` target for you." ) return GoBinaryMainPackage(wrapped_specified_tgt.target.address) - # TODO: fix this to account for `_go_internal_package` being generated. - build_dir_targets = await Get(Targets, AddressSpecs([SiblingAddresses(addr.spec_path)])) - internal_pkg_targets = [ - tgt for tgt in build_dir_targets if tgt.has_field(GoInternalPackageSourcesField) + candidate_targets = await Get(Targets, AddressSpecs([AscendantAddresses(addr.spec_path)])) + relevant_pkg_targets = [ + tgt + for tgt in candidate_targets + if ( + tgt.has_field(GoInternalPackageSubpathField) + and tgt[GoInternalPackageSubpathField].full_dir_path == addr.spec_path + ) ] - if len(internal_pkg_targets) == 1: - return GoBinaryMainPackage(internal_pkg_targets[0].address) + if len(relevant_pkg_targets) == 1: + return GoBinaryMainPackage(relevant_pkg_targets[0].address) wrapped_tgt = await Get(WrappedTarget, Address, addr) alias = wrapped_tgt.target.alias - if not internal_pkg_targets: + if not relevant_pkg_targets: raise ResolveError( - f"The `{alias}` target {addr} requires that there is a `go_package` " - "target in the same directory, but none were found." + f"The `{alias}` target {addr} requires that there is a `_go_internal_package` " + f"target for its directory {addr.spec_path}, but none were found.\n\n" + "Have you added a `go_mod` target (which will generate `_go_internal_package` targets)?" ) raise ResolveError( - f"There are multiple `go_package` targets in the same directory of the `{alias}` " - f"target {addr}, so it is ambiguous what to use as the `main` package.\n\n" + f"There are multiple `_go_internal_package` targets for the same directory of the " + "`{alias}` target {addr}: {addr.spec_path}. It is ambiguous what to use as the `main` " + "package.\n\n" f"To fix, please either set the `main` field for `{addr} or remove these " - "`go_package` targets so that only one remains: " - f"{sorted(tgt.address.spec for tgt in internal_pkg_targets)}" + "`_go_internal_package` targets so that only one remains: " + f"{sorted(tgt.address.spec for tgt in relevant_pkg_targets)}" ) 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 39344900fc5..a62312033e5 100644 --- a/src/python/pants/backend/go/target_type_rules_test.py +++ b/src/python/pants/backend/go/target_type_rules_test.py @@ -263,29 +263,29 @@ def gen_external_tgt(mod_path: str, version: str, import_path: str) -> GoExterna # ----------------------------------------------------------------------------------------------- -@pytest.mark.xfail def test_determine_main_pkg_for_go_binary(rule_runner: RuleRunner) -> None: rule_runner.write_files( { - "explicit/BUILD": dedent( + "go.mod": dedent( """\ - go_package(name='pkg', sources=[]) - go_binary(main=':pkg') - """ - ), - "inferred/BUILD": dedent( - """\ - go_package(name='pkg', sources=[]) - go_binary() + module example.com/foo + go 1.17 """ ), - "ambiguous/BUILD": dedent( + "BUILD": "go_mod(name='mod')", + "explicit/f.go": "", + "explicit/BUILD": "go_binary(main='//:mod#./explicit')", + "inferred/f.go": "", + "inferred/BUILD": "go_binary()", + "ambiguous/f.go": "", + "ambiguous/go.mod": dedent( """\ - go_package(name='pkg1', sources=[]) - go_package(name='pkg2', sources=[]) - go_binary() + module example.com/ambiguous + go 1.17 """ ), + "ambiguous/BUILD": "go_binary()", + # Note there are no `.go` files in this dir, so no package targets will be created. "missing/BUILD": "go_binary()", "explicit_wrong_type/BUILD": dedent( """\ @@ -307,12 +307,18 @@ def get_main(addr: Address) -> Address: assert [main_addr] == list(injected_addresses) return main_addr - assert get_main(Address("explicit")) == Address("explicit", target_name="pkg") - assert get_main(Address("inferred")) == Address("inferred", target_name="pkg") + assert get_main(Address("explicit")) == Address( + "", target_name="mod", generated_name="./explicit" + ) + assert get_main(Address("inferred")) == Address( + "", target_name="mod", generated_name="./inferred" + ) with engine_error(ResolveError, contains="none were found"): get_main(Address("missing")) - with engine_error(ResolveError, contains="There are multiple `go_package` targets"): + with engine_error(ResolveError, contains="There are multiple `_go_internal_package` targets"): get_main(Address("ambiguous")) - with engine_error(InvalidFieldException, contains="must point to a `go_package` target"): + with engine_error( + InvalidFieldException, contains="must point to a `_go_internal_package` target" + ): get_main(Address("explicit_wrong_type")) diff --git a/src/python/pants/backend/go/target_types.py b/src/python/pants/backend/go/target_types.py index a8449f54568..63edbe561e4 100644 --- a/src/python/pants/backend/go/target_types.py +++ b/src/python/pants/backend/go/target_types.py @@ -142,7 +142,7 @@ class GoInternalPackageDependenciesField(Dependencies): pass -class GoInternalPackageSubpathField(StringField): +class GoInternalPackageSubpathField(StringField, AsyncFieldMixin): alias = "subpath" help = ( "The path from the owning `go.mod` to this package's directory, e.g. `subdir`.\n\n" @@ -152,6 +152,19 @@ class GoInternalPackageSubpathField(StringField): required = True value: str + @property + def full_dir_path(self) -> str: + """The full path to this package's directory, relative to the build root.""" + # NB: Assumes that the `spec_path` points to the ancestor `go.mod`, which will be the + # case when `go_mod` targets generate. + if not self.address.is_generated_target: + # TODO: Make this error more eager via target validation. + raise AssertionError( + f"Target was manually created, but expected to be generated: {self.address}" + ) + go_mod_path = self.address.spec_path + return os.path.join(go_mod_path, self.value) + class GoInternalPackageTarget(Target): alias = "_go_internal_package" @@ -211,9 +224,9 @@ class GoExternalPackageTarget(Target): class GoBinaryMainPackageField(StringField, AsyncFieldMixin): alias = "main" help = ( - "Address of the `go_package` with the `main` for this binary.\n\n" - "If not specified, will default to the `go_package` in the same directory as this target's " - "BUILD file." + "Address of the `_go_internal_package` with the `main` for this binary.\n\n" + "If not specified, will default to the `_go_internal_package` for the same " + "directory as this target's BUILD file." ) value: str