Fix setting of PEX_PATH in ./pants run (v2 backend) #4969
Conversation
… to pants runner pex info; bump pex version.
| @@ -130,6 +119,9 @@ def create_pex(self, pex_info=None): | |||
|
|
|||
| extra_pex_paths = [pex.path() for pex in pexes if pex] | |||
|
|
|||
| if extra_pex_paths: | |||
| pex_info.pex_path = ':'.join(extra_pex_paths) | |||
There was a problem hiding this comment.
how about using the new PexInfo.merge_pex_path() method here instead of the if + set?
|
|
||
| def run(self, *args, **kwargs): | ||
| kwargs_copy = copy(kwargs) | ||
| env = copy(kwargs_copy.get('env')) if 'env' in kwargs_copy else {} | ||
| env[self._PEX_PATH_ENV_VAR_NAME] = self._pex_path() | ||
| kwargs_copy['env'] = env | ||
| return self._pex.run(*args, **kwargs_copy) |
There was a problem hiding this comment.
the only reason I can see to do all of the kwargs + env copying here would be to mutate that - so I'm thinking that's probably no longer necessary?
i.e. return self._pex.run(*args, **kwargs) is probably a sufficient method body.
| return self._pex.run(*args, **kwargs_copy) | ||
|
|
||
| def _pex_path(self): | ||
| return ':'.join(self._extra_pex_paths) |
There was a problem hiding this comment.
seems like this was the last usage of self._extra_pex_paths in this class, so that should probably get yanked from __init__'s kwargs, docstring and body.
| @@ -130,6 +116,9 @@ def create_pex(self, pex_info=None): | |||
|
|
|||
| extra_pex_paths = [pex.path() for pex in pexes if pex] | |||
|
|
|||
| if extra_pex_paths: | |||
| pex_info.merge_pex_path(':'.join(extra_pex_paths)) | |||
|
|
|||
| with safe_concurrent_creation(path) as safe_path: | |||
| builder = PEXBuilder(safe_path, interpreter, pex_info=pex_info) | |||
| builder.freeze() | |||
There was a problem hiding this comment.
also seems like you no longer need the extra_pex_paths_file_path handling down below (lines 126-133) both inside and outside the invalidated block, since WrappedPex no longer does anything with that and it should all be written directly into the pex/chroot's PEX-INFO.
| @@ -130,16 +114,11 @@ def create_pex(self, pex_info=None): | |||
|
|
|||
There was a problem hiding this comment.
can kill this unused line above (line 95):
extra_pex_paths_file_path = path + '.extra_pex_paths'
|
looks like you're gonna need to fixup the |
| return '{env_var_name}={pex_path} {cmdline}'.format(env_var_name=self._PEX_PATH_ENV_VAR_NAME, | ||
| pex_path=pex_path, | ||
| cmdline=cmdline) | ||
| return '{env_var_name}={pex_path} {cmdline}'.format(env_var_name=self._PEX_PATH_ENV_VAR_NAME, |
| @@ -53,7 +53,7 @@ def cmdline(self, args=()): | |||
| cmdline = ' '.join(self._pex.cmdline(args)) | |||
| pex_path = self._pex_path() | |||
| if pex_path: | |||
| return '{env_var_name}={pex_path} {cmdline}'.format(env_var_name=self._PEX_PATH_ENV_VAR_NAME, | |||
| return '{env_var_name}={pex_path} {cmdline}'.format(env_var_name=self._PEX_PATH_ENV_VAR_NAME, | |||
| pex_path=pex_path, | |||
| cmdline=cmdline) | |||
There was a problem hiding this comment.
these hanging lines should align to the parent too.
…gston/pants into clivingston/pex-path-to-pex-info
|
merging on green CI mod the jdk8 failure. |
Problem
App servers that need to re-execute (apps watching for file systems changes, jupyter notebooks, etc.) throw an ImportError when being ran with
./pants runbecause the pants runner pex is losing context of where to find the requirements pex, the sources pex, and any other pexes to merge into the runtime environment. This context is encapsulated in a search path calledPEX_PATH, which is being loaded into thePEX_PATHenvironment variable and is consequently scrubbed out of the environment upon re-exec by pex internal logic.Solution
Completely remove any setting of
PEX_PATHin the environment upon construction of the pants runner pex and instead pass this extra pex path information to thepex_infometadata that is used by thePEXBuilderobject to construct the pants runner pex.Result
Applications that need to re-execute will now be able to locate the sources module and entry point because the search path to resolve them (
PEX_PATH) will be persisted in the pants runner pex's PEX-INFO metadata.