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

When action StampWithIjar runs it does not allow environment variables to be passed in using --action_env #1232

Closed
adam-singer opened this issue Feb 26, 2021 · 6 comments

Comments

@adam-singer
Copy link
Contributor

When action StampWithIjar runs it does not allow environment variables to be passed in using --action_env command line flag. This can lead to problems when ijar is executed on systems that have compiled it with a tool chain picked up from the environment but not available on the host system in standard locations.

A common solution applied is to allow durning action run use_default_shell_env = True as done in twitter-forks#8

@liucijus
Copy link
Collaborator

liucijus commented Mar 1, 2021

@adam-singer do you need your scala imports to be stamped? If not can you disable it with a setting:
https://github.com/bazelbuild/rules_scala/blob/master/docs/scala_import.md#scala_import

If you don't need stamping, I would prefer to hold off from applying use_default_shell_env = True to prevent non-deterministic builds. If stamping is something you really need now, then we need to ask this to be released bazelbuild/bazel#12730 and, maybe it's ok for a short time to enable use_default_shell_env = True to unblock you.

@adam-singer
Copy link
Contributor Author

@liucijus let's say we don't ship this, long term what would be the solution or approach used? Based on some of the source code reading and commit logs, it seemed this feature was under development and forked a bit from the provided implementation in rules_java code base, so I think it would be smart for us to avoid it. My understanding is we do prevent some amount of rebuilding if we do use stamped imports.

@liucijus
Copy link
Collaborator

liucijus commented Mar 2, 2021

Can you send a PR with a use_default_shell_env = True? If you need stamping, there are no other working alternatives that I know.

@wisechengyi
Copy link
Contributor

We were able to get our code working by turning off stamping for scala_import, since ijar is no longer invoked.

That said, I think @adam-singer has found use_default_shell_env = True to be common pattern for native tools. So if ijar was compiled with access to env vars, I'd say we should set use_default_shell_env = True when invoking it as well to maintain the consistency.

@adam-singer
Copy link
Contributor Author

@wisechengyi I'm testing out build --@io_bazel_rules_scala//scala/settings:stamp_scala_import=False in the .bazelrc file, locally it worked, but I would like to confirm on CI. Some discussion moved from slack https://bazelbuild.slack.com/archives/CA31HN1T3/p1614634162181200 back to this github thread.

@adam-singer
Copy link
Contributor Author

@liucijus build --@io_bazel_rules_scala//scala/settings:stamp_scala_import=False in the .bazelrc worked great after we removed use_default_shell_env = True. Huge Thanks !

wisechengyi pushed a commit to twitter-forks/rules_scala that referenced this issue Mar 2, 2021
### Description

Originally it was unknown to me that we could use flags to directly disable the stamping of scala imports using 
`build --@io_bazel_rules_scala//scala/settings:stamp_scala_import=False` in the `.bazelrc` file. Discussion in 
 bazelbuild#1232 cleared that up. 

### Motivation

Reverting #8 as it is not needed.
cattibrie pushed a commit to twitter-forks/rules_scala that referenced this issue Mar 9, 2021
### Description

Originally it was unknown to me that we could use flags to directly disable the stamping of scala imports using 
`build --@io_bazel_rules_scala//scala/settings:stamp_scala_import=False` in the `.bazelrc` file. Discussion in 
 bazelbuild#1232 cleared that up. 

### Motivation

Reverting #8 as it is not needed.
cattibrie pushed a commit to twitter-forks/rules_scala that referenced this issue Mar 11, 2021
### Description

Originally it was unknown to me that we could use flags to directly disable the stamping of scala imports using 
`build --@io_bazel_rules_scala//scala/settings:stamp_scala_import=False` in the `.bazelrc` file. Discussion in 
 bazelbuild#1232 cleared that up. 

### Motivation

Reverting #8 as it is not needed.
adam-singer added a commit to twitter-forks/rules_scala that referenced this issue Oct 5, 2021
### Description

Originally it was unknown to me that we could use flags to directly disable the stamping of scala imports using 
`build --@io_bazel_rules_scala//scala/settings:stamp_scala_import=False` in the `.bazelrc` file. Discussion in 
 bazelbuild#1232 cleared that up. 

### Motivation

Reverting #8 as it is not needed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants