Re: [PATCH liburing 3/3] spec: Fedora RPM cleanups

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 11/4/19 5:05 AM, Stefan Hajnoczi wrote:
> From: Jeff Moyer <jmoyer@xxxxxxxxxx>
> 
> Cole Robinson and Fabio Valenti made a number of suggestions for the
> .spec file:
> https://bugzilla.redhat.com/show_bug.cgi?id=1766157
> 
>   * Release should be Release: 1%{?dist} so the .fcXX bits get appended to the version string
>   * Source: should be a pointer to the upstream URL that hosts the release. In this case I think it should be https://github.com/axboe/liburing/archive/%{name}-%{version}.tar.gz#%{name}-%{name}-%{version}.tar.gz, the ending weirdness is due to github renaming the archive strangely. You might need to pass '-n %{name}-%{name}-%{version}' to %setup/%autosetup to tell it what the extracted archive name is
>   * The %defattr lines should be removed: https://pagure.io/packaging-committee/issue/77
>   * The Group: lines should be removed
>   * All the BuildRoot and RPM_BUILD_ROOT lines should be removed. %clean should be removed
>   * The ./configure line should be replaced with just %configure
>   * The 'make' call should be %make_build
>   * The 'make install' call should be %make_install
>   * The %pre and %post sections can be entirely removed, ldconfig is done automatically: https://fedoraproject.org/wiki/Changes/Removing_ldconfig_scriptlets
>   * The devel package 'Requires: liburing' should instead be: Requires: %{name} = %{version}-%{release}
>   * The devel package should also have Requires: pkgconfig
>   * I think all the %attr usage can be entirely removed, unless they are doing something that the build system isn't doing.
>   * The Provides: liburing.so.1 shouldn't be necessary, I'm pretty sure RPM automatically adds annotations like this
>   * Replace %setup with %autosetup, which will automatically apply any listed Patch: in the spec if anything is backported in the future. It's a small maintenace optimization
> 
> These changes work on Fedora 31 and openSUSE Leap 15.1.  Therefore they
> are likely to work on other rpm-based distributions too.

This looks like a spec file changelog. Nothing wrong with that, but it
should be no wider than 72 characters to avoid making git log look like
a mess. Normally I'd just fix that up, but another comment below.

> -URL: http://git.kernel.dk/cgit/liburing/
> +URL: http://brick.kernel.dk/snaps/%{name}-%{version}.tar.gz
> +BuildRequires: gcc

I enabled snapshots in cgit, so maybe we can just point at those?
Something like:

URL: https://git.kernel.dk/cgit/liburing/snapshot/%{name}-%{version}.tar.gz

Yes, and there's also https now, finally got that enabled on kernel.dk
after running that site for more than two decades.

>   %changelog
> +* Thu Oct 31 2019 Jeff Moyer <jmoyer@xxxxxxxxxx> - 0.2-1
> +- Initial fedora package.
> +

Maybe keep this distro agnostic?

-- 
Jens Axboe




[Index of Archives]     [Linux RAID]     [Linux SCSI]     [Linux ATA RAID]     [IDE]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Device Mapper]

  Powered by Linux