Bug 191594 - Review Request: gtkglextmm
Review Request: gtkglextmm
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Ralf Corsepius
Fedora Package Reviews List
:
: 193107 (view as bug list)
Depends On:
Blocks: FE-ACCEPT
  Show dependency treegraph
 
Reported: 2006-05-13 12:43 EDT by Gilles Gagniard
Modified: 2007-11-30 17:11 EST (History)
1 user (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-05-22 18:48:00 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)

  None (edit)
Description Gilles Gagniard 2006-05-13 12:43:28 EDT
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-13 23:44:43 EDT
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 14:43:39 EDT
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 03:20:38 EDT
(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 14:47:07 EDT
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 03:21:20 EDT
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 03:08:00 EDT
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 07:14:54 EDT
*** 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.