pantsbuild / pants

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

Improve safety of using `environments` during `BUILD` evaluation time #17354

Open stuhood opened 1 year ago

stuhood commented 1 year ago

Currently, build graph calculations and BUILD file evaluation are pinned to the __local__ environment (see #17129), meaning that it will not run in a docker, or remote environment. This is accomplished by "masking" the environment at graph and BUILD file evaluation boundaries, and then explicit injecting the ChosenLocalEnvironmentName as the current EnvironmentName.

While that process is enforced (by the masked_type annotations) and fairly safe, it is not type safe for @rule authors who might think that they can use the Platform or EnvironmentVars during BUILD file evaluation time or dependency inference. Because they can access those types in those locations, but they will not get the appropriate values to use at runtime: they will instead get the values for the __local__ environment.

If you know that, then you can manually avoid consuming environment-specific types in those @rules, and defer the work to, say, codegen (as @thejcannon demonstrates here), but making this type safe instead would definitely be preferable.


One way to help guide users would be to overload all of the environment-aware intrinsics (Process running, Platform calculation, EnvironmentVars calculation, etc) to consume either:

The latter type would be available to @rules which were part of BUILD file evaluation and dependency calculation, while the former type would be available to codegen and etc. This would bifurcate @rules by type, and maybe help to guide @rule authors to avoid consuming the "wrong" Platform. But because dependency inference still needs to be able to run processes, it would not represent true type safety.

stuhood commented 1 year ago

The latter type would be available to @rules which were part of BUILD file evaluation and dependency calculation, while the former type would be available to codegen and etc. This would bifurcate @rules by putting constraints on what could be done in each subgraph, but it would prevent accidentally consuming the "wrong" Platform.

Mm... unfortunately, this would still only act as a guideline/hint not to produce platform-specific outputs in these subgraphs, rather than providing actual type safety. Dependency calculations need to be able to run processes (and locate interpreters, etc), so they would need to be able to consume all of the platform-specific types. But they would need to guarantee that they produced platform-independent/agnostic outputs, because otherwise the computed dependencies could be unintentionally different on different __local__ platforms.