-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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 --incompatible_merge_fixed_and_default_shell_env
#18235
Conversation
2e5a466
to
867f751
Compare
Cc @oquenchil as this fixes an issue with |
I think @comius could best triage? |
bump, this would be super nice to have |
With the new flag, Starlark actions that specify both `env` and `use_default_shell_env` will no longer have `env` ignored. Instead, the values of `env` will override the default shell environment. This allows Starlark actions to both pick up user-configured variables such as `PATH` from the shell environment as well as set variables to fixed values required for the action, e.g., variables provided by the C++ toolchain. Rationale for having `env` override the default shell env: The rules know best which values they have to set specific environment variables to in order to successfully execute an action, so a situation where users break an action by a globally applied `--action_env` is prevented. If users really do have to be able to modify an environment variables fixed by the rule, the rule can always make this configurable via an attribute. Work towards bazelbuild#5980
@comius It would be great if you could take a look or reassign to someone else. Flipping this flag is required for my ongoing work on the all Starlark |
would definitely still love to see this one land! |
@comius Friendly ping, could you take a look or assign to someone else? |
@bazelbuild/triage |
@comius will try and review this tmrw cc: @fmeum @brentleyjones |
Since this is behind a flag, you think it's cherry-pickable @fmeum? |
It would be if it applies cleanly. I recall some merge conflicts, but we can give it a try. |
@bazel-io flag |
@bazel-io fork 6.4.0 |
@fmeum @brentleyjones @keith @gregestren @comius From In the current release-6.4.0 branch, we have
but it really should be
before the change. cc: @bazelbuild/triage |
With the new flag, Starlark actions that specify both `env` and `use_default_shell_env` will no longer have `env` ignored. Instead, the values of `env` will override the default shell environment. This allows Starlark actions to both pick up user-configured variables such as `PATH` from the shell environment as well as set variables to fixed values required for the action, e.g., variables provided by the C++ toolchain. Rationale for having `env` override the default shell env: The rules know best which values they have to set specific environment variables to in order to successfully execute an action, so a situation where users break an action by a globally applied `--action_env` is prevented. If users really do have to be able to modify an environment variables fixed by the rule, the rule can always make this configurable via an attribute. Work towards bazelbuild#5980 Fixes bazelbuild#12049 Closes bazelbuild#18235. PiperOrigin-RevId: 559506535 Change-Id: I7ec6ae17b076bbca72fab8394f3a8b3e4f9ea9d8
I submitted a manual cherry-pick as #19319. |
With the new flag, Starlark actions that specify both `env` and `use_default_shell_env` will no longer have `env` ignored. Instead, the values of `env` will override the default shell environment. This allows Starlark actions to both pick up user-configured variables such as `PATH` from the shell environment as well as set variables to fixed values required for the action, e.g., variables provided by the C++ toolchain. Rationale for having `env` override the default shell env: The rules know best which values they have to set specific environment variables to in order to successfully execute an action, so a situation where users break an action by a globally applied `--action_env` is prevented. If users really do have to be able to modify an environment variables fixed by the rule, the rule can always make this configurable via an attribute. Work towards bazelbuild#5980 Fixes bazelbuild#12049 Closes bazelbuild#18235. PiperOrigin-RevId: 559506535 Change-Id: I7ec6ae17b076bbca72fab8394f3a8b3e4f9ea9d8
With the new flag, Starlark actions that specify both `env` and `use_default_shell_env` will no longer have `env` ignored. Instead, the values of `env` will override the default shell environment. This allows Starlark actions to both pick up user-configured variables such as `PATH` from the shell environment as well as set variables to fixed values required for the action, e.g., variables provided by the C++ toolchain. Rationale for having `env` override the default shell env: The rules know best which values they have to set specific environment variables to in order to successfully execute an action, so a situation where users break an action by a globally applied `--action_env` is prevented. If users really do have to be able to modify an environment variables fixed by the rule, the rule can always make this configurable via an attribute. Work towards #5980 Fixes #12049 Closes #18235. Commit d1fdc53 PiperOrigin-RevId: 559506535 Change-Id: I7ec6ae17b076bbca72fab8394f3a8b3e4f9ea9d8 Fixes #19312 --------- Co-authored-by: Ivo List <[email protected]> Co-authored-by: Ian (Hee) Cha <[email protected]>
With the new flag, Starlark actions that specify both
env
anduse_default_shell_env
will no longer haveenv
ignored. Instead, the values ofenv
will override the default shell environment.This allows Starlark actions to both pick up user-configured variables such as
PATH
from the shell environment as well as set variables to fixed values required for the action, e.g., variables provided by the C++ toolchain.Rationale for having
env
override the default shell env: The rules know best which values they have to set specific environment variables to in order to successfully execute an action, so a situation where users break an action by a globally applied--action_env
is prevented. If users really do have to be able to modify an environment variables fixed by the rule, the rule can always make this configurable via an attribute.Work towards #5980
Fixes #12049