Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 191594 - Review Request: gtkglextmm
Summary: Review Request: gtkglextmm
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Ralf Corsepius
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
: 193107 (view as bug list)
Depends On:
Blocks: FE-ACCEPT
TreeView+ depends on / blocked
 
Reported: 2006-05-13 16:43 UTC by Gilles Gagniard
Modified: 2007-11-30 22:11 UTC (History)
1 user (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2006-05-22 22:48:00 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)

Description Gilles Gagniard 2006-05-13 16:43:28 UTC
Spec URL: http://jaile.ath.cx/gilles/fedora_core_5/gtkglextmm.spec
SRPM URL: http://jaile.ath.cx/gilles/fedora_core_5/gtkglextmm-1.2.0-1.src.rpm
Description: gtkglextmm is a C++ wrapper for GtkGlExt, an OpenGL extension to GTK+ (See http://gtkglext.sourceforge.net for more information).

Please also note that this is my first package and that I need a sponsor.

Comment 1 Ralf Corsepius 2006-05-14 03:44:43 UTC
Some comments:
* The spec "BuildRequires: gtkext >= 1.2.0".
AFAICT, 1.0.0 should be sufficient. At least, I am not aware about any API
changes between GtkGlExt-1.0.0 and 1.2.0 making this requirement necessary.

A fact confirming this, is gtkglextmm's configure script to only check for
gtkglext >= 1.0.0.

* The spec explicitly 
Requires: gtkglext
Requires: gtkmm24

This shouldn't be necessary.


* Please explain /usr/lib/gtkglextmm-1.2/proc/m4/*
I don't know what these files are (Look like some m4 macros to help converting
some types), how they are supposed to be used and why they need to be shipped.

AFAIS, they don't they seem to be used by anything in gtkglextmm.



Comment 2 Gilles Gagniard 2006-05-14 18:43:39 UTC
Alright, I reuploaded a new version of the spec.

> * The spec "BuildRequires: gtkext >= 1.2.0".
> AFAICT, 1.0.0 should be sufficient. At least, I am not aware about any API
> changes between GtkGlExt-1.0.0 and 1.2.0 making this requirement necessary.

> A fact confirming this, is gtkglextmm's configure script to only check for
> gtkglext >= 1.0.0.

Yup, you're right. As both of these packages have been released almost together,
I thought that it was necessary ;)

> * The spec explicitly 
> Requires: gtkglext
> Requires: gtkmm24

> This shouldn't be necessary.

Ok, I removed them. But for my personal education, could you explain me why ? It
is the first rpm spec I write, I only have a previous experience with gentoo
ebuilds ... Is it because rpm added automatically a dependency on the
*librairies* in the gtkglext package by checking undefined symbols ?

> * Please explain /usr/lib/gtkglextmm-1.2/proc/m4/*
> I don't know what these files are (Look like some m4 macros to help converting
> some types), how they are supposed to be used and why they need to be shipped.

> AFAIS, they don't they seem to be used by anything in gtkglextmm.

Well, I use this library as an app developper, and I don't need these files
either. However, directly from the README file in the source package in tools/m4 :

"This directory contains additional type conversions for gtkglextmm.
The convert.m4 file overrides the file of the same name in gtkmm.

Like the gtkmm m4 conversion files, these files are also installed, for use by
other libraries."

So I guess some people have a use for it ...

Comment 3 Ralf Corsepius 2006-05-15 07:20:38 UTC
(In reply to comment #2)
> Alright, I reuploaded a new version of the spec.
Please increment the release-tag. 

I am going to continue the review once you do.

> > * The spec explicitly 
> > Requires: gtkglext
> > Requires: gtkmm24
> 
> > This shouldn't be necessary.
> 
> Ok, I removed them. But for my personal education, could you explain me why ?
The main package is a pure run-time library package. Rpms of applications using
these libraries will automatically be added dependencies on these shared
libraries and their dependencies, when *building* rpms of these applications.
(i.e. these Requires are necessary in *-devel rpms, but not in runtime rpms).

> is the first rpm spec I write, I only have a previous experience with gentoo
> ebuilds ... Is it because rpm added automatically a dependency on the
> *librairies* in the gtkglext package by checking undefined symbols ?
Almost. Rpm adds dependencies on the libraries (It adds "Requires:
libfoo.so.1"), not on the package ("Requires: foo"). 
Depsolvers/Installers such as yum/apt etc. will translate "Requires:
libfoo.so.1" into packages.
 
> > * Please explain /usr/lib/gtkglextmm-1.2/proc/m4/*
> > I don't know what these files are (Look like some m4 macros to help converting
> > some types), how they are supposed to be used and why they need to be shipped.
> 
> > AFAIS, they don't they seem to be used by anything in gtkglextmm.
> 
> Well, I use this library as an app developper,
I am heavily using GtkGLExt with C++ and have never found gtkmm/gtkglextmm to be
attractive ;)

> and I don't need these files
> either. However, directly from the README file in the source package in tools/m4 :
..
> So I guess some people have a use for it ...
Hmm, seems like an historic artefact (== upstream bug) or a packaging bug in FE
gtkmm to me. AFAICT, older versions of gtkmm seem to have shipped scripts using
the macros, newer versions don't seem to do so.

Anyway, this is not a blocker for this review.

Comment 4 Gilles Gagniard 2006-05-17 18:47:07 UTC
The release tag is incremented. Here are now the new URLs :

Spec URL: http://jaile.ath.cx/gilles/fedora_core_5/gtkglextmm.spec
SRPM URL: http://jaile.ath.cx/gilles/fedora_core_5/gtkglextmm-1.2.0-2.src.rpm

Comment 5 Ralf Corsepius 2006-05-18 07:21:20 UTC
Package looks fine, except one issue remaining:
You are shipping libtool archives (*.la).

The PackageGuideLines Gods want you to remove them. I for one consider this part
of the package guidelines as in error, and therefore will not force anybody to
remove *.la, but will leave such a decision to the packager. 

I.e. decide on yourself if you want to ship them or not.


APPROVED.

To get sponsored, please proceed with "Get a Fedora Account" on
http://fedoraproject.org/wiki/Extras/Contributors


Comment 6 Ralf Corsepius 2006-05-24 07:08:00 UTC
Gilles,

1. In CVS, you are using the same Release:-tag for devel and FC-5.
In future, please make sure these are different and that the Release tag of
devel is greater than that of FC-5, otherwise upgrades from one FC-release to
another one are likely to fail.

I recommend using %{?dist}. As this is your first package, I've applied
corresponding changes to devel and FC-5, and triggered a rebuild for devel.
(ATM, there is no need to rebuild FC-5)

2. You should have closed this PR, when the packages had been released.

3. Please remove FE-NEEDSPONSOR from this PR and all others you might have added
it to.


Comment 7 Ville Skyttä 2006-05-25 11:14:54 UTC
*** Bug 193107 has been marked as a duplicate of this bug. ***


Note You need to log in before you can comment on or make changes to this bug.