Add env and env_inherit to native_binary and native_test (using @bazel_features)
#484
Conversation
redsun82
requested review from
brandjon,
tetromino,
comius and
c-mita
as code owners
redsun82
changed the title
env and env_inherit to native_binary and native_testenv and env_inherit to native_binary and native_test (using @bazel_features)
Conflicts: MODULE.bazel docs/native_binary_doc.md rules/native_binary.bzl
FYI, going by https://bazel.build/release#support-matrix , v5.4.1 is now the minimum supported version, so if (not a full review, I'll defer to @tetromino / @brandjon for that) |
According to https://bazel.build/release#support-matrix, we can drop support for Bazel 4.x (so we can use `runfiles.merge_all`) and Bazel <5.3 (so we can use `RunEnvironmentInfo` unconditionally, which simplifies the change significantly).
|
@hvadehra thanks for the heads up, that makes things way easier. @tetromino / @brandjon, the PR should be ready for another review pass now 🚀 |
| @@ -10,7 +10,7 @@ register_toolchains( | |||
| "//toolchains/unittest:bash_toolchain", | |||
| ) | |||
|
|
|||
| bazel_dep(name = "platforms", version = "0.0.4") | |||
| bazel_dep(name = "platforms", version = "0.0.9") | |||
There was a problem hiding this comment.
there was a warning about a dependency pulling this version in
tests/native_binary/assertenv.cc
Outdated
| #define OS_VAR "HOME" | ||
| #endif | ||
|
|
||
| enum vars_to_be_found { |
There was a problem hiding this comment.
Could you implement this test in a less hacky way?
For example, get rid of the enum, and just search for 3 strings. Use set found to true/false.
|
@comius sorry for the late update on this, I've cleaned up the test now. |
|
@comius any chance you could have another look at this? |
Closes #409