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

Finalise environments targets and documentation #17095

Merged
Merged
2 changes: 1 addition & 1 deletion BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ files(name="files", sources=["BUILD_ROOT", "pants.toml"])
python_test_utils(name="test_utils")

# Used for experimenting with the new Docker support.
_docker_environment(
docker_environment(
name="docker_env",
image="python:3.9",
)
66 changes: 46 additions & 20 deletions src/python/pants/core/util_rules/environments.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,9 @@ class EnvironmentsSubsystem(Subsystem):
linux_ci = "build-support:linux_ci_env"
macos_ci = "build-support:macos_ci_env"

TODO(#7735): explain how names are used once they are consumed.
To use an environment for a given target, specify the name in the `_environment` field
on that target. Pants will consume the environment target at the address mapped from
that name.

Pants will ignore any environment targets that are not given a name via this option.
"""
Expand All @@ -81,8 +83,16 @@ class EnvironmentField(StringField):
default = LOCAL_ENVIRONMENT_MATCHER
value: str
help = softwrap(
"""
TODO(#7735): fill this in.
f"""
Specify which environment target to consume environment-sensitive options from.

Once environments are defined in `[environments-preview].names`, you can specify the environment
for this target by its name. Any fields that are defined in that environment will override
the values from options set by `pants.toml`, command line values, or environment variables.

You can specify multiple valid environments by using `parametrize`. If
{LOCAL_ENVIRONMENT_MATCHER} specified, Pants will fall back to the `local_environment`
chrisjrn marked this conversation as resolved.
Show resolved Hide resolved
defined for the current platform, or no environment if no such environment exists.
"""
)

Expand Down Expand Up @@ -131,13 +141,20 @@ class LocalFallbackEnvironmentField(FallbackEnvironmentField):


class LocalEnvironmentTarget(Target):
alias = "_local_environment"
alias = "local_environment"
core_fields = (*COMMON_TARGET_FIELDS, CompatiblePlatformsField, LocalFallbackEnvironmentField)
help = softwrap(
"""
Configuration of environment variables and search paths for running Pants locally.
f"""
Configuration of environment-sensitive options, such as environment variables and search
paths when Pants runs subprocesses locally.

TODO(#7735): Explain how this gets used once we allow targets to set environment.
To use this environment, map this target's address with a memorable name in
`[environments-preview].names`. You can then consume this environment by specifying the name in
the `_environment` field defined on other targets.

Only one `local_environment` may be defined in `[environments-preview].names` per platform, and
when `{LOCAL_ENVIRONMENT_MATCHER}` is specified as the environment, the
`local_environment` that matches the current platform (if defined) will be selected.
Comment on lines +155 to +157
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All of the environment targets should include a doc URL for #17096 in their help: but if you're planning on landing this today, we can do that in a followup.

"""
)

Expand All @@ -148,10 +165,11 @@ class DockerImageField(StringField):
value: str
help = softwrap(
"""
The docker image ID to use when this environment is loaded, e.g. `centos6:latest`.
The docker image ID to use when this environment is loaded.

chrisjrn marked this conversation as resolved.
Show resolved Hide resolved
TODO: expectations about what are valid IDs, e.g. if they must come from DockerHub vs other
registries.
This value may be any image identifier that the local Docker installation can accept.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should possibly remind users to use a SHA for reproducibility?

This includes image names with or without tags (e.g. `centos6` or `centos6:latest`), or
image names with an immutable digest (e.g. `centos@sha256:<some_sha256_value>`).
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't specify an actual sha256 value here, as I think these docs are probably going to be subject to bitrot (basically preserving bugs or vulnerabilities that are out of our control within our docs). I imagine people familiar with Docker will be able to figure out what a good value here would be.

"""
)

Expand Down Expand Up @@ -213,7 +231,7 @@ class DockerFallbackEnvironmentField(FallbackEnvironmentField):


class DockerEnvironmentTarget(Target):
alias = "_docker_environment"
alias = "docker_environment"
core_fields = (
*COMMON_TARGET_FIELDS,
DockerImageField,
Expand All @@ -222,10 +240,13 @@ class DockerEnvironmentTarget(Target):
)
help = softwrap(
"""
Configuration of a Docker image used for building your code, including the environment
variables and search paths used by Pants.
Configuration of a Docker environment used for building your code, including the Docker
image, along with environment-sensitive options, which include environment variables and
search paths, used by Pants to execute processes in this Docker environment.
Comment on lines +243 to +245
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This sentence has too many commas in it I think: not sure how to fix that. Break it up until multiple sentences, or make part of it parenthetical maybe?


TODO(#7735): Explain how this gets used once we allow targets to set environment.
To use this environment, map this target's address with a memorable name in
`[environments-preview].names`. You can then consume this environment by specifying the name in
the `_environment` field defined on other targets.
"""
)

Expand Down Expand Up @@ -272,7 +293,7 @@ class RemoteFallbackEnvironmentField(FallbackEnvironmentField):


class RemoteEnvironmentTarget(Target):
alias = "_remote_environment"
alias = "remote_environment"
core_fields = (
*COMMON_TARGET_FIELDS,
RemotePlatformField,
Expand All @@ -282,12 +303,17 @@ class RemoteEnvironmentTarget(Target):
help = softwrap(
"""
Configuration of a remote execution environment used for building your code, including the
environment variables and search paths used by Pants.
environment-sensitive options, which include environment variables and
search paths, used by Pants to execute processes in this remote environment.
Comment on lines 305 to +307
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto too many commas.


Note that you must also configure remote execution with the global options like
`remote_execution` and `remote_execution_address`.

Often, it is only necessary to have a single `_remote_environment` target for your
To use this environment, map this target's address with a memorable name in
`[environments-preview].names`. You can then consume this environment by specifying the name in
the `_environment` field defined on other targets.

Often, it is only necessary to have a single `remote_environment` target for your
repository, but it can be useful to have >1 so that you can set different
`extra_platform_properties`. For example, with some servers, you could use this to
configure a different Docker image per environment.
Expand Down Expand Up @@ -419,7 +445,7 @@ async def determine_local_environment(
raise AmbiguousEnvironmentError(
softwrap(
f"""
Multiple `_local_environment` targets from `[environments-preview].names`
Multiple `local_environment` targets from `[environments-preview].names`
are compatible with the current platform `{platform.value}`, so it is ambiguous
which to use:
{sorted(tgt.address.spec for _name, tgt in compatible_name_and_targets)}
Expand Down Expand Up @@ -615,8 +641,8 @@ async def get_target_for_environment_name(
raise ValueError(
softwrap(
f"""
Expected to use the address to a `_local_environment`, `_docker_environment`, or
`_remote_environment` target in the option `[environments-preview].names`, but the name
Expected to use the address to a `local_environment`, `docker_environment`, or
`remote_environment` target in the option `[environments-preview].names`, but the name
`{env_name.val}` was set to the target {address.spec} with the target type
`{tgt.alias}`.
"""
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ def test_unrecognized_build_file_symbols_during_bootstrap() -> None:
# TODO(#7735): Once we migrate the Shell backend to use environments, add one of its
# plugin fields here
_local_environment(name='env')
local_environment(name='env')
"""
)
with setup_tmpdir({"BUILD": build_file}) as tmpdir:
Expand All @@ -32,7 +32,7 @@ def test_unrecognized_build_file_symbols_during_bootstrap() -> None:


def test_environment_sensitive_option_fields_exist() -> None:
pants = run_pants(["help", "_local_environment"])
pants = run_pants(["help", "local_environment"])
pants.assert_success()
assert "python_bootstrap_names" in pants.stdout
assert "python_bootstrap_search_path" in pants.stdout
30 changes: 15 additions & 15 deletions src/python/pants/core/util_rules/environments_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -145,10 +145,10 @@ def test_all_environments(rule_runner: RuleRunner) -> None:
{
"BUILD": dedent(
"""\
_local_environment(name='e1')
_local_environment(name='e2')
_local_environment(name='no-name')
_docker_environment(name='docker', image="centos6:latest")
local_environment(name='e1')
local_environment(name='e2')
local_environment(name='no-name')
docker_environment(name='docker', image="centos6:latest")
"""
)
}
Expand All @@ -169,10 +169,10 @@ def test_choose_local_environment(rule_runner: RuleRunner) -> None:
{
"BUILD": dedent(
"""\
_local_environment(name='e1')
_local_environment(name='e2')
_local_environment(name='not-compatible', compatible_platforms=[])
_docker_environment(name='docker', docker_image="centos6:latest")
local_environment(name='e1')
local_environment(name='e2')
local_environment(name='not-compatible', compatible_platforms=[])
docker_environment(name='docker', docker_image="centos6:latest")
"""
)
}
Expand Down Expand Up @@ -203,14 +203,14 @@ def test_resolve_environment_name(rule_runner: RuleRunner) -> None:
{
"BUILD": dedent(
"""\
_local_environment(name='local')
_local_environment(
local_environment(name='local')
local_environment(
name='local-fallback', compatible_platforms=[], fallback_environment='local'
)
_docker_environment(name='docker', image="centos6:latest")
_remote_environment(name='remote-no-fallback')
_remote_environment(name='remote-fallback', fallback_environment="docker")
_remote_environment(name='remote-bad-fallback', fallback_environment="fake")
docker_environment(name='docker', image="centos6:latest")
remote_environment(name='remote-no-fallback')
remote_environment(name='remote-fallback', fallback_environment="docker")
remote_environment(name='remote-bad-fallback', fallback_environment="fake")
"""
)
}
Expand Down Expand Up @@ -365,7 +365,7 @@ def create_docker_tgt(


def test_resolve_environment_tgt(rule_runner: RuleRunner) -> None:
rule_runner.write_files({"BUILD": "_local_environment(name='env')"})
rule_runner.write_files({"BUILD": "local_environment(name='env')"})
rule_runner.set_options(
["--environments-preview-names={'env': '//:env', 'bad-address': '//:fake'}"]
)
Expand Down
2 changes: 1 addition & 1 deletion src/python/pants/engine/internals/platform_rules_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ def test_docker_complete_env_vars() -> None:
{
"BUILD": dedent(
"""\
_docker_environment(
docker_environment(
name='docker',
image='centos@sha256:a1801b843b1bfaf77c501e7a6d3f709401a1e0c83863037fa3aab063a7fdb9dc',
platform='linux_x86_64',
Expand Down