Bug 1774417

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

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.

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.

> Sounds like you're using some kind of buggy sandbox

See also bz1770154.

Reviewer: Put into your ~/.config/mock.cfg

config_opts['use_nspawn'] = False


In order to workaround the issue.

Taking this review...

(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.

(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...

Did it work? Is there anything I can do to help you review this?

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?

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.

See https://src.fedoraproject.org/rpms/python3/pull-request/154

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.

(fedscm-admin):  The Pagure repository was created at https://src.fedoraproject.org/rpms/python39

Thanks, Neal.