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

Re: New Package: fish



Hello Michael,

Thank you for your input. I'm sorry for the delay in my reply, but
while I was writing my reply, my appendix burst, and I've spent the
last few days recovering from surgery.

On 6/20/05, Michael Schwendt <bugs michael gmx net> 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.

Sorry, blame it on inexperience. Removed the line again.

> 
> > The new version is available here:
> >
> > http://roo.no-ip.org/fish/fedora/fish-1.11.1-2.src.rpm
> 
> In addition to the comments done by earlier reviewers, quite some
> clean-up is needed:

I think all previous comments are taken care of, but you have found
quite a few more problems. I try to addresws all of them in this
email.

> 
>  * No Packager or Vendor tags in a spec file, please. These belong
>    into your local build environment only.

Removed.

> 
>  * "License: GPL"? No. The C source files disagree. => LGPL

All code written by me is released under the GPL. This is the vast
majority of the code. Fish also includes code from other sources:

* The mimedb command uses the xdgmime library which is dual licensed
under Academic and LGPL.

* The xsel command is relesed under the MIT license.

* Fish contains two functions that are derived from BSD functions.

If I'm not mistaken, all the above can be relicensed under the GPL, so
long as a copyright notice remains. The license section of the fish
documentation, which is distributed in the RPM, notify the user of
this, as the license requires.

Have I missed something?

> 
>  * Use %configure instead of "./configure --prefix=...". rpmlint even
>    warns about that. The %configure macro sets prefix, libdir and
>    friends for you.

Ok. Thank you for the suggestion.

> 
>  * %install section is missing "rm -rf $RPM_BUILD_ROOT" at the
>    beginning.

Fixed. Thank you for the suggestion.

> 
>  * Your %postun script will break upgrades, as it will be executed
>    last. Look like e.g. "bash" does it.

I run fedora myself, and I've upgraded hundreds of time over the last
few months, without any issues. The fish entry is not removed after an
upgrade.

I checked what bash does, and the only difference is that the editing
of /etc/shells is conditioned on

if [ "$1" = 0 ]; then

which I don't know what it does, since I haven't been able to find out
what parameters are sent to the postun script. Sorry, but I don't own
any books on RPM.

Can you elaborate on the problem?

>  * Manual pages are installed into %_mandir and are flagged %doc
>    automatically. Use %_mandir not /usr/man

Changed.

> 
>  * Use the other macros like %_datadir (/usr/share), %_bindir (/usr/bin)
>    %_sysconfdir (/etc)

Done.

> 
>  * Package doesn't build with $RPM_OPT_FLAGS, because it overrides CFLAGS
>    with its own flags.

fish uses some c99 features, I've had issues with some gcc version
unless --std=gnu99 is specified. fno-strict-aliasing is there to
remove warnings on Alpha. I fear that fish will not build correctly in
all cases without gnu99.

> 
>  * Package fails to build. Missing at least "Buildrequires: ncurses-devel"

Added ncurses-devel and xorg-x11-devel, which is also needed by the
xsel command. Thank you for pointing this out.

> 
>  * (RFE: Is it possible to install documentation files into %_docdir like
>    most other packages? i.e. %_datadir/doc/%name-%version)

This required minor changes to the source, but the documentation
directory is now specified by;

%configure docdir=%_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

Oops. Fixed. Thanks.

> 
> --
> Fedora Core release 4 (Stentz) - Linux 2.6.11-1.1369_FC4
> loadavg: 1.75 1.18 1.06
> 
> --
> fedora-extras-list mailing list
> fedora-extras-list redhat com
> https://www.redhat.com/mailman/listinfo/fedora-extras-list

The new srpm can be found here:

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

Let me know if there are any more issues.

--
Axel


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