Skip to content

Commit

Permalink
[internal] Don't store derived values on go_first_party_package tar…
Browse files Browse the repository at this point in the history
…gets (#13676)

We now compute the `import_path` and `subpath`, rather than storing them on the target via target generation.

This unblocks #13488. We do not want users to have to explicitly set these values. Even if `tailor` could set it automatically, that would be brittle to users moving around where code is located or renaming their `go.mod`'s module.
  • Loading branch information
Eric-Arellano authored Nov 19, 2021
1 parent 8826bf0 commit eb4f4d6
Show file tree
Hide file tree
Showing 7 changed files with 126 additions and 79 deletions.
15 changes: 5 additions & 10 deletions src/python/pants/backend/go/goals/test.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
from typing import Sequence

from pants.backend.go.subsystems.gotest import GoTestSubsystem
from pants.backend.go.target_types import GoFirstPartyPackageSourcesField, GoImportPathField
from pants.backend.go.target_types import GoFirstPartyPackageSourcesField
from pants.backend.go.util_rules.build_pkg import (
BuildGoPackageRequest,
FallibleBuildGoPackageRequest,
Expand All @@ -21,13 +21,11 @@
from pants.backend.go.util_rules.import_analysis import ImportConfig, ImportConfigRequest
from pants.backend.go.util_rules.link import LinkedGoBinary, LinkGoBinaryRequest
from pants.backend.go.util_rules.tests_analysis import GeneratedTestMain, GenerateTestMainRequest
from pants.build_graph.address import Address
from pants.core.goals.test import TestDebugRequest, TestFieldSet, TestResult, TestSubsystem
from pants.engine.fs import EMPTY_FILE_DIGEST, Digest, MergeDigests
from pants.engine.internals.selectors import Get, MultiGet
from pants.engine.internals.selectors import Get
from pants.engine.process import FallibleProcessResult, Process, ProcessCacheScope
from pants.engine.rules import collect_rules, rule
from pants.engine.target import WrappedTarget
from pants.engine.unions import UnionRule
from pants.util.logging import LogLevel
from pants.util.ordered_set import FrozenOrderedSet, OrderedSet
Expand Down Expand Up @@ -118,9 +116,8 @@ def transform_test_args(args: Sequence[str]) -> tuple[str, ...]:
async def run_go_tests(
field_set: GoTestFieldSet, test_subsystem: TestSubsystem, go_test_subsystem: GoTestSubsystem
) -> TestResult:
maybe_pkg_info, wrapped_target = await MultiGet(
Get(FallibleFirstPartyPkgInfo, FirstPartyPkgInfoRequest(field_set.address)),
Get(WrappedTarget, Address, field_set.address),
maybe_pkg_info = await Get(
FallibleFirstPartyPkgInfo, FirstPartyPkgInfoRequest(field_set.address)
)

if maybe_pkg_info.info is None:
Expand All @@ -135,9 +132,7 @@ async def run_go_tests(
output_setting=test_subsystem.output,
)
pkg_info = maybe_pkg_info.info

target = wrapped_target.target
import_path = target[GoImportPathField].value
import_path = pkg_info.import_path

testmain = await Get(
GeneratedTestMain,
Expand Down
31 changes: 23 additions & 8 deletions src/python/pants/backend/go/target_type_rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
GoBinaryMainPackageField,
GoBinaryMainPackageRequest,
GoFirstPartyPackageSourcesField,
GoFirstPartyPackageSubpathField,
GoFirstPartyPackageTarget,
GoImportPathField,
GoModPackageSourcesField,
Expand All @@ -25,6 +24,8 @@
from pants.backend.go.util_rules import first_party_pkg, import_analysis
from pants.backend.go.util_rules.first_party_pkg import (
FallibleFirstPartyPkgInfo,
FirstPartyPkgImportPath,
FirstPartyPkgImportPathRequest,
FirstPartyPkgInfoRequest,
)
from pants.backend.go.util_rules.go_mod import GoModInfo, GoModInfoRequest
Expand Down Expand Up @@ -68,7 +69,11 @@ class AllGoTargets(Targets):

@rule(desc="Find all Go targets in project", level=LogLevel.DEBUG)
def find_all_go_targets(tgts: AllTargets) -> AllGoTargets:
return AllGoTargets(t for t in tgts if t.has_field(GoImportPathField))
return AllGoTargets(
t
for t in tgts
if t.has_field(GoImportPathField) or t.has_field(GoFirstPartyPackageSourcesField)
)


@dataclass(frozen=True)
Expand All @@ -79,9 +84,22 @@ class ImportPathToPackages:
@rule(desc="Map all Go targets to their import paths", level=LogLevel.DEBUG)
async def map_import_paths_to_packages(go_tgts: AllGoTargets) -> ImportPathToPackages:
mapping: dict[str, list[Address]] = defaultdict(list)
first_party_addresses = []
first_party_gets = []
for tgt in go_tgts:
import_path = tgt[GoImportPathField].value
mapping[import_path].append(tgt.address)
if tgt.has_field(GoImportPathField):
import_path = tgt[GoImportPathField].value
mapping[import_path].append(tgt.address)
else:
first_party_addresses.append(tgt.address)
first_party_gets.append(
Get(FirstPartyPkgImportPath, FirstPartyPkgImportPathRequest(tgt.address))
)

first_party_import_paths = await MultiGet(first_party_gets)
for import_path_info, addr in zip(first_party_import_paths, first_party_addresses):
mapping[import_path_info.import_path].append(addr)

frozen_mapping = FrozenDict({ip: tuple(tgts) for ip, tgts in mapping.items()})
return ImportPathToPackages(frozen_mapping)

Expand Down Expand Up @@ -217,15 +235,12 @@ async def generate_targets_from_go_mod(

def create_first_party_package_tgt(dir: str) -> GoFirstPartyPackageTarget:
subpath = fast_relpath(dir, generator_addr.spec_path)
import_path = f"{go_mod_info.import_path}/{subpath}" if subpath else go_mod_info.import_path

return GoFirstPartyPackageTarget(
{
GoImportPathField.alias: import_path,
GoFirstPartyPackageSubpathField.alias: subpath,
GoFirstPartyPackageSourcesField.alias: tuple(
sorted(os.path.join(subpath, f) for f in dir_to_filenames[dir])
),
)
},
# E.g. `src/go:mod#./subdir`.
generator_addr.create_generated(f"./{subpath}"),
Expand Down
14 changes: 2 additions & 12 deletions src/python/pants/backend/go/target_type_rules_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
GoBinaryMainPackageRequest,
GoBinaryTarget,
GoFirstPartyPackageSourcesField,
GoFirstPartyPackageSubpathField,
GoFirstPartyPackageTarget,
GoImportPathField,
GoModTarget,
Expand Down Expand Up @@ -186,13 +185,7 @@ def test_generate_package_targets(rule_runner: RuleRunner) -> None:

def gen_first_party_tgt(rel_dir: str, sources: list[str]) -> GoFirstPartyPackageTarget:
return GoFirstPartyPackageTarget(
{
GoImportPathField.alias: (
os.path.join("example.com/src/go", rel_dir) if rel_dir else "example.com/src/go"
),
GoFirstPartyPackageSubpathField.alias: rel_dir,
GoFirstPartyPackageSourcesField.alias: tuple(sources),
},
{GoFirstPartyPackageSourcesField.alias: tuple(sources)},
Address("src/go", generated_name=f"./{rel_dir}"),
residence_dir=os.path.join("src/go", rel_dir).rstrip("/"),
)
Expand Down Expand Up @@ -234,10 +227,7 @@ def gen_third_party_tgt(import_path: str) -> GoThirdPartyPackageTarget:

def test_package_targets_cannot_be_manually_created() -> None:
with pytest.raises(InvalidTargetException):
GoFirstPartyPackageTarget(
{GoImportPathField.alias: "foo", GoFirstPartyPackageSubpathField.alias: "foo"},
Address("foo"),
)
GoFirstPartyPackageTarget({}, Address("foo"))
with pytest.raises(InvalidTargetException):
GoThirdPartyPackageTarget(
{GoImportPathField.alias: "foo"},
Expand Down
33 changes: 10 additions & 23 deletions src/python/pants/backend/go/target_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,17 +25,6 @@
)
from pants.option.global_options import FilesNotFoundBehavior


class GoImportPathField(StringField):
alias = "import_path"
help = (
"Import path in Go code to import this package.\n\n"
"This field should not be overridden; use the value from target generation."
)
required = True
value: str


# -----------------------------------------------------------------------------------------------
# `go_mod` target generator
# -----------------------------------------------------------------------------------------------
Expand Down Expand Up @@ -143,22 +132,10 @@ class GoFirstPartyPackageDependenciesField(Dependencies):
pass


class GoFirstPartyPackageSubpathField(StringField, AsyncFieldMixin):
alias = "subpath"
help = (
"The path from the owning `go.mod` to this package's directory, e.g. `subdir`.\n\n"
"This field should not be overridden; use the value from target generation."
)
required = True
value: str


class GoFirstPartyPackageTarget(Target):
alias = "go_first_party_package"
core_fields = (
*COMMON_TARGET_FIELDS,
GoImportPathField,
GoFirstPartyPackageSubpathField,
GoFirstPartyPackageDependenciesField,
GoFirstPartyPackageSourcesField,
)
Expand All @@ -184,6 +161,16 @@ def validate(self) -> None:
# -----------------------------------------------------------------------------------------------


class GoImportPathField(StringField):
alias = "import_path"
help = (
"Import path in Go code to import this package.\n\n"
"This field should not be overridden; use the value from target generation."
)
required = True
value: str


class GoThirdPartyPackageDependenciesField(Dependencies):
pass

Expand Down
12 changes: 5 additions & 7 deletions src/python/pants/backend/go/util_rules/build_pkg_target.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,10 @@
from __future__ import annotations

import dataclasses
import os
from dataclasses import dataclass

from pants.backend.go.target_types import (
GoFirstPartyPackageSourcesField,
GoFirstPartyPackageSubpathField,
GoImportPathField,
GoThirdPartyPackageDependenciesField,
)
Expand Down Expand Up @@ -52,7 +50,6 @@ async def setup_build_go_package_target_request(
) -> FallibleBuildGoPackageRequest:
wrapped_target = await Get(WrappedTarget, Address, request.address)
target = wrapped_target.target
import_path = target[GoImportPathField].value

if target.has_field(GoFirstPartyPackageSourcesField):
_maybe_first_party_pkg_info = await Get(
Expand All @@ -61,16 +58,15 @@ async def setup_build_go_package_target_request(
if _maybe_first_party_pkg_info.info is None:
return FallibleBuildGoPackageRequest(
None,
import_path,
_maybe_first_party_pkg_info.import_path,
exit_code=_maybe_first_party_pkg_info.exit_code,
stderr=_maybe_first_party_pkg_info.stderr,
)
_first_party_pkg_info = _maybe_first_party_pkg_info.info

digest = _first_party_pkg_info.digest
subpath = os.path.join(
target.address.spec_path, target[GoFirstPartyPackageSubpathField].value
)
import_path = _first_party_pkg_info.import_path
subpath = _first_party_pkg_info.subpath
minimum_go_version = _first_party_pkg_info.minimum_go_version

go_file_names = _first_party_pkg_info.go_files
Expand All @@ -82,6 +78,8 @@ async def setup_build_go_package_target_request(
s_file_names = _first_party_pkg_info.s_files

elif target.has_field(GoThirdPartyPackageDependenciesField):
import_path = target[GoImportPathField].value

_go_mod_address = target.address.maybe_convert_to_target_generator()
_go_mod_info = await Get(GoModInfo, GoModInfoRequest(_go_mod_address))
_third_party_pkg_info = await Get(
Expand Down
Loading

0 comments on commit eb4f4d6

Please sign in to comment.