PR#7: Automatically call %python_provide - rpms/python-rpm-generators

rpms / python-rpm-generators

#7 Automatically call %python_provide

Merged 3 years ago by churchyard. Opened 3 years ago by churchyard.

rpms/ churchyard/python-rpm-generators auto_python_provides into master

Automatically call %python_provide

Miro Hrončok • 3 years ago

bbfe493

python-rpm-generators.spec

file modified

+7 -2

		`@@ -10,7 +10,8 @@`
		`Source0: https://raw.githubusercontent.com/rpm-software-management/rpm/102eab50b3d0d6546dfe082eac0ade21e6b3dbf1/COPYING`
		`Source1: python.attr`
		`Source2: pythondist.attr`
		`- Source3: pythondistdeps.py`
		`+ Source3: pythonname.attr`
		`+ Source4: pythondistdeps.py`

		`BuildArch: noarch`

		`@@ -22,6 +23,8 @@`
		`Requires: python3-setuptools`
		`# We have parametric macro generators, we need RPM 4.16 (4.15.90+ is 4.16 alpha)`
		`Requires: rpm > 4.15.90-0`
		`+ # We use %%python_provide`
		`+ Requires: python-rpm-macros`

		`%description -n python3-rpm-generators`
		`%{summary}.`
		`@@ -31,19 +34,21 @@`
		`cp -a %{sources} .`

		`%install`
		`- install -Dpm0644 -t %{buildroot}%{_fileattrsdir} python.attr pythondist.attr`
		`+ install -Dpm0644 -t %{buildroot}%{_fileattrsdir} *.attr`
		`install -Dpm0755 -t %{buildroot}%{_rpmconfigdir} pythondistdeps.py`

		`%files -n python3-rpm-generators`
		`%license COPYING`
		`%{_fileattrsdir}/python.attr`
		`%{_fileattrsdir}/pythondist.attr`
		`+ %{_fileattrsdir}/pythonname.attr`
		`%{_rpmconfigdir}/pythondistdeps.py`

		`%changelog`
		`* Wed Apr 01 2020 Miro Hrončok <mhroncok@redhat.com> - 11-1`
		`- Rewrite python(abi) generators to Lua to make them faster`
		`- RPM 4.16+ is needed`
		`+ - Automatically call %%python_provide`

		`* Thu Jan 30 2020 Fedora Release Engineering <releng@fedoraproject.org> - 10-4`
		`- Rebuilt for https://fedoraproject.org/wiki/Fedora_32_Mass_Rebuild`

pythonname.attr

file added

+20

		`@@ -0,0 +1,20 @@`
		`+ %__pythonname_provides() %{lua:`
		`+ -- this macro is called for each file in a package, the path being in %1`
		`+ -- but we don't need to know the path, so we would get for each file: Macro %1 defined but not used within scope`
		`+ -- in here, we expand %name conditionally on %1 to suppress the warning`
		`+ local name = rpm.expand('%{?1:%{name}}')`
		`+ -- a structure that knows what names were already processed, so we can end early`
		`+ if __pythonname_beenthere == nil then`
		`+ __pythonname_beenthere = {}`
		`+ end`
		`+ -- we save ourselves a trip to %python_provide if we have already been there`
		`+ if __pythonname_beenthere[name] == nil then`
		`+ local python_provide = rpm.expand('%{?python_provide:%python_provide %{name}}')`
		`+ for provides in python_provide:gmatch('Provides:[ \\t]+([^\\n]+)') do`
		`+ print(provides .. " ")`
		`+ end`
		`+ __pythonname_beenthere[name] = true`
		`+ end`
		`+ }`
		`+`
		`+ %__pythonname_path ^/`

tests/pythonname.sh

file added

+28

		`@@ -0,0 +1,28 @@`
		`+ #!/usr/bin/bash -eux`
		`+ rpmbuild -ba pythonname.spec`
		`+`
		`+ XY=$(rpm --eval '%python3_version_nodots')`
		`+ RPMDIR=$(rpm --eval '%_topdir')/RPMS/noarch`
		`+`
		`+ echo "Provides for python${XY}-foo"`
		`+ rpm -qp --provides ${RPMDIR}/python${XY}-foo-0-0.noarch.rpm`
		`+ rpm -qp --provides ${RPMDIR}/python${XY}-foo-0-0.noarch.rpm \| grep -q '^python-foo = 0-0$'`
		`+ rpm -qp --provides ${RPMDIR}/python${XY}-foo-0-0.noarch.rpm \| grep -q '^python3-foo = 0-0$'`
		`+`
		`+ echo "Provides for python3-foo"`
		`+ rpm -qp --provides ${RPMDIR}/python3-foo-0-0.noarch.rpm`
		`+ rpm -qp --provides ${RPMDIR}/python3-foo-0-0.noarch.rpm \| grep -q '^python-foo = 0-0$'`
		`+ rpm -qp --provides ${RPMDIR}/python3-foo-0-0.noarch.rpm \| grep -q '^python'${XY}'-foo = 0-0$'`
		`+`
		`+ echo "Provides for python2-foo"`
		`+ rpm -qp --provides ${RPMDIR}/python2-foo-0-0.noarch.rpm`
		`+ rpm -qp --provides ${RPMDIR}/python2-foo-0-0.noarch.rpm \| grep -vq '^python-foo = 0-0$'`
		`+`
		`+ echo "Provides for python-foo"`
		`+ rpm -qp --provides ${RPMDIR}/python-foo-0-0.noarch.rpm`
		`+ rpm -qp --provides ${RPMDIR}/python-foo-0-0.noarch.rpm \| grep -vq '^python2-foo = 0-0$'`
		`+`
		`+ echo "Provides for python35-foo"`
		`+ rpm -qp --provides ${RPMDIR}/python35-foo-0-0.noarch.rpm`
		`+ rpm -qp --provides ${RPMDIR}/python35-foo-0-0.noarch.rpm \| grep -vq '^python-foo = 0-0$'`
		`+ rpm -qp --provides ${RPMDIR}/python35-foo-0-0.noarch.rpm \| grep -vq '^python3-foo = 0-0$'`

tests/pythonname.spec

file added

+62

		`@@ -0,0 +1,62 @@`
		`+ Name: pythonname`
		`+ Version: 0`
		`+ Release: 0`
		`+ Summary: ...`
		`+ License: MIT`
		`+ BuildArch: noarch`
		`+`
		`+ %description`
		`+ ...`
		`+`
		`+ %install`
		`+ touch %{buildroot}/something`
		`+ touch %{buildroot}/something_else`
		`+ touch %{buildroot}/something_completely_different`
		`+`
		`+`
		`+ %package -n python-foo`
		`+ Summary: ...`
		`+ %description -n python-foo`
		`+ ...`
		`+ %files -n python-foo`
		`+ /*`
		`+`
		`+`
		`+ %package -n python2-foo`
		`+ Summary: ...`
		`+ %description -n python2-foo`
		`+ ...`
		`+ %files -n python2-foo`
		`+ /*`
		`+`
		`+`
		`+ %package -n python3-foo`
		`+ Summary: ...`
		`+ %description -n python3-foo`
		`+ ...`
		`+ %files -n python3-foo`
		`+ /*`
		`+`
		`+`
		`+ %package -n python%{python3_version_nodots}-foo`
		`+ Summary: ...`
		`+ %description -n python%{python3_version_nodots}-foo`
		`+ ...`
		`+ %files -n python%{python3_version_nodots}-foo`
		`+ /*`
		`+`
		`+`
		`+ %package -n python35-foo`
		`+ Summary: ...`
		`+ %description -n python35-foo`
		`+ ...`
		`+ %files -n python35-foo`
		`+ /*`
		`+`
		`+`
		`+ %package -n ruby-foo`
		`+ Summary: ...`
		`+ %description -n ruby-foo`
		`+ ...`
		`+ %files -n ruby-foo`
		`+ /*`

tests/tests.yml

file modified

		`@@ -16,6 +16,9 @@`
		`- pythonabi:`
		`dir: .`
		`run: ./pythonabi.sh`
		`+ - pythonname:`
		`+ dir: .`
		`+ run: ./pythonname.sh`
		`required_packages:`
		`- rpm-build`
		`- python3-devel`

churchyard commented 3 years ago

That should allow us to drop the %python_provide macro for most packages,
except where we want to use it for virtual provides or empty packages.

cc @ignatenkobrain @ngompa @pviktori @torsava

ngompa commented 3 years ago

:thumbsup:

pviktori commented 3 years ago

The idea sounds good, but it's the first time I see RPM generators...

pviktori commented on line 3 of pythonname.attr 3 years ago

https://rpm.org/user_doc/dependency_generators.html says that "new generators get called once per each file of a type". Does ^/ only match the root, or is pythonname.sh called for every installed file?

churchyard commented 3 years ago

If I understand things correctly, RPM pipes the list of matching files into the script. The script here ignores stdin completely.

churchyard commented 3 years ago

Does ^/ only match the root.

That would not work unless the package owns the root. This matches everything.

churchyard commented 3 years ago

Reading the docs more, this indeed seem to be called for every file.

Let's wait for rpm 4.16 and rewrite it as a parametric macro generator in lua?

Alternatively, we might just match dist-info / egg-info files to make this faster, but it will only work for the wast majority of python3-foo called packages (not all).

Edited 3 years ago by churchyard

churchyard commented 3 years ago

@pmatilai, @ffesti Assuming the RPM 4.16 change gets approved, do you have an idea when the first builds can hit rawhide (aka is the change process the only thing blocking this)?

churchyard commented 3 years ago

I've looked at the Lua macros and I think we can reuse the %python_provide macro there, not to have the "rules" defined in two places \o/

rebased onto 6b4d0da7970097e25fbd34a34fb954b039ae1df7

3 years ago

churchyard commented 3 years ago

I've reworked this to use the parametric macro generator that calls %python_provide internally. I've nerdslipped and prematurely optimized this as well. Also, there is a CI test, because historically testing this package has been a horror.

I've removed the changelog entry, I intent to rebase this on top of https://src.fedoraproject.org/rpms/python-rpm-generators/pull-request/9

Edited 3 years ago by churchyard

rebased onto df5402791d3685ca97701f4043b43357db15f186

3 years ago

churchyard commented 3 years ago

I also decided to not do this with the obsoletes, because I don't consider them vital at this point and packagers who will ditch explicit %python_provide calls will see if something goes to hell without them.

(The obsoletes were good when the mass renaming form python-foo to python2-foo happened and when the macro changed from python2-foo to python3-foo being "default". We can have another discussion to remove them from %python_provide now.)

I intent to go more verbose in the commit message (and mention this) when rebasing.

Edited 3 years ago by churchyard

rebased onto ce2e2bb7bfcefc6033074b40969e3a5281a1f96c

3 years ago

rebased onto 52d08e3321879ffdd780ab4a7f7722e1340e7a98

3 years ago

rebased onto b3369cbb532a3b9c6fa91521e1bd8a64b9bcdfed

3 years ago

ngompa commented 3 years ago

This looks okay to me.

torsava commented on line 3 of pythonname.attr 3 years ago

And %1 will always be there, because the macro is always launched with some file name (which we ignore) as %1, right? If so, I think it deserves a comment for readability.

churchyard commented on line 3 of pythonname.attr 3 years ago

yes and yes and ok

torsava commented 3 years ago

Besides that, the code looks good!

rebased onto bbfe493

3 years ago

churchyard commented 3 years ago

I've changed the comments while rebasing this.

churchyard commented 3 years ago

I'll also enable @zuul

torsava commented 3 years ago

Comments looks good. I say merge at will, @churchyard.

churchyard commented 3 years ago

I say we build a package that has bazillion of files. and see how slow this is, before we ship it.

churchyard commented 3 years ago

100 000 files and you can blink twice. It takes ten times that to even create the files.

Here you go in case you are interested:

Name:           python3-speedcheck
Version:        0
Release:        0
Summary:        ...
License:        MIT
BuildArch:      noarch
BuildRequires:  python3-devel

# turn off all BRP scripts, some of them are way to slow for this
%global __arch_install_post %{nil}
%global __os_install_post %{nil}

%description
...

%prep
cat > create_files.py << EOF
import pathlib
for i in range(100000):
    pathlib.Path(f'%{buildroot}%{python3_sitelib}/{i}.py').touch() 
EOF

%install
mkdir -p %{buildroot}%{python3_sitelib}
%{python3} create_files.py

%files
%{python3_sitelib}/*.py

(Don't even try to run this with pythondist.sh.)

Would try with a million, but mock says rpmbuild ends with exit code 255.

Edited 3 years ago by churchyard

churchyard commented 3 years ago

Will build, ship, announce and amend the packaging guidelines.

Pull-Request has been merged by churchyard

3 years ago