https://bugzilla.redhat.com/show_bug.cgi?id=2106611 Daniel Berrangé <berrange@xxxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |berrange@xxxxxxxxxx --- Comment #9 from Daniel Berrangé <berrange@xxxxxxxxxx> --- (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) 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. > # 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. > %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 > %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. 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 > %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. -- 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
