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

Allow testing.TestEnvironment / TestEnvironmentInfo to be read from Starlark #15224

Closed
rickeylev opened this issue Apr 12, 2022 · 1 comment
Closed
Labels
P4 This is either out of scope or we don't have bandwidth to review a PR. (No assignee) type: bug

Comments

@rickeylev
Copy link
Contributor

Description of the bug:

Given a target that returns the testing.TestEnvironment / TestEnvironmentInfo, that provider cannot be read from the target.

This makes it impossible to easily test that the rule constructs an appropriate TestEnvironmentInfo object.

What's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

def _provides_info(ctx):
  return [testing.TestEnvironment({})]

provides_info = rule(implementation = _provides_info)

def _reads_info(ctx):
  print(ctx.attr.target[testing.TestEnvironment])

reads_info = rule(implementation=_reads_info, attrs={"target": attr.label()})

provides_info(name="provides")
reads_info(name="reads", target=":provides")

Actual behavior:

An error like below occurs:

Error: Type Target only supports indexing by object constructors, got builtin_function_or_method instead

Expected behavior:

The TestEnvironmentInfo provider instance is returned.

Which operating system are you running Bazel on?

Linux

What is the output of bazel info release?

google build

If bazel info release returns development version or (@non-git), tell us how you built Bazel.

Google build

What's the output of git remote get-url origin; git rev-parse master; git rev-parse HEAD ?

n/a

Have you found anything relevant by searching the web?

None found

Any other information, logs, or outputs that you want to share?

No response

@sgowroji sgowroji added untriaged team-OSS Issues for the Bazel OSS team: installation, release processBazel packaging, website labels Apr 13, 2022
@meteorcloudy meteorcloudy added team-Build-Language and removed team-OSS Issues for the Bazel OSS team: installation, release processBazel packaging, website labels Apr 13, 2022
@brandjon
Copy link
Member

At a glance, it looks like this provider is consumed natively but indeed not accessible from Starlark. The above example fails because the testing.TestEnvironment object is a builtin factory function that produces TestEnvironment instances, yet not itself the actual provider object.

@brandjon brandjon changed the title testing.TestEnvironment / TestEnvironmentInfo cannot be read from a target Allow testing.TestEnvironment / TestEnvironmentInfo to be read from Starlark Apr 28, 2022
@brandjon brandjon added P4 This is either out of scope or we don't have bandwidth to review a PR. (No assignee) and removed untriaged labels Apr 28, 2022
fmeum added a commit to fmeum/bazel that referenced this issue 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 issue 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);
      ^
```
JamesMBartlett added a commit to JamesMBartlett/rules_go that referenced this issue Aug 22, 2022
Works the same as the `go_cross_binary` rule.
Because of [this](bazelbuild/bazel#15224) bazel issue,
there's no way to access the underlying `go_test` rule's `env`
info provider. So until bazel 5.3.0 becomes the `rules_go` minimum bazel
version, I see no way of passing through the `go_test` rule's `env` to
the `go_cross_test`. This means any `env` set will not be used when the
`go_cross_test` test is run, which is a little unfortunate. However, I
think `go_cross_test` is still useful in the meantime despite this
limitation.

Signed-off-by: James Bartlett <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P4 This is either out of scope or we don't have bandwidth to review a PR. (No assignee) type: bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants