Let Starlark executable rules specify their environment #15232
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
fmeum
force-pushed
the
run-environment-info
branch
2 times, most recently
from
8f48123
to
2b981c0
Compare
|
@comius Alternative names I considered are |
fmeum
force-pushed
the
run-environment-info
branch
2 times, most recently
from
5480d51
to
61a9f58
Compare
|
Ivo, are you the right person for this or is it @brandjon ? |
fmeum
changed the title
fmeum
force-pushed
the
run-environment-info
branch
from
61a9f58
to
93ac9a6
Compare
I've reviewed the |
rickeylev
reviewed
May 3, 2022
src/main/java/com/google/devtools/build/lib/analysis/RunEnvironmentInfo.java
Outdated
Show resolved
Hide resolved
rickeylev
reviewed
May 3, 2022
src/main/java/com/google/devtools/build/lib/starlarkbuildapi/RunEnvironmentInfoApi.java
Outdated
Show resolved
Hide resolved
rickeylev
reviewed
May 3, 2022
src/main/java/com/google/devtools/build/lib/starlarkbuildapi/test/TestEnvironmentInfoApi.java
Outdated
Show resolved
Hide resolved
|
I'm interested in using this functionality, so I reviewed what I could. I'm not too familiar with apis, so my review wasn't very deep. |
The new RunEnvironmentInfo provider allows any executable or test Starlark rule to specify the environment for when it is executed, either as part of a test action or via the run command. Refactors testing.TestEnvironment to construct a RunEnvironmentInfo and adds a warning (but not an error) if the provider constructed in this way is returned from a non-executable non-test rule. If a RunEnvironmentInfo is constructed directly via the Starlark constructor, this warning becomes an error.
fmeum
force-pushed
the
run-environment-info
branch
from
93ac9a6
to
9dfd5e5
Compare
Thanks for the comments @rickeylev, I pushed a new commit to address them (and resolved the merge conflicts). |
|
LGTM. @comius can you review? |
sgowroji
added
the
awaiting-review
comius
approved these changes
May 10, 2022
|
@bazel-io flag |
bazel-io
added
the
potential release blocker
|
@bazel-io fork 5.3.0 |
bazel-io
removed
the
potential release blocker
|
Hello @fmeum, I cherry-picked these changes and raised a PR against release-5.3.0 branch. Presubmit checks are failing in that PR. Could you please cherry picked with appropriate commits and raise a PR against release-5.3.0? Thanks! |
fmeum
added a commit
to fmeum/bazel
that referenced
this pull request
Jun 29, 2022
The new RunEnvironmentInfo provider allows any executable or test Starlark rule to specify the environment for when it is executed, either as part of a test action or via the run command. Refactors testing.TestEnvironment to construct a RunEnvironmentInfo and adds a warning (but not an error) if the provider constructed in this way is returned from a non-executable non-test rule. If a RunEnvironmentInfo is constructed directly via the Starlark constructor, this warning becomes an error. Fixes bazelbuild#7364 Fixes bazelbuild#15224 Fixes bazelbuild#15225 Closes bazelbuild#15232. PiperOrigin-RevId: 448185352
ckolli5
pushed a commit
that referenced
this pull request
Jun 29, 2022
* Specify fixedEnv/inheritedEnv interaction in ActionEnvironment Previously, ActionEnvironment did not publicly document how fixed and inherited environment variables interact, but still cautioned users to keep the two sets disjoint without enforcing this. As a result, neither could users rely on the interaction nor could ActionEnvironment benefit from the additional freedom of not specifying the behavior. With this commit, ActionEnvironment explicitly specifies that the values of environment variable inherited from the client environment take precedence over fixed values and codifies this behavior in a test. This has been the effective behavior all along and has the advantage that users can provide overrideable defaults for environment variables. Closes #15170. PiperOrigin-RevId: 439315634 * Intern trivial ActionEnvironment and EnvironmentVariables instances When an ActionEnvironment is constructed out of an existing one, the ActionEnvironment and EnvironmentVariables instances, which are immutable, can be reused if no variables are added. Also renames addVariables and addFixedVariables to better reflect that they are operating on an immutable type. Closes #15171. PiperOrigin-RevId: 440312159 * Let Starlark executable rules specify their environment The new RunEnvironmentInfo provider allows any executable or test Starlark rule to specify the environment for when it is executed, either as part of a test action or via the run command. Refactors testing.TestEnvironment to construct a RunEnvironmentInfo and adds a warning (but not an error) if the provider constructed in this way is returned from a non-executable non-test rule. If a RunEnvironmentInfo is constructed directly via the Starlark constructor, this warning becomes an error. Fixes #7364 Fixes #15224 Fixes #15225 Closes #15232. PiperOrigin-RevId: 448185352 * Fix strict deps violation ``` src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleClassFunctions.java:990: error: [strict] Using type com.google.devtools.build.lib.cmdline.LabelValidator from an indirect dependency (TOOL_INFO: "//src/main/java/com/google/devtools/build/lib/cmdline:LabelValidator"). See command below ** LabelValidator.parseAbsoluteLabel(labelString); ^ ```
ShreeM01
removed
the
awaiting-review
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The new RunEnvironmentInfo provider allows any executable or test
Starlark rule to specify the environment for when it is executed, either
as part of a test action or via the run command.
Refactors testing.TestEnvironment to construct a RunEnvironmentInfo and
adds a warning (but not an error) if the provider constructed in this
way is returned from a non-executable non-test rule. If a
RunEnvironmentInfo is constructed directly via the Starlark constructor,
this warning becomes an error.
Fixes #7364
Fixes #15224
Fixes #15225