New member, package review, questions about template .spec

[Ken Dreyer]

On Mon, Jan 23, 2012 at 12:45 AM, Philip Prindeville
<philipp_subx at redfish-solutions.com> wrote:
> I've been developing packages for 6 or 7 years now, and I'm a contributor to Fedora (and occasionally EPEL).

Welcome to the RPM Fusion dev community!

> Anyway, so I've taken their driver and repackaged it for rpmfusion.  My first cut is here:
>
> ftp://ftp.redfish-solutions.com/pub/perle-serial-kmod-3.8.0-6.1.fc16.1.src.rpm

Thank you for your interest in contributing. Would you mind signing up
for bugzilla.rpmfusion.org, and submitting a new review request? We
can continue the review for your package there.  The steps for setting
up a new review bug are almost identical to Fedora. The instructions
are at http://rpmfusion.org/Contributors, under "3.2. Create a package
review request".


> Anyway, I noticed the following things I had to change to the .spec file I cut and pasted from the Wiki to get it to work:
>
> (1) added the lines
>
> %define repo   rpmfusion
> %define _name  foo
>
> as %{repo} is undefined otherwise. This might be a glitch in my installing the rpmfusion stuff improperly.

Yeah, I think this may be an anachronism from the time when several
repos were merging together. I see some packages have just hardcoded
"--repo rpmfusion" in the kmodtool invocation.

Anyone else have opinions on this? Should "rpmfusion" be hardcoded in
the kmodtool invocation, or should we use a %define at the top of each
file? My vote is for hardcoding, because it's less to read.


> (2) modified the line
>
> Release:        1{?dist}.1
>
> there's a missing "%" before the "{".

Yes, this should be changed on the wiki. Done, in
http://rpmfusion.org/Packaging/KernelModules/Kmods2?action=recall&rev=19

> Name:          %{_name}-kmod

I don't see the string "_name" anywhere on the wiki page, so I'm not
sure about this one... I see openafs uses "%define kmod_name openafs",
and other packages just hardcode the shortname everywhere. I think
it's probably up to you what you want to do for the Package's
"shortname".


> (3) modified the lines:
>
> # needed for plague to make sure it builds for i586 and i686
> ExclusiveArch:  i586 i686 x86_64 ppc ppc64
>
> not sure why "ppc" and "ppc64" are included... that seems to contradict the accompanying comment (which probably should mention x86_64 but doesn't).

Yeah, I'm not a Plague expert, so I am not sure why this is needed, or
if it is still necessary.


> (4) added the lines:
>
> %package common

So the issue here is that "-common" is supposed to be provided by the
userland package. The -kmod package doesn't build or provide this at
all. See openafs.spec for an example:

55 	Provides: %{name}-kmod-common = %{version}

Maybe openafs isn't the best example here, since that builds a few
different userland subpackages. open-vm-tools.spec is another example:

36 	Provides: open-vm-tools-kmod-common = %{version}

Basically the main userland package should "Provide" the -common package.


> (5) commented out this line:

No need to comment that out that I can see.


> (6) modified the loop:
>
> for kernel_version in %{?kernel_versions} ; do
>    cp -a foo-%{version} _kmod_build_${kernel_version%%___*}
> done
>
> to use %{_name} instead of "foo".

As mentioned above, as far as I know that's a style thing, up to you.


> (7) modified the loop:
>
> for kernel_version in %{?kernel_versions}; do
>    make install DESTDIR=${RPM_BUILD_ROOT} KMODPATH=%{kmodinstdir_prefix}/${kernel_version%%___*}/%{kmodinstdir_postfix}
>    # install -D -m 755 _kmod_build_${kernel_version%%___*}/foo/foo.ko  ${RPM_BUILD_ROOT}%{kmodinstdir_prefix}/${kernel_version%%___*}/%{kmodinstdir_postfix}/foo.kmod
> done
>
> this shouldn't install into .../foo.kmod but into ".../foo.ko" instead.

Good catch; I've fixed this in
http://rpmfusion.org/Packaging/KernelModules/Kmods2?action=recall&rev=20.


> (8) added the lines to the %install section:
>
> mkdir -p ${RPM_BUILD_ROOT}%{_sysconfdir}/modprobe.d/
> %{__cp} -p %{SOURCE1} ${RPM_BUILD_ROOT}%{_sysconfdir}/modprobe.d/%{_name}.conf

That is appropriate for your package, but not all packages will need
to do this. For example OpenAFS loads the module in the init script.
You can feel free to add a section to the Kmods2 wiki page describing
this technique, though.

- Ken