-
Notifications
You must be signed in to change notification settings - Fork 219
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add pypy3.7 7.3.5 #1103
Add pypy3.7 7.3.5 #1103
Conversation
@mattip, could you review this PR please ? |
LINK_VERSION=$(pip${PYVERS} -V) | ||
REAL_VERSION=$(${PYTHON} -m pip -V) | ||
test "${LINK_VERSION%% from *}" = "${REAL_VERSION%% from *}" | ||
fi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't these sanity checks be run on PyPy too? If they fail there is a problem with the PyPy build ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can't run the sanity check of the pipx.y
symlink in /usr/local/bin
since there are no symlink for PyPy pip in there. As mentioned in an earlier comment, going to remove those anyway.
For sqlite3, this is specific to cpython being rebuilt from source with a specific version (well should be specific, this has been an issue before). Given PyPY comes as a full featured binary, this check isn't right (& fails because the embedded sqlite3 is version 3.28 or 3.29 - I don't remember exactly)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the embedded sqlite3 is version 3.28 or 3.29
Hmm. Is it documented somewhere what sqlite3 a specific python version expects? If cpython is statically linking to 3.35 that would suggest PyPy should as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The minimum sqlite3 version required by cpython is fairly low: https://github.com/python/cpython/blob/220dd80a2671f57486055955d5422163cf73ed33/Modules/_sqlite/module.c#L33
The default version in manylinux1 was even older than this requirement though:
It's been updated since then in order not to get issue reported because of CVE on older versions.
Windows/macOS installers of CPython also update sqlite3 regularly as noted in their release notes.
Thanks. I plan on releasing PyPy 7.3.5 in the next two days, so maybe merge only after that is released and can be used here? |
Seems like a good plan. |
PyPy 3.7.5 has been released. The checksums are available at https://www.pypy.org/checksums.html |
Is there a way to test that this is working? |
The tests in the manylinux repo are really minimal & PR images aren't uploaded anywhere. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the delay, @mayeut. I don't know the manylinux images that well, but yeah, this PR seems to make sense to me :-)
Fix #1099