--test_env/--action_env only for specific targets

[Globegitter]

Description of the problem / feature request:

We are using container_test from rules_docker and for ci need to pass in DOCKER_HOST, DOCKER_TLS_VERIFY and DOCKER_CERT_PATH so these can work. But passing that flag like bazel test ... --test_env=DOCKER_HOST or putting it into the .bazelrc passes that into all test sandbox environments. It would be nice to have a way of specifying which target these environment variables should be passed in. We had a similar use-case once for action_env but could work around that, though I still think it would be nice to have it there also.

Feature requests: what underlying problem are you trying to solve with this feature?

By passing these environment variables to all tests/actions invoked in that one command it invalidates the cache and reruns tests/actions that do not actually need the env variables. Further in our case that also means that the remote cache for all affected targets can not be shared between ci and non-ci.

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

Have a workspace with multiple tests, run bazel test ... and then bazel test --test_env=SOME_ENV=a ... and see that all tests get re-executed. It behaves similarly for --action_env

What operating system are you running Bazel on?

Ubuntu 16.04

What's the output of bazel info release?

release 0.17.0rc1

[ittaiz]

+1

…

On Tue, 28 Aug 2018 at 11:38 Markus Padourek ***@***.***> wrote: Description of the problem / feature request: We are using container_test from rules_docker and for ci need to pass in DOCKER_HOST, DOCKER_TLS_VERIFY and DOCKER_CERT_PATH so these can work. But passing that flag like bazel test ... --test_env=DOCKER_HOST or putting it into the .bazelrc passes that into all test sandbox environments. It would be nice to have a way of specifying which target these environment variables should be passed in. We had a similar use-case once for action_env but could work around that, though I still think it would be nice to have it there also. Feature requests: what underlying problem are you trying to solve with this feature? By passing these environment variables to all tests/actions invoked in that one command it invalidates the cache and reruns tests/actions that do not actually need the env variables. Further in our case that also means that the remote cache for all affected targets can not be shared between ci and non-ci. Bugs: what's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible. Have a workspace with multiple tests, run bazel test ... and then bazel test --test_env=SOME_ENV=a ... and see that all tests get re-executed. What operating system are you running Bazel on? Ubuntu 16.04 What's the output of bazel info release? release 0.17.0rc1 — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#6011>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABUIF1TnF42SwDispnrc-O6z1FQ-ANchks5uVQGNgaJpZM4WPKUI> .

[Globegitter]

Also as a note, what I actually instinctively thought would be possible at the beginning is putting something like

test //my/target:test --test_env=something

into the .bazelrc and it would just apply to the listed targets.

[katre]

The bazelrc only accepts the same flags that would exist on the command line, so if this is added for the command line it would then work in a bazelrc.

We have some flags that perform like this, for example see --runs_per_test:

If a single number is specified, all tests will run that many times. Alternatively, a regular expression may be specified using the syntax regex@number. This constrains the effect of --runs_per_test to targets which match the regex (e.g. "--runs_per_test=^//pizza:.*@4" runs all tests under //pizza/ 4 times). This form of --runs_per_test may be specified more than once.

I'm not entirely sure if this syntax would work well with the more complex data given by --test_env. Please comment on the syntax you'd like to see, and I am handing this off to the Execution team to continue triage.

[Globegitter]

@katre Thanks for the response, well bazelrc does have its own syntax also with allowing e.g. build:config. But in fact the syntax I suggested above is also valid syntax on the command-line, so what would be the issue with supporting that?

Otherwise yeah, having the same regex syntax as runs_per_test for test_env/action_env would also solve that specific issue and would be totally fine by me.

[katre]

Bazel (like many command-line programs) differentiates between flag-based options, and non-flag-based arguments. The target patterns used to determine what to build (or test, run, etc) are arguments to the command. The bazelrc only allows the setting of options, not arguments. If it did, then (by the logic of how bazelrc works), everytime you built, you would build the targets specified in the bazelrc, which is probably not what anyone wants.

[Globegitter]

Would it be very complex to change the behaviour there? Or would it possibly have a big impact on performance? Or the other way around, would adding regex support to test_env/action_env be more trivial?

[katre]

Adding regex support to test_env and action_env would almost certainly be easier. However, I think the Execution team needs to triage this, so I will mark it as Untriaged and they can take a look.

[jmmv]

The changes to the bazelrc file are not really feasible as they would imply significantly changing how it works and this file is already complex enough. It doesn't help that this file is purely parsed on the Bazel client side...

That said, what's the argument for putting these in the configuration file? If you want certain tests to have certain flags, shouldn't those be in their corresponding BUILD files?

Aside from that, the regexp solution may work... but will it be sufficient? The --test_env et. al. currently affect the test configuration fragment. Changes to this flag, no matter how they are "tagged", invalidate the whole analysis cache. (There are changes on the way to resolve this, but the essential problem remains: the flag's content is seen as a unit and cannot be attached to individual tests.)

It'd be good to have @serynth input here.

[programmablereya]

Hmmm... These are definitely important concerns! :) There are two concerns here, re-analysis and re-execution.

As far as re-execution goes... The regex solution should help with that, I believe? Since then you would get an action cache hit, even if the analysis cache is dropped.

As far as re-analysis goes... Once the regex solution exists, it's theoretically possible that a future version of configuration trimming could trim down at such a fine level - if an individual element of test_env does not apply to a target and its transitive dependencies, then it's removed from test_env for that target and its transitive dependencies. This would allow for cache hits on unaffected targets.

But that solution would be quite some ways off - right now even trimming full fragments (groups of flags) doesn't exist, let alone trimming one flag, let alone trimming part of a flag.

A workaround which works today is to have a test (e.g., sh_test) which has a data dependency on the test you want to run. You can set these environment variables and then exec the real test executable. (A Starlark macro may help in creating these wrappers.) If you can afford to have a CI wrapper for each test you want to run this way, you can run these for your CI in place of the unwrapped ones!

[Globegitter]

I think just the fact of saving re-execution would imo be very valuable. Saving re-analysis would be great as well but does sound like an optimisation to me.

But good pointer with wrapping it into a bash script.

I am just wondering, which kind of env vars, apart from maybe PATH I would want to be added to all my tests in any case?

[Globegitter]

Another feature that would help at least for the test scenario is: #7364

Then I could be using the common env attribute rather than specifying --test_env. It still would not help with build actions though.

[pauldraper]

DOCKER_HOST, DOCKER_TLS_VERIFY and DOCKER_CERT_PATH

FWIW, these specific variables would ideally not be part of the cache key for any action...your artifacts/tests likely don't vary by their values.

---

My workaround is to avoid --action_env altogether and put these in workspace status.

And I have a helper executable with ctx.version_file as a runfile that it reads and sets env vars.

usage: workspace-status [-h] command [args [args ...]]

Run command with volatiles as environment variables

positional arguments:
  command
  args

optional arguments:
  -h, --help  show this help message and exit

Then I can run commands with the workspace values as env vars, or load a variables in shell scripts . <(...executable... env), and they don't affect caching. (At least...it works locally, haven't tried remote caching.)

[burdiyan]

The Please build system which is inspired by Bazel has a configuration option PassUnsafeEnv which passes environment variables to actions without affecting the cache key. I really wish Bazel had something similar!

Although it can be dangerous, for certain use cases it really helps a lot.

As far as I understand --action_env, --define and even using volatile workspace status file - they all change the cache key, so changing a value would be a cache miss. Is that true?

[burdiyan]

This issue also seem to be talking about a similar problem: #10075

[github-actions]

Thank you for contributing to the Bazel repository! This issue has been marked as stale since it has not had any activity in the last 1+ years. It will be closed in the next 14 days unless any other activity occurs or one of the following labels is added: "not stale", "awaiting-bazeler". Please reach out to the triage team (@bazelbuild/triage) if you think this issue is still relevant or you are interested in getting the issue resolved.

[burdiyan]

@bazelbuild/triage