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

Re: New Package: fish



On 06/27/2005 10:09 PM, Axel Liljencrantz wrote:
Hello Oliver,

Thanks for the comments. I'm sorry for not answering your message any
sooner, but I've been unwell.

On 6/21/05, Oliver Falk <oliver linux-kernel at> wrote:

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


Sorry, I removed the offending line.


Please do also remove the Tags Packager and Vendor.


Done.


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.


Where, how? Is there any preferred indentation style, etc?


* Remove the blank line after %description - please.


Done.


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.


Fixed.


* 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}.


Ok.


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


Done. I also made sure the contents of the above directory is set to %config.


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


You are correct. Fixed.


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


Ok. I thought that would be the default, but I added it anyway.


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.


I have. The new version is:

http://roo.no-ip.org/fish/fedora/fish-1.11.1-4.src.rpm

If there are any more issues, please let me know.


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

And I forgot, the release tag is missing the %{?dist} macro.

Best,
 Oliver


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