Skip directories when symlinking libraries for PyPy3 #2204
Conversation
The PyPy3 logic creates symlinks for all files from the library directory existing alongside the PyPy executable. This is meant to ensure that the bundled libraries to which PyPy is linked can also be found from inside the virtualenv. However, this logic also symlinks all directories which is unnecessary and causes library directory collisions with the new install layout. Change to logic to symlink non-directories only. A similar fix has been applied to the internal venv module in PyPy3.8: https://foss.heptapod.net/pypy/pypy/-/commit/713b2af9abd2b9453e12c60143e17431a1aefb33 Fixes pypa#2182
Codecov Report
@@ Coverage Diff @@
## main #2204 +/- ##
==========================================
- Coverage 93.38% 88.87% -4.52%
==========================================
Files 88 88
Lines 4402 4404 +2
==========================================
- Hits 4111 3914 -197
- Misses 291 490 +199
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
| @@ -44,6 +44,8 @@ def sources(cls, interpreter): | |||
| host_lib = Path(interpreter.system_prefix) / "lib" | |||
| if host_lib.exists() and host_lib.is_dir(): | |||
| for path in host_lib.iterdir(): | |||
| if path.is_dir(): | |||
| continue | |||
There was a problem hiding this comment.
Once upon a time we needed to copy any shared object files in base/lib but these days we use RPATH in the libpypy3-c.so to find the lib dir, and that main shared object is now symlinked.
So in short, iterating over base/lib and copying the files is no longer needed, so lines 45-48 can be deleted. That means this whole method can be deleted.
There was a problem hiding this comment.
... as long as os.symlink is available where the "portable PyPy" downloads (the ones from https://www.pypy.org/download.html) are used. Linux and brew build PyPy against system libraries, so they do not need the shared objects in the base/lib directory .
There was a problem hiding this comment.
@mattip for 3.6 we still need this, not? What about Windows?
There was a problem hiding this comment.
No, 3.6 also uses symlinks and RPATH. Windows never needed this.
@mgorny could you change line 30, 31 to use pypy instead of python ?
|
I validated this (both the minimal change first suggested and my more extensive suggested change) against the upcoming PyPy3.8 rc1 and against PyPy 3.7. I don't think the failures are connected to this change. |
|
I tried to emit a PR to update this from mattip/virtualenv but for some reason could not make a PR to mgorny/virtualenv. While this version is fine, I think removing the whole |
I'll just close my PR and please submit yours instead ;-). |
The PyPy3 logic creates symlinks for all files from the library
directory existing alongside the PyPy executable. This is meant
to ensure that the bundled libraries to which PyPy is linked can also
be found from inside the virtualenv. However, this logic also symlinks
all directories which is unnecessary and causes library directory
collisions with the new install layout. Change to logic to symlink
non-directories only.
A similar fix has been applied to the internal venv module in PyPy3.8:
https://foss.heptapod.net/pypy/pypy/-/commit/713b2af9abd2b9453e12c60143e17431a1aefb33
Fixes #2182
tox -e fix_lint)docs/changelogfolder