openSUSE / python-rpm-macros

Multi-Python, Single-Spec macros generator
Other
22 stars 20 forks source link

Install libalts with python_clone instead of `%pre` #135

Closed bnavigator closed 2 years ago

bnavigator commented 2 years ago

Closes #133. Although I would not have called that "wrong".

AdamMajer commented 2 years ago

Something is not correct. The files seem to not be created.

[   26s]     Directory not found: /home/abuild/rpmbuild/BUILDROOT/python-chardet-4.0.0-79.1.x86_64/usr/share/libalternatives/chardetect
[   26s]     File not found: /home/abuild/rpmbuild/BUILDROOT/python-chardet-4.0.0-79.1.x86_64/usr/share/libalternatives/chardetect/38.conf

https://build.opensuse.org/project/show/home:adamm:branches:devel:languages:python

bnavigator commented 2 years ago

Chardet had some unorthodox alternatives creation. The automatism for python stuff is only in %python_clone -a, not %prepare_alternative: https://build.opensuse.org/request/show/986862

AdamMajer commented 2 years ago

This looks much much better now, thank you.

Just one possible nit-pick is to remove the unused scriptlet sections altogether. Is this possible? Now I see,

> rpm -qp --scripts python310-chardet-4.0.0-81.1.noarch.rpm
preinstall scriptlet (using /bin/sh):
# If libalternatives is used: Removing old update-alternatives entries.

if [ "$1" -gt 0 ] && [ -f /usr/sbin/update-alternatives ]; then 
    update-alternatives --quiet --remove "chardetect" "/usr/bin/chardetect-3.10" 
fi
postinstall scriptlet (using /bin/sh):

: # no install scriptlet action for libalternatives
postuninstall scriptlet (using /bin/sh):

: # no uninstall scriptlet action for libalternatives

Ideally the %pre stays while the %post and %postun are completely removed. I'm assuming that if these scriptlets are present, they get unnecessarily run, or am I wrong on this?

bnavigator commented 2 years ago

Ideally the %pre stays while the %post and %postun are completely removed. I'm assuming that if these scriptlets are present, they get unnecessarily run, or am I wrong on this?

They are no-ops. Their non-emptyness is necessary, because otherwise you would have

%post

%files

Which throws an error.

And if you remove the %python_install_alternatives from the specfile, you stop supporting older distributions without libalternatives (the bcond is there for a reason).

AdamMajer commented 2 years ago

I meant, when the %post and %postun would be empty, don't write it to the spec file. Then they are no longer in the RPM.

And yes, of course, they should be there when using update-alternatives.

bnavigator commented 2 years ago

We can't encapsulate the %post and %postun directives into the alternatives macros. The subpackages rewriter needs to see them in the source specfile. Note that the effective build is from a generated

%post -n pythonXY-foo
%pythonXY_install_alternative cmd
bnavigator commented 2 years ago

Of course you can always do

%if %{with libalternatives}
%pre
%python_libalternatives_reset_alternative cmd

%else

%post
%python_install_alternative cmd

%postun
%python_uninstall_alternative cmd

%endif

But that is package specific and can't be introduced in general by the macros

AdamMajer commented 2 years ago

Yes, of course. Somehow my brain didn't see the %post and %postun in the spec file itself. :exploding_head:

AdamMajer commented 2 years ago

This patch works well and definitely addresses the main issue with these files being generated at the wrong time.

mcepl commented 2 years ago

@bnavigator Thank you.