https://pantsbuild.org/ logo
#general
Title
# general
c

curved-television-6568

10/07/2021, 7:12 AM
The docs for subprocess env (https://www.pantsbuild.org/docs/reference-subprocess-environment) is a bit vague, as I thought it would apply to all processes, but it seems to only apply for Python processes, invoked using PEX..
e

enough-analyst-54434

10/07/2021, 2:15 PM
They apply to any rule code that asks for a SubprocessEnvironment and then uses that to set up a Process. You'll have to do a little work. If you see a useful way to DRY that up, fire away.
c

curved-television-6568

10/07/2021, 2:47 PM
Right, I figured that the rule executing the
Process -> ProcessResult
would inject that.. but I see that you’ll have to
Get
the SubprocessEnvironment when creating the
Process
if you want to have it included. Not saying this is a bad thing, only that it is not clear from the documentation that it is up to each
Process
request whether to use the extra sub proc env vars. If we’d want it for all processes, I think it could be doable, but there’d need to be an extra level of indirection between the
Process
request, and the rule actually executing it (looks like it is in the rust engine), so I’ve no idea how big a change that would be (and probably not worth it). So, I suggest to perhaps add a short sentence to the docs, highlighting the current state, that it is up to each process invocation to respect the configuration (if desired), and currently it is only the subprocesses invoking pex’es that does so.
Or, it could be done as a pre-step to
Process
Like there is a
VenvPexProcess -> Process
rule, there could be a more generic
ProcessRequest -> Process
that adds in the SubprocessEnvironment for you.. mm, but I’m not convinced that this is a good idea…
w

witty-crayon-22786

10/07/2021, 4:23 PM
i like https://github.com/pantsbuild/pants/pull/13146 better, yea… the
[subprocess-environment]
should probably be renamed and bounded in scope, because any sort of “set an environment variable _everywhere_” option is likely to be too big a hammer
👍 2
3 Views