Subject: Re: bug#44656: [PATCH] Upgrade pypy3
From: Maxim Cournoyer <[email-masked]>
To: Lars-Dominik Braun <[email-masked]>
Cc: [email-masked]
Date: Thu, 05 Aug 2021 12:54:47 -0400
List-Id: <debbugs-submit.debbugs.gnu.org>

Hello again,

Maxim Cournoyer <[email-masked]> writes:

> Hi Lars,
>
> Lars-Dominik Braun <[email-masked]> writes:
>
>> Hi,
>>
>> the first patch in this series upgrades pypy3 to 7.3.2 and the second
>> adjusts several aspects of the package.
>>
>> Lars
>>
>>
>>>>From caf5e4bf9acd10f91de2e6a9c60185475144a1b9 Mon Sep 17 00:00:00 2001
>> From: Lars-Dominik Braun <[email-masked]>
>> Date: Sun, 15 Nov 2020 10:54:26 +0100
>> Subject: [PATCH 2/2] gnu: pypy3: Various package fixes.
>>
>> * gnu/packages/python.scm (pypy3) [patches]: Add new patch.
>> [inputs]: Remove bash-minimal and add nss-certs.
>> [native-inputs]: Remove nss-certs here.
>
> In Guix we leave the choice of the TLS certs to the users; meaning we
> don't hard-code their location in packages and instead have the users
> explicitly install them (and in the case of OpenSSL, set the necessary
> environment variables), the same we do for icons and other 'choosable'
> things; while convenient the above goes against this tradition.
>
>> [arguments]: Use gdbm compat library, add 2to3 binary.
>> [native-search-paths]: Add search path.
>> * gnu/packages/patches/pypy3-7.3.1-ssl-paths.patch: New file.
>> * gnu/local.mk: Add it.
>> ---
>>  gnu/local.mk                                  |   1 +
>>  .../patches/pypy3-7.3.1-ssl-paths.patch       |  41 +++++++
>>  gnu/packages/python.scm                       | 109 ++++++++++++------
>>  3 files changed, 116 insertions(+), 35 deletions(-)
>>  create mode 100644 gnu/packages/patches/pypy3-7.3.1-ssl-paths.patch
>>
>> diff --git a/gnu/local.mk b/gnu/local.mk
>> index 91a3295e75..b644391a11 100644
>> --- a/gnu/local.mk
>> +++ b/gnu/local.mk
>> @@ -1536,6 +1536,7 @@ dist_patch_DATA =						\
>>    %D%/packages/patches/python-unittest2-remove-argparse.patch	\
>>    %D%/packages/patches/python-waitress-fix-tests.patch		\
>>    %D%/packages/patches/pypy3-7.3.1-fix-tests.patch		\
>> +  %D%/packages/patches/pypy3-7.3.1-ssl-paths.patch		\
>>    %D%/packages/patches/qemu-build-info-manual.patch		\
>>    %D%/packages/patches/qemu-glibc-2.27.patch 			\
>>    %D%/packages/patches/qrcodegen-cpp-make-install.patch		\
>> diff --git a/gnu/packages/patches/pypy3-7.3.1-ssl-paths.patch b/gnu/packages/patches/pypy3-7.3.1-ssl-paths.patch
>> new file mode 100644
>> index 0000000000..d21133b4ae
>> --- /dev/null
>> +++ b/gnu/packages/patches/pypy3-7.3.1-ssl-paths.patch
>> @@ -0,0 +1,41 @@
>> +Fix default certificate search path, still allowing the user to override it
>> +with environment variables.
>> +
>> +--- a/lib_pypy/_cffi_ssl/_stdssl/__init__.py
>> ++++ b/lib_pypy/_cffi_ssl/_stdssl/__init__.py
>> +@@ -1679,20 +1679,9 @@ def get_default_verify_paths():
>> +      https://golang.org/src/crypto/x509/root_linux.go (for the files)
>> +     '''
>> +     certFiles = [
>> +-        "/etc/ssl/certs/ca-certificates.crt",                # Debian/Ubuntu/Gentoo etc.
>> +-        "/etc/pki/tls/certs/ca-bundle.crt",                  # Fedora/RHEL 6
>> +-        "/etc/ssl/ca-bundle.pem",                            # OpenSUSE
>> +-        "/etc/pki/tls/cacert.pem",                           # OpenELEC
>> +-        "/etc/pki/ca-trust/extracted/pem/tls-ca-bundle.pem", # CentOS/RHEL 7
>> +-        "/etc/ssl/cert.pem",                                 # Alpine Linux
>> +     ]
>> +     certDirectories = [
>> +-        "/etc/ssl/certs",               # SLES10/SLES11
>> +-        "/system/etc/security/cacerts", # Android
>> +-        "/usr/local/share/certs",       # FreeBSD
>> +-        "/etc/pki/tls/certs",           # Fedora/RHEL
>> +-        "/etc/openssl/certs",           # NetBSD
>> +-        "/var/ssl/certs",               # AIX
>> ++        "@GUIX_CERT_PATH@",
>> +     ]
>> +
>> +     # optimization: reuse the values from a local varaible
>> +@@ -1707,9 +1696,10 @@ def get_default_verify_paths():
>> +     ofile = _cstr_decode_fs(lib.X509_get_default_cert_file())
>> +     odir = _cstr_decode_fs(lib.X509_get_default_cert_dir())
>> +
>> +-    if os.path.exists(ofile) and os.path.exists(odir):
>> +-        get_default_verify_paths.retval = (ofile_env, ofile, odir_env, odir)
>> +-        return get_default_verify_paths.retval
>> ++    if not os.path.exists(ofile):
>> ++        ofile = None
>> ++    if not os.path.exists(odir):
>> ++        odir = None
>> +
>> +     # OpenSSL didn't supply the goods. Try some other options
>> +     for f in certFiles:
>
> As mentioned above, the choice of certs should not be hard coded the in
> the package definition.  The correct behavior of honoring SSL_CERT_DIR
> and SSL_CERT_FILE environment variables, and falling back to the system
> provided location is sane, so the above patch is unwelcome, IMHO.
>
>> diff --git a/gnu/packages/python.scm b/gnu/packages/python.scm
>> index 8ef8ae2e1d..c0bd3335e3 100644
>> --- a/gnu/packages/python.scm
>> +++ b/gnu/packages/python.scm
>> @@ -696,7 +696,8 @@ ease from the desktop to a microcontroller or embedded system.")
>>                (sha256
>>                 (base32
>>                  "03f1fdw6yk2mypa9pbmgk26r8y1hhmw801l6g36zry9zsvz7aqgx"))
>> -              (patches (search-patches "pypy3-7.3.1-fix-tests.patch"))))
>> +              (patches (search-patches "pypy3-7.3.1-fix-tests.patch"
>> +                                       "pypy3-7.3.1-ssl-paths.patch"))))
>>      (build-system gnu-build-system)
>>      (native-inputs
>>       `(("python-2" ,python-2)
>> @@ -704,13 +705,13 @@ ease from the desktop to a microcontroller or embedded system.")
>>         ("tar" ,tar)                     ; Required for package.py
>>         ("python2-pycparser" ,python2-pycparser)
>>         ("python2-hypothesis" ,python2-hypothesis)
>> -       ("nss-certs" ,nss-certs)         ; For ssl tests
>>         ("gzip" ,gzip)))
>>      (inputs
>>       `(("libffi" ,libffi)
>>         ("zlib" ,zlib)
>>         ("ncurses" ,ncurses)
>>         ("openssl" ,openssl)
>> +       ("nss-certs" ,nss-certs)         ; For ssl module
>
> This change is not needed without the TLS patching above.
>
>>         ("expat" ,expat)
>>         ("bzip2" ,bzip2)
>>         ("sqlite" ,sqlite)
>> @@ -718,10 +719,9 @@ ease from the desktop to a microcontroller or embedded system.")
>>         ("tcl" ,tcl)
>>         ("tk" ,tk)
>>         ("glibc" ,glibc)
>> -       ("bash-minimal" ,bash-minimal)   ; Used as /bin/sh
>>         ("xz" ,xz)))                     ; liblzma
>>      (arguments
>> -     `(#:tests? #f     ;FIXME: Disabled for now, there are many tests failing.
>> +     `(#:tests? #f ;FIXME: Disabled for now, there are many tests failing.
>>         #:modules ((ice-9 ftw) (ice-9 match)
>>                    (guix build utils) (guix build gnu-build-system))
>>         #:phases (modify-phases %standard-phases
>> @@ -750,6 +750,11 @@ ease from the desktop to a microcontroller or embedded system.")
>>                        (substitute* '("lib_pypy/_curses_build.py")
>>                          ;; Find curses
>>                          (("/usr/local") (assoc-ref inputs "ncurses")))
>> +                      (substitute* '("lib_pypy/_dbm.py")
>> +                        ;; Use gdbm compat library, so we don’t need to pull in bdb
>> +                        (("ctypes.util.find_library\\('db'\\)")
>> +                         (string-append "'" (assoc-ref inputs "gdbm")
>> +                                        "/lib/libgdbm_compat.so'")))
>
> OK.
>
>>                        (substitute* '("lib_pypy/_sqlite3_build.py")
>>                          ;; Always use search paths
>>                          (("sys\\.platform\\.startswith\\('freebsd'\\)") "True")
>> @@ -761,11 +766,18 @@ ease from the desktop to a microcontroller or embedded system.")
>>                                          "/lib/libsqlite3.so.0'")))
>>                        (substitute* '("lib-python/3/subprocess.py")
>>                          ;; Fix shell path
>> -                        (("/bin/sh")
>> -                         (string-append (assoc-ref inputs "bash-minimal") "/bin/sh")))
>> +                        (("/bin/sh") (which "sh")))
>
> OK.
>
>>                        (substitute* '("lib-python/3/distutils/unixccompiler.py")
>>                          ;; gcc-toolchain does not provide symlink cc -> gcc
>>                          (("\"cc\"") "\"gcc\""))
>> +                      (substitute* '("lib_pypy/_cffi_ssl/_stdssl/__init__.py")
>> +                        ;; Add nss-certs to default certificate search path,
>> +                        ;; otherwise every packages has to specify nss-certs and
>> +                        ;; openssl as input to set the proper env variables.
>> +                        ;; Depends on -ssl-paths.patch.
>> +                        (("@GUIX_CERT_PATH@")
>> +                         (string-append (assoc-ref inputs "nss-certs")
>> +                                        "/etc/ssl/certs")))
>
> Not every package; the required SSL environment variables it should be
> set in the environment by the user or by via a native search path, and
> the certs manually provided (installed) by the user.
>
>>                        #t))
>>                    (add-after
>>                        'unpack 'set-source-file-times-to-1980
>> @@ -785,7 +797,8 @@ ease from the desktop to a microcontroller or embedded system.")
>>                                  (string-append "--make-jobs="
>>                                                 (number->string (parallel-job-count)))
>>                                  "-Ojit"
>> -                                "targetpypystandalone"))
>> +                                "targetpypystandalone"
>> +                                "--allworkingmodules"))
>>                        ;; Build c modules and package everything, so tests work.
>>                        (with-directory-excursion "pypy/tool/release"
>>                          (unsetenv "PYTHONPATH") ; Do not use the system’s python libs:
>> @@ -793,7 +806,12 @@ ease from the desktop to a microcontroller or embedded system.")
>>                                          ; attribute 'IntFlag'
>>                          (invoke "python2" "package.py"
>>                                  "--archive-name" "pypy-dist"
>> -                                "--builddir" (getcwd)))))
>> +                                "--builddir" (getcwd))
>> +                        ;; install pip and setuptools into the dist directory.
>> +                        ;; XXX: Breaks virtualenv, because it does not set +w
>> +                        ;; on files copied from the store.
>> +                        ;(invoke "pypy-dist/bin/pypy3" "-m" "ensurepip")
>> +                        )))
>
> Since the above attempt to bundle pip failed and is new, I'd just leave
> it out.
>
>>                    (replace 'check
>>                      (lambda* (#:key tests? #:allow-other-keys)
>>                        (if tests?
>> @@ -811,32 +829,54 @@ ease from the desktop to a microcontroller or embedded system.")
>>                        #t))
>>                    (replace 'install
>>                      (lambda* (#:key inputs outputs #:allow-other-keys)
>> -                      (with-directory-excursion "pypy/tool/release"
>> -                        ;; Delete test data.
>> -                        (for-each
>> -                         (lambda (x)
>> -                           (delete-file-recursively (string-append
>> -                                                     "pypy-dist/lib-python/3/" x)))
>> -                         '("tkinter/test"
>> -                           "test"
>> -                           "sqlite3/test"
>> -                           "lib2to3/tests"
>> -                           "idlelib/idle_test"
>> -                           "distutils/tests"
>> -                           "ctypes/test"
>> -                           "unittest/test"))
>> -                        ;; Patch shebang referencing python2
>> -                        (substitute* '("pypy-dist/lib-python/3/cgi.py"
>> -                                       "pypy-dist/lib-python/3/encodings/rot_13.py")
>> -                          (("#!.+/bin/python")
>> -                           (string-append "#!" (assoc-ref outputs "out") "/bin/pypy3")))
>> -                        (with-fluids ((%default-port-encoding "ISO-8859-1"))
>> -                          (substitute* '("pypy-dist/lib_pypy/_md5.py"
>> -                                         "pypy-dist/lib_pypy/_sha1.py")
>> -                            (("#!.+/bin/python")
>> -                             (string-append "#!" (assoc-ref outputs "out") "/bin/pypy3"))))
>> -                        (copy-recursively "pypy-dist" (assoc-ref outputs "out")))
>> -                      #t)))))
>> +                      (let* ((out (assoc-ref outputs "out"))
>> +                             (bin-pypy3 (string-append out "/bin/pypy3"))
>> +                             (shebang-match-python "#!.+/bin/python")
>> +                             (shebang-pypy3 (string-append "#!" bin-pypy3))
>> +                             (dist-dir "pypy/tool/release/pypy-dist"))
>> +                        (with-directory-excursion dist-dir
>> +                          ;; Delete test data.
>> +                          (for-each
>> +                           (lambda (x)
>> +                             (delete-file-recursively (string-append
>> +                                                       "lib-python/3/" x)))
>> +                           '("tkinter/test"
>> +                             "test"
>> +                             "sqlite3/test"
>> +                             "lib2to3/tests"
>> +                             "idlelib/idle_test"
>> +                             "distutils/tests"
>> +                             "ctypes/test"
>> +                             "unittest/test"))
>> +                          ;; Patch shebang referencing python2
>> +                          (substitute* '("lib-python/3/cgi.py"
>> +                                         "lib-python/3/encodings/rot_13.py")
>> +                            ((shebang-match-python) shebang-pypy3))
>> +                          (with-fluids ((%default-port-encoding "ISO-8859-1"))
>> +                                       (substitute* '("lib_pypy/_md5.py"
>> +                                                      "lib_pypy/_sha1.py")
>> +                                         ((shebang-match-python) shebang-pypy3))))
>> +                        (copy-recursively dist-dir out)
>> +                        ;; Make sure pypy3 is callable as python/python3, so we
>> +                        ;; don’t have to patch every single package.
>> +                        (symlink bin-pypy3 (string-append out "/bin/python"))
>> +                        (symlink bin-pypy3 (string-append out "/bin/python3"))
>> +                        ;; 2to3 is missing from pypy3, create it.
>> +                        (let ((2to3 (string-append out "/bin/2to3")))
>> +                          (call-with-output-file 2to3
>> +                            (lambda (port)
>> +                              (format port "#!~a~%" (string-append out "/bin/pypy3"))
>> +                              (format port "
>> +import sys
>> +from lib2to3.main import main
>> +
>> +sys.exit(main('lib2to3.fixes'))")))
>> +                          (chmod 2to3 #o755))
>> +                        #t))))))
>
> I'm unconvinced about the above symlinks; it seems preferable for users
> to be able to unambiguously run both pypy3 and python3 (cpython) in the
> same profile without conflicts.  The pypy3 package on Debian contains
> the following, for example:
>
> $ apt-file show pypy3
> pypy3: /usr/bin/pypy3
> pypy3: /usr/bin/pypy3clean
> pypy3: /usr/bin/pypy3compile
> pypy3: /usr/lib/libpypy3-c.so
> pypy3: /usr/lib/pypy3/bin/libpypy3-c.so
> pypy3: /usr/lib/pypy3/bin/pypy3-c
> pypy3: /usr/lib/pypy3/include/pypy_decl.h
> pypy3: /usr/lib/pypy3/include/pypy_macros.h
> pypy3: /usr/lib/pypy3/include/pypy_marshal_decl.h
> pypy3: /usr/lib/pypy3/include/pypy_structmember_decl.h
> pypy3: /usr/lib/pypy3/lib_pypy/_audioop_cffi.pypy36-pp73-x86_64-linux-gnu.so
> pypy3: /usr/lib/pypy3/lib_pypy/_blake2/_blake2b_cffi.pypy36-pp73-x86_64-linux-gnu.so
> pypy3: /usr/lib/pypy3/lib_pypy/_blake2/_blake2s_cffi.pypy36-pp73-x86_64-linux-gnu.so
> pypy3: /usr/lib/pypy3/lib_pypy/_curses_cffi.pypy36-pp73-x86_64-linux-gnu.so
> pypy3: /usr/lib/pypy3/lib_pypy/_decimal_cffi.pypy36-pp73-x86_64-linux-gnu.so
> pypy3: /usr/lib/pypy3/lib_pypy/_gdbm_cffi.pypy36-pp73-x86_64-linux-gnu.so
> pypy3: /usr/lib/pypy3/lib_pypy/_lzma_cffi.pypy36-pp73-x86_64-linux-gnu.so
> pypy3: /usr/lib/pypy3/lib_pypy/_pwdgrp_cffi.pypy36-pp73-x86_64-linux-gnu.so
> pypy3: /usr/lib/pypy3/lib_pypy/_pypy_openssl.pypy36-pp73-x86_64-linux-gnu.so
> pypy3: /usr/lib/pypy3/lib_pypy/_resource_cffi.pypy36-pp73-x86_64-linux-gnu.so
> pypy3: /usr/lib/pypy3/lib_pypy/_sha3/_sha3_cffi.pypy36-pp73-x86_64-linux-gnu.so
> pypy3: /usr/lib/pypy3/lib_pypy/_sqlite3_cffi.pypy36-pp73-x86_64-linux-gnu.so
> pypy3: /usr/lib/pypy3/lib_pypy/_syslog_cffi.pypy36-pp73-x86_64-linux-gnu.so
> pypy3: /usr/share/doc/pypy3/TODO.Debian
> pypy3: /usr/share/doc/pypy3/changelog.Debian.gz
> pypy3: /usr/share/doc/pypy3/copyright
> pypy3: /usr/share/lintian/overrides/pypy3
> pypy3: /usr/share/man/man1/pypy3.1.gz
>
>> +    (native-search-paths
>> +     (list (search-path-specification
>> +            (variable "PYTHONPATH")
>> +            (files '("lib/pypy3.6/site-packages")))))
>
> About this search path, how it is supposed to work?  The version is
> wrong (3.6) and the package doesn't include this directory, and we do
> not have a pypy build system or other pypy packages that would make use
> of it.  Am I missing something?
>
> Thank you!
>
> Maxim

I've upgraded pypy to version 7.3.5 and pushed the non-controversial
changes from your 'small fixes' commit above as
7e05fda6e8dd72b63009181072a282ad15c25c4f.  We can fix the search path
after the next core-updates merge, where we'll be able to uniformize how
it works with Python (GUIX_PYTHONPATH).

Thanks,

Closing.

Maxim