Skip to content

Commit

Permalink
[internal] Fix go_binary's main field inference to work with gene…
Browse files Browse the repository at this point in the history
…rated targets

[ci skip-rust]

[ci skip-build-wheels]
  • Loading branch information
Eric-Arellano committed Oct 8, 2021
1 parent df02e30 commit 215a9f9
Show file tree
Hide file tree
Showing 4 changed files with 65 additions and 41 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -163,8 +163,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')
"""
),
}
Expand Down
40 changes: 23 additions & 17 deletions src/python/pants/backend/go/target_type_rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
from pants.backend.go.util_rules.go_pkg import ResolvedGoPackage, ResolveGoPackageRequest
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
Expand Down Expand Up @@ -291,34 +291,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)}"
)


Expand Down
42 changes: 24 additions & 18 deletions src/python/pants/backend/go/target_type_rules_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
"""\
Expand All @@ -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"))
21 changes: 17 additions & 4 deletions src/python/pants/backend/go/target_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,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"
Expand All @@ -144,6 +144,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"
Expand Down Expand Up @@ -203,9 +216,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

Expand Down

0 comments on commit 215a9f9

Please sign in to comment.