https://bugzilla.redhat.com/show_bug.cgi?id=2106611 --- Comment #10 from Stefano Brivio <sbrivio@xxxxxxxxxx> --- (In reply to Daniel Berrangé from comment #9) > (In reply to Stefano Brivio from comment #8) > > Thanks for all the reviews! I made a number of changes, and I finally have > > an updated spec files hopefully addressing all the pending comments. > > > > Spec URL: > > https://download.copr.fedorainfracloud.org/results/sbrivio/passt/fedora- > > rawhide-x86_64/04752883-passt/passt.spec > > SRPM URL: > > https://download.copr.fedorainfracloud.org/results/sbrivio/passt/fedora- > > rawhide-x86_64/04752883-passt/passt-0.git.2022_08_21.7b71094-1.fc38.src.rpm > > This versioning scheme doesn't match documented rules IMHO: > > https://docs.fedoraproject.org/en-US/packaging-guidelines/Versioning/ > #_snapshots > > My interpretation of the rules is that it is expected to look like: > > 0^20220821.git7b71094 > > ('git' can be reduced to just 'g' if desired) I see now -- I didn't consider the "Snapshots" guidelines and observed the "Simple versioning" rules, but upstream git tags don't really qualify as releases I guess. Patch posted upstream. > In the first line of the spec it says > > > # SPDX-License-Identifier: AGPL-3.0-or-later > > While it is allowed, it is really very unusual to put an explicit license on > the spec file. Every spec I've ever looked at has no license and is thus > considered to be MIT licensed > > https://fedoraproject.org/wiki/Licensing:Main#License_of_Fedora_SPEC_Files > > I think that it is preferrable to stick with normal Fedora practice, given > that parts of the spec file are commonly cargo cult copied across packages > when new rules are applied. Dropped (patch posted upstream). This is something I originally added to make the project compliant with the REUSE specification (https://reuse.software/spec/), but there are now a few other files where adding explicit licensing information isn't really practical. I guess I'll try again later on with that. > > # contrib/fedora/passt.spec - Example spec file for fedora > > This line can go, it isn't an example spec, it is the actual official spec. > > > > Source: https://passt.top/passt/snapshot/passt-7b710946b152fabab0f3c838e5518576beb9020c.tar.xz > > This commit hash is used in multiple places, so best practice would be to > define this at the top of the file: > > %global git_hash 7b710946b152fabab0f3c838e5518576beb9020c > %global git_shorthash 7b71094 > > and then reference %{git_hash} or %{git_shorthash} throughout the file. Patch posted, defining %{git_hash}, not %{git_shorthash} because it appears only once, and it's actually populated by an rpkg macro that outputs the full version, so splitting that out looks a bit awkward. > > %package selinux > > BuildArch: noarch > > Summary: SELinux support for passt and pasta > > Requires: %{name} = %{version} > > This Requires should be fully versioned, ie > > Requires: %{name} = %{version}.%{release} > > See > https://docs.fedoraproject.org/en-US/packaging-guidelines/ > #_requiring_base_package Fixed (with "%{version}-%{release}"), patch posted upstream. > > %if 0%{?suse_version} > 910 > > %make_install DESTDIR=%{buildroot} prefix=%{_prefix} docdir=%{_prefix}/share/doc/packages/passt > > %else > > %make_install DESTDIR=%{buildroot} prefix=%{_prefix} > > %endif > > Fedora specs would generally only be expected to contain conditionals for > Fedora or RHEL, not any other RPM based distro, since Fedora & RHEL share > the same packaging guidelines, but other distros do their own thing. I'm using the same spec file on Copr also for CentOS, Mageia and OpenSUSE Tumbleweed builds, and I find that really convenient, but sure, I see your point. In any case, I dropped that, because: > Also setting prefix only affects one of the many variables your makefile > uses: > > prefix ?= /usr/local > exec_prefix ?= $(prefix) > bindir ?= $(exec_prefix)/bin > datarootdir ?= $(prefix)/share > docdir ?= $(datarootdir)/doc/passt > mandir ?= $(datarootdir)/man > man1dir ?= $(mandir)/man1 > > I'd expect to override the ones relevant to 'install' functionality at > least, even if Makefile's current defaults happen to match Fedora's > > %make_install prefix=%{_prefix} bindir=%{_bindir} mandir=%{_mandir} > docdir=%{_docdir}/passt ...this suggestion should actually fix the issue I had with OpenSUSE Tumbleweed as well. Patch pending upstream now. > > %changelog > > * Sun Aug 21 2022 Stefano Brivio <sbrivio@xxxxxxxxxx> - 0.git.2022_08_21.7b71094-0 > > - Use more GNU-style directory variables, explicit docdir for OpenSUSE > > ...snip... > > Consider if you wish to use 'Release: %autorelease' and %autochangelog > which populates from git commit messages. I didn't know about those, thanks for mentioning them. %autochangelog, however, is a bit limited compared to the current rpkg macro I'm using: most importantly, it doesn't generate links to upstream changes, which, I think, are a nice addition. I left this part as it was. -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component https://bugzilla.redhat.com/show_bug.cgi?id=2106611 _______________________________________________ package-review mailing list -- package-review@xxxxxxxxxxxxxxxxxxxxxxx To unsubscribe send an email to package-review-leave@xxxxxxxxxxxxxxxxxxxxxxx Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/package-review@xxxxxxxxxxxxxxxxxxxxxxx Do not reply to spam, report it: https://pagure.io/fedora-infrastructure/new_issue
