Bug 192363 - Review Request: GTS - Gnu Triangulated Surface Library
Review Request: GTS - Gnu Triangulated Surface Library
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
:
Depends On:
Blocks: FE-ACCEPT
  Show dependency treegraph
 
Reported: 2006-05-18 22:04 EDT by Ed Hill
Modified: 2011-11-29 12:47 EST (History)
2 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-05-22 15:10:49 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
limburgher: fedora‑cvs+


Attachments (Terms of Use)
patch to address Makefile.am applying bogus INCLUDES (6.13 KB, patch)
2006-05-19 08:45 EDT, Ralf Corsepius
no flags Details | Diff

  None (edit)
Description Ed Hill 2006-05-18 22:04:42 EDT
Spec URL: http://mitgcm.org/eh3/fedora_misc/gts.spec
SRPM URL: http://mitgcm.org/eh3/fedora_misc/gts-0.7.6-1.src.rpm
Description: 
GTS provides a set of useful functions to deal with 3D surfaces meshed
with interconnected triangles including collision detection,
multiresolution models, constrained Delaunay triangulations and robust
set operations (union, intersection, differences).
Comment 1 Ralf Corsepius 2006-05-18 23:42:27 EDT
I've been packaging gts for many years, so ...

NEEDSWORK:

- Shipping static libs
Add --disable-static to %configure

- Bogus "Provides: gts-devel" at the beginning of the spec.

- Some of the binaries' names are too general and likely to conflict with other
packages:
/usr/bin/delaunay
/usr/bin/happrox
/usr/bin/transform
I propose to rename them into gts<name>

- Mispackaged file:
 /usr/share/gts/gts.m4
This file is an autoconf support macro and belongs into /usr/share/aclocal

- The html docs contained in gts-*.rpm are devel docs.
They should be packaged into the gts-devel-*.rpm.
Comment 2 Ed Hill 2006-05-19 07:59:30 EDT
Hi Ralf, thank you for all the helpful comments!  Here are the updated files
that incorporate all of your fixes:

  http://mitgcm.org/eh3/fedora_misc/gts-0.7.6-2.src.rpm
  http://mitgcm.org/eh3/fedora_misc/gts.spec
Comment 3 Ralf Corsepius 2006-05-19 08:42:53 EDT
OK, now to the nasty parts (Gts's sources are quite dirty ;) ):

1. gts-config is buggy:

Compare this (buggy):
$ ./gts-config --cflags
-I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include -I/usr/include
$ ./gts-config --libs
-L/usr/lib -lgts -lglib-2.0 -lm

to this:
$ pkg-config --cflags gts
-pthread -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include
$ pkg-config --libs gts
-pthread -Wl,--export-dynamic -lgts -lgthread-2.0 -lgmodule-2.0 -ldl -lglib-2.0

I'd propose to patch gts-config to behave as a wrapper to the corresponding
calls to pkg-config, at least for --cflags and --libs, or to patch out the
-I/usr/include rsp. -L/usr/lib by brute force.

2. The package's Makefiles are broken.
They explictily pass -I$(includedir) through INCLUDES. This overrides the system
include path and can cause nasty side-effects.

3. Building the packages causes GCC to emit quite a significant number of
"punned pointer/strict-alignment warnings". These are likely to break the
package at run-time, esp. on 64bit platforms.

Also worth noting is the configure script playing tricks with CFLAGS. AFAIS,
they seem to be harmless at least on FC5/i386.

So, be prepared to watch PRs coming in ;)

I consider 1. to be a MUST-fix, 2. to be a "I recommend to fix it" and 3. headsup.

APPROVED, provided you fix 1. I am going to attach a patch to address issue 2.
to this PR.

[Besides this, this package is one of those packages applying the weird (IMO
sense-free) Drepper-style shared library names. As this package is an ordinary
library, I am not going to fret over this, this time.]
Comment 4 Ralf Corsepius 2006-05-19 08:45:12 EDT
Created attachment 129599 [details]
patch to address Makefile.am applying bogus INCLUDES
Comment 5 Ed Hill 2006-05-19 11:23:16 EDT
Hi Ralf, thank you again for all the help.  I'm new (*very* new) to gts 
and am hoping to learn about it and use it for some future projects.  You 
are obviously a lot more familiar with it.  Would you be interested in 
"owning" it in Extras?  I'd be glad to do that and would be equally happy 
to try and maintain it myself.  Either way is cool, I'd just like to have 
a usable version of it in Extras.
Comment 6 Ralf Corsepius 2006-05-19 11:48:06 EDT
(In reply to comment #5)
> You are obviously a lot more familiar with it.
Well, it's a dependency of other packages I package for local use.

>  Would you be interested in "owning" it in Extras?
Not really ;) I am not actively using it, just passively (You might have noticed
that I am packaging several 3D related packages).

It's just that I have an (unclean) gts-0.7.4.spec myself, which I didn't intend
to publish, because my interest in it is limited and because I feel, I already
maintain too many packages.

>  I'd be glad to do that and would be equally happy 
> to try and maintain it myself.  Either way is cool, I'd just like to have 
> a usable version of it in Extras.
Exactly, what I want. So, this again is a case, I am willing to co-maintain and
to team up, but I do not want to take the lead.

Comment 7 Ed Hill 2006-05-21 16:35:55 EDT
Hi Ralf, I've included your patch (comment #4), added one for gts-config,
and imported it into CVS.  Please let me know if you see any problems.  
I'll request builds on FC-4/5 and devel in day or two.

And co-maintainership is fine with me -- please feel free to make whatever 
changes you want to it in CVS at any time.
Comment 8 Ed Hill 2006-05-22 15:10:49 EDT
Successfully built on FC-4, FC-5, and devel.  Thanks again for the help!
Comment 9 Paul F. Johnson 2006-07-25 18:50:36 EDT
Can you post up a new srpm and spec file? I can get down to reviewing it
properly then.
Comment 10 Ralf Corsepius 2006-07-25 23:03:05 EDT
(In reply to comment #9)
> Can you post up a new srpm and spec file? I can get down to reviewing it
> properly then.
... this package's review had been completed long ago. 
Comment 11 Jonathan Underwood 2011-11-29 12:39:55 EST
Package Change Request
======================
Package Name: gts
New Branches: el6
Owners: jgu
InitialCC:
Comment 12 Gwyn Ciesla 2011-11-29 12:47:00 EST
Git done (by process-git-requests).

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