Bug 585719

Just a few comments:

- You need to write in the specfile, how you got the sources and not 'I did 
  something and here they are'.

  e.g. there is a nice snipped /usr/bin/fedora-getsvn, maybe that's the easiest 
  way:
  'fedora-getsvn wxformbuilder $URL-TOSVN $REV'


- You could do %{__mkdir} -p %{buildroot}%{_datadir}/icons/hicolor/48x48/mimetypes
  Instead all the mkdirs above

- You should not use a vendor tag:
  https://fedoraproject.org/wiki/Packaging/Guidelines#desktop-file-install_usage

(In reply to comment #1)
> Just a few comments:
> 
> - You need to write in the specfile, how you got the sources and not 'I did 
>   something and here they are'.
> 
>   e.g. there is a nice snipped /usr/bin/fedora-getsvn, maybe that's the easiest 
>   way:
>   'fedora-getsvn wxformbuilder $URL-TOSVN $REV'

I can't use fedora-getsvn because I can't set the current version, but I added some infos on how to get the packaged revision.

> - You could do %{__mkdir} -p
> %{buildroot}%{_datadir}/icons/hicolor/48x48/mimetypes
>   Instead all the mkdirs above

Done

> - You should not use a vendor tag:
>  
> https://fedoraproject.org/wiki/Packaging/Guidelines#desktop-file-install_usage    

Done
Spec URL: http://dionysos.fedorapeople.org/SPECS/wxformbuilder.spec
SRPM URL:
http://dionysos.fedorapeople.org/SRPMS/wxformbuilder-3.1.68-2.fc12.src.rpm

> > - You could do %{__mkdir} -p
> > %{buildroot}%{_datadir}/icons/hicolor/48x48/mimetypes
> >   Instead all the mkdirs above
> 
> Done

Or add -D to install, which will create dirs as needed e.g.

%{__mkdir} -p %{buildroot}%{_datadir}/icons/hicolor/48x48/apps
install -p install/linux/data/gnome/usr/share/pixmaps/%{name}.png \
           %{buildroot}%{_datadir}/icons/hicolor/48x48/apps/%{name}.png

could be done as:

install -D -p install/linux/data/gnome/usr/share/pixmaps/%{name}.png \
           %{buildroot}%{_datadir}/icons/hicolor/48x48/apps/%{name}.png

you might add -m as well, to set correct mode:

install -m 0644 -D -p  install/linux/data/gnome/usr/share/pixmaps/%{name}.png \
           %{buildroot}%{_datadir}/icons/hicolor/48x48/apps/%{name}.png

One more comment,

when you create the tarball from svn sources I don't see the value
in providing a url to the tarball. In fact that can be misleading,
giving the impresssion http://dionysos.fedorapeople.org/SOURCES/
is some kind of offical tarball repo. 

I would just simplify to

Source0:         %{name}-%{version}.tar.bz2
Source1:         %{name}-ld.conf

%changelog
* Mon Apr 26 2010  Alain Portal <alain.portal[AT]univ-montp2[DOT]fr> 3.1.68-3
  - Remove url in source path
  - Improve some files installations using -D install option

Spec URL: http://dionysos.fedorapeople.org/SPECS/wxformbuilder.spec
SRPM URL:
http://dionysos.fedorapeople.org/SRPMS/wxformbuilder-3.1.68-3.fc12.src.rpm

Thanks.

- you are mixing  $RPM_BUILD_ROOT and %{buildroot}.
- use %{_prefix} for /usr in %install.

Still some work needed here:

rpmlint:

 - E: empty-debuginfo-package. 
   A related note: the build process is silent and it's not possible 
   to verify that correct build opts are being used during build.
   Please fix both.

 - W: no-cleaning-of-buildroot %install
   Not needed in later Fedoras?

 - W: invalid-url URL: http://wxformbuilder.org/ BadStatusLine('',)
   Strange, can't reproduce
  
 - E: binary-or-shlib-defines-rpath /usr/lib64/wxformbuilder/libwxadditions-mini.so ['$ORIGIN']
 - E: binary-or-shlib-defines-rpath /usr/bin/wxformbuilder ['$ORIGIN/../lib/wxformbuilder']
   Must be fixed

 - W: non-conffile-in-etc /etc/ld.so.conf.d/wxformbuilder.conf
   Add %config tag in %files

 - W: invalid-url Source0: wxformbuilder-3.1.68.tar.bz2
   ignore.


A first look on license indicate GPLv2+, not GPLv2. More on that later.

You can use xz (tar cJvf ...) to compress the tarball, saves 10%. 

And, in fact, the package don't build in koji, seems DSO related, logs here:
  http://koji.fedoraproject.org/koji/taskinfo?taskID=2140121

(In reply to comment #7)
> Still some work needed here:

It would be nice if you report problems once, and not only when I succeed to fix the one you reported before...

> rpmlint:
> 
>  - E: empty-debuginfo-package.

Unable to fix :-(

>    A related note: the build process is silent and it's not possible 
>    to verify that correct build opts are being used during build.
>    Please fix both.

I found a way to make the build process more verbose, but I don't think that help us...

>  - W: no-cleaning-of-buildroot %install
>    Not needed in later Fedoras?

I don't know if this is needed in later Fedora, but I sure if there is a cleanup before, install is not possible:

+ /bin/rm -rf /home/alain/Fedora/rpmbuild/BUILDROOT/wxformbuilder-3.1.68-4.fc12.x86_64
+ install/linux/wxfb_export.sh /home/alain/Fedora/rpmbuild/BUILDROOT/wxformbuilder-3.1.68-4.fc12.x86_64/usr
mkdir: cannot create directory `/home/alain/Fedora/rpmbuild/BUILDROOT/wxformbuilder-3.1.68-4.fc12.x86_64/usr': No such file or directory

>  - W: invalid-url URL: http://wxformbuilder.org/ BadStatusLine('',)
>    Strange, can't reproduce

I can :-(
Many problems to broese this site :-(

>  - E: binary-or-shlib-defines-rpath
> /usr/lib64/wxformbuilder/libwxadditions-mini.so ['$ORIGIN']
>  - E: binary-or-shlib-defines-rpath /usr/bin/wxformbuilder
> ['$ORIGIN/../lib/wxformbuilder']
>    Must be fixed

I don't how.

>  - W: non-conffile-in-etc /etc/ld.so.conf.d/wxformbuilder.conf
>    Add %config tag in %files

Fixed

>  - W: invalid-url Source0: wxformbuilder-3.1.68.tar.bz2
>    ignore.
> 
> 
> A first look on license indicate GPLv2+, not GPLv2. More on that later.

GGPL sayed: Version 2, June 1991, not "or later"..
I would be please...

> You can use xz (tar cJvf ...) to compress the tarball, saves 10%. 

????
x is for extract...

z is for bzip, j (as I used) is for bzip2...

> And, in fact, the package don't build in koji, seems DSO related, logs here:
>   http://koji.fedoraproject.org/koji/taskinfo?taskID=2140121    

Package build successfully in koji!
F12 : http://koji.fedoraproject.org/koji/taskinfo?taskID=2147057
F11 : http://koji.fedoraproject.org/koji/taskinfo?taskID=2147075

(In reply to comment #8)
> (In reply to comment #7)
> > rpmlint:
> > 
> >  - E: empty-debuginfo-package.
> 
> Unable to fix :-(

This package has a strange buildsystem, and doesn't honor %{optflags}, if it would, there would most likely be a debuginfo-package.
Maybe you could try to patch the buildsystem to respect it
(Just exporting CFLAGS or something is unlikely to work, but I didn't try that.)

> >  - E: binary-or-shlib-defines-rpath
> > /usr/lib64/wxformbuilder/libwxadditions-mini.so ['$ORIGIN']
> >  - E: binary-or-shlib-defines-rpath /usr/bin/wxformbuilder
> > ['$ORIGIN/../lib/wxformbuilder']
> >    Must be fixed
> 
> I don't how.

$ rpmlint -I binary-or-shlib-defines-rpath
binary-or-shlib-defines-rpath:
The binary or shared library defines `RPATH'. Usually this is a bad thing
because it hardcodes the path to search libraries and so makes it difficult to
move libraries around.  Most likely you will find a Makefile with a line like:
gcc test.o -o test -Wl,--rpath.  Also, sometimes configure scripts provide a
--disable-rpath flag to avoid this.

and: https://fedoraproject.org/wiki/Packaging/Guidelines#Removing_Rpath

> 
> >  - W: non-conffile-in-etc /etc/ld.so.conf.d/wxformbuilder.conf
> >    Add %config tag in %files
> 
> Fixed

(Not yet in the spec linked above)

> 
> >  - W: invalid-url Source0: wxformbuilder-3.1.68.tar.bz2
> >    ignore.
> > 
> > 
> > A first look on license indicate GPLv2+, not GPLv2. More on that later.
> 
> GGPL sayed: Version 2, June 1991, not "or later"..
> I would be please...

Look at a random *.h or *.cpp file. In the header you can find:
 * This program is free software; you can redistribute it and/or

 * modify it under the terms of the GNU General Public License

 * as published by the Free Software Foundation; either version 2

 * of the License, or (at your option) any later version.

> 
> > You can use xz (tar cJvf ...) to compress the tarball, saves 10%. 
> 
> ????
> x is for extract...
> 
> z is for bzip, j (as I used) is for bzip2...

He means: http://tukaani.org/xz/ another compressing format and not bzip.
All rpms are compressed with that and that compressing format is better that the other. If you choose to use xz, you could maybe not use that on EL (once had a problem with that, don't know if that's resolved.)

I created some patches to fix the debuginfo package, using correct buildflags
and some of the rpath problems, it's available here:

 http://terjeros.fedorapeople.org/wxformbuilder/

At first glance this seems like a simple package, however looking deeper there was a number of issues.

(In reply to comment #9)
> (In reply to comment #8)
> > (In reply to comment #7)
> > > rpmlint:
> > > 
> > >  - E: empty-debuginfo-package.
> > 
> > Unable to fix :-(
> 
> This package has a strange buildsystem,

Yes, it has!!!
It use premake (http://industriousone.com/premake) which use lua (http://www.lua.org/)

> and doesn't honor %{optflags}, if it
> would, there would most likely be a debuginfo-package.
> Maybe you could try to patch the buildsystem to respect it
> (Just exporting CFLAGS or something is unlikely to work, but I didn't try
> that.)
> 
> > >  - E: binary-or-shlib-defines-rpath
> > > /usr/lib64/wxformbuilder/libwxadditions-mini.so ['$ORIGIN']
> > >  - E: binary-or-shlib-defines-rpath /usr/bin/wxformbuilder
> > > ['$ORIGIN/../lib/wxformbuilder']
> > >    Must be fixed
> > 
> > I don't how.
> 
> $ rpmlint -I binary-or-shlib-defines-rpath
> binary-or-shlib-defines-rpath:
> The binary or shared library defines `RPATH'. Usually this is a bad thing
> because it hardcodes the path to search libraries and so makes it difficult to
> move libraries around.  Most likely you will find a Makefile with a line like:
> gcc test.o -o test -Wl,--rpath.  Also, sometimes configure scripts provide a
> --disable-rpath flag to avoid this.

I would be happy if there was a configure script...

> and: https://fedoraproject.org/wiki/Packaging/Guidelines#Removing_Rpath

Thank to remember me that this page evolves...

> > 
> > >  - W: non-conffile-in-etc /etc/ld.so.conf.d/wxformbuilder.conf
> > >    Add %config tag in %files
> > 
> > Fixed
> 
> (Not yet in the spec linked above)

Of course ;)
I didn't commit it. A such poor fix...

> > 
> > >  - W: invalid-url Source0: wxformbuilder-3.1.68.tar.bz2
> > >    ignore.
> > > 
> > > 
> > > A first look on license indicate GPLv2+, not GPLv2. More on that later.
> > 
> > GGPL sayed: Version 2, June 1991, not "or later"..
> > I would be please...
> 
> Look at a random *.h or *.cpp file. In the header you can find:
>  * This program is free software; you can redistribute it and/or
> 
>  * modify it under the terms of the GNU General Public License
> 
>  * as published by the Free Software Foundation; either version 2
> 
>  * of the License, or (at your option) any later version.


My apologies, I didn't investigate enough, you (and Terje) are right.

> > 
> > > You can use xz (tar cJvf ...) to compress the tarball, saves 10%. 
> > 
> > ????
> > x is for extract...
> > 
> > z is for bzip, j (as I used) is for bzip2...
> 
> He means: http://tukaani.org/xz/ another compressing format and not bzip.
> All rpms are compressed with that and that compressing format is better that
> the other. If you choose to use xz, you could maybe not use that on EL (once
> had a problem with that, don't know if that's resolved.)    

This is a misunderstood,I didn't know xz.
But I won't use it as I get enough problems with el5...
http://koji.fedoraproject.org/koji/taskinfo?taskID=2150700

(In reply to comment #10)
> I created some patches to fix the debuginfo package, using correct buildflags
> and some of the rpath problems, it's available here:
> 
>  http://terjeros.fedorapeople.org/wxformbuilder/
> 
> At first glance this seems like a simple package, however looking deeper there
> was a number of issues.    

Thanks for your help,patches fix all problems you reported :-)

Spec URL: http://dionysos.fedorapeople.org/SPECS/wxformbuilder.spec
SRPM URL:
http://dionysos.fedorapeople.org/SRPMS/wxformbuilder-3.1.68-5.fc12.src.rpm 

Still problem on F-13 :-(
http://koji.fedoraproject.org/koji/taskinfo?taskID=2150676

French adage:
- "Mieux vaut tard que jamais"
A translation could be "Better late than never"
- "Tout vient à point à qui sait attendre"
A translation could be "Things come to those who wait" (google...)

I could fix indirectly with the help from Dennis
http://lists.fedoraproject.org/pipermail/devel/2010-May/136445.html

http://dionysos.fedorapeople.org/SPECS/wxformbuilder.spec
http://dionysos.fedorapeople.org/SRPMS/wxformbuilder-3.1.68-6.fc12.src.rpm

%changelog
* Thu Jun 24 2010  Alain Portal <alain.portal[AT]univ-montp2[DOT]fr> 3.1.68-7.rev1750
  - Update source file to svn rev 1750

Spec URL: http://dionysos.fedorapeople.org/SPECS/wxformbuilder.spec
SRPM URL:
http://dionysos.fedorapeople.org/SRPMS/wxformbuilder-3.1.68-7.rev1750.fc12.src.rpm

Do you intend to end this review or not?

Hi Alain, sorry about the delay. 

I started on a complete analyse of the license status.
That job was a bit more complicated than expected (still more surprises in this package). Let's start with what I have so far.

 o sdk/tinyxml/
  -> seems to be a private copy of a lib already in Fedora, not allowed.

 o src/codegen/codeparser.h
   src/codegen/codeparser.cpp 
  -> no information about license, 

 o controls/src/wxScintilla/scintilla/src/RESearch.h
   controls/src/wxScintilla/scintilla/src/StyleContext.h 
  -> Public Domain

 o src/controls/include/wx/propgrid/*
   src/controls/include/wx/wxFlatNotebook/*
   src/controls/include/wx/wxScintilla/*    
  -> wxWindows license? is that GPLv2+?

 o src/controls/src/wxScintilla/scintilla/src/* a strange mix of 
   -> BSD, wxWin, Public Domain, GPLv2+?

Would be nice you could comment on this findings and contact upstream about
this fuzzy license status.

I would ask upstream about the src/codegen/codeparser.* files, it seems odd that they do not contain license attribution when the rest of the files in that directory do.

wxScintilla has an independent upstream (albeit, a very dead one), so it should be packaged separately:
http://sourceforge.net/projects/wxscintilla/

Same is true for:

wxFlatNotebook:
http://sourceforge.net/projects/wxflatnotebook/files/wxflatnotebook/

wxPropertyGrid:
http://wxpropgrid.sourceforge.net/

There is also a copy of boost in boost/, and it should be using the system copy.
The same is true for tinyxml, as pointed out above.

The wxWindows license is now known as the "wxWidgets" license, since the upstream renamed.

I don't know if this review will come back to life, since it appears to be stale for more than a year, but hopefully this information will help anyone who wishes to take it on.

Blocking FE-Legal.

If somebody wants to continue: I need too un-bundle wxpropgrid for saga, so I'll probably package it.

That should have been "I need to un-bundle", of course!

wxpropgrid is on review now:

https://bugzilla.redhat.com/show_bug.cgi?id=767082

I think we can close this one.