Skip to content

Conversation

@cognifloyd
Copy link
Member

@cognifloyd cognifloyd commented Apr 15, 2024

In our standard st2 virtualenv there are a variety of .pth files that add modules to the sys.path when python starts up (via the site module: https://docs.python.org/3/library/site.html):

$ ls -1 virtualenv/lib/python3.8/site-packages/*.pth
virtualenv/lib/python3.8/site-packages/distutils-precedence.pth
virtualenv/lib/python3.8/site-packages/easy-install.pth
virtualenv/lib/python3.8/site-packages/oslo.config-1.12.1-py2.7-nspkg.pth
virtualenv/lib/python3.8/site-packages/repoze.lru-0.7-py3.6-nspkg.pth
virtualenv/lib/python3.8/site-packages/sphinxcontrib_devhelp-1.0.2-py3.8-nspkg.pth
virtualenv/lib/python3.8/site-packages/sphinxcontrib_jsmath-1.0.1-py3.7-nspkg.pth
virtualenv/lib/python3.8/site-packages/sphinxcontrib_qthelp-1.0.3-py3.8-nspkg.pth
virtualenv/lib/python3.8/site-packages/sphinxcontrib_serializinghtml-1.1.5-py3.9-nspkg.pth
virtualenv/lib/python3.8/site-packages/sphinxcontrib_websupport-1.2.4-py3.8-nspkg.pth

Today, however, none of those .pth files affect pack venvs because the parent st2 venv is not in the standard list of site-packages locations.

We probably haven't encountered issues with the lack of .pth file loading in pack venvs because oslo.config and repoze.lru are the only things that could have an impact, and the impact would be minimal at best.

This becomes more pronounced when using a virtualenv generated by pants+pex (instead of via our Makefile) because all of our first party sources get installed with PEP 660 wheels. The legacy calls to python setup.py develop trigger creating a symlink to st2common, st2auth, st2* and the runner directories under the virtualenv's site-packages directory. PEP 660 wheels use .pth files instead of symlinks to achieve the same thing.

So, we have to ensure .pth files get loaded whenever interacting with a pack venv. This PR does that by injecting a .pth file into the pack venv that uses the site module to load any .pth files from the parent st2 venv.

Also, I adjusted the GHA CI workflow to work when our sources are added to the venv via PEP 660 editable wheels. Apparently, PEP 660 only installs entry point scripts, but most of our st2*/bin/* scripts are just scripts not entry_points. https://peps.python.org/pep-0660/#limitations

Finally, I adjusted how st2-run-pack-tests so that it can handle hermetic scripts. A hermetic script has a python shebang with the -sE args that do not load user site-packages (-s) and ignore all PYTHON* vars (-E) like PYTHONPATH. Once we update to a version of pants that includes pantsbuild/pants#20763, we can just disable how pants+pex changes the virtualenv shebang lines. Until then, we need to run nosetests in a way that lets PYTHONPATH continue to affect it.

I extracted this from #6130 where I was testing what changes are required to use a pants-built venv to run tests.

importing site has side effects which were messing up sys.path

I wasn't sure what else to import, so I imported sys. An import is required
to trigger the exec().
avoid duplicate entries and hopefully avoid
adding system paths before the st2 paths.
The system lib paths are already in the sys.path before site gets imported.
Then, all .pth files append after that.
So, this is expected.
By default, pants+pex adds `-sE` to the virtualenv/bin scripts which prevents
PYTHONPATH and other PYTHON* vars from affecting the program.

st2-run-pack-tests uses PYTHONPATH, however, so bypass the script if it is hermetic.
If the virtualenv is built by pants, the PEP 660 wheels will not include
any of the scripts (like st2-run-pack-tests). Only entry_point scripts
will be installed in the virtualenv. So, do not assume that a dev venv
will have st2-run-pack-tests. Also, adjust path assumptions to work with
either kind of venv.
@cognifloyd cognifloyd added this to the pants milestone Apr 15, 2024
@cognifloyd cognifloyd self-assigned this Apr 15, 2024
@pull-request-size pull-request-size bot added the size/L PR that changes 100-499 lines. Requires some effort to review. label Apr 15, 2024
@cognifloyd cognifloyd requested a review from a team April 18, 2024 22:30
@cognifloyd cognifloyd requested a review from a team April 19, 2024 08:12
@cognifloyd cognifloyd enabled auto-merge April 19, 2024 16:45
@cognifloyd cognifloyd merged commit a56f527 into master Apr 23, 2024
@cognifloyd cognifloyd deleted the venv-pth-files branch April 23, 2024 15:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature packs pantsbuild python3 size/L PR that changes 100-499 lines. Requires some effort to review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants