https://bugzilla.redhat.com/show_bug.cgi?id=2106611
--- Comment #10 from Stefano Brivio <sbrivio(a)redhat.com> ---
(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-7b710946b152fabab0f3c838e5518576be...
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(a)redhat.com> -
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