Bug 1774417 - Review Request: python39 - Version 3.9 of the Python interpreter
Summary: Review Request: python39 - Version 3.9 of the Python interpreter
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Neal Gompa
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2019-11-20 09:42 UTC by Miro Hrončok
Modified: 2019-11-26 20:22 UTC (History)
2 users (show)

Fixed In Version: python39-3.9.0~a1-1.fc32
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2019-11-26 20:22:57 UTC
Type: ---
ngompa13: fedora-review+


Attachments (Terms of Use)

Description Miro Hrončok 2019-11-20 09:42:36 UTC
Spec URL: https://churchyard.fedorapeople.org/SRPMS/python39.spec
SRPM URL: https://churchyard.fedorapeople.org/SRPMS/python39-3.9.0~a1-1.fc32.src.rpm

Description:
Python 3.9 package for developers.

This package exists to allow developers to test their code against a newer
version of Python. This is not a full Python stack and if you wish to run
your applications with Python 3.9, update your Fedora to a newer
version once Python 3.9 is stable.

Fedora Account System Username: churchayrd

Our patches: https://github.com/fedora-python/cpython/commits/fedora-3.9

Changes from python38: https://github.com/fedora-python/python39-spec/commits/master

Comment 1 Miro Hrončok 2019-11-20 09:48:18 UTC
The uploaded spec and srpm are in bootstrap mode to be able to build.

It's mostly about "make regen-all".

Running a Koji scratch build for tests: https://koji.fedoraproject.org/koji/taskinfo?taskID=39125778

Comment 2 Miro Hrončok 2019-11-20 10:36:17 UTC
In my local mock, I always get:

======================================================================
FAIL: test_pidfd_open (test.test_posix.PosixTester)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/builddir/build/BUILD/Python-3.9.0a1/Lib/test/test_posix.py", line 1479, in test_pidfd_open
    self.assertEqual(cm.exception.errno, errno.EINVAL)
AssertionError: 1 != 22

----------------------------------------------------------------------


When not bootstrapping. This has not reproduced in Koji (with bootstrapping). Will try in mock, with bootstrapping.

Comment 3 Miro Hrončok 2019-11-20 10:56:45 UTC
Ok, Koji has an older Kernel form Fedora 29 and this test is skipped.

I've notified upstream via the pull request that added this feature: https://github.com/python/cpython/pull/17063#issuecomment-555947310

Nathaniel says: Sounds like you're using some kind of buggy sandbox that's returning EPERM for unrecognized syscalls: https://bugs.python.org/issue38692#msg356235

Will try to talk to mock people to see if this is the case.


In the meantime, please use the Koji built packages to do the review.

Comment 4 Miro Hrončok 2019-11-20 12:24:35 UTC
> Sounds like you're using some kind of buggy sandbox

See also bz1770154.

Comment 5 Miro Hrončok 2019-11-20 12:38:39 UTC
Reviewer: Put into your ~/.config/mock.cfg

config_opts['use_nspawn'] = False


In order to workaround the issue.

Comment 6 Neal Gompa 2019-11-20 13:52:20 UTC
Taking this review...

Comment 7 Miro Hrončok 2019-11-21 09:46:19 UTC
(In reply to Miro Hrončok from comment #5)
> Reviewer: Put into your ~/.config/mock.cfg
> 
> config_opts['use_nspawn'] = False
> 
> 
> In order to workaround the issue.

Update to at least:

libseccomp-2.4.2-1.fc31
systemd-243.4-1.fc31

To fix it.

Comment 8 Neal Gompa 2019-11-23 17:16:18 UTC
(In reply to Miro Hrončok from comment #7)
> (In reply to Miro Hrončok from comment #5)
> > Reviewer: Put into your ~/.config/mock.cfg
> > 
> > config_opts['use_nspawn'] = False
> > 
> > 
> > In order to workaround the issue.
> 
> Update to at least:
> 
> libseccomp-2.4.2-1.fc31
> systemd-243.4-1.fc31
> 
> To fix it.

The systemd and libseccompp updates have landed in stable updates, so now I'm going to try this again...

Comment 9 Miro Hrončok 2019-11-25 09:40:07 UTC
Did it work? Is there anything I can do to help you review this?

Comment 10 Neal Gompa 2019-11-25 12:02:38 UTC
The packaging is mostly fine, but I do have some specific questions:

> # 00001 #
> # Fixup distutils/unixccompiler.py to remove standard library path from rpath:
> # Was Patch0 in ivazquez' python3000 specfile:
> Patch1:         00001-rpath.patch

There's no upstreaming status here? What's up with that?

> # 00102 #
> # Change the various install paths to use /usr/lib64/ instead or /usr/lib
> # Only used when "%%{_lib}" == "lib64"
> # Not yet sent upstream.
> Patch102: 00102-lib64.patch

I'm pretty sure I've seen variations of this patch for almost a decade now. Why hasn't this been upstreamed?

> # 00111 #
> # Patch the Makefile.pre.in so that the generated Makefile doesn't try to build
> # a libpythonMAJOR.MINOR.a
> # See https://bugzilla.redhat.com/show_bug.cgi?id=556092
> # Downstream only: not appropriate for upstream
> Patch111: 00111-no-static-lib.patch

Why patch instead of just deleting or subpackaging the file?

> # 00189 #
> # Instead of bundled wheels, use our RPM packaged wheels from
> # /usr/share/python-wheels
> Patch189: 00189-use-rpm-wheels.patch

I know this isn't upstreamable, but could you please mark it as such and why?

> # 00251
> # Set values of prefix and exec_prefix in distutils install command
> # to /usr/local if executable is /usr/bin/python* and RPM build
> # is not detected to make pip and distutils install into separate location
> # Fedora Change: https://fedoraproject.org/wiki/Changes/Making_sudo_pip_safe
> Patch251: 00251-change-user-install-location.patch

This is not upstreamable either?

> # 00274 #
> # Upstream uses Debian-style architecture naming. Change to match Fedora.
> Patch274: 00274-fix-arch-names.patch

This is actually the GCC names. I'm not sure what's going on here, but I *think* our GCC builds have the architectures mangled. It's not a particular priority, but I'd like if someone checked with our GCC folks to see what's up here...

> # 00328 #
> # Restore pyc to TIMESTAMP invalidation mode as default in rpmbubild
> # See https://src.fedoraproject.org/rpms/redhat-rpm-config/pull-request/57#comment-27426
> Patch328: 00328-pyc-timestamp-invalidation-mode.patch

I know this isn't upstreamable, but please mark it as such and why.

> # In Fedora 31, /usr/bin/pydoc was moved here from Python 2.
> # Ideally we'd have an explicit conflict with "/usr/bin/pydoc < 3",
> # but file provides aren't versioned and the file moved across packages.
> # Instead, we rely on the conflict in python3-libs.
> 

This comment exists with no stuff (Conflicts, etc.) underneath it. Was there something there before?

Comment 11 Miro Hrončok 2019-11-25 12:17:25 UTC
Generally, we track those efforts via this tracker: bz1287556.

I will try to answer your questions here, but please don't block the review on it:

> > # 00001 #
> > # Fixup distutils/unixccompiler.py to remove standard library path from rpath:
> > # Was Patch0 in ivazquez' python3000 specfile:
> > Patch1:         00001-rpath.patch
> 
> There's no upstreaming status here? What's up with that?

We don't want to upstream this, we want to get rid of it, see https://pagure.io/packaging-committee/issue/886


> > # 00102 #
> > # Change the various install paths to use /usr/lib64/ instead or /usr/lib
> > # Only used when "%%{_lib}" == "lib64"
> > # Not yet sent upstream.
> > Patch102: 00102-lib64.patch
> 
> I'm pretty sure I've seen variations of this patch for almost a decade now.
> Why hasn't this been upstreamed?

We are working on it. https://github.com/python/cpython/pull/11755


> > # 00111 #
> > # Patch the Makefile.pre.in so that the generated Makefile doesn't try to build
> > # a libpythonMAJOR.MINOR.a
> > # See https://bugzilla.redhat.com/show_bug.cgi?id=556092
> > # Downstream only: not appropriate for upstream
> > Patch111: 00111-no-static-lib.patch
> 
> Why patch instead of just deleting or subpackaging the file?

Historical reasons. Possibly faster build, but not sure. The idea was to get rid of this patch via https://fedoraproject.org/wiki/Changes/PythonStaticSpeedup but that is not happening.

We will revisit this one day, but it's not a big priority.


> > # 00189 #
> > # Instead of bundled wheels, use our RPM packaged wheels from
> > # /usr/share/python-wheels
> > Patch189: 00189-use-rpm-wheels.patch
> 
> I know this isn't upstreamable, but could you please mark it as such and why?


I'll submit a python3 PR shortly.

> > # 00251
> > # Set values of prefix and exec_prefix in distutils install command
> > # to /usr/local if executable is /usr/bin/python* and RPM build
> > # is not detected to make pip and distutils install into separate location
> > # Fedora Change: https://fedoraproject.org/wiki/Changes/Making_sudo_pip_safe
> > Patch251: 00251-change-user-install-location.patch
> 
> This is not upstreamable either?


Not really in this form. Ideally, we would PEP this, but time is low... :(


> > # 00274 #
> > # Upstream uses Debian-style architecture naming. Change to match Fedora.
> > Patch274: 00274-fix-arch-names.patch
> 
> This is actually the GCC names. I'm not sure what's going on here, but I
> *think* our GCC builds have the architectures mangled. It's not a particular
> priority, but I'd like if someone checked with our GCC folks to see what's
> up here...


Possibly, once everything else is done :D

> > # 00328 #
> > # Restore pyc to TIMESTAMP invalidation mode as default in rpmbubild
> > # See https://src.fedoraproject.org/rpms/redhat-rpm-config/pull-request/57#comment-27426
> > Patch328: 00328-pyc-timestamp-invalidation-mode.patch
> 
> I know this isn't upstreamable, but please mark it as such and why.


PR incoming.

> > # In Fedora 31, /usr/bin/pydoc was moved here from Python 2.
> > # Ideally we'd have an explicit conflict with "/usr/bin/pydoc < 3",
> > # but file provides aren't versioned and the file moved across packages.
> > # Instead, we rely on the conflict in python3-libs.
> > 
> 
> This comment exists with no stuff (Conflicts, etc.) underneath it. Was there
> something there before?

No, the line that starts with "Instead" describes where to look for those.

Comment 13 Neal Gompa 2019-11-25 12:31:37 UTC
Review notes:

- Packaging follows Fedora Packaging Guidelines
- Package builds and installs
- No serious rpmlint issues
- Licensing is declared properly and license files are installed and marked correctly

PACKAGE APPROVED.

Comment 14 Gwyn Ciesla 2019-11-25 14:21:21 UTC
(fedscm-admin):  The Pagure repository was created at https://src.fedoraproject.org/rpms/python39

Comment 15 Miro Hrončok 2019-11-26 20:22:57 UTC
Thanks, Neal.


Note You need to log in before you can comment on or make changes to this bug.