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