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: agg - C++ rendering framework, move from core to shared

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

pertusus@free.fr changed:

What |Removed |Added ---------------------------------------------------------------------------- CC| |pertusus@free.fr

------- Additional Comments From pertusus@free.fr 2007-01-06 13:59 EST ------- * I have spotted a licensing issue: gpc is shipped with agg but seems to be covered by a GPL incompatible license:

You may not use this software, in whole or in part, in support of any commercial product without the express consent of the author.

* I think the summary could be ameliorated, for example with 'general purpose 2D graphical library' or 'graphical rendering engine'

* in the patch, I guess the @FREETYPE_LIBS@ should be ion the libaggfontfreetype_la_LIBADD line. Not a big deal. Has the patch been submitted to 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: agg - C++ rendering framework, move from core to shared

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

tibbs@math.uh.edu changed:

What |Removed |Added ---------------------------------------------------------------------------- CC| |tibbs@math.uh.edu

------- Additional Comments From tibbs@math.uh.edu 2007-01-06 14:22 EST ------- Forgive my intrusion, but mention of gpc perked my interest. Can I assume that the gpc referenced here has no relation to the GNU Pascal Compiler?

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: agg - C++ rendering framework, move from core to shared

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

------- Additional Comments From pertusus@free.fr 2007-01-06 18:57 EST ------- Some other issues I spotted:

Not blockers (personal preferences, in fact ;-): * the news file could be shipped even though it only points to a web page * I would personally replace rm -f $RPM_BUILD_ROOT/%{_libdir}/*.la by rm $RPM_BUILD_ROOT/%{_libdir}/*.la * and in clean I would have used rm -rf $RPM_BUILD_ROOT * I tend to add a trailing / to directories in %files to show that these are directories and not files, like %{_includedir}/agg2/

Serious issues: * there are dependencies missing for agg-devel: freetype-devel (for ft2build.h), pkgconfig

* %{_libdir}/pkgconfig/ and %{_datadir}/aclocal/ are not owned. pkgconfig owns %{_libdir}/pkgconfig/, automake owns %{_datadir}/aclocal/ (for the aclocal dir there are more than one way to fix the issue, but it must be fixed).

* in libagg.pc there is -Wl,-rpath,${exec_prefix}/lib Is it really needed?

Still in libagg.pc there is no reference to freetype2. Maybe there should be a Requires.private: freetype2 ?

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: agg - C++ rendering framework, move from core to shared

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

------- Additional Comments From bugs.michael@gmx.net 2007-01-07 07:14 EST -------

• I would personally replace

rm -f $RPM_BUILD_ROOT/%{_libdir}/*.la by rm $RPM_BUILD_ROOT/%{_libdir}/*.la

The latter returns False if the target files don't exist. The former returns True in that case and doesn't break the build.

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: agg - C++ rendering framework, move from core to shared

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

------- Additional Comments From bugs.michael@gmx.net 2007-01-07 08:04 EST ------- %exclude %{_libdir}/*.la in the %files list would achieve the same.

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: agg - C++ rendering framework, move from core to shared

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

------- Additional Comments From pertusus@free.fr 2007-01-07 12:43 EST ------- Please post the link of the updated srpm...

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: agg - C++ rendering framework, move from core to shared

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

------- Additional Comments From pertusus@free.fr 2007-01-08 12:57 EST ------- (In reply to comment #11)

The substantive issue is gpc, and in that case we're building with --disable-gpc to disable building and including gpc in the final rpm

It isn't enough, the srpm should not contain non free or legally problematic code. This should be reported upstream anyway, upstream is violating gpc license and/or the GPL.

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: agg - C++ rendering framework, move from core to shared

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

bugzilla@redhat.com changed:

What |Removed |Added ---------------------------------------------------------------------------- Product|Fedora Extras |Fedora

tibbs@math.uh.edu changed:

What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |NEEDINFO OtherBugsDependingO|163776 | nThis| | Flag| |needinfo?(caolanm@redhat.com | |)

------- Additional Comments From tibbs@math.uh.edu 2007-07-08 12:45 EST ------- It's been six months since the last comment by the submitter; setting NEEDINFO. I will close this ticket soon if there is no further response.

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: agg - C++ rendering framework, move from core to shared

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

------- Additional Comments From tibbs@math.uh.edu 2007-07-08 14:10 EST ------- Oh, joy. Well, then it's essentially a merge review. And so it may come down to this package being pulled from the distro if we can't get any response to review comments.

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: agg - C++ rendering framework, move from core to shared

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

caolanm@redhat.com changed:

What |Removed |Added ---------------------------------------------------------------------------- Status|NEEDINFO |NEW Flag|needinfo?(caolanm@redhat.com| |) |

------- Additional Comments From caolanm@redhat.com 2008-02-05 11:56 EST ------- Sure, you can even be the sole maintainer if you want.

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: agg - C++ rendering framework, move from core to shared

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

------- Additional Comments From pertusus@free.fr 2008-02-05 17:32 EST ------- I have taken care of all my comments (and a bit more).

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: agg - C++ rendering framework, move from core to shared

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

------- Additional Comments From caolanm@redhat.com 2008-02-23 05:46 EST -------

c++filt _Z8agg_mainiPPc

agg_main(int, char**)

i.e. I believe that idea is that apps using those libraries are supposed to provide their own "agg_main" entry point

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=221717

Jason Tibbitts tibbs@math.uh.edu changed:

What |Removed |Added ---------------------------------------------------------------------------- CC| |kevin@tigcc.ticalc.org

--- Comment #26 from Jason Tibbitts tibbs@math.uh.edu 2008-12-17 13:48:40 EDT --- Looks like Kevin now owns this package; updating the CC. I never found the time to get back to this, but I should be able to do so soon.

The rpmlint complaint list is currently:

agg.x86_64: E: binary-or-shlib-defines-rpath /usr/lib64/libaggplatformsdl.so.2.0.4 ['/usr/lib64'] agg.x86_64: E: binary-or-shlib-defines-rpath /usr/lib64/libaggplatformX11.so.2.0.4 ['/usr/lib64'] agg.x86_64: E: binary-or-shlib-defines-rpath /usr/lib64/libaggfontfreetype.so.2.0.4 ['/usr/lib64'] These are new since my last comment, and need tto be fixed up.

agg.x86_64: W: undefined-non-weak-symbol /usr/lib64/libaggplatformsdl.so.2.0.4 _Z8agg_mainiPPc agg.x86_64: W: undefined-non-weak-symbol /usr/lib64/libaggplatformX11.so.2.0.4 _Z8agg_mainiPPc These are OK as discussed above.

agg.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libaggplatformsdl.so.2.0.4 /lib64/libpthread.so.0 agg.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libaggplatformsdl.so.2.0.4 /lib64/libm.so.6 agg.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libaggplatformX11.so.2.0.4 /lib64/libm.so.6 These are not a big deal as discussed above, but are easy to get rid of.

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=221717

Jason Tibbitts tibbs@math.uh.edu changed:

What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED AssignedTo|nobody@fedoraproject.org |tibbs@math.uh.edu Flag| |fedora-review?

--- Comment #28 from Jason Tibbitts tibbs@math.uh.edu 2009-01-15 23:07:08 EDT --- Any update? It would really nice to get rid of this ancient review ticket. Actually, I think I'll just go ahead and do a full review.

I was about to complain about a missing pkgconfig dependency for the -devel package, but then noted that rpm automatically generates a /usr/bin/pkg-config dependency. That seems to solve the issue, although I'm not quite sure where the dependency comes from.

So really it's just the rpmlint complaints above that could use fixing.

* source files match upstream (verified manually). * package meets naming and versioning guidelines. * specfile is properly named, is cleanly written and uses macros consistently. * summaries are OK. * descriptions are OK. * dist tag is present. * build root is OK. * license field matches the actual license. * license is open source-compatible. * license text included in package. * BuildRequires are proper. * compiler flags are appropriate. * %clean is present. * package builds in mock (rawhide, x86_64). * package installs properly. * debuginfo package looks complete. X rpmlint has some valid complaints. * final provides and requires are sane: agg-2.5-6.fc11.x86_64.rpm libagg.so.2()(64bit) libaggfontfreetype.so.2()(64bit) libaggplatformX11.so.2()(64bit) libaggplatformsdl.so.2()(64bit) agg = 2.5-6.fc11 agg(x86-64) = 2.5-6.fc11 = /sbin/ldconfig libSDL-1.2.so.0()(64bit) libX11.so.6()(64bit) libagg.so.2()(64bit) libaggfontfreetype.so.2()(64bit) libaggplatformX11.so.2()(64bit) libaggplatformsdl.so.2()(64bit) libfreetype.so.6()(64bit) libgcc_s.so.1()(64bit) libgcc_s.so.1(GCC_3.0)(64bit) libstdc++.so.6()(64bit) libstdc++.so.6(CXXABI_1.3)(64bit) libstdc++.so.6(GLIBCXX_3.4)(64bit)

agg-devel-2.5-6.fc11.x86_64.rpm pkgconfig(libagg) = 2.5.0 agg-devel = 2.5-6.fc11 agg-devel(x86-64) = 2.5-6.fc11 = /usr/bin/pkg-config agg = 2.5-6.fc11 automake freetype-devel libagg.so.2()(64bit) libaggfontfreetype.so.2()(64bit) libaggplatformX11.so.2()(64bit) libaggplatformsdl.so.2()(64bit) pkgconfig(freetype2)

* shared libraries present. ldconfig called properly unversioned .so links are in the -devel package. * owns the directories it creates. * doesn't own any directories it shouldn't. * no duplicates in %files. * file permissions are appropriate. * no generically named files. * scriptlets are OK (ldconfig). * code, not content. * documentation is small, so no -doc subpackage is necessary. * %docs are not necessary for the proper functioning of the package. * headers are in the -devel package. * pkgconfig files present; pkgconfig dependency is there (via /usr/bin/pkg-config auto-dep). * no static libraries. * no libtool .la files.

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=221717

--- Comment #30 from Kevin Kofler kevin@tigcc.ticalc.org 2009-01-16 00:04:09 EDT --- Hmmm, but it can't be the new libtool, as agg was last built in February 2008. WTF?!

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=221717

--- Comment #32 from Jason Tibbitts tibbs@math.uh.edu 2009-01-17 11:05:03 EDT --- Yes, I always do a local mock build. If you want, just think of it as a predictor of problems you'll see when you next rebuild.

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=221717

--- Comment #34 from Jason Tibbitts tibbs@math.uh.edu 2009-01-20 14:09:42 EDT --- You're welcome to try and get libtool changed to do what you want. From my perspective, however, this package fails to meet the packaging guidelines and I cannot approve it until it does. Were it a new package it would simply stay out of the distribution until it were fixed, but since this is a merge review I can't really do anything about it and you of course have no incentive to fix it.

I guess this ticket will just stay open until something gives.

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=221717

Jason Tibbitts tibbs@math.uh.edu changed:

What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |CLOSED Resolution| |RAWHIDE Flag|fedora-review? |fedora-review+

--- Comment #36 from Jason Tibbitts tibbs@math.uh.edu 2009-02-19 14:26:42 EDT --- I went and whined in that bug and it seems to have been fixed. I'm not sure why someone who understood the problem didn't do that, but in any case the rpath problems are gone from this package after a rebuild (which will happen automatically next week anyway).

So, APPROVED. I'll close this out.

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=221717

--- Comment #38 from Kevin Kofler kevin@tigcc.ticalc.org 2010-07-19 20:04:54 EDT --- As far as I'm concerned (i.e. unless one of the comaintainers wants it), feel free to maintain it in EPEL, I have little to no interest in EPEL.

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=221717

--- Comment #40 from Kevin Fenzi kevin@tummy.com 2010-07-21 01:03:48 EDT --- CVS done (by process-cvs-requests.py).