Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
https://bugzilla.redhat.com/show_bug.cgi?id=431665
Summary: Review Request: fox-devel - A C++ library for GUI development Product: Fedora Version: rawhide Platform: All OS/Version: Linux Status: NEW Severity: medium Priority: medium Component: Package Review AssignedTo: nobody@fedoraproject.org ReportedBy: marc@mwiriadi.id.au QAContact: extras-qa@fedoraproject.org CC: fedora-package-review@redhat.com,notting@redhat.com
Spec URL: http://mwiriadi.fedorapeople.org/packages/fox-devel/fox-devel.spec SRPM URL: http://mwiriadi.fedorapeople.org/packages/fox-devel/fox-devel-1.7.15-1.fc8.s...
Description: FOX is a C++-based library for graphical user interface development FOX supports modern GUI features, such as drag-and-drop, tooltips, tab books, tree lists, icons, multiple document interfaces (MDI), timers, idle processing, automatic GUI updating, as well as OpenGL/Mesa for 3D graphics. Subclassing of basic FOX widgets allows for easy extension beyond the built-in widgets by application writers. The fox-devel package contains the files necessary to develop applications using the FOX GUI toolkit: the header files, the reswrap resource compiler, manual pages, and HTML documentation.
I'm not to sure if there should be a separate doc package. I'm also unsure as to whether to create a separate fox package or rename it libfox?
Only rpmlint issues.
[marc@localhost i386]$ rpmlint fox-devel-1.7.15-1.fc8.i386.rpm fox-devel.i386: W: file-not-utf8 /usr/share/doc/fox-1.7/html/styles.css fox-devel.i386: W: file-not-utf8 /usr/share/doc/fox-1.7/html/menu.css fox-devel.i386: W: no-dependency-on fox
Other packages built contain no rpmlint errors/warnings
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: fox-devel - A C++ library for GUI development
https://bugzilla.redhat.com/show_bug.cgi?id=431665
panemade@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |panemade@gmail.com Summary|Review Request: fox-devel - |Review Request: fox-devel - |A C++ library for GUI |A C++ library for GUI |development |development
------- Additional Comments From panemade@gmail.com 2008-02-06 05:53 EST ------- you better drop -devel in package name and add subpackage -devel like you did for other subpackages -calculator, -pathfinder
make fox-devel sub-package own *.h *.so and .pc files and main fox package .so* files
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: fox-devel - A C++ library for GUI development
https://bugzilla.redhat.com/show_bug.cgi?id=431665
------- Additional Comments From pertusus@free.fr 2008-02-06 06:07 EST ------- In addition to whar Parag said, I thing that calculator should better be called fox-calculator and pathfinder be called fox-pathfinder (with corresponding modifications in man pages). The names are much too common.
There are dots missing at the end of %descriptions.
Since they are in te same tarball, adie, pathfinder and so on should have the same version than fox itself. Using specific versions for subpackages will cause much pain.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: fox-devel - A C++ library for GUI development
https://bugzilla.redhat.com/show_bug.cgi?id=431665
------- Additional Comments From marc@mwiriadi.id.au 2008-02-06 06:57 EST ------- Updated.
Hopefully it is fine.
http://mwiriadi.fedorapeople.org/packages/fox-devel/fox.spec http://mwiriadi.fedorapeople.org/packages/fox-devel/fox-1.7.15-2.fc8.src.rpm
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: fox - A C++ library for GUI development
https://bugzilla.redhat.com/show_bug.cgi?id=431665
panemade@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Summary|Review Request: fox-devel - |Review Request: fox - A C++ |A C++ library for GUI |library for GUI development |development |
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: fox - A C++ library for GUI development
https://bugzilla.redhat.com/show_bug.cgi?id=431665
panemade@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- AssignedTo|nobody@fedoraproject.org |panemade@gmail.com Status|NEW |ASSIGNED Flag| |fedora-review?
------- Additional Comments From panemade@gmail.com 2008-02-07 05:41 EST ------- 1)Good if you will split BR lines to 80 chars say 2)Missing Requires: pkgconfig on -devel package. 3) missing Requires: %{name} = %{version}-%{release} on -adie, -calculator, -pathfinder, -shutterbug packages. 4) fox library used license LGPLv3+ and other subpackages are using GPLv3+
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: fox - A C++ library for GUI development
https://bugzilla.redhat.com/show_bug.cgi?id=431665
------- Additional Comments From marc@mwiriadi.id.au 2008-02-07 06:12 EST ------- Fixed.
http://mwiriadi.fedorapeople.org/packages/fox-devel/fox.spec http://mwiriadi.fedorapeople.org/packages/fox-devel/fox-1.7.15-3.fc8.src.rpm
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: fox - A C++ library for GUI development
https://bugzilla.redhat.com/show_bug.cgi?id=431665
------- Additional Comments From pertusus@free.fr 2008-02-08 05:17 EST ------- There are dots missing at the end of %descriptions.
The files in %_bindir also share a common namespace, so in there too they should be fox-PathFinder and fox-calculator. (and corresponding man pages).
Adie.stx looks like a config files. It should be in %_datadir, and it should be possible to override it with a file in %_sysconfdir and a file in $HOME/.something.
fox-config should be a wrapper around pkg-config calls to avoid multilib conflicts.
the pkgconfig file for fox is wrong. There is certainly no need to link against all those libs to link against fox. Libs: should only contain link flags for libraries that will be used directly by applications linking against fox. Libs.static: should contain link flags for the remaining libraries linked against the fox library (but not needed directly by applications linking against the fox library).
You can have an idea about that by doing
ldd -u -r /usr/bin/adie
it will show all the overlinking in that example. Looking at the fox headers, it seems to me that fox completly hides the underlying X/GL and image libs, so that Libs should only be:
Libs: -L${libdir} ${FOX_LIBS}
Regarding the include files there are very strange things, namely there are autoconf conditionals in headers, like in fx3d.h #ifdef HAVE_GLU_H
This is not right, these conditionals should be in .cpp files, not in the API.
The file xincs.h is especially full of these, and also full of include files for other APIs. However it doesn't seems to be included in any other file, so it may certainly be dropped completely from the fox API.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: fox - A C++ library for GUI development
https://bugzilla.redhat.com/show_bug.cgi?id=431665
------- Additional Comments From marc@mwiriadi.id.au 2008-02-10 05:46 EST ------- I'm having a chat to upstream about the issues relating to the code to get feedback.
I have linked the review request in my email he has advised that he will take a look and comment so I'll await his comment.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: fox - A C++ library for GUI development
https://bugzilla.redhat.com/show_bug.cgi?id=431665
------- Additional Comments From panemade@gmail.com 2008-02-27 21:55 EST ------- ping?
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: fox - A C++ library for GUI development
https://bugzilla.redhat.com/show_bug.cgi?id=431665
------- Additional Comments From marc@mwiriadi.id.au 2008-02-28 04:53 EST ------- Still waiting for upstream. Going to contact them again sorry about this.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: fox - A C++ library for GUI development
https://bugzilla.redhat.com/show_bug.cgi?id=431665
------- Additional Comments From marc@mwiriadi.id.au 2008-03-06 06:02 EST ------- (In reply to comment #6)
There are dots missing at the end of %descriptions.
The files in %_bindir also share a common namespace, so in there too they should be fox-PathFinder and fox-calculator. (and corresponding man pages).
Upstream has advised me NOT to change it.
Adie.stx looks like a config files. It should be in %_datadir, and it should be possible to override it with a file in %_sysconfdir and a file in $HOME/.something.
fox-config should be a wrapper around pkg-config calls to avoid multilib conflicts.
fox-config can be dropped in fedora since it is mainly for other distro's.
the pkgconfig file for fox is wrong. There is certainly no need to link against all those libs to link against fox. Libs: should only contain link flags for libraries that will be used directly by applications linking against fox. Libs.static: should contain link flags for the remaining libraries linked against the fox library (but not needed directly by applications linking against the fox library).
You can have an idea about that by doing
ldd -u -r /usr/bin/adie
it will show all the overlinking in that example. Looking at the fox headers, it seems to me that fox completly hides the underlying X/GL and image libs, so that Libs should only be:
Libs: -L${libdir} ${FOX_LIBS}
Regarding the include files there are very strange things, namely there are autoconf conditionals in headers, like in fx3d.h #ifdef HAVE_GLU_H
This is not right, these conditionals should be in .cpp files, not in the API.
The condition could probably be removed, if the classes not dependent on OpenGL are moved back to fx.h [FXVec** and so on]. These could be useful in a 2D environment w/o GL.
The file xincs.h is especially full of these, and also full of include files for other APIs. However it doesn't seems to be included in any other file, so it may certainly be dropped completely from the fox API.
This file is needed to write custom API's advised by upstream.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: fox - A C++ library for GUI development
https://bugzilla.redhat.com/show_bug.cgi?id=431665
------- Additional Comments From marc@mwiriadi.id.au 2008-03-08 00:18 EST ------- (In reply to comment #6)
Adie.stx looks like a config files. It should be in %_datadir, and it should be possible to override it with a file in %_sysconfdir and a file in $HOME/.something.
It is needed to be useful although an RFE will change that in the future not sure when though.
fox-config should be a wrapper around pkg-config calls to avoid multilib conflicts.
I've removed it now in a newer version thats nearly ready.
the pkgconfig file for fox is wrong. There is certainly no need to link against all those libs to link against fox. Libs: should only contain link flags for libraries that will be used directly by applications linking against fox. Libs.static: should contain link flags for the remaining libraries linked against the fox library (but not needed directly by applications linking against the fox library).
You can have an idea about that by doing
ldd -u -r /usr/bin/adie
I have removed the static libraries using the *.la how do you propose I go about fixing this?
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: fox - A C++ library for GUI development
https://bugzilla.redhat.com/show_bug.cgi?id=431665
------- Additional Comments From pertusus@free.fr 2008-03-08 03:46 EST ------- (In reply to comment #10)
(In reply to comment #6)
The files in %_bindir also share a common namespace, so in there too they should be fox-PathFinder and fox-calculator. (and corresponding man pages).
Upstream has advised me NOT to change it.
What are their arguments? The bindir is shared between all applications, the names of commands should be specific.
fox-config can be dropped in fedora since it is mainly for other distro's.
If other upstream programs use fox-config it should be in fedora.
Regarding the include files there are very strange things, namely there are autoconf conditionals in headers, like in fx3d.h #ifdef HAVE_GLU_H
This is not right, these conditionals should be in .cpp files, not in the API.
The condition could probably be removed, if the classes not dependent on OpenGL are moved back to fx.h [FXVec** and so on]. These could be useful in a 2D environment w/o GL.
Whatever solution is used, the conditionals should go. An API should never be conditional. But indeed part of the api could be factored out in files not installed if that part of the api isn't compiled in the library.
The file xincs.h is especially full of these, and also full of include files for other APIs. However it doesn't seems to be included in any other file, so it may certainly be dropped completely from the fox API.
This file is needed to write custom API's advised by upstream.
How can they advise that? Do they advise relying on autoconf conditionals? It is plain wrong.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: fox - A C++ library for GUI development
https://bugzilla.redhat.com/show_bug.cgi?id=431665
------- Additional Comments From pertusus@free.fr 2008-03-08 04:04 EST ------- (In reply to comment #11)
(In reply to comment #6)
Adie.stx looks like a config files. It should be in %_datadir, and it should be possible to override it with a file in %_sysconfdir and a file in $HOME/.something.
It is needed to be useful although an RFE will change that in the future not sure when though.
In the mean time Adie.stx should be better located.
fox-config should be a wrapper around pkg-config calls to avoid multilib conflicts.
I've removed it now in a newer version thats nearly ready.
I don't think this is the right way. Other packages building against fox may need it. Better is to propose upstream a fox-config-pkgconfig which would be a wrapper around pkgconfig.
the pkgconfig file for fox is wrong. There is certainly no need to link against all those libs to link against fox. Libs: should only contain link flags for libraries that will be used directly by applications linking against fox. Libs.static: should contain link flags for the remaining libraries linked against the fox library (but not needed directly by applications linking against the fox library).
You can have an idea about that by doing
ldd -u -r /usr/bin/adie
I have removed the static libraries using the *.la how do you propose I go about fixing this?
The .la files are not static libs. They are text files created by libtool to help linking/dlopening on platforms with insufficient shared library support, and are therefore not needed on linux in general and in fedora.
The issue of the pkgconfig file is completly different. The content of the .pc file is wrong. Indeed when linking against shared libraries you should only link against the library which provides the symbols you are using in your application. But if you look at the fox .pc file you'll see that all the libraries that are linked against fox are brought in: pkg-config fox --libs -lFOX-1.7 -lXext -lX11 -lXcursor -lXrender -lXrandr -lXfixes -lXi -lpthread -lrt -ljpeg -lpng -ltiff -lz -lbz2 -lm -lGLU -lGL
Those are not needed for an application linking against fox .the only link flag needed is -lFOX-1.7.
Now when you link statically, you have to precise all the libraries in the link command. Therefore pkgconfig has a way to distinguish between what is needed to link dynamically and what is needed to link statically.
in man pkg-config you can look at Libs and Libs.private in the METADATA FILE SYNTAX section.
As a side note there is also a Requires.private which has the same relation ship with Requires than Libs.private with Libs.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: fox - A C++ library for GUI development
https://bugzilla.redhat.com/show_bug.cgi?id=431665
panemade@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- AssignedTo|panemade@gmail.com |nobody@fedoraproject.org
------- Additional Comments From panemade@gmail.com 2008-04-03 23:40 EST ------- I better move away from this review. Patrice, feel free to review this package.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: fox - A C++ library for GUI development
https://bugzilla.redhat.com/show_bug.cgi?id=431665
panemade@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag|fedora-review? |
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: fox - A C++ library for GUI development
https://bugzilla.redhat.com/show_bug.cgi?id=431665
------- Additional Comments From pertusus@free.fr 2008-04-04 03:57 EST ------- Ok, I'll do.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: fox - A C++ library for GUI development
https://bugzilla.redhat.com/show_bug.cgi?id=431665
------- Additional Comments From marc@mwiriadi.id.au 2008-04-05 00:36 EST ------- Created an attachment (id=301361) --> (https://bugzilla.redhat.com/attachment.cgi?id=301361&action=view) fox pc patch
pc patch
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: fox - A C++ library for GUI development
https://bugzilla.redhat.com/show_bug.cgi?id=431665
------- Additional Comments From marc@mwiriadi.id.au 2008-04-05 00:37 EST ------- Created an attachment (id=301362) --> (https://bugzilla.redhat.com/attachment.cgi?id=301362&action=view) fox-config patch
fox-config.patch it should fix the links.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: fox - A C++ library for GUI development
https://bugzilla.redhat.com/show_bug.cgi?id=431665
------- Additional Comments From marc@mwiriadi.id.au 2008-04-05 00:46 EST ------- I have updated the spec file and also the src.rpm file. It is still rough so I acknowledge that clean up is required.
http://mwiriadi.fedorapeople.org/packages/fox-devel/fox.spec http://mwiriadi.fedorapeople.org/packages/fox-devel/fox-1.7.15-3.fc9.src.rpm
What I need to know is content wise what needs to change to get it approved.
Adie.stx needs to be done. Is there anything else?
Apologies for the delay in patching stuff I had to read a lot since I've never patched the pc file or fox-config file so I'm a bit slow.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: fox - A C++ library for GUI development
https://bugzilla.redhat.com/show_bug.cgi?id=431665
------- Additional Comments From pertusus@free.fr 2008-04-16 17:55 EST ------- I won't accept a package that ships binary named PathFinder or calculator.
There should be dots at the end of the %description texts.
Why the Requires: bzip2, zlib The libs should be auto-detected, so why this Requires?
For the .pc patch, unless I am missing something, there are still some libs that are only linked through fox (though the double quote seems to confuse pkgconfig, and now the libs needed when statically linking are not present anymore. In my opinion, it should end up like (also note that the double quotes should not be there, I removed them):
.... LIBS=@LIBS@ FOX_LIBS=-lFOX-@FOX_MAJOR_VERSION@.@FOX_MINOR_VERSION@ X_LIBS=@X_LIBS@ X_BASE_LIBS=@X_BASE_LIBS@ X_EXTRA_LIBS=@X_EXTRA_LIBS@ GL_LIBS=@GL_LIBS@
Name: FOX Description: The FOX Toolkit URL: www.fox-toolkit.com Version: @FOX_MAJOR_VERSION@.@FOX_MINOR_VERSION@.@FOX_PATCH_LEVEL@ Libs: ${FOX_LIBS} Libs.private: ${X_LIBS} ${X_BASE_LIBS} ${X_EXTRA_LIBS} ${GL_LIBS} ${LIBS} Cflags: -I${includedir}
Also the fedora opt flags are not used.
With a release, the autoconf call is not needed (and may be harmful).
Regarding the fox-config file, I think it should better be completly rewritten as a pkgconfig wrapper. I'll attach it. You can then propose it to upstream to install it as, for example fox-config-pkgconfig, and then in the fedora package you ship fox-config-pkgconfig as fox-config.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: fox - A C++ library for GUI development
https://bugzilla.redhat.com/show_bug.cgi?id=431665
------- Additional Comments From pertusus@free.fr 2008-04-16 17:56 EST ------- Created an attachment (id=302678) --> (https://bugzilla.redhat.com/attachment.cgi?id=302678&action=view) pkgconfig config file wrapper
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: fox - A C++ library for GUI development
https://bugzilla.redhat.com/show_bug.cgi?id=431665
------- Additional Comments From pertusus@free.fr 2008-04-16 18:07 EST ------- Regarding the license isn't it LGPLv3+ and GPLv3+? In any case, the library seems to be covered by LGPL while the binaries are GPL covered, so it should be along:
For main package License: LGPLv3
Nothing for the devel package (same license)
For adie, calculator...
License: GPLv3
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: fox - A C++ library for GUI development
https://bugzilla.redhat.com/show_bug.cgi?id=431665
tibbs@math.uh.edu changed:
What |Removed |Added ---------------------------------------------------------------------------- OtherBugsDependingO| |201449 nThis| | Status|ASSIGNED |CLOSED Resolution| |NOTABUG
------- Additional Comments From tibbs@math.uh.edu 2008-07-02 21:57 EST ------- Closing as detailed in bug 429809.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=431665
Terje Røsten terjeros@phys.ntnu.no changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |terjeros@phys.ntnu.no
--- Comment #23 from Terje Røsten terjeros@phys.ntnu.no 2010-01-02 13:30:12 EDT --- I might have some interest in this package, spec file links seems dead, any private copy floating around?
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=431665
--- Comment #24 from Terje Røsten terjeros@phys.ntnu.no 2010-01-02 17:24:15 EDT --- Redid the thing with help from a wieers rpm:
koji: http://koji.fedoraproject.org/koji/taskinfo?taskID=1899111 spec: http://terjeros.fedorapeople.org/fox/fox.spec srpm: http://terjeros.fedorapeople.org/fox/fox-1.6.37-1.fc12.src.rpm
This is for the current stable version.
TODO: pkgconfig issue.
I don't see the Adie.stx location as blocker, it's more a bug in the app, not a thing to stop approval?
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=431665
--- Comment #25 from Terje Røsten terjeros@phys.ntnu.no 2010-01-03 16:21:38 EDT --- More fixes, should be mostly ok now, built and tested a FOX based application without issue.
spec: http://terjeros.fedorapeople.org/fox/fox.spec srpm: http://terjeros.fedorapeople.org/fox/fox-1.6.37-1.fc10.src.rpm
package-review@lists.fedoraproject.org
-
Red Hat Bugzilla