Refactor Pex working directory handling. #12479
Conversation
Encapsulate working directory handling in PexEnvironment and CompletePexEnvironment and for passing a value for the working directory. Also refactor the venv pex script to fully handle working directory issues on its own. # 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]
|
I'm not sure this is any better, but see what you think. |
| append_only_caches=FrozenDict({cls._PEX_ROOT_DIRNAME: str(pex_root)}), | ||
| ) | ||
|
|
||
|
|
||
| class WorkspacePexEnvironment(CompletePexEnvironment): |
There was a problem hiding this comment.
Do we need this class at all? Couldn't in_workspace() return a CompletePexEnvironment? If not for some reason I can't see, then possibly we want a similarly trivial SandboxPexEnvironment, for symmetry?
There was a problem hiding this comment.
We do if you hold being explicit about working directory as a goal, which I was. That said, I could churn a bit more and just eliminate the presence of any rule to produce any CompletePexEnvironment.
There was a problem hiding this comment.
Thanks! I would have also been fine with keeping WorkspacePexEnvironment but also having SandboxPexEnvironment. It just seemed odd to have one and not the other.
There was a problem hiding this comment.
Yeah - that wouldn't really work since they used to be symmetric in requiring no input parameters (well both required just a PexEnvironment) but now are not. You must provide an additional input parameter to get a SandboxPexEnvironment now - the working directory - and so it cannot be a rule parameter like WorkspacePexEnvironment could be and still can be; so this was really the only route.
Instead have all injections be of PexEnvironment and force all usages to call the appropriate in_... method. # 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]
This was missed in pantsbuild#12479. # 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]
Encapsulate working directory handling in PexEnvironment and
CompletePexEnvironment and force passing a value for the working
directory. Also refactor the venv pex script to fully handle
working directory issues on its own.
[ci skip-rust]
[ci skip-build-wheels]