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

Add __defaults__ BUILD file symbol #15836

Merged
merged 23 commits into from
Jun 23, 2022
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
57 changes: 56 additions & 1 deletion docs/markdown/Using Pants/concepts/targets.md
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ python_tests(
>
> When would you do this? Sometimes you may have conflicting metadata for the same source file, such as wanting to check that a Shell test works with multiple shells. Normally, you should prefer Pants's `parametrize` mechanism to do this. See the below section "Parametrizing Targets".
>
> Often, however, it is not intentional when multiple targets on the same file. For example, this often happens when using `**` globs, like this:
> Often, however, it is not intentional when multiple targets own the same file. For example, this often happens when using `**` globs, like this:
>
> ```python
> # project/BUILD
Expand Down Expand Up @@ -154,6 +154,61 @@ You only need to declare direct dependencies. Pants will pull in _transitive dep
>
> Transitive excludes can only be used in target types that conventionally are not dependend upon by other targets, such as `pex_binary` and `python_test` / `python_tests`. This is meant to limit confusion, as using `!!` in something like a `python_source` / `python_sources` target could result in surprising behavior for everything that depends on it. (Pants will print a helpful error when using `!!` when it's not legal.)

Field default values
====================

As mentioned above in [BUILD files](doc:targets#build-files), most fields use sensible defaults. And
for specific cases it is easy to provide some other value to a specific target. The issue is if you
want to apply a specific non-default value for a field on many targets. This can get unwieldy, error
prone and hard to maintain. Enter `__defaults__`.

Default field values per target are set using the `__defaults__` BUILD file symbol, and apply to the
current subtree.

The defaults are provided as a dictionary mapping targets to the default field values. Multiple
targets may share the same set of default field values, when grouped together in parenthesis (as a
Python tuple).

Use the `all` keyword argument to provide default field values that should apply to all targets.

The `extend=True` keyword argument allows to add to any existing default field values set by a
previous `__defaults__` call rather than replacing them.

Default fields and values are validated against their target types, except when provided using the
`all` keyword, in which case only values for fields applicable to each target are validated.

This means, that it is legal to provide a default value for `all` targets, even if it is only a
subset of targets that actually supports that particular field.

Examples:

```python src/example/BUILD
# Provide default `tags` to all targets in this subtree, and skip black, where applicable.
__defaults__(all=dict(tags=["example"], skip_black=True))
```

Subdirectories may override defaults from a parent BUILD file:

```python src/example/override/BUILD
# For `files` and `resources` targets, we want to use some other defaults.
__defaults__({
(files, resources): dict(tags=["example", "overridden"], description="Our assets")
})
```

Use the `extend=True` keyword to update defaults rather than replace them, for any given target.

```python src/example/extend/BUILD
# Add a default description to all types, in addition to the inherited default tags.
__defaults__(extend=True, all=dict(description="Add default description to the defaults."))
```

To reset any modified defaults, simply override with the empty dict:

```python src/example/nodefaults/BUILD
__defaults__(all={})
```

Target generation
=================

Expand Down
43 changes: 39 additions & 4 deletions src/python/pants/engine/internals/build_files.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,13 @@
)
from pants.engine.engine_aware import EngineAwareParameter
from pants.engine.fs import DigestContents, GlobMatchErrorBehavior, PathGlobs, Paths
from pants.engine.internals.defaults import BuildFileDefaults, BuildFileDefaultsParserState
from pants.engine.internals.mapper import AddressFamily, AddressMap
from pants.engine.internals.parser import BuildFilePreludeSymbols, Parser, error_on_imports
from pants.engine.internals.target_adaptor import TargetAdaptor, TargetAdaptorRequest
from pants.engine.rules import Get, collect_rules, rule
from pants.engine.target import RegisteredTargetTypes
from pants.engine.unions import UnionMembership
from pants.option.global_options import GlobalOptions
from pants.util.frozendict import FrozenDict
from pants.util.strutil import softwrap
Expand Down Expand Up @@ -117,13 +120,28 @@ def debug_hint(self) -> str:
return self.path


@dataclass(frozen=True)
class OptionalAddressFamily:
path: str
address_family: AddressFamily | None = None


@rule
async def ensure_address_family(request: OptionalAddressFamily) -> AddressFamily:
if request.address_family is None:
raise ResolveError(f"Directory '{request.path}' does not contain any BUILD files.")
return request.address_family


@rule(desc="Search for addresses in BUILD files")
async def parse_address_family(
parser: Parser,
build_file_options: BuildFileOptions,
prelude_symbols: BuildFilePreludeSymbols,
directory: AddressFamilyDir,
) -> AddressFamily:
registered_target_types: RegisteredTargetTypes,
union_membership: UnionMembership,
) -> OptionalAddressFamily:
"""Given an AddressMapper and a directory, return an AddressFamily.

The AddressFamily may be empty, but it will not be None.
Expand All @@ -138,13 +156,30 @@ async def parse_address_family(
),
)
if not digest_contents:
raise ResolveError(f"Directory '{directory.path}' does not contain any BUILD files.")
return OptionalAddressFamily(directory.path)

defaults = BuildFileDefaults({})
parent_dir = os.path.dirname(directory.path)
if parent_dir != directory.path:
maybe_parent = await Get(OptionalAddressFamily, AddressFamilyDir(parent_dir))
if maybe_parent.address_family is not None:
defaults = maybe_parent.address_family.defaults

defaults_parser_state = BuildFileDefaultsParserState.create(
directory.path, defaults, registered_target_types, union_membership
)
address_maps = [
AddressMap.parse(fc.path, fc.content.decode(), parser, prelude_symbols)
AddressMap.parse(
fc.path, fc.content.decode(), parser, prelude_symbols, defaults_parser_state
)
for fc in digest_contents
]
return AddressFamily.create(directory.path, address_maps)
return OptionalAddressFamily(
directory.path,
AddressFamily.create(
directory.path, address_maps, defaults_parser_state.get_frozen_defaults()
),
)


@rule
Expand Down
57 changes: 55 additions & 2 deletions src/python/pants/engine/internals/build_files_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,22 @@
from pants.engine.internals.build_files import (
AddressFamilyDir,
BuildFileOptions,
OptionalAddressFamily,
evaluate_preludes,
parse_address_family,
)
from pants.engine.internals.parser import BuildFilePreludeSymbols, Parser
from pants.engine.internals.scheduler import ExecutionError
from pants.engine.internals.target_adaptor import TargetAdaptor, TargetAdaptorRequest
from pants.engine.target import Dependencies, MultipleSourcesField, StringField, Tags, Target
from pants.engine.target import (
Dependencies,
MultipleSourcesField,
RegisteredTargetTypes,
StringField,
Tags,
Target,
)
from pants.engine.unions import UnionMembership
from pants.testutil.rule_runner import (
MockGet,
QueryRule,
Expand All @@ -35,22 +44,33 @@

def test_parse_address_family_empty() -> None:
"""Test that parsing an empty BUILD file results in an empty AddressFamily."""
af = run_rule_with_mocks(
optional_af = run_rule_with_mocks(
parse_address_family,
rule_args=[
Parser(build_root="", target_type_aliases=[], object_aliases=BuildFileAliases()),
BuildFileOptions(("BUILD",)),
BuildFilePreludeSymbols(FrozenDict()),
AddressFamilyDir("/dev/null"),
RegisteredTargetTypes({}),
UnionMembership({}),
],
mock_gets=[
MockGet(
output_type=DigestContents,
input_type=PathGlobs,
mock=lambda _: DigestContents([FileContent(path="/dev/null/BUILD", content=b"")]),
),
MockGet(
output_type=OptionalAddressFamily,
input_type=AddressFamilyDir,
mock=lambda _: OptionalAddressFamily("/dev"),
),
],
)
assert optional_af.path == "/dev/null"
assert optional_af.address_family is not None
af = optional_af.address_family
assert af.namespace == "/dev/null"
assert len(af.name_to_target_adaptors) == 0


Expand Down Expand Up @@ -212,6 +232,39 @@ def test_target_adaptor_parsed_correctly(target_adaptor_rule_runner: RuleRunner)
assert target_adaptor.type_alias == "mock_tgt"


def test_target_adaptor_defaults_applied(target_adaptor_rule_runner: RuleRunner) -> None:
target_adaptor_rule_runner.write_files(
{
"helloworld/dir/BUILD": dedent(
"""\
__defaults__({mock_tgt: dict(resolve="mock")}, all=dict(tags=["24"]))
mock_tgt(tags=["42"])
mock_tgt(name='t2')
"""
)
}
)
target_adaptor = target_adaptor_rule_runner.request(
TargetAdaptor,
[TargetAdaptorRequest(Address("helloworld/dir"), description_of_origin="tests")],
)
assert target_adaptor.name is None
assert target_adaptor.kwargs["resolve"] == "mock"
assert target_adaptor.kwargs["tags"] == ["42"]

target_adaptor = target_adaptor_rule_runner.request(
TargetAdaptor,
[
TargetAdaptorRequest(
Address("helloworld/dir", target_name="t2"), description_of_origin="tests"
)
],
)
assert target_adaptor.name == "t2"
assert target_adaptor.kwargs["resolve"] == "mock"
assert target_adaptor.kwargs["tags"] == ["24"]


def test_target_adaptor_not_found(target_adaptor_rule_runner: RuleRunner) -> None:
with pytest.raises(ExecutionError) as exc:
target_adaptor_rule_runner.request(
Expand Down
Loading