add libffi-7.dll to pypy on windows#2141
Conversation
Codecov Report
@@ Coverage Diff @@
## main #2141 +/- ##
=======================================
Coverage 93.72% 93.72%
=======================================
Files 88 88
Lines 4382 4382
=======================================
Hits 4107 4107
Misses 275 275
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
| @classmethod | ||
| def _shared_libs(cls): | ||
| return ["libpypy-c.dll"] | ||
| return ["libpypy-c.dll", "libffi-7.dll"] |
There was a problem hiding this comment.
Wouldn't this need to be version limited to newer pypy versions?
There was a problem hiding this comment.
no, the code to copy ignores missing files
There was a problem hiding this comment.
There was a problem hiding this comment.
Will it work without this DLL? Should we make it hard dependency rather than soft that it is now?
There was a problem hiding this comment.
Currently this DLL is soft depdency (so assuming that's not a bug), if in the future this is not the case we should make it hard dependency and version guard the hard dependency part.
There was a problem hiding this comment.
It would be nice to avoid this code altogether in favor of a more robust solution
There was a problem hiding this comment.
That wasn't my question. Can we say a usecase where pypy would work without this DLL with the current less robust solution? If yes, we can keep it a soft dependency. If not, it must be hard depedency.
There was a problem hiding this comment.
I am not sure what you mean, sorry. PyPy v7.3.5 will not have the libffi-7.dll and will work without it. Once the branch to add the dll gets merged, any future PyPy versions will require the dll. I am hopeful that branch will be merged before the PyPy v7.3.6 release, but it is dependent on a new release of virtualenv with this PR or one similar.
There was a problem hiding this comment.
On the other hand, I can envision a scenario where a packager like conda uses some OS-level utilities to avoid copying the dll. In this case even though the dlls are required, they are not copied by this code into the new virtualenv.
|
Is the pypy2 windows test run known to be flaky or did I mess something up? |
| @classmethod | ||
| def _shared_libs(cls): | ||
| return ["libpypy-c.dll"] | ||
| return ["libpypy-c.dll", "libffi-7.dll"] |
There was a problem hiding this comment.
Will it work without this DLL? Should we make it hard dependency rather than soft that it is now?
Signed-off-by: Bernát Gábor <bgabor8@bloomberg.net>
|
Thanks |
|
Wow, that was fast, thanks! |
Thanks for contributing, make sure you address all the checklists (for details on how see
development documentation)!
tox -e fix_lint)docs/changelogfolderThis is a continuation of PyPy issue 1922 where virtualenv has the shared objects needed to run PyPy hardcoded, and PyPy has not yet adopted CPython's launchers on windows. I am not sure how best to solve this. I was hoping a change in PyPy's venv module would have helped, but it seems not.
The immediate problem is that PyPy wishes to add
libffi-7.dllto the windows build for the next release (in 6 weeks or so?). I have tested this fix locally for the yet-as-unmerged-to-master change in PyPy's sources.