New Package: fish

Oliver Falk oliver at linux-kernel.at
Tue Jun 21 07:14:33 UTC 2005


Michael Schwendt wrote:
> On Mon, 20 Jun 2005 16:58:07 +0200, Axel Liljencrantz wrote:
> 
>>Aside from the above changes, I've also added
>>
>>provides: fish xsel mimedb
>>
>>to the spec file.
> 
> And these require an explanation, at least in the spec file.
> 
> For example, "Provides: %name = %version-%release" is automatic,
> so your one would be redundant and less accurate (= non-versioned).
> 
> [...]
> 
> Btw, caution! Adding things during a review often asks for trouble as your
> current reviewer may not agree with changes he didn't propose.

Correct. fish in provides is absolutely useless. xsel is a binary in 
your RPM /usr/bin/xsel, so if you really need to Require it somewhere 
else, it's better to require /usr/bin/xsel, as RPM dep checking or the 
dep resolving mechs from yum/up2date are intelligent enough to find the 
package where this binary is included. Same for mimedb! Please

Please do also remove the Tags Packager and Vendor.

BuildRoot has been documented in Wiki this way:
%{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)

Btw, indentations - I think - would be fine so it's better readable.

* Remove the blank line after %description - please.

For the %doc-section I'm currently not sure what's in Wiki, but I 
believe, that /usr/share/doc/%{name} is not good. And %doc-ing the 
man-packages is not needed, as this is done automatically.

* Make use of macros whereever you can. /etc/ is better written as 
%{_sysconfdir} and /usr/bin is %{_bindir}. /usr/man should be moved to 
/usr/share/man and it's macroed as %{_mandir}.

* /etc/fish.d should not be %config, but %dir.

* /etc/fish and /etc/fish_inputrc might be also missing a %config directive.

* /usr/bin/* should get an %attr(0755,root,root) I think.

>>The new version is available here:
>>
>>http://roo.no-ip.org/fish/fedora/fish-1.11.1-2.src.rpm

Please check the above mentioned fixes. And the fixes down here added by 
Michael - I had no look at them, might be, that some of my point are 
redundant with those.

> In addition to the comments done by earlier reviewers, quite some
> clean-up is needed:
> 
>  * No Packager or Vendor tags in a spec file, please. These belong
>    into your local build environment only.
> 
>  * "License: GPL"? No. The C source files disagree. => LGPL
> 
>  * Use %configure instead of "./configure --prefix=...". rpmlint even
>    warns about that. The %configure macro sets prefix, libdir and
>    friends for you.
> 
>  * %install section is missing "rm -rf $RPM_BUILD_ROOT" at the
>    beginning.
> 
>  * Your %postun script will break upgrades, as it will be executed
>    last. Look like e.g. "bash" does it.
> 
>  * Manual pages are installed into %_mandir and are flagged %doc
>    automatically. Use %_mandir not /usr/man
> 
>  * Use the other macros like %_datadir (/usr/share), %_bindir (/usr/bin)
>    %_sysconfdir (/etc)
> 
>  * Package doesn't build with $RPM_OPT_FLAGS, because it overrides CFLAGS
>    with its own flags.
> 
>  * Package fails to build. Missing at least "Buildrequires: ncurses-devel"
> 
>  * (RFE: Is it possible to install documentation files into %_docdir like
>    most other packages? i.e. %_datadir/doc/%name-%version)
> 
>  * Package fails to install:
> 
> Preparing...                ########################################### [100%]
>    1:fish                   ########################################### [100%]
> /home/qa/tmp/rpm/tmp/rpm-tmp.28773: line 11: syntax error: unexpected end of file
> error: %post(fish-1.11.1-2.i386) scriptlet failed, exit status 2
> 

Best,
  Oliver




More information about the fedora-extras-list mailing list