Closed glyph closed 3 years ago
tianon
commented
3 years ago Looks like this was broken in the 7.3.3 release. :disappointed:
Trying to figure out what happened and how far this goes -- so far, both the Python 3.6 and 3.7 branches are definitely affected, but 2.7 isn't (which isn't really unexpected).
(Not seeing anything obviously related already reported to https://foss.heptapod.net/pypy/pypy/-/issues.)
tianon
commented
3 years ago Confirmed that pypy:3-7.3.2 works fine, so this is definitely something related to 7.3.3.
Not seeing anything super obvious in the released tarballs from https://downloads.python.org/pypy/:
tianon
commented
3 years ago It seems like https://foss.heptapod.net/pypy/pypy/-/commit/36b29935f3af98f071873218fb3d15200a54bc80 might be the culprit (adding import thread to crypt where it didn't exist previously, and that commit doesn't appear to exist until 7.3.3.rc1)
glyph
commented
3 years ago Reported upstream here: https://foss.heptapod.net/pypy/pypy/-/issues/3395
mattip
commented
3 years ago This was fixed on PyPy HEAD and will be part of the next release. It can be patched in the pure-python file lib_pypy/_crypt/__init__.py (there was another bug fixed in https://foss.heptapod.net/pypy/pypy/-/issues/3378):
diff -r 40928263b3ca -r 7414ff6076ed lib_pypy/_crypt/__init__.py
--- a/lib_pypy/_crypt/__init__.py Sun Oct 11 18:23:06 2020 +0300
+++ b/lib_pypy/_crypt/__init__.py Sat Feb 06 20:53:19 2021 +0200
@@ -4,8 +4,8 @@
import sys
import cffi
-import thread
-_lock = thread.allocate_lock()
+import _thread
+_lock = _thread.allocate_lock()
try: from __pypy__ import builtinify
except ImportError: builtinify = lambda f: f
@@ -22,12 +22,20 @@
@builtinify
def crypt(word, salt):
+ # Both arguments must be str on CPython, but are interpreted as
+ # utf-8 bytes. The result is also a str. For backward
+ # compatibility with previous versions of the logic here
+ # we also accept directly bytes (and then return bytes).
with _lock:
- if isinstance(word, str):
- word = word.encode('ascii')
+ arg_is_str = isinstance(word, str)
+ if arg_is_str:
+ word = word.encode('utf-8')
if isinstance(salt, str):
- salt = salt.encode('ascii')
+ salt = salt.encode('utf-8')
res = lib.crypt(word, salt)
if not res:
return None
- return ffi.string(res)
+ res = ffi.string(res)
+ if arg_is_str:
+ res = res.decode('utf-8')
+ return res
Thanks, I'll see about applying that -- that's a combination of https://foss.heptapod.net/pypy/pypy/-/commit/16faa2be85839e6ab4fb8ee09298a4d934aab81f and https://foss.heptapod.net/pypy/pypy/-/commit/c63da169246ed972fe90e1c289fc2378236fa852 then, correct?
Separately, do you expect a new release soonish? (We don't do much "packaging" beyond downloading the releases from https://downloads.python.org/pypy/ which is why this affects us and likely affects others outside Docker -- just recompiling some of the shared libraries where necessary. :sweat_smile:)
tianon
commented
3 years ago Also, it seems clear to me that https://foss.heptapod.net/pypy/pypy/-/commit/16faa2be85839e6ab4fb8ee09298a4d934aab81f applies to 3.6 and 3.7 (but not 2.7), but https://foss.heptapod.net/pypy/pypy/-/commit/c63da169246ed972fe90e1c289fc2378236fa852 seems like it might apply on 2.7 as well; is that accurate?
mattip
commented
3 years ago We will probably do a 3.7/2.7 release in March. There is a slight chance it may include a first alpha of 3.8, but I wouldn't rush to put out a docker image that includes it.
Someone should confirm that the issue occurs on 2.7, and file an issue if needed. I am not sure what the correct typing/encoding of the arguments to crypt.crypt() is on 2.7.
We are not planning another 3.6 release, do you see many people using the 3.6 docker image? CPython 3.6 is in security-fix-only mode, and it is difficult for us to maintain more than one python3 version.
tianon
commented
3 years ago We will probably do a 3.7/2.7 release in March. There is a slight chance it may include a first alpha of 3.8, but I wouldn't rush to put out a docker image that includes it.
For things like this we try to include a (non-default) variant for folks to try it out as soon as we can (the intent being that for a lot of users, docker run pypy:3.8 is going to be easier than the alternatives to try it out and hopefully send feedback / help find bugs :sweat_smile:).
Someone should confirm that the issue occurs on 2.7, and file an issue if needed. I am not sure what the correct typing/encoding of the arguments to
crypt.crypt()is on 2.7.
Ok, fair enough; thanks for the insight - I'll see about getting it applied only on 3.x. :+1:
We are not planning another 3.6 release, do you see many people using the 3.6 docker image? CPython 3.6 is in security-fix-only mode, and it is difficult for us to maintain more than one python3 version.
Unfortunately, we don't get much in the way of per-tag stats, but the way the images are tagged all the 3 (and other similarly non-specific) aliases are currently pointing at 3.6 because it was our understanding (from https://morepypy.blogspot.com/2020/11/pypy-733-triple-release-python-37-36.html) that 3.7 is still "beta" so we didn't think it should be the default yet.
Totally understandable on the support levels; I can only imagine the level of effort for supporting any of this often borders on herculean. :grimacing: :heart:
mattip
commented
3 years ago hmm. As long as we are calling it "beta" that makes sense. I will definitely drop the beta tag on the next release. Thanks for packaging this.
This breaks other parts of the standard library.
It broke at some point in the last week.