Bug 585719

Summary: Review Request: wxformbuilder - The OpenSource wxWidgets Designer, GUI Builder, and RAD Tool
Product: [Fedora] Fedora Reporter: Alain Portal <alain.portal>
Component: Package ReviewAssignee: Terje Røsten <terjeros>
Status: CLOSED NOTABUG QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: alain.portal, dan, fedora-package-review, mzdunek, notting, pahan, tcallawa, terjeros, tomspur, volker27
Target Milestone: ---Flags: terjeros: fedora‑review?
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2012-06-04 02:51:48 EDT Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Bug Depends On: 767082    
Bug Blocks: 182235    

Description Alain Portal 2010-04-25 14:59:11 EDT
Spec URL: http://dionysos.fedorapeople.org/SPECS/wxformbuilder.spec
SRPM URL: http://dionysos.fedorapeople.org/SRPMS/wxformbuilder-3.1.68-1.fc12.src.rpm
Description:
wxFormBuilder aims to be an application that as well as enabling visual
development and generating the corresponding code, allow the inclusion of 
non-graphical components, as well as providing facilities for extending the
set of widgets easily via plugins, like other applications such as qt-designer.
Comment 1 Thomas Spura 2010-04-25 16:18:35 EDT
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
Comment 2 Alain Portal 2010-04-26 14:56:43 EDT
(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
Comment 3 Terje Røsten 2010-04-26 16:11:52 EDT
> > - 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
Comment 4 Terje Røsten 2010-04-26 16:22:56 EDT
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
Comment 5 Alain Portal 2010-04-26 17:15:27 EDT
%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
Comment 6 Terje Røsten 2010-04-27 01:25:58 EDT
Thanks.

- you are mixing  $RPM_BUILD_ROOT and %{buildroot}.
- use %{_prefix} for /usr in %install.
Comment 7 Terje Røsten 2010-04-27 03:24:26 EDT
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
Comment 8 Alain Portal 2010-04-29 17:25:36 EDT
(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
Comment 9 Thomas Spura 2010-04-29 17:57:24 EDT
(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.)
Comment 10 Terje Røsten 2010-04-30 04:07:27 EDT
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.
Comment 11 Alain Portal 2010-04-30 17:09:33 EDT
(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
Comment 12 Alain Portal 2010-04-30 17:16:57 EDT
(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
Comment 13 Alain Portal 2010-05-20 15:02:41 EDT
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
Comment 14 Alain Portal 2010-06-24 15:00:38 EDT
%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
Comment 15 Alain Portal 2010-07-01 18:33:32 EDT
Do you intend to end this review or not?
Comment 16 Terje Røsten 2010-07-02 15:47:41 EDT
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.
Comment 17 Tom "spot" Callaway 2011-10-17 14:10:21 EDT
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.
Comment 18 Volker Fröhlich 2011-10-30 17:02:48 EDT
If somebody wants to continue: I need too un-bundle wxpropgrid for saga, so I'll probably package it.
Comment 19 Volker Fröhlich 2011-12-13 14:49:34 EST
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
Comment 20 Volker Fröhlich 2012-06-04 02:51:48 EDT
I think we can close this one.