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

Intern trivial ActionEnvironment and EnvironmentVariables instances #15171

Closed
wants to merge 1 commit into from

Conversation

fmeum
Copy link
Collaborator

@fmeum fmeum commented Apr 4, 2022

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.

@fmeum fmeum requested a review from lberki as a code owner April 4, 2022 13:17
@fmeum fmeum marked this pull request as draft April 4, 2022 13:17
@fmeum
Copy link
Collaborator Author

fmeum commented Apr 4, 2022

@comius This is the second follow-up to #14849 and based on #15170, which is why I labeled it as a draft.

@fmeum fmeum force-pushed the intern-action-environment branch 2 times, most recently from d749055 to 0ebad39 Compare April 4, 2022 13:57
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.
@fmeum fmeum force-pushed the intern-action-environment branch from 0ebad39 to 1b225bf Compare April 4, 2022 15:37
@fmeum fmeum marked this pull request as ready for review April 4, 2022 15:39
@fmeum
Copy link
Collaborator Author

fmeum commented Apr 4, 2022

@comius I rebased onto master and marked this as ready for review.

@sgowroji sgowroji added the team-Rules-Java Issues for Java rules label Apr 5, 2022
@bazel-io bazel-io closed this in 01d337f Apr 8, 2022
@fmeum fmeum deleted the intern-action-environment branch April 8, 2022 12:00
@fmeum
Copy link
Collaborator Author

fmeum commented Apr 8, 2022

@comius Is there a public way to quickly benchmark the rough memory impact of a Bazel change?

@comius
Copy link
Contributor

comius commented Apr 8, 2022

@comius Is there a public way to quickly benchmark the rough memory impact of a Bazel change?

If you have a sizable project you can check used-heap-size-after-gc displayed by bazel info. See #14890

I don't know about any public scripts that would do it for you.

cc @meisterT

@meisterT
Copy link
Member

meisterT commented Apr 8, 2022

fmeum added a commit to fmeum/bazel that referenced this pull request Jun 29, 2022
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 bazelbuild#15171.

PiperOrigin-RevId: 440312159
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);
      ^
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team-Rules-Java Issues for Java rules
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants