PR#4297: koji.spec Makefile /usr/sbin everywhere - koji

koji

#4297 koji.spec Makefile /usr/sbin everywhere

Merged a month ago by tkopecek. Opened 2 months ago by jcupova.

jcupova/koji issue-4296 into master

koji.spec Makefile /usr/sbin everywhere

Jana Librova • a month ago

f67b9db

koji.spec

file modified

+9 -3

		`@@ -385,6 +385,11 @@`

		`%install`
		`rm -rf $RPM_BUILD_ROOT`
		`+ # The Makefiles hardcode /usr/sbin everywhere`
		`+ %if "%{_sbindir}" == "%{_bindir}"`
		`+ mkdir -p %{buildroot}%{_prefix}`
		`+ ln -sf --relative %{buildroot}%{_bindir} %{buildroot}/usr/sbin`
		`+ %endif`

		`%if 0%{py2_support} < 2 && 0%{py3_support} < 2`
		`echo "At least one python must be built with full support"`
		`@@ -497,6 +502,7 @@`
		`rm -rf $RPM_BUILD_ROOT`

		`%files`
		`+ %exclude /usr/sbin`
		`%config(noreplace) /etc/koji.conf`
		`%dir /etc/koji.conf.d`
		`%doc docs Authors COPYING LGPL`
		`@@ -709,7 +715,7 @@`
		`- PR#4164: better default handling for getMultiArchInfo`
		`- PR#4181: Fix a typo in the kiwi image type attribute override patch`
		`- PR#4184: kiwi: Add support for overriding version and releasever`
		`- - PR#4186: Basic tests for kiwi plugin`
		`+ - PR#4186: Basic tests for kiwi plugin`
		`- PR#4192: allow None in repoInfo for backwards compat`

		`* Mon May 6 2024 Tomas Kopecek <tkopecek at redhat.com> - 1.34.1-1`
		`@@ -1095,7 +1101,7 @@`
		`- PR#3104: Make setup.py executable`
		`- PR#3113: error function instead of print with sys.exit in CLI commands`
		`- PR#3115: Add and update CLI unit tests`
		`- - PR#3118: handle dictionary parameter in get_tag()`
		`+ - PR#3118: handle dictionary parameter in get_tag()`
		`- PR#3138: doc: improve protonmsg SSL parameter descriptions`
		`- PR#3139: www: style channelinfo hosts table`
		`- PR#3142: devtools: print fakeweb listening URL`
		`@@ -1120,7 +1126,7 @@`
		`- PR#2898 hub: fix SQL condition`
		`- PR#2900 kojiweb - Fix getting tag ID for buildMaven taskinfo page.`
		`- PR#2906 lib: return taskLabel for unknown tasks`
		`- - PR#2916 [policy] use "name" in result of lookup_name for CGs`
		`+ - PR#2916 [policy] use "name" in result of lookup_name for CGs`

		`* Mon May 10 2021 Tomas Kopecek <tkopecek at redhat.com> - 1.25.0-1`
		`- PR#2844: protonmsg: use consistent data format for messages`

jcupova commented 2 months ago

Fixes: https://pagure.io/koji/issue/4296

rebased onto b7d193c

2 months ago

mikem commented 2 months ago

See comments in #4296

rebased onto b7d193c

2 months ago

jcupova commented 2 months ago

Sorry, I didn't have any time for fix it earlier than today. But now, it fixed. I tried to build it for fedora-rawhide, f41, f40, f39, rhel-7, rhel8, rhel9 and everything is working now :-)

Edited 2 months ago by jcupova

mikem commented 2 months ago

We need a clearer problem statement before we move forward with changes. The Fedora change is not supposed to necessitate sweeping packaging changes.

zbyszek commented a month ago

No, please don't do this.

We want the files listed in package contents to move from /usr/sbin to /usr/bin. This is necessary so that upgraded systems can finish the sbin-merge. Once all files in /usr/sbin are symlinks, the directory itself is replaced by a symlink. If we have packages that keep installing files under /usr/sbin, this process will never finish.

Instead, fix the installation process to put files in the configured location. In some cases it's just a matter of setting some additional option on the %make_install line. Othertimes it's just easier to do a mv at the end %install.

For files that are packaged and move from /usr/sbin, both /usr/sbin/foo and /usr/bin/foo will remain equivalent, during the merge and after. So it's fine if scripts or systemd unit files, etc., use the old path.

zbyszek commented a month ago

I started to write a PoC patch, but then I found a commit from last year ;(
https://src.fedoraproject.org/rpms/koji/pull-request/25 seems to solve the issue.

mikem commented a month ago

thanks, that's much easier than fixing the Makefiles ;)

mikem commented a month ago

@zbyszek this version was definitely not going to get merged. See comments in #4296

mikem commented a month ago

@tkopecek the change that zbyszek linked above looks like a reasonable approach to me, short of a significant Makefile overhaul. wdyt?

rebased onto 5e1584e

a month ago

jcupova commented a month ago

I tested @zbyszek changes in koji.spec on our side and it's working, so I updated this PR like @zbyszek idea.

salimma commented a month ago

See https://pagure.io/koji/pull-request/4324 for an alternate implementation that allows the Makefile to be passed an SBINDIR override

salimma commented a month ago

(I did not update the sample spec in the Koji repo yet, but I tested it with my spec PR and that works fine https://src.fedoraproject.org/rpms/koji/pull-request/26

Either way we should probably pick one implementation :)

mikem commented a month ago

My gut feeling is "first one then the other"

This one seems to work. It's easy and safe, and the f42 freeze is just around the corner.

PR #4324 is a better approach, but somewhat more complicated and needs more work. Taking this doesn't block us from adopting the other solution in the future (perhaps along with additional macro values honored by the makefiles)

tkopecek commented a month ago

Agreed, let's use this one for now (1.36) and work further in #4324 for next release.

Metadata Update from @tkopecek:
- Pull-request tagged with: testing-ready

a month ago

tkopecek commented a month ago

In light of f42 freeze - do we want to push this one (with no_qe) to 1.35.2?

jcupova commented a month ago

@tkopecek I guess that we can say, that this is tested. I run jenkins job, which setup Koji branch with all testing-ready, testing-done and no_qe tests and when this is setup, it start copr build with F40, F41, F42, F-rawhide, rhel7, rhel8, rhel9 and it looks working. Without this commit, F42 and F-rawhide failed. So I thing, we can move it to 1.35.2.
https://copr.devel.redhat.com/coprs/brew-qe-team/koji-biweekly/build/118941/

Metadata Update from @tkopecek:
- Pull-request tagged with: testing-done

a month ago

tkopecek commented a month ago

Commit 28159f6 fixes this pull-request

Pull-Request has been merged by tkopecek

a month ago