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

Add env attr to native_binary and native_test #409

Open
cameron-martin opened this issue Nov 7, 2022 · 9 comments · May be fixed by #484 or #482
Open

Add env attr to native_binary and native_test #409

cameron-martin opened this issue Nov 7, 2022 · 9 comments · May be fixed by #484 or #482

Comments

@cameron-martin
Copy link

native_binary and native_test currently have no way to specify the environment that the target runs with. A RunEnvironmentInfo provider could be returned from these, based on a passed-in env attr.

@cameron-martin
Copy link
Author

This env param does work even without RunEnvironmentInfo (similar to args).

@fmeum
Copy link
Contributor

fmeum commented Jul 30, 2023

@cameron-martin Are you sure that's true? As far as I know the args parameter is treated specially for all rules, but env is only for native rules.

@cameron-martin
Copy link
Author

Looks like I'm too eager to clean up my issues list. I'll reopen this as you're probably right about this.

@redsun82
Copy link

redsun82 commented Dec 20, 2023

Hi! I would also like this! For now I'm working around this with a patch, using this rather small change:

https://github.com/bazelbuild/bazel-skylib/compare/main...redsun82:bazel-skylib:env-native-binary?expand=1

I would have contributed it as a PR, but I'm a bit unsure about:

  • setting up tests
  • bazel version compatibility (is RunEnvironmentInfo supported across all versions bazel-skylib must support? If not, how can that code be made compatible across versions?)

@fmeum
Copy link
Contributor

fmeum commented Dec 20, 2023

@redsun82 You could use bazel_features, which provides you a way to test for certain global symbols: https://github.com/bazel-contrib/bazel_features/tree/main?tab=readme-ov-file#accessing-globals
RunEnvironmentInfo is already supported: https://github.com/bazel-contrib/bazel_features/blob/443861571a389ddc16d17690ab8e46ee87b4ea57/private/globals.bzl#L5

@redsun82
Copy link

ah, nice one @fmeum

@redsun82
Copy link

redsun82 commented Dec 20, 2023

For the moment I've opened this draft PR, I'll update it sometime soon with the RunEnvironmentInfo check and have a look at the existing native_binary tests.

@redsun82
Copy link

I've updated the PR, which turned out a bit trickier than I thought. For the moment I decided to replicate part of what bazel_features does to avoid complicating the legacy workflow-based integration instructions, which is not ideal. If you deem that we should rather go for bazel_features to simplify the code but update the instructions to integrate bazel_skylib into a WORKFLOW file accordingly with the new dependency, I can update the PR further.

@redsun82
Copy link

I've opened another PR based on the previous one but using bazel_features (and modifying the release instructions for WORKSPACE setup). There was a bit of gymnastics involved in getting the doc update right, but I guess the two approaches can be compared now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants