New Package: fish

[Axel Liljencrantz]

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
> >
> 
> Best,
>   Oliver
> 
> --
> fedora-extras-list mailing list
> fedora-extras-list at redhat.com
> https://www.redhat.com/mailman/listinfo/fedora-extras-list
> 


--
Axel