pantsbuild
Add global option `process_extra_env` for every Process call
Precompiled binaries that were not created for NixOS usually have a so-called link-loader hardcoded into them. On Linux/x86_64 this is for example /lib64/ld-linux-x86-64.so.2. for glibc. NixOS, on the other hand, usually has its dynamic linker in the glibc package in the Nix store and therefore cannot run these binaries.
The solution is to use nix-ld, but it only works if you set NIX_LD env variable for every process call.
This pr adds a global option process_extra_env to set arbitrary env variables for every pants Process call.
Thanks for tackling this!
So we actually already have an option for this: https://www.pantsbuild.org/dev/reference/subsystems/subprocess-environment
The problem is that we only actually apply those env vars in some Python-related processes. But the documentation of that option is very clear that it should apply to all processes. So this is simply broken today.
Therefore, instead of the new option, could you make this existing option apply to all processes, and remove the special handling in the python cases? This would also close https://github.com/pantsbuild/pants/issues/16565 .
@benjyw I'm getting a bunch of rule errors. It looks like SubprocessEnvironment uses EnvironmentAware class which afaik needs an EnvironmentName which means that I can't use it in the -> Process rule, is this correct? If so, shall we create a different option for Process env vars?
I've asked @stuhood to take a look at this, as his engine expertise is second to none.
Thanks for the ping. @stuhood found a minimal repro and workaround for the underlying graph issue, so perhaps that can move this along by allowing this option to be EnvironmentAware.
The remaining question is whether we should in fact make SubprocessEnvironment live up to its documentation by applying to all processes, or whether, since that has been Python-only in practice for a long time, we should deprecate/rename it and start with a new subsystem that either applies globally or can be scoped somehow. See discussion here: https://github.com/pantsbuild/pants/issues/9760#issuecomment-662241354
If/when this PR can progress, we've just branched for ~2.25 2.26 2.27~ 2.28, so merging this pull request now will come out in ~2.26 2.27 2.28~ 2.29, please move the release notes updates to docs/notes/2.29.x.md. Thank you!
Thanks for the contribution. We've just branched for 2.30.x, so merging this pull request now will come out in 2.31.x (but be backported as requested). Please move the release notes updates to docs/notes/2.31.x.md if that's appropriate.
That said, now that we're entirely call-by-name you should be able to avoid graph issues.