Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[internal] Don't store derived values on go_first_party_package targets #13676

Merged
merged 2 commits into from
Nov 19, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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