Skip to content

Commit

Permalink
Do not choke on values from macros on environment targets during boot…
Browse files Browse the repository at this point in the history
…strap. (pantsbuild#20191)

Clear all unrecognized field values during bootstrap. (Such as from
calls to macros, which are not loaded during that phase.)

Add note to docs discouraging use of macros for environment targets.

Fixes pantsbuild#20178
  • Loading branch information
kaos authored Nov 16, 2023
1 parent 645ebbb commit 47d27ae
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 0 deletions.
5 changes: 5 additions & 0 deletions docs/markdown/Using Pants/environments.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,11 @@ docker_environment(
)
```

> 🚧 Environment targets and macros
>
> Environment targets are loaded before regular targets in a bootstrap phase, during which macros are unavailable. As such any required field values must be fully defined in the BUILD file without referencing any macros. For optional fields, the use of macros are still discouraged as it may or may not work and Pants makes no guarantees that it will not break in a future version if it were to currently work.

### Environment-aware options

Environment targets have fields ([target](doc:targets) arguments) which correspond to [options](doc:options) which are marked "environment-aware". When an option is environment-aware, the value of the option that will be used in an environment can be overridden by setting the corresponding field value on the associated environment target. If an environment target does not set a value, it defaults to the value which is set globally via options values.
Expand Down
36 changes: 36 additions & 0 deletions src/python/pants/engine/internals/build_files_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -775,6 +775,42 @@ def test_default_plugin_field_bootstrap() -> None:
assert dict(tags=("ok",)) == dict(address_family.defaults["mock_tgt"])


def test_environment_target_macro_field_value() -> None:
rule_runner = RuleRunner(
rules=[QueryRule(AddressFamily, [AddressFamilyDir])],
target_types=[MockTgt],
is_bootstrap=True,
)
rule_runner.set_options(
args=("--build-file-prelude-globs=prelude.py",),
)
rule_runner.write_files(
{
"prelude.py": dedent(
"""
def tags():
return ["foo", "bar"]
"""
),
"BUILD": dedent(
"""
mock_tgt(name="tgt", tags=tags())
"""
),
}
)

# Parse the root BUILD file.
address_family = rule_runner.request(AddressFamily, [AddressFamilyDir("")])
tgt = address_family.name_to_target_adaptors["tgt"][1]
# We're pretending that field values returned from a called macro function doesn't exist during
# bootstrap. This is to allow the semi-dubios use of macro calls for environment target field
# values that are not required, and depending on how they are used, it may work to only have
# those field values set during normal lookup.
assert not tgt.kwargs
assert tgt == TargetAdaptor("mock_tgt", "tgt", "BUILD:2")


def test_build_file_env_vars(target_adaptor_rule_runner: RuleRunner) -> None:
target_adaptor_rule_runner.write_files(
{
Expand Down
6 changes: 6 additions & 0 deletions src/python/pants/engine/internals/parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,12 @@ def __str__(self) -> str:
return self._type_alias

def __call__(self, **kwargs: Any) -> TargetAdaptor:
if self._parse_state.is_bootstrap and any(
isinstance(v, _UnrecognizedSymbol) for v in kwargs.values()
):
# Remove any field values that are not recognized during the bootstrap phase.
kwargs = {k: v for k, v in kwargs.items() if not isinstance(v, _UnrecognizedSymbol)}

# Target names default to the name of the directory their BUILD file is in
# (as long as it's not the root directory).
if "name" not in kwargs:
Expand Down

0 comments on commit 47d27ae

Please sign in to comment.