Added run_under_env argument to RunEnvironmentInfo

[UebelAndre]

Executable Starlark rules can use the run_under_env parameter of RunEnvironmentInfo to define an environment variable with the value of --run_under when it's set. This is useful in cases where executable targets execute some sort of process-wrapper which either sub-processes or execs into the expected process. Before, --run_under would happen on the process wrapper which made debuggers and profiling tools harder to use as the process they were attached to was not the process users cared about.

After this change, by having an executable Starlark rule return RunEnvironmentInfo(run_under_env = "RUN_UNDER_VALUE"), run or test invocations which pass --run_under='strace -c' will cause the executable to have RUN_UNDER_VALUE='strace -c' set in it's environment.

RELNOTES: Executable Starlark rules can use the run_under_env parameter of RunEnvironmentInfo to define an environment variable with the value of --run_under when it's set. This allows Starlark rule authors to control how this value is used.

closes #16232

[UebelAndre]

Some open questions for this draft:

• How are integration tests written for Bazel?
• Should the value set to run_under_env include the shell executable?

[fmeum]

• How are integration tests written for Bazel?

Given that the existing RunEnvironmentInfo tests are in

bazel/src/test/shell/bazel/bazel_rules_test.sh

Line 670 in b5db379

function test_starlark_rule_with_run_environment() {

, you should probably add shell tests there.

• Should the value set to run_under_env include the shell executable?

That is a good question I don't have a good answer to - I yet have to use --run_under in a non-trivial way. Do Starlark rules have access to the shell toolchain?

[UebelAndre]

Do Starlark rules have access to the shell toolchain?

I don't think so. I'm not aware of shell_toolchain being something that exists. I think if I could get an integration test it might lead to an answer here.

[UebelAndre]

@fmeum I noticed you did not have a change like like this for arugments in your PR. Was that intentional? Is this needed?

[fmeum]

That was intentional: Adding such a method/struct field would give Starlark rules access to this information in the provider. I wasn't sure whether arguments is something that other rules should be able to inspect, so I decided not to expose it for now.

Do you see a reason for depending rules to access the value of run_under_env? If there is no clear use case, it may be better not to expose it for now. API creep is real and incompatible changes removing functionality are painful.

Note that depending rules can always just forward the provider unchanged, so not exposing this doesn't prevent e.g. transition wrappers.

[UebelAndre]

@fmeum with the first passing build (which took quite a bit of time and effort), I think it's worth a thorough review. Would you be willing to take another look?

[fmeum]

Looks very good now, thanks for investing the time!

Given that he also reviewed the other RunEnvironmentInfo PRs, @comius would likely be in the best position to review this PR.

[UebelAndre]

@fmeum I think I've addressed all the feedback and the PR is ready for another review.

[fmeum]

Looks great, nothing to add :-)

This still has to be reviewed and imported by a Googler though.

[UebelAndre]

@comius Hello again, would you have time this week to review this as well? 😅

[UebelAndre]

I don't have time to do a rebase which is sad because I think these changes would be really good. Closing so the changes don't appear to be in work.