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

Re: New Package: fish



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


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