pantsbuild / pants

The Pants Build System
https://www.pantsbuild.org
Apache License 2.0
3.27k stars 627 forks source link

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

Open Eric-Arellano opened 2 years ago

Eric-Arellano commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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

stuhood commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

?

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 commented 2 years ago

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 commented 2 years ago

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