Bug 2134021 - Review Request: mingw-pyproject-rpm-macros - RPM macros for PEP 517 MinGW Python packages
Summary: Review Request: mingw-pyproject-rpm-macros - RPM macros for PEP 517 MinGW Pyt...
Keywords:
Status: CLOSED WONTFIX
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Nobody's working on this, feel free to take it
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2022-10-12 07:21 UTC by Sandro Mani
Modified: 2022-10-19 17:31 UTC (History)
4 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2022-10-19 17:31:03 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)

Description Sandro Mani 2022-10-12 07:21:19 UTC
Spec URL: https://smani.fedorapeople.org/review/mingw-pyproject-rpm-macros.spec
SRPM URL: https://smani.fedorapeople.org/review/mingw-pyproject-rpm-macros-1-1.fc38.src.rpm
Description: RPM macros for PEP 517 MinGW Python packages
Fedora Account System Username: smani

Comment 1 Maxwell G 2022-10-12 16:25:56 UTC
Some initial drive by comments:

Have you communicated with the pyproject-rpm-macros maintainers about this?  Where are the %mingw_python3* macros defined? Are these macros supposed to depend on pyproject-rpm-macros? This seems to rely on some files and macros from there (e.g. %{_rpmconfigdir}/redhat/pyproject_save_files.py in %pyproject_save_files or %{_pyproject_ghost_distinfo}). pyproject-rpm-macros also has solid unit and integration tests which seem to be removed here.

Comment 2 Miro Hrončok 2022-10-12 17:20:42 UTC
> Have you communicated with the pyproject-rpm-macros maintainers about this?

Actually yes. And I was reluctant to maintain this within pyproject-rpm-macros due to my (close to) zero understanding of MinGW packaging. Hence we agreed to package it separately, but I haven't yet seen how it is done.

Comment 3 Sandro Mani 2022-10-12 20:18:08 UTC
The %mingw_python3 macros come from macros.mingw{32,64}-python3 as installed by mingw{32,64}-python3. mingw{32,64}-python3 pulls in python3-devel, which in turn pulls in the pyproject-rpm-macros. But to make it more explicit, I could also add an explicit Requires: pyproject-rpm-macros to the mingw{32,64}-pyproject-rpm-macros packages.

Comment 4 Miro Hrončok 2022-10-12 22:56:52 UTC
Random thoughts:

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

I agree that the dependency on pyproject-rpm-macros should be explicit and better yet versioned (somehow).

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

The version of this package seems quite arbitrary. Should it follow the pyproject-rpm-macros version instead?

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

%pyproject_buildrequires is missing entirely?

There are the following runtime requires hardcoded:

Requires:       mingwXX-python3-pip
Requires:       mingwXX-python3-setuptools
Requires:       mingwXX-python3-wheel

That is against the spirit of PEP 517 which this package mentions in the summary and description.

If you just need a way to build a wheel with setuptools and install it, aren't %py3_build_wheel and %py3_install_wheel better suited for the task?

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

LICENSE. The original macros are:
	
  Copyright 2019 pyproject-rpm-macros contributors

This one is:

  Copyright (c) 2015 Sandro Mani <manisandro>

The year is probably bogus and changing the name seems a bit weird.

Comment 5 Sandro Mani 2022-10-13 05:34:00 UTC
> I agree that the dependency on pyproject-rpm-macros should be explicit and better yet versioned (somehow).

Ok - versioned to make sure that manual intervention is required to make sure that a new version of pyproject-rpm-macros works correctly I presume?


> The version of this package seems quite arbitrary. Should it follow the pyproject-rpm-macros version instead?

Yes can make sense


> %pyproject_buildrequires is missing entirely?

Yes, I haven't yet looked into this one TBH. Priority for now is to have a possibility to build packages which don't have setup.py.

> There are the following runtime requires hardcoded:

> Requires:       mingwXX-python3-pip
> Requires:       mingwXX-python3-setuptools
> Requires:       mingwXX-python3-wheel

> That is against the spirit of PEP 517 which this package mentions in the summary and description.

> If you just need a way to build a wheel with setuptools and install it, aren't %py3_build_wheel and %py3_install_wheel better suited for the task?

This is probably me lacking a good understanding of python building post setuptools. Ultimately I was just looking to mirror the packaging of native packages, but I'm open to any solution which allows me to build the packages.

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

> LICENSE. The original macros are:
	
>   Copyright 2019 pyproject-rpm-macros contributors

> This one is:
> 
>   Copyright (c) 2015 Sandro Mani <manisandro>
> 
> The year is probably bogus and changing the name seems a bit weird.

Sloppiness on my part, I copied the license from rpm-mpi-hooks without realizing that it contained also the copyright line. Obviously the copyright should be the one of the original macros.

Comment 6 Miro Hrončok 2022-10-13 08:50:29 UTC
(In reply to Sandro Mani from comment #5)
> > I agree that the dependency on pyproject-rpm-macros should be explicit and better yet versioned (somehow).
> 
> Ok - versioned to make sure that manual intervention is required to make
> sure that a new version of pyproject-rpm-macros works correctly I presume?

New or old. I was thinking something like:

Requires: (pyproject-rpm-macros >= 1.4.0 with pyproject-rpm-macros < 1.5~~)



> Priority for now is to have a possibility to build packages which don't have setup.py.

But still use setuptools?



> ...I'm open to any solution which allows me to build the packages.

So you have some examples I can examine?

Comment 7 Maxwell G 2022-10-13 18:20:10 UTC
(In reply to Miro Hrončok from comment #4)
> %pyproject_buildrequires is missing entirely?
> 
> There are the following runtime requires hardcoded:
> 
> Requires:       mingwXX-python3-pip
> Requires:       mingwXX-python3-setuptools
> Requires:       mingwXX-python3-wheel
> 
> That is against the spirit of PEP 517 which this package mentions in the
> summary and description.

I guess there isn't a Python dist Provides generator or a dependency generator for mingw python packages. The generator would have to create Provides like mingw32-python3dist(foo) or mingw64-python3dist(foo).

> If you just need a way to build a wheel with setuptools and install it, aren't %py3_build_wheel and %py3_install_wheel better suited for the task?

Yeah, I also wonder if there's a better choice if you don't want to implement the other plubming. If you built python3-build for mingw, you could also run %mingwXX_python3 -m build -w to build wheels for packages that use other build systems. %py3_build_wheel only works with legacy setup.py files. 


(In reply to Miro Hrončok from comment #6)
> (In reply to Sandro Mani from comment #5)
> > ...I'm open to any solution which allows me to build the packages.
> 
> So you have some examples I can examine?

https://copr.fedorainfracloud.org/coprs/smani/mingw-python3-3.11

Comment 8 Sandro Mani 2022-10-13 20:26:11 UTC
I actually gave python-build a try a while ago, but I ran into a chicken-egg problem: python3-build requires python3-tomli, which does not have a setup.py - so I'm not sure how I'm supposed to build it? Therefore I assumed that going for the full pyproject macros was the better solution.

As far as other concrete examples without setup.py, there are

mingw-python-flit_core - https://copr.fedorainfracloud.org/coprs/smani/mingw-python3-3.11/build/4917383/
mingw-python-pyparsing - https://copr.fedorainfracloud.org/coprs/smani/mingw-python3-3.11/build/4917414/

Comment 9 Miro Hrončok 2022-10-13 21:53:30 UTC
for flit-core, you can follow what EPEL 8 is doing.
for python-tomli, see the bootstrap bcond in Fedora.

Comment 10 Maxwell G 2022-10-13 22:01:08 UTC
(In reply to Sandro Mani from comment #8)
> I actually gave python-build a try a while ago, but I ran into a chicken-egg
> problem: python3-build requires python3-tomli

python3-build only requires tomli on Python 3.10 and below. On Python 3.11 and above, it uses tomllib[1] from the stdlib. I think you said you were building for Python 3.11 in your ML post.

[1]: https://docs.python.org/3.11/library/tomllib.html / https://peps.python.org/pep-0680/

> python3-tomli, which does not have a
> setup.py - so I'm not sure how I'm supposed to build it? Therefore I assumed
> that going for the full pyproject macros was the better solution.

Either way, you can build flit-core itself or any project that uses it with %python3 -m flit_core.wheel. This is what I used to build flit-core, tomli, and testpath for EPEL 8 that doesn't have build, pyproject-rpm-macros, or support for pep517 build backends in its old pip. See https://src.fedoraproject.org/rpms/python-flit-core/blob/epel8/f/python-flit-core.spec or https://src.fedoraproject.org/rpms/python-tomli/blob/epel8/f/python-tomli.spec.

> mingw-python-flit_core

This should be named mingw-python-flit_core.

Comment 11 Maxwell G 2022-10-13 22:21:30 UTC
(In reply to Maxwell G from comment #10) 
> > mingw-python-flit_core
> 
> This should be named mingw-python-flit_core.

Doh, I wrote the same thing twice. I meant this should be mingw-python-flit-core (no underscore) as per the python naming guidelines.

Comment 12 Sandro Mani 2022-10-14 07:51:45 UTC
I am indeed building for python 3.11. python-build indeed directly requires tomli only with python < 3.11, but python-build also requires python-pep517 which unconditionally requires tomli. Hmm let me look into this again...

Comment 13 Sandro Mani 2022-10-14 11:57:21 UTC
The python-build approach is turning out to be pretty complex, as it is based on virtual environments, which currently don't work for cross-compilation with mingw-python3.

Currently mingw-python works by tweaking the environment basically as follows:

pylibdynload=`/usr/bin/python3.11 -c 'import sysconfig; import os; print(os.path.join(sysconfig.get_path("stdlib"), "lib-dynload"))'`; 
_PYTHON_HOST_PLATFORM=mingw \
_PYTHON_SYSCONFIGDATA_NAME="_sysconfigdata__win32_" \
PYTHONHOME=/usr/x86_64-w64-mingw32/sys-root/mingw \
PYTHONPATH=$PYTHONPATH:/usr/x86_64-w64-mingw32/lib/python3.11:/usr/x86_64-w64-mingw32/lib/python3.11/site-packages:$pylibdynload:/usr/x86_64-w64-mingw32/sys-root/mingw/lib/python3.11:/usr/x86_64-w64-mingw32/sys-root/mingw/lib/python3.11/site-packages \
PYTHONPLATLIBDIR=lib \
/usr/bin/python3.11

I'd need to figure out how to patch venv to ensure that this environment is properly preserved.

Before I go down the route of patching venv, might the current pyproject approach be the better/simpler solution after all?

Comment 14 Maxwell G 2022-10-14 17:01:29 UTC
Virtual environments are a pretty core feature that you should implement, but they're not actually needed here.

Use the -n / --no-isolation flag to disable venv build isolation. I think the build isolation mode downloads dependencies from PyPI, so you'd need to disable that for RPM builds even if venvs did work properly. I should've mentioned that in the first place :).

> might the current pyproject approach be the better/simpler solution after all?

I guess that's up to you. To me, using simpler tools like python-build and just modifying %py3_install_wheel sounds less maintenance intensive over the long term than reimplementing parts of pyprpoject-rpm-macros and keeping up with its changes, especially if you're not implementing dynamic buildrequires which is pyproject-rpm-macros' main selling point.

Comment 15 Sandro Mani 2022-10-14 20:05:28 UTC
Thanks, that worked with -n!

Last question: any tips on the best practice to install the wheel to the buildroot?

Comment 16 Petr Viktorin (pviktori) 2022-10-17 08:31:41 UTC
> python-build also requires python-pep517 which unconditionally requires tomli

Not in Rawhide (0.13.0 upstream). Do you need that backported?

> any tips on the best practice to install the wheel to the buildroot?

With python3-installer. (Or pip, which pyproject-rpm-macros still does.)

If you don't have python3-pip or python3-installer yet, you could unzip a wheel.
The spec is here: https://packaging.python.org/en/latest/specifications/binary-distribution-format/#details
By simply unzipping, you will miss:
- Version check for the wheel format
- Spreading files from a top-level *.data directory (e.g. scripts that should go to /usr/bin)
- byte-compilation, which fully-bootstrapped RPM macros should do for you
- removing the *.dist-info/RECORD file (which you have to do even after installing with pip/installer)

So it's best to only use this to bootstrap an installer.

Another possibility might be to run pip directly from its wheel. This does not work in general [0], but pip currently supports it [1].
(The wheel is cross-platform, so I guess you could even use Fedora's main python-pip-wheel package?)

> using simpler tools like python-build and just modifying %py3_install_wheel sounds less maintenance intensive over the long term

Hm, I should find time to switch pyproject-rpm-macros to the simpler tools, shouldn't I.


[0] https://packaging.python.org/en/latest/specifications/binary-distribution-format/#is-it-possible-to-import-python-code-directly-from-a-wheel-file
[1] https://github.com/pypa/pip/blob/a8ba0eec6ac3c1f6cf23f1e2e4c64954bd7a08ed/src/pip/__main__.py#L12

Comment 17 Sandro Mani 2022-10-18 09:59:06 UTC
Thanks for the python-installer tip, I'm giving it a go in [1], but looks like it's the last missing piece of the puzzle!

[1] https://copr.fedorainfracloud.org/coprs/smani/mingw-python3-3.11-build/

Comment 18 Sandro Mani 2022-10-19 17:31:03 UTC
It's working fine with python-build + python-installer, hence I'm abandoning the pyproject approach. Thanks for all your inputs! If someone has time, I'd appreciate review of the following packages to complete the process:

https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=2136235 - mingw-python-build
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=2136236 - mingw-python-flit-core
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=2136237 - mingw-python-installer
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=2136238 - mingw-python-pep517

Thanks!


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