From e8aa5d7ef81809f6a1cb23677d15ae4a84ea0e1a Mon Sep 17 00:00:00 2001 From: Eric Arellano Date: Tue, 5 Oct 2021 07:56:00 -0700 Subject: [PATCH 1/2] [internal] Infer `main` field for `go_binary` # Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels] # Conflicts: # testprojects/src/go/pants_test/BUILD --- .../pants/backend/experimental/go/register.py | 11 ++- .../pants/backend/go/goals/package_binary.py | 22 ++--- .../goals/package_binary_integration_test.py | 12 +-- .../go/lint/gofmt/rules_integration_test.py | 4 +- .../pants/backend/go/target_type_rules.py | 76 ++++++++++++++- .../backend/go/target_type_rules_test.py | 97 +++++++++++++++++-- src/python/pants/backend/go/target_types.py | 49 ++++++++-- .../util_rules/assembly_integration_test.py | 6 +- testprojects/src/go/pants_test/BUILD | 6 +- 9 files changed, 228 insertions(+), 55 deletions(-) diff --git a/src/python/pants/backend/experimental/go/register.py b/src/python/pants/backend/experimental/go/register.py index c1de6393bd3..371bbccfb0f 100644 --- a/src/python/pants/backend/experimental/go/register.py +++ b/src/python/pants/backend/experimental/go/register.py @@ -2,13 +2,17 @@ # Licensed under the Apache License, Version 2.0 (see LICENSE). from pants.backend.go import target_type_rules -from pants.backend.go import target_types as go_target_types from pants.backend.go.goals import custom_goals, package_binary, tailor from pants.backend.go.lint import fmt from pants.backend.go.lint.gofmt import skip_field as gofmt_skip_field from pants.backend.go.lint.gofmt.rules import rules as gofmt_rules from pants.backend.go.subsystems import golang -from pants.backend.go.target_types import GoBinary, GoExternalPackageTarget, GoModTarget, GoPackage +from pants.backend.go.target_types import ( + GoBinaryTarget, + GoExternalPackageTarget, + GoModTarget, + GoPackage, +) from pants.backend.go.util_rules import ( assembly, build_go_pkg, @@ -24,7 +28,7 @@ def target_types(): - return [GoBinary, GoPackage, GoModTarget, GoExternalPackageTarget] + return [GoModTarget, GoPackage, GoExternalPackageTarget, GoBinaryTarget] def rules(): @@ -34,7 +38,6 @@ def rules(): *compile.rules(), *external_module.rules(), *golang.rules(), - *go_target_types.rules(), *import_analysis.rules(), *go_mod.rules(), *go_pkg.rules(), diff --git a/src/python/pants/backend/go/goals/package_binary.py b/src/python/pants/backend/go/goals/package_binary.py index 66c710b5fa4..650ff390966 100644 --- a/src/python/pants/backend/go/goals/package_binary.py +++ b/src/python/pants/backend/go/goals/package_binary.py @@ -4,11 +4,14 @@ from dataclasses import dataclass from pathlib import PurePath -from pants.backend.go.target_types import GoBinaryMainAddress +from pants.backend.go.target_types import ( + GoBinaryMainPackage, + GoBinaryMainPackageField, + GoBinaryMainPackageRequest, +) from pants.backend.go.util_rules.build_go_pkg import BuildGoPackageRequest, BuiltGoPackage from pants.backend.go.util_rules.import_analysis import ImportConfig, ImportConfigRequest from pants.backend.go.util_rules.link import LinkedGoBinary, LinkGoBinaryRequest -from pants.build_graph.address import Address, AddressInput from pants.core.goals.package import ( BuiltPackage, BuiltPackageArtifact, @@ -23,23 +26,16 @@ @dataclass(frozen=True) class GoBinaryFieldSet(PackageFieldSet): - required_fields = (GoBinaryMainAddress,) + required_fields = (GoBinaryMainPackageField,) - main_address: GoBinaryMainAddress + main: GoBinaryMainPackageField output_path: OutputPathField @rule async def package_go_binary(field_set: GoBinaryFieldSet) -> BuiltPackage: - main_go_package_address = await Get( - Address, - AddressInput, - AddressInput.parse(field_set.main_address.value, relative_to=field_set.address.spec_path), - ) - - built_package = await Get( - BuiltGoPackage, BuildGoPackageRequest(main_go_package_address, is_main=True) - ) + main_pkg = await Get(GoBinaryMainPackage, GoBinaryMainPackageRequest(field_set.main)) + built_package = await Get(BuiltGoPackage, BuildGoPackageRequest(main_pkg.address, is_main=True)) main_pkg_path = built_package.import_paths_to_pkg_a_files["main"] import_config = await Get( ImportConfig, ImportConfigRequest(built_package.import_paths_to_pkg_a_files) 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 9468add1023..9ed5cf87179 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 @@ -12,7 +12,7 @@ from pants.backend.go import target_type_rules from pants.backend.go.goals import package_binary from pants.backend.go.goals.package_binary import GoBinaryFieldSet -from pants.backend.go.target_types import GoBinary, GoModTarget, GoPackage +from pants.backend.go.target_types import GoBinaryTarget, GoModTarget, GoPackage from pants.backend.go.util_rules import ( assembly, build_go_pkg, @@ -50,7 +50,7 @@ def rule_runner() -> RuleRunner: *sdk.rules(), QueryRule(BuiltPackage, (GoBinaryFieldSet,)), ], - target_types=[GoBinary, GoPackage, GoModTarget], + target_types=[GoBinaryTarget, GoPackage, GoModTarget], ) rule_runner.set_options([], env_inherit={"PATH"}) return rule_runner @@ -88,8 +88,8 @@ def test_package_simple(rule_runner: RuleRunner) -> None: "BUILD": dedent( """\ go_mod(name='mod') - go_package(name='main') - go_binary(name='bin', main=':main') + go_package(name='pkg') + go_binary(name='bin') """ ), } @@ -165,8 +165,8 @@ def test_package_with_dependencies(rule_runner: RuleRunner) -> None: "BUILD": dedent( """\ go_mod(name='mod') - go_package(name='main') - go_binary(name='bin', main=':main') + go_package(name='pkg') + go_binary(name='bin') """ ), } diff --git a/src/python/pants/backend/go/lint/gofmt/rules_integration_test.py b/src/python/pants/backend/go/lint/gofmt/rules_integration_test.py index ef37f2e8b0a..fe464cf3404 100644 --- a/src/python/pants/backend/go/lint/gofmt/rules_integration_test.py +++ b/src/python/pants/backend/go/lint/gofmt/rules_integration_test.py @@ -10,7 +10,7 @@ from pants.backend.go.lint import fmt from pants.backend.go.lint.gofmt.rules import GofmtFieldSet, GofmtRequest from pants.backend.go.lint.gofmt.rules import rules as gofmt_rules -from pants.backend.go.target_types import GoBinary, GoPackage +from pants.backend.go.target_types import GoBinaryTarget, GoPackage from pants.core.goals.fmt import FmtResult from pants.core.goals.lint import LintResult, LintResults from pants.core.util_rules import source_files @@ -24,7 +24,7 @@ @pytest.fixture() def rule_runner() -> RuleRunner: return RuleRunner( - target_types=[GoBinary, GoPackage], + target_types=[GoBinaryTarget, GoPackage], rules=[ *fmt.rules(), *gofmt_rules(), diff --git a/src/python/pants/backend/go/target_type_rules.py b/src/python/pants/backend/go/target_type_rules.py index b0d15d909ce..04ff50d3462 100644 --- a/src/python/pants/backend/go/target_type_rules.py +++ b/src/python/pants/backend/go/target_type_rules.py @@ -8,6 +8,10 @@ from dataclasses import dataclass from pants.backend.go.target_types import ( + GoBinaryDependenciesField, + GoBinaryMainPackage, + GoBinaryMainPackageField, + GoBinaryMainPackageRequest, GoExternalModulePathField, GoExternalModuleVersionField, GoExternalPackageDependencies, @@ -33,15 +37,16 @@ ) 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, MaybeEmptyDescendantAddresses, MaybeEmptySiblingAddresses, + SiblingAddresses, ) -from pants.build_graph.address import Address -from pants.engine.internals.selectors import Get, MultiGet -from pants.engine.rules import collect_rules, rule +from pants.engine.addresses import Address, AddressInput +from pants.engine.rules import Get, MultiGet, collect_rules, rule from pants.engine.target import ( GeneratedTargets, GenerateTargetsRequest, @@ -49,6 +54,7 @@ InferredDependencies, InjectDependenciesRequest, InjectedDependencies, + InvalidFieldException, Targets, WrappedTarget, ) @@ -285,6 +291,69 @@ def create_tgt( ) +# ----------------------------------------------------------------------------------------------- +# The `main` field for `go_binary` +# ----------------------------------------------------------------------------------------------- + + +@rule +async def determine_main_pkg_for_go_binary( + request: GoBinaryMainPackageRequest, +) -> GoBinaryMainPackage: + addr = request.field.address + if request.field.value: + wrapped_specified_tgt = await Get( + WrappedTarget, + AddressInput, + AddressInput.parse(request.field.value, relative_to=addr.spec_path), + ) + if not wrapped_specified_tgt.target.has_field(GoPackageSources): + raise InvalidFieldException( + f"The {repr(GoBinaryMainPackageField.alias)} field in target {addr} must point to " + "a `go_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." + ) + return GoBinaryMainPackage(wrapped_specified_tgt.target.address) + + 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(GoPackageSources)] + if len(internal_pkg_targets) == 1: + return GoBinaryMainPackage(internal_pkg_targets[0].address) + + wrapped_tgt = await Get(WrappedTarget, Address, addr) + alias = wrapped_tgt.target.alias + if not internal_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." + ) + 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"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)}" + ) + + +class InjectGoBinaryMainDependencyRequest(InjectDependenciesRequest): + inject_for = GoBinaryDependenciesField + + +@rule +async def inject_go_binary_main_dependency( + request: InjectGoBinaryMainDependencyRequest, +) -> InjectedDependencies: + wrapped_tgt = await Get(WrappedTarget, Address, request.dependencies_field.address) + main_pkg = await Get( + GoBinaryMainPackage, + GoBinaryMainPackageRequest(wrapped_tgt.target[GoBinaryMainPackageField]), + ) + return InjectedDependencies([main_pkg.address]) + + def rules(): return ( *collect_rules(), @@ -293,5 +362,6 @@ def rules(): UnionRule(InjectDependenciesRequest, InjectGoPackageDependenciesRequest), UnionRule(InferDependenciesRequest, InferGoPackageDependenciesRequest), UnionRule(InjectDependenciesRequest, InjectGoExternalPackageDependenciesRequest), + UnionRule(InjectDependenciesRequest, InjectGoBinaryMainDependencyRequest), UnionRule(GenerateTargetsRequest, GenerateGoExternalPackageTargetsRequest), ) 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 34ee0ae66e4..a268ebe5dd0 100644 --- a/src/python/pants/backend/go/target_type_rules_test.py +++ b/src/python/pants/backend/go/target_type_rules_test.py @@ -1,6 +1,9 @@ # Copyright 2021 Pants project contributors (see CONTRIBUTORS.md). # Licensed under the Apache License, Version 2.0 (see LICENSE). -import textwrap + +from __future__ import annotations + +from textwrap import dedent import pytest @@ -8,8 +11,13 @@ from pants.backend.go.target_type_rules import ( GenerateGoExternalPackageTargetsRequest, InferGoPackageDependenciesRequest, + InjectGoBinaryMainDependencyRequest, ) from pants.backend.go.target_types import ( + GoBinaryMainPackage, + GoBinaryMainPackageField, + GoBinaryMainPackageRequest, + GoBinaryTarget, GoExternalModulePathField, GoExternalModuleVersionField, GoExternalPackageImportPathField, @@ -20,7 +28,9 @@ GoPackageSources, ) from pants.backend.go.util_rules import external_module, go_mod, go_pkg, sdk +from pants.base.exceptions import ResolveError from pants.build_graph.address import Address +from pants.core.target_types import GenericTarget from pants.engine.addresses import Addresses from pants.engine.rules import QueryRule from pants.engine.target import ( @@ -28,10 +38,12 @@ DependenciesRequest, GeneratedTargets, InferredDependencies, + InjectedDependencies, + InvalidFieldException, Target, Targets, ) -from pants.testutil.rule_runner import RuleRunner +from pants.testutil.rule_runner import RuleRunner, engine_error from pants.util.ordered_set import FrozenOrderedSet @@ -45,8 +57,16 @@ def rule_runner() -> RuleRunner: *sdk.rules(), *target_type_rules.rules(), QueryRule(Addresses, [DependenciesRequest]), + QueryRule(GoBinaryMainPackage, [GoBinaryMainPackageRequest]), + QueryRule(InjectedDependencies, [InjectGoBinaryMainDependencyRequest]), + ], + target_types=[ + GoPackage, + GoModTarget, + GoExternalPackageTarget, + GoBinaryTarget, + GenericTarget, ], - target_types=[GoPackage, GoModTarget, GoExternalPackageTarget], ) rule_runner.set_options([], env_inherit={"PATH"}) return rule_runner @@ -86,7 +106,7 @@ def test_go_package_dependency_inference(rule_runner: RuleRunner) -> None: ( { "foo/BUILD": "go_mod()", - "foo/go.mod": textwrap.dedent( + "foo/go.mod": dedent( """\ module go.example.com/foo go 1.17 @@ -94,7 +114,7 @@ def test_go_package_dependency_inference(rule_runner: RuleRunner) -> None: require github.com/google/go-cmp v0.4.0 """ ), - "foo/go.sum": textwrap.dedent( + "foo/go.sum": dedent( """\ github.com/google/go-cmp v0.4.0 h1:xsAVV57WRhGj6kEIi8ReJzQlHHqcBYCElAvkovg3B/4= github.com/google/go-cmp v0.4.0/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= @@ -103,7 +123,7 @@ def test_go_package_dependency_inference(rule_runner: RuleRunner) -> None: """ ), "foo/pkg/BUILD": "go_package()\n", - "foo/pkg/foo.go": textwrap.dedent( + "foo/pkg/foo.go": dedent( """\ package pkg import "github.com/google/go-cmp/cmp" @@ -113,7 +133,7 @@ def test_go_package_dependency_inference(rule_runner: RuleRunner) -> None: """ ), "foo/cmd/BUILD": "go_package()\n", - "foo/cmd/main.go": textwrap.dedent( + "foo/cmd/main.go": dedent( """\ package main import ( @@ -151,7 +171,7 @@ def test_generate_go_external_package_targets(rule_runner: RuleRunner) -> None: rule_runner.write_files( { "src/go/BUILD": "go_mod()\n", - "src/go/go.mod": textwrap.dedent( + "src/go/go.mod": dedent( """\ module example.com/src/go go 1.17 @@ -162,7 +182,7 @@ def test_generate_go_external_package_targets(rule_runner: RuleRunner) -> None: ) """ ), - "src/go/go.sum": textwrap.dedent( + "src/go/go.sum": dedent( """\ github.com/google/go-cmp v0.4.0 h1:xsAVV57WRhGj6kEIi8ReJzQlHHqcBYCElAvkovg3B/4= github.com/google/go-cmp v0.4.0/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= @@ -231,3 +251,62 @@ def gen_tgt(mod_path: str, version: str, import_path: str) -> GoExternalPackageT ) assert list(generated.keys()) == list(expected.keys()) assert generated == expected + + +# ----------------------------------------------------------------------------------------------- +# The `main` field for `go_binary` +# ----------------------------------------------------------------------------------------------- + + +def test_determine_main_pkg_for_go_binary(rule_runner: RuleRunner) -> None: + rule_runner.write_files( + { + "explicit/BUILD": dedent( + """\ + go_package(name='pkg', sources=[]) + go_binary(main=':pkg') + """ + ), + "inferred/BUILD": dedent( + """\ + go_package(name='pkg', sources=[]) + go_binary() + """ + ), + "ambiguous/BUILD": dedent( + """\ + go_package(name='pkg1', sources=[]) + go_package(name='pkg2', sources=[]) + go_binary() + """ + ), + "missing/BUILD": "go_binary()", + "explicit_wrong_type/BUILD": dedent( + """\ + target(name='dep') + go_binary(main=':dep') + """ + ), + } + ) + + def get_main(addr: Address) -> Address: + tgt = rule_runner.get_target(addr) + main_addr = rule_runner.request( + GoBinaryMainPackage, [GoBinaryMainPackageRequest(tgt[GoBinaryMainPackageField])] + ).address + injected_addresses = rule_runner.request( + InjectedDependencies, [InjectGoBinaryMainDependencyRequest(tgt[Dependencies])] + ) + 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") + + with engine_error(ResolveError, contains="none were found"): + get_main(Address("missing")) + with engine_error(ResolveError, contains="There are multiple `go_package` targets"): + get_main(Address("ambiguous")) + with engine_error(InvalidFieldException, contains="must point to a `go_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 c5d288bebac..6645890a6e8 100644 --- a/src/python/pants/backend/go/target_types.py +++ b/src/python/pants/backend/go/target_types.py @@ -1,12 +1,18 @@ # Copyright 2021 Pants project contributors (see CONTRIBUTORS.md). # Licensed under the Apache License, Version 2.0 (see LICENSE). + +from __future__ import annotations + import os +from dataclasses import dataclass from typing import Sequence from pants.core.goals.package import OutputPathField -from pants.engine.rules import collect_rules +from pants.engine.addresses import Address +from pants.engine.engine_aware import EngineAwareParameter from pants.engine.target import ( COMMON_TARGET_FIELDS, + AsyncFieldMixin, Dependencies, InvalidFieldException, Sources, @@ -140,18 +146,41 @@ class GoExternalPackageTarget(Target): # ----------------------------------------------------------------------------------------------- -class GoBinaryMainAddress(StringField): +class GoBinaryMainPackageField(StringField, AsyncFieldMixin): alias = "main" - required = True - help = "Address of the main Go package for this binary." + help = ( + "Address of the `go_package` with the `main` for this binary.\n\n" + "If left off, will default to the `go_package` in the same directory as this target's " + "BUILD file." + ) value: str -class GoBinary(Target): - alias = "go_binary" - core_fields = (*COMMON_TARGET_FIELDS, OutputPathField, GoBinaryMainAddress) - help = "A Go binary." +@dataclass(frozen=True) +class GoBinaryMainPackage: + address: Address + + +@dataclass(frozen=True) +class GoBinaryMainPackageRequest(EngineAwareParameter): + field: GoBinaryMainPackageField + def debug_hint(self) -> str: + return self.field.address.spec -def rules(): - return collect_rules() + +class GoBinaryDependenciesField(Dependencies): + # This is only used to inject a dependency from the `GoBinaryMainPackageField`. Users should + # add any explicit dependencies to the `go_package`. + alias = "_dependencies" + + +class GoBinaryTarget(Target): + alias = "go_binary" + core_fields = ( + *COMMON_TARGET_FIELDS, + OutputPathField, + GoBinaryMainPackageField, + GoBinaryDependenciesField, + ) + help = "A Go binary." diff --git a/src/python/pants/backend/go/util_rules/assembly_integration_test.py b/src/python/pants/backend/go/util_rules/assembly_integration_test.py index b44ba247983..d26ca8634ee 100644 --- a/src/python/pants/backend/go/util_rules/assembly_integration_test.py +++ b/src/python/pants/backend/go/util_rules/assembly_integration_test.py @@ -12,7 +12,7 @@ from pants.backend.go import target_type_rules from pants.backend.go.goals import package_binary from pants.backend.go.goals.package_binary import GoBinaryFieldSet -from pants.backend.go.target_types import GoBinary, GoModTarget, GoPackage +from pants.backend.go.target_types import GoBinaryTarget, GoModTarget, GoPackage from pants.backend.go.util_rules import ( assembly, build_go_pkg, @@ -50,7 +50,7 @@ def rule_runner() -> RuleRunner: *sdk.rules(), QueryRule(BuiltPackage, (GoBinaryFieldSet,)), ], - target_types=[GoBinary, GoPackage, GoModTarget], + target_types=[GoBinaryTarget, GoPackage, GoModTarget], ) rule_runner.set_options([], env_inherit={"PATH"}) return rule_runner @@ -114,7 +114,7 @@ def test_build_package_with_assembly(rule_runner: RuleRunner) -> None: """\ go_mod(name="mod") go_package(name="pkg") - go_binary(name="bin", main=":pkg") + go_binary(name="bin") """ ), } diff --git a/testprojects/src/go/pants_test/BUILD b/testprojects/src/go/pants_test/BUILD index 30a8a6fbe06..cbdeec03429 100644 --- a/testprojects/src/go/pants_test/BUILD +++ b/testprojects/src/go/pants_test/BUILD @@ -2,8 +2,4 @@ # Licensed under the Apache License, Version 2.0 (see LICENSE). go_package() - -go_binary( - name="bin", - main=":pants_test", -) +go_binary(name="bin") From 265b82239aee48720b0752d2840737e1694328bf Mon Sep 17 00:00:00 2001 From: Eric Arellano Date: Tue, 5 Oct 2021 08:19:02 -0700 Subject: [PATCH 2/2] Review feedback # Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels] --- src/python/pants/backend/go/target_types.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/python/pants/backend/go/target_types.py b/src/python/pants/backend/go/target_types.py index 6645890a6e8..5c4cf6b1100 100644 --- a/src/python/pants/backend/go/target_types.py +++ b/src/python/pants/backend/go/target_types.py @@ -150,7 +150,7 @@ class GoBinaryMainPackageField(StringField, AsyncFieldMixin): alias = "main" help = ( "Address of the `go_package` with the `main` for this binary.\n\n" - "If left off, will default to the `go_package` in the same directory as this target's " + "If not specified, will default to the `go_package` in the same directory as this target's " "BUILD file." ) value: str