PR#174: Add BuildArch: noarch by default - rpms/python-rpm-macros

rpms / python-rpm-macros

#174 Add BuildArch: noarch by default

Opened a month ago by lecris. Modified a month ago

rpms/ lecris/python-rpm-macros extras-noarch into rawhide

Add option -a to include BuilArch: noarch

Cristian Le • a month ago

da464d7

macros.python-srpm

file modified

+7 -2

		`@@ -225,11 +225,12 @@`
		`end`
		`}`

		`- %python_extras_subpkg(n:i:f:F) %{expand:%{lua:`
		`+ %python_extras_subpkg(n:i:f:Fa) %{expand:%{lua:`
		`local option_n = '-n (name of the base package)'`
		`local option_i = '-i (buildroot path to metadata)'`
		`local option_f = '-f (builddir path to a filelist)'`
		`local option_F = '-F (skip %%files section)'`
		`+ local option_a = '-a (insert BuildArch: noarch)'`
		`local value_n = rpm.expand('%{-n*}')`
		`local value_i = rpm.expand('%{-i*}')`
		`local value_f = rpm.expand('%{-f*}')`
		`@@ -254,6 +255,10 @@`
		`rpm.expand('%{error:%%%0 requires at least one argument with "extras" name}')`
		`end`
		`local requires = 'Requires: ' .. value_n .. ' = %{?epoch:%{epoch}:}%{version}-%{release}'`
		`+ local noarch = ''`
		`+ if opt.a then`
		`+ noarch = 'BuildArch: noarch'`
		`+ end`
		`for extras in args:gmatch('[^%s,]+') do`
		`local rpmname = value_n .. '+' .. extras`
		`local pkgdef = '%package -n ' .. rpmname`
		`@@ -277,7 +282,7 @@`
		`elseif value_f ~= '' then`
		`files = '%files -n ' .. rpmname .. ' -f ' .. value_f`
		`end`
		`- for i, line in ipairs({pkgdef, summary, requires, description, files, ''}) do`
		`+ for i, line in ipairs({pkgdef, summary, requires, noarch, description, files, ''}) do`
		`print(line .. '\\\n')`
		`end`
		`end`

python-rpm-macros.spec

file modified

+4 -1

		`@@ -53,7 +53,7 @@`
		`end`
		`}`
		`Version: %{__default_python3_version}`
		`- Release: 9%{?dist}`
		`+ Release: 10%{?dist}`

		`BuildArch: noarch`

		`@@ -163,6 +163,9 @@`


		`%changelog`
		`+ * Wed Apr 10 2024 Cristian Le <fedora@lecris.me> - 3.12-10`
		`+ - %%python_extras_subpkg: Add option -a to include BuildArch: noarch`
		`+`
		`* Thu Mar 28 2024 Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl> - 3.12-9`
		`- Minor improvements to brp-fix-pyc-reproducibility`

tests/test_evals.py

file modified

+26

		`@@ -545,6 +545,7 @@`
		`%package -n python3-setuptools_scm+toml`
		`Summary: Metapackage for python3-setuptools_scm: toml extras`
		`Requires: python3-setuptools_scm = 6-7`
		`+`
		`%description -n python3-setuptools_scm+toml`
		`This is a metapackage bringing in toml extras requires for`
		`python3-setuptools_scm.`
		`@@ -556,6 +557,7 @@`
		`%package -n python3-setuptools_scm+yaml`
		`Summary: Metapackage for python3-setuptools_scm: yaml extras`
		`Requires: python3-setuptools_scm = 6-7`
		`+`
		`%description -n python3-setuptools_scm+yaml`
		`This is a metapackage bringing in yaml extras requires for`
		`python3-setuptools_scm.`
		`@@ -574,6 +576,7 @@`
		`%package -n python3-setuptools_scm+toml`
		`Summary: Metapackage for python3-setuptools_scm: toml extras`
		`Requires: python3-setuptools_scm = 6-7`
		`+`
		`%description -n python3-setuptools_scm+toml`
		`This is a metapackage bringing in toml extras requires for`
		`python3-setuptools_scm.`
		`@@ -584,6 +587,7 @@`
		`%package -n python3-setuptools_scm+yaml`
		`Summary: Metapackage for python3-setuptools_scm: yaml extras`
		`Requires: python3-setuptools_scm = 6-7`
		`+`
		`%description -n python3-setuptools_scm+yaml`
		`This is a metapackage bringing in yaml extras requires for`
		`python3-setuptools_scm.`
		`@@ -601,6 +605,7 @@`
		`%package -n python3-setuptools_scm+toml`
		`Summary: Metapackage for python3-setuptools_scm: toml extras`
		`Requires: python3-setuptools_scm = 6-7`
		`+`
		`%description -n python3-setuptools_scm+toml`
		`This is a metapackage bringing in toml extras requires for`
		`python3-setuptools_scm.`
		`@@ -611,6 +616,7 @@`
		`%package -n python3-setuptools_scm+yaml`
		`Summary: Metapackage for python3-setuptools_scm: yaml extras`
		`Requires: python3-setuptools_scm = 6-7`
		`+`
		`%description -n python3-setuptools_scm+yaml`
		`This is a metapackage bringing in yaml extras requires for`
		`python3-setuptools_scm.`
		`@@ -619,6 +625,22 @@`
		`assert lines == expected`


		`+ def test_python_extras_subpkg_a():`
		`+ lines = rpm_eval('%python_extras_subpkg -n python3-setuptools_scm -a -F toml',`
		`+ version='6', release='7')`
		`+ expected = textwrap.dedent(f"""`
		`+ %package -n python3-setuptools_scm+toml`
		`+ Summary: Metapackage for python3-setuptools_scm: toml extras`
		`+ Requires: python3-setuptools_scm = 6-7`
		`+ BuildArch: noarch`
		`+ %description -n python3-setuptools_scm+toml`
		`+ This is a metapackage bringing in toml extras requires for`
		`+ python3-setuptools_scm.`
		`+ It makes sure the dependencies are installed.`
		`+ """).lstrip().splitlines()`
		`+ assert lines == expected`
		`+`
		`+`
		`def test_python_extras_subpkg_underscores():`
		`lines = rpm_eval('%python_extras_subpkg -n python3-webscrapbook -F adhoc_ssl',`
		`version='0.33.3', release='1.fc33')`
		`@@ -626,6 +648,7 @@`
		`%package -n python3-webscrapbook+adhoc_ssl`
		`Summary: Metapackage for python3-webscrapbook: adhoc_ssl extras`
		`Requires: python3-webscrapbook = 0.33.3-1.fc33`
		`+`
		`%description -n python3-webscrapbook+adhoc_ssl`
		`This is a metapackage bringing in adhoc_ssl extras requires for`
		`python3-webscrapbook.`
		`@@ -649,6 +672,7 @@`
		`%package -n python3-hypothesis+cli`
		`Summary: Metapackage for python3-hypothesis: cli extras`
		`Requires: python3-hypothesis = 6.6.0-1.fc35`
		`+`
		`%description -n python3-hypothesis+cli`
		`This is a metapackage bringing in cli extras requires for python3-hypothesis.`
		`It makes sure the dependencies are installed.`
		`@@ -658,6 +682,7 @@`
		`%package -n python3-hypothesis+ghostwriter`
		`Summary: Metapackage for python3-hypothesis: ghostwriter extras`
		`Requires: python3-hypothesis = 6.6.0-1.fc35`
		`+`
		`%description -n python3-hypothesis+ghostwriter`
		`This is a metapackage bringing in ghostwriter extras requires for`
		`python3-hypothesis.`
		`@@ -668,6 +693,7 @@`
		`%package -n python3-hypothesis+pytz`
		`Summary: Metapackage for python3-hypothesis: pytz extras`
		`Requires: python3-hypothesis = 6.6.0-1.fc35`
		`+`
		`%description -n python3-hypothesis+pytz`
		`This is a metapackage bringing in pytz extras requires for python3-hypothesis.`
		`It makes sure the dependencies are installed.`

lecris commented a month ago

Based on some discussions on matrix chatroom, I've tried the approach of making the base package archful and the actual distributed pacakges noarch, but I have encountered rpmlint issues when using %pyproject_extras_subpkg due to the lack of BuildArch: noarch

This PR adds the BuildArch: noarch by default and option_A (as in archful, not good with names) to override the behavior

Not sure why value_F was used there, I've opted for opt.A

@music @decathorpe thoughts on the implementation?

zuul commented a month ago

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci
https://fedora.softwarefactory-project.io/zuul/buildset/9625ac501df44ae082f9f0c3f6f3c259

check-for-arches : SUCCESS in 1m 44s
check-for-eln : SUCCESS in 21s
eln-rpm-scratch-build : SUCCESS in 10m 50s
rpm-scratch-build : SUCCESS in 8m 46s
rpm-scratch-build-s390x : SUCCESS in 8m 22s (non-voting)
rpm-scratch-build-ppc64le : SUCCESS in 9m 49s (non-voting)
rpm-scratch-build-i686 : SUCCESS in 8m 43s (non-voting)
rpm-scratch-build-aarch64 : SUCCESS in 9m 38s (non-voting)
rpm-linter : SUCCESS in 4m 16s (non-voting)
rpm-rpminspect : SUCCESS in 7m 50s (non-voting)
check-for-sti-tests : SUCCESS in 16s
check-for-fmf-tests : SUCCESS in 17s
rpm-install-test : SUCCESS in 1m 06s
rpm-sti-test : FAILURE in 8m 16s
Skipped 1 job

rebased onto 8125cea

a month ago

zuul commented a month ago

check-for-arches : SUCCESS in 1m 43s
check-for-eln : SUCCESS in 19s
eln-rpm-scratch-build : SUCCESS in 10m 08s
rpm-scratch-build : SUCCESS in 11m 20s
rpm-scratch-build-s390x : SUCCESS in 11m 04s (non-voting)
rpm-scratch-build-ppc64le : SUCCESS in 14m 47s (non-voting)
rpm-scratch-build-i686 : SUCCESS in 10m 21s (non-voting)
rpm-scratch-build-aarch64 : SUCCESS in 10m 47s (non-voting)
rpm-linter : SUCCESS in 4m 10s (non-voting)
rpm-rpminspect : SUCCESS in 8m 37s (non-voting)
check-for-sti-tests : SUCCESS in 17s
check-for-fmf-tests : SUCCESS in 16s
rpm-install-test : SUCCESS in 1m 09s
rpm-sti-test : FAILURE in 8m 13s
Skipped 1 job

rebased onto 63c20b4

a month ago

zuul commented a month ago

Build succeeded.
https://fedora.softwarefactory-project.io/zuul/buildset/d741d45979444ad28584d68aabb97dbb

check-for-arches : SUCCESS in 1m 31s
check-for-eln : SUCCESS in 17s
eln-rpm-scratch-build : SUCCESS in 11m 14s
rpm-scratch-build : SUCCESS in 9m 39s
rpm-scratch-build-s390x : SUCCESS in 11m 00s (non-voting)
rpm-scratch-build-ppc64le : SUCCESS in 10m 54s (non-voting)
rpm-scratch-build-i686 : SUCCESS in 10m 51s (non-voting)
rpm-scratch-build-aarch64 : SUCCESS in 10m 46s (non-voting)
rpm-linter : SUCCESS in 4m 57s (non-voting)
rpm-rpminspect : SUCCESS in 8m 32s (non-voting)
check-for-sti-tests : SUCCESS in 16s
check-for-fmf-tests : SUCCESS in 16s
rpm-install-test : SUCCESS in 1m 09s
rpm-sti-test : SUCCESS in 8m 16s
Skipped 1 job

churchyard commented a month ago

Making the package noarch by default is a very breaking change. Any package that uses the macro and ships extension modules will have to add -A or else it will produce different norach packages on i686 and on other arches.

What exactly are you trying to solve?

Edited a month ago by churchyard

churchyard commented a month ago

Would the ability to insert BuildArch: noarch (e.g. with -a) solve your use case? (In other words, make this opt-in rather than opt-out.)

lecris commented a month ago

Pretty much the reasoning was that the extra dependency packages are (generally) pure metadata packages that simply provide the Requires dependencies. The only situation where this is not the case is if additional entries are injected into %files at which point additional flags are needed for %pyproject_extras_subpkg. @music also felt that there are some weird setup that are not considered, @churchyard maybe you know of some?

Would the ability to insert BuildArch: noarch (e.g. with -a) solve your use case? (In other words, make this opt-in rather than opt-out.)

Indeed that would also be an option, and I am a bit indifferent about either approach. The few points for adding BuildArch: noarch:

More common defaults for most python packages
More backwards compatible macros. For a while, either -a or -A (opt-in/opt-out) flags would have to be introduced based on Fedora version, and the question would be which is the more common case?

The cases we need to consider are:

Python-only packages: Should already have BuildArch: noarch at the top-level, so they are unaffected by either approach
Arched extra packages, i.e. they are inserting/splitting architecture dependent files. Unaffected if going with -a, but require fixing with -A. Works currently.
Generally these packages should be very rare. Should also consider that the installation is not equivalent with pip install since there would not have such optional packaging
header-only like packages, i.e. wanting to run the build on all architectures, but otherwise a python-only package, e.g. scikit-build-core, a build backend for python+CMake. Requires fixing -a, but unaffected if going with -A. Requires either approach.
Quite common for numerical and scientific packages.

@decathorpe seems to be in favor of BuildArch: noarch as default, maybe there are more comments on that?

Edited a month ago by lecris

churchyard commented a month ago

The extra dependency packages have the dist-info (or egg-info) directory listed as %ghost. The directory may be in %{python_sitelib} (/usr/lib/python3.12/site-packages) for pure Python packages or in %{python_sitearch} (/usr/lib64/python3.12/site-packages on 64bit, /usr/lib/python3.12/site-packages on 32bit) for packages with extension modules.

Extra subpackages for archful Python packages MUST be archful. Despite being metapackages they contain %ghost files with arch-specific path.

Indeed, pure Python packages are probably more common than packages with extension modules. However, extension modules are not exactly rare to come by, or "special cases".

OTOH arched packages with noarch python subpackages are not very common. I'd consider them "special cases".

Edited a month ago by churchyard

churchyard commented a month ago

Examples:

$ repoquery -q --repo=rawhide -l python3-sqlalchemy1.4+mysql-1.4.52-1.fc41.x86_64
/usr/lib64/python3.12/site-packages/SQLAlchemy-1.4.52.dist-info
/usr/lib64/python3.12/site-packages/SQLAlchemy-1.4.52.dist-info/INSTALLER
/usr/lib64/python3.12/site-packages/SQLAlchemy-1.4.52.dist-info/LICENSE
/usr/lib64/python3.12/site-packages/SQLAlchemy-1.4.52.dist-info/METADATA
/usr/lib64/python3.12/site-packages/SQLAlchemy-1.4.52.dist-info/WHEEL
/usr/lib64/python3.12/site-packages/SQLAlchemy-1.4.52.dist-info/top_level.txt

This cannot be noarch. It has files in /usr/lib64 on x86_64.

However:

$ repoquery -q --repo=rawhide -l python3-trimesh+easy-4.2.4-1.fc41.x86_64
/usr/lib/python3.12/site-packages/trimesh-4.2.4.dist-info
/usr/lib/python3.12/site-packages/trimesh-4.2.4.dist-info/INSTALLER
/usr/lib/python3.12/site-packages/trimesh-4.2.4.dist-info/LICENSE.md
/usr/lib/python3.12/site-packages/trimesh-4.2.4.dist-info/METADATA
/usr/lib/python3.12/site-packages/trimesh-4.2.4.dist-info/WHEEL
/usr/lib/python3.12/site-packages/trimesh-4.2.4.dist-info/top_level.txt

This is clearly wrong, python3-trimesh+easy should be noarch, as is python3-trimesh.

Some stats.

There are 68 packages matching python3-.+\+.+\.x86_64. This omits packages with Python extras not called python3-foo+bar but foo+bar, but let's roll with it.

$ repoquery -q --repo=rawhide -a | grep -E 'python3-.+\+.+\.x86_64' | wc -l
68

Those packages belong to 20 "base" packages:

$ repoquery -q --repo=rawhide -a | grep -E 'python3-.+\+.+\.x86_64' | cut -d+ -f1 | sort -u | wc -l
20

Out of those 20, only 2 can (and should) be noarch:

$ repoquery -q --repo=rawhide -l $(repoquery -q --repo=rawhide -a | grep -E 'python3-.+\+.+\.x86_64') | grep 'dist-info$'
/usr/lib/python3.12/site-packages/dask-2024.2.1.dist-info
/usr/lib/python3.12/site-packages/trimesh-4.2.4.dist-info
/usr/lib64/python3.12/site-packages/Cartopy-0.22.0.dist-info
/usr/lib64/python3.12/site-packages/SQLAlchemy-1.4.52.dist-info
/usr/lib64/python3.12/site-packages/SQLAlchemy-2.0.29.dist-info
/usr/lib64/python3.12/site-packages/ZEO-6.0.0.dist-info
/usr/lib64/python3.12/site-packages/aiohttp-3.9.3.dist-info
/usr/lib64/python3.12/site-packages/ezdxf-1.2.0.dist-info
/usr/lib64/python3.12/site-packages/fastavro-1.9.4.dist-info
/usr/lib64/python3.12/site-packages/lxml-5.2.1.dist-info
/usr/lib64/python3.12/site-packages/pandas-2.2.1.dist-info
/usr/lib64/python3.12/site-packages/pyshtools-4.11.10.dist-info
/usr/lib64/python3.12/site-packages/qtile-0.23.0.dist-info
/usr/lib64/python3.12/site-packages/rapidfuzz-3.5.2.dist-info
/usr/lib64/python3.12/site-packages/rasterio-1.3.9.dist-info

I'd rather change the 2 than the remaining 18 (also, any future ones as well).

Considering also packages not named python3-, we only see one more (matrix_synapse), rightfully arched:

$ repoquery -q --repo=rawhide -l $(repoquery -q --repo=rawhide -a | grep -E '.+\+.+\.x86_64') | grep 'dist-info$'
/usr/lib/python3.12/site-packages/dask-2024.2.1.dist-info
/usr/lib/python3.12/site-packages/trimesh-4.2.4.dist-info
/usr/lib64/python3.12/site-packages/Cartopy-0.22.0.dist-info
/usr/lib64/python3.12/site-packages/SQLAlchemy-1.4.52.dist-info
/usr/lib64/python3.12/site-packages/SQLAlchemy-2.0.29.dist-info
/usr/lib64/python3.12/site-packages/ZEO-6.0.0.dist-info
/usr/lib64/python3.12/site-packages/aiohttp-3.9.3.dist-info
/usr/lib64/python3.12/site-packages/ezdxf-1.2.0.dist-info
/usr/lib64/python3.12/site-packages/fastavro-1.9.4.dist-info
/usr/lib64/python3.12/site-packages/lxml-5.2.1.dist-info
/usr/lib64/python3.12/site-packages/matrix_synapse-1.104.0.dist-info
/usr/lib64/python3.12/site-packages/pandas-2.2.1.dist-info
/usr/lib64/python3.12/site-packages/pyshtools-4.11.10.dist-info
/usr/lib64/python3.12/site-packages/qtile-0.23.0.dist-info
/usr/lib64/python3.12/site-packages/rapidfuzz-3.5.2.dist-info
/usr/lib64/python3.12/site-packages/rasterio-1.3.9.dist-info

lecris commented a month ago

Oh, ok, thanks for the insight on the issue. I don't suppose there is a way to copy the archfullness from the -n package passed to %pyproject_extras_subpkg?

churchyard commented a month ago

That's unfortunately just a string, there is no way of querying anything here :(

churchyard commented a month ago

I forgot to grep for egg-info packages, but there are only 3 relevant ones and they are all in lib64:

$ repoquery -q --repo=rawhide -l $(repoquery -q --repo=rawhide -a | grep -E '.+\+.+\.x86_64') | grep 'egg-info$'
/usr/lib64/python3.12/site-packages/coverage-7.3.2-py3.12.egg-info
/usr/lib64/python3.12/site-packages/fonttools-4.50.0-py3.12.egg-info
/usr/lib64/python3.12/site-packages/grpcio-1.48.4-py3.12.egg-info

lecris commented a month ago

Ok, so for this one, I will invert the macro so that patching those 2 packages will be easier. Patching it for previous versions would be tricky unless we cherry-pick this for the previous? Also does OpenSuse also use %pyproject_extras_subpkg that we would need to synchronize with?

churchyard commented a month ago

Ok, so for this one, I will invert the macro so that patching those 2 packages will be easier.

Please do.

Patching it for previous versions would be tricky unless we cherry-pick this for the previous?

Either cherry-pick for older Fedoras, or let me do it, either works fine. f40 can still be fast-forwarded from rawhide.

No idea about openSUSE, do they even generate provides from upstream metadata?

churchyard commented a month ago

The new flag will need to be added to pyproject-rpm-macros as well.

rebased onto da464d7

a month ago

zuul commented a month ago

Merge Failed.

This change or one of its cross-repo dependencies was unable to be automatically merged with the current state of its repository. Please rebase the change and upload a new patchset.

churchyard commented a month ago

A little bikeshedding: is -a the best name for noarch? Should it be -A for noarch, -a for arch?

music commented a month ago

I'd rather change the 2 than the remaining 18 (also, any future ones as well).

I’m surprised there are only two! I think there have been more in the past, but some of them have been due to conditionals and special cases that have been resolved over time. I’ve also encountered a few cases that were so idiosyncratic that the metapackages had to be defined manually without using %pyproject_extra_subpkg; I don’t think those are worth talking about here.

Would the ability to insert BuildArch: noarch (e.g. with -a) solve your use case? (In other words, make this opt-in rather than opt-out.)

This would be a nice feature.

I have considered the arched-ness of the python-trimesh extras metapackages a “wart” rather than a serious problem – not worth defining them manually instead of using %pyproject_extras_subpkg, but definitely worth fixing if it becomes straightforward to do so.

lecris commented a month ago

A little bikeshedding: is -a the best name for noarch? Should it be -A for noarch, -a for arch?

No particular opinion on that, words are hard. It does kinda feel like -a and arch are intuitive, but noarch has me stumped.

Another option could be to use a global macro and not support selectively injecting BuildArch: noarch. Or maybe you could do it by re-defining the macro

churchyard commented a month ago

Thoughts from others?