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

Fix up our environment handling options now that we have multiple languages #14809

Open
Eric-Arellano opened this issue Mar 16, 2022 · 10 comments

Comments

@Eric-Arellano
Copy link
Contributor

Our options for setting environment variables is confusing.

We do have a consistent mechanism for setting env vars for tests via [test].env_vars, at least.

I propose that we move to each language backend ([python], [golang], [shell-setup]) having its own --env-vars option. Rather than a single subsystem for every single process. Why? Env vars mess up caching—especially remote caching i.e. sharing across machines—so we want things to be as granular as possible. Go might need to set env vars that Python doesn't care about. (Related, this allows us to better ban setting certain env vars like PEX_EXTRA_SYS_PATH that only one language cares about.)

Having to duplicate certain env vars like HTTPS_PROXY across multiple options is unfortunate, but probably worth it? A middle-ground is to have both language-specific options & also a global option, but to heavily encourage the language-specific onees?

@stuhood
Copy link
Sponsor Member

stuhood commented Mar 16, 2022

I propose that we move to each language backend ([python], [golang], [shell-setup]) having its own --env-vars option.

The environment variables that you need to fetch things off the network or to compile things might be significantly different. I'm not sure whether that suggests "per-language" isolation so much as "per task" isolation (compiling a C extension vs downloading vs running tests). But maybe it's both.

@Eric-Arellano
Copy link
Contributor Author

I think figuring this out should probably block making the new languages (Go, Java, Scala) stable.

@stuhood
Copy link
Sponsor Member

stuhood commented Mar 16, 2022

This also relates to #7735. But most likely these task/language options should be created regardless, and then marked cross-platform/environment by a facility from #7735.

@kaos
Copy link
Member

kaos commented Mar 16, 2022

I like the idea of per language and task --env-vars.

I think there are only a few env vars that could be applicable to multiple sections, and as such are worth repeating in those cases.

@Eric-Arellano
Copy link
Contributor Author

I don't love having a proliferation of subsystems, so I'd encourage we bundle all the language options together.

These are totally straw-person, but maybe something like this?

[python].env_vars_all_tasks
[python].env_vars_dependency_resolution
[python].env_vars_code_execution

Not sure if the all_tasks makes sense, it's meant to help w/ DRY.

@kaos
Copy link
Member

kaos commented Mar 16, 2022

Agree with avoiding new subsystems. I imagined there'd be existing subsystem that could house env-vars.

Also, come to think of it, download is perhaps the only task that is disjoint from any particular language. So my vote is perhaps on per language env vars, and for downloads. (shying away from the sheer number of env_vars_* in the straw man example :D )

@Eric-Arellano
Copy link
Contributor Author

I imagined there'd be existing subsystem that could house env-vars.

The issue here is, for better-or-worse, we don't have "plugin options" like we have "plugin fields". So a generic env-vars could not house all these different granular options. This has also caused weirdness with [tailor], that Python-related tailor options live in [python] when maybe they should live in [tailor] instead. cc @thejcannon

@kaos
Copy link
Member

kaos commented Mar 16, 2022

?

I'm not sure I see the issue with having the various lang env-vars options registered in the corresponding subsystems. Like for python, register env-vars there, no need for it to be registered by another via some plugin hook. Feels like I'm missing something, or we're talking past each other..?

@Eric-Arellano
Copy link
Contributor Author

Eric-Arellano commented Mar 16, 2022

A generic [env-vars] subsystem in core/subsystems has no idea that Python or Go backends exists. We could leak it by defining Python-related options in core/subsystems/, but that's bad coupling. It's not friendly to plugin authors who can't change core/subsystems. It also means [env-vars] will have options for languages you aren't even using and don't care about.

With the Target API, this is why we have "plugin fields". We only add python_resolve to the protobuf_source target if you're using Python. No such equivalent exists with the options system.

@kaos
Copy link
Member

kaos commented Mar 17, 2022

What I was thinking wasn't a global [env-vars] subsystem, but to have [python].env_vars, [go].env_vars, [jvm].env_vars, etc...

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