New Package: fish

[Oliver Falk]

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 at 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