Refactors PythonBootstrap to pre-calculate expanded interpreter paths
#17030
Conversation
# Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]
# Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]
# Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]
# Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]
# Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]
# Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]
chrisjrn
requested review from
stuhood and
Eric-Arellano
and removed request for
stuhood
chrisjrn
added
the
category:internal
| @@ -29,9 +28,12 @@ class PexSubsystem(Subsystem): | |||
| options_scope = "pex" | |||
| help = "How Pants uses Pex to run Python subprocesses." | |||
|
|
|||
| class EnvironmentAware: | |||
| class EnvironmentAware(Subsystem.EnvironmentAware): | |||
There was a problem hiding this comment.
Is the meta-class supposed to make explicitly subclassing like this unnecessary?
There was a problem hiding this comment.
You need to subclass Subsystem.EnvironmentAware if you want to be able to type check against properties of Subsystem.EnvironmentAware i.e. you have properties that depend on environment variables.
In cases where you aren't post-processing based on environment variables, you do not need to subclass.
There was a problem hiding this comment.
(This is a case of nuance that we'll need to document, but I think the net benefit of not needing to pass in an EnvironmentVars to expand PATHs and the like is a significant win for clarity)
There was a problem hiding this comment.
Kk. Possibly relevant to look at how @thejcannon handled inner classes for lint/fmt... essentially, separate class definitions for TYPE_CHECKING vs not.
Previously,
PythonBootstraphad a method that would calculate the expanded interpreter paths when they are first consumed. This change moves the calculation of those expanded interpreter paths to the initial creation ofPythonBootstrap.Relatedly, a bunch of static methods defined on
PythonBootstrapare now private functions at module scope. This is prework that will significantly improve the clarity of my next piece of work onPythonBootstrap, namely converting all of the various filesystem-dependent functions to@rules.None of the helper methods have been changed
Addresses #16800.