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 210781 - Review Request: libctl - Guile-based support for flexible control files
Summary: Review Request: libctl - Guile-based support for flexible control files
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Jason Tibbitts
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On:
Blocks: FE-ACCEPT
TreeView+ depends on / blocked
 
Reported: 2006-10-14 22:44 UTC by Ed Hill
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-10-17 17:40:37 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)

Description Ed Hill 2006-10-14 22:44:49 UTC
Spec URL: http://mitgcm.org/eh3/fedora_misc/libctl.spec
SRPM URL: http://mitgcm.org/eh3/fedora_misc/libctl-3.0.2-3.fc5.src.rpm
Description: 
The libctl package is a Guile-based library that provides support for
flexible control files in scientific simulations.

Comment 1 Ed Hill 2006-10-14 22:59:30 UTC
Please note that the patch in the above SRPM has already been submitted 
to the upstream folks.  I hope to remove it and the resulting autotools 
dependency from future versions.

Comment 2 Jason Tibbitts 2006-10-15 02:49:15 UTC
The only thing that gives me any pause about this package is /usr/include/ctl.h,
which does seem a bit generic and prone to conflicts in the future.  I don't
really know what to do about it, though.  If it were moved into a subdirectory
of /usr/include or renamed, then Fedora would differ from every other
installation.  And there's no convenient .pc file to use to relocate things.
Any thoughts on the matter?

* source files match upstream:
   dff367aa94e68a507678f0f3d48b1165  libctl-3.0.2.tar.gz
* package meets naming and packaging guidelines.
* specfile is properly named, is cleanly written and uses macros consistently.
* dist tag is present.
* build root is correct.
* license field matches the actual license.
* license is open source-compatible.  License text included in package.
* latest version is being packaged.
* BuildRequires are proper.
* compiler flags are appropriate.
* %clean is present.
* package builds in mock (development, x86_64).
* package installs properly
* debuginfo package looks complete.
* rpmlint is silent.
* final provides and requires are sane:
  libctl-3.0.2-3.fc6.x86_64.rpm
   libctl.so.3()(64bit)
   libctlgeom.so.3()(64bit)
   libctl = 3.0.2-3.fc6
  =
   /sbin/ldconfig
   guile
   libctl.so.3()(64bit)
   libctlgeom.so.3()(64bit)
   libguile.so.17()(64bit)

  libctl-devel-3.0.2-3.fc6.x86_64.rpm
   libctl-devel = 3.0.2-3.fc6
  =
   /bin/sh
   libctl = 3.0.2-3.fc6
   libctl.so.3()(64bit)
   libctlgeom.so.3()(64bit)
  
* %check is not present; no test suite upstream.
* shared libraries are present; ldconfig is called properly.
* owns the directories it creates.
* doesn't own any directories it shouldn't.
* no duplicates in %files.
* file permissions are appropriate.
* scriptlets are OK (ldconfig)
* code, not content.
* documentation is small, so no -docs subpackage is necessary.
* %docs are not necessary for the proper functioning of the package.
* headers are in the -devel package.
* no pkgconfig files.
* no libtool .la droppings.

Comment 3 Ed Hill 2006-10-15 03:28:08 UTC
Hi Jason, many thanks for the rapid review!

So I can easily push the three header files into a /usr/include/ctl or 
similarly named directory.  And then use -I/usr/include/ctl for the 
dependant packages (which will be submitted next).  If I do that, will 
you have any objections?

Alternatively, we could look through all the packages currently in Core 
and Extras and convince ourselves that nothing else owns the admittedly
very generic-looking /usr/include/ctl.h file -- so that its OK for it to 
be there.

I'm leaning towards the first one but will be OK with either solution 
(or maybe even an alternative?).  Please let me know what you prefer and 
I'll probably just go ahead and implement it.  :-)


Comment 4 Jason Tibbitts 2006-10-15 03:53:50 UTC
Well, moving things to a directory is a reasonable solution.  I guess the
question is whether /usr/include/ctl is really any better than
/usr/include/ctl.h and whether it's worth the trouble at all.  I mean, every
downstream package will need work to accommodate.  And normally I wouldn't bring
it up at all, except that there has been plenty of discussion about this issue
recently.

It's easy to check to see if anything else owns the file:
   repoquery --whatprovides /usr/include/ctl.h
which shows that indeed nothing owns it.  (I had checked this earlier.)

I guess that putting things in a directory under /usr/include is conceptually
quite clean and should be adopted by more packages.  There shouldn't be any
objections as long as it's easy to fix the dependent packages.

Comment 5 Patrice Dumas 2006-10-15 08:52:27 UTC
My personnal opinion is that it is bad upstream practice to use
names that can clash, but I don't think this is an issue we should
solve at the packaging level except when a real conflict occurs.
I think that reporting the issue upstream should be a must item,
however.

Comment 6 Ed Hill 2006-10-15 18:42:42 UTC
OK, here's an update that puts the three headers in a subdir:

  http://mitgcm.org/eh3/fedora_misc/libctl-3.0.2-4.src.rpm

and I'll deal with the "-I/usr/include/ctl" for the packages that 
use it.  Its easy enough to do!

Comment 7 Jason Tibbitts 2006-10-15 20:57:34 UTC
OK, that looks fine to me.

APPROVED

Comment 8 Ed Hill 2006-10-17 17:40:03 UTC
Hi Jason, thank you for the review!  It built successfully for FC-5 and 
devel so I'm closing this bug.  Thanks again!


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