Expand $(location) and "Make variables" in env

[mbland]

Description

Implements the "values are subject to $(location) and "Make variable" substitution" contract for the common env attribute for binary and test rules. Fixes #1770.

Motivation

Makes env attribute semantics for Scala binary and test targets conformant with those documented in the Bazel docs:

• https://bazel.build/reference/be/common-definitions#common-attributes-binaries
• https://bazel.build/reference/be/common-definitions#common-attributes-tests
• https://bazel.build/reference/be/make-variables#use
• https://bazel.build/rules/lib/providers/RunEnvironmentInfo

Most of the new functions could live in a separate repository at some point.

As for why we're using the "deprecated" ctx.expand_make_variables:

• Need replacement for ctx.expand_make_variables() bazelbuild/bazel#5859
• Adding expansion.bzl for Fully Expanding Environment Variables bazelbuild/bazel-skylib#486

The "deprecated" comment in the ctx.expand_make_variables docstring has existed from the beginning of the file's existence (2018-05-11):

• bazelbuild/bazel@abbb900

[WojciechMazur]

LGTM

[mbland]

@WojciechMazur Thanks for the quick turnaround. I've actually just pushed three small new changes; I'll merge once CI passes:

• I updated the run_environment_info API to take a list of attr.label_list instead of a list of string. That should prevent silent breakages if a user misspells an attribute name.

• I added a new shell test, test_scala_test_env_attribute_with_env_inherit_and_test_env, to clarify the expected interations between env, env_inherit, and --test_env. Basically, precedence is env, --test_env, env_inherit, except on macOS, where env_inherit overrides env. I'll file a Bazel bug eventually.

• I added .gemini/config.yaml to disable Gemini Code Assist for this repo.

@WojciechMazur @simuons After this lands, I'll start a new version bump pull request. Once that lands, what do you think about pushing v7.2.0? (Or should it be v7.1.2, if we consider implementing this behavior a bugfix rather than a new feature?)

[mbland]

• Basically, precedence is env, --test_env, env_inherit, except on macOS, where env_inherit overrides env. I'll file a Bazel bug eventually.

Wow, I was wrong about this being a Bazel bug on macOS. I ended up going on a bit of a journey to fix test_env_attribute_expansion.sh on Windows, but it's all working now. Will merge once CI finishes.

As for why I was wrong:

• For starters, I had one of the environment variables still set to the empty string in my macOS environment, which is why I thought there was a problem. But then the macOS CI build broke, prompting me to find the stale environment variable in my shell as the culprit.

• As it turns out, env_inherit will override env, not the other way around.

• On Windows, when Bazel is in the path as bazel.exe, then MSYS2 Bash will not pass its environment variables to the Bazel process, breaking the test cases that validate env_inherit behavior. As you can see, I had to add code to test_env_attribute_expansion.sh to find the Bazel executable, then wrap bazel.exe in a .bat file to get the correct behavior in test_scala_test_env_attribute_with_env_inherit_and_test_env.

I have no idea why the Buildkite Windows environment uses bazel, and my Windows VM uses bazel.exe. I studied bazelbuild/continuous-integration a bit, especially buildkite/bazelci.py, and can't yet really see how it does it. But that test is finally working everywhere, so I'm happy for now.