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 222347 - Review Request: g-wrap - A tool for creating Scheme interfaces to C libraries
Summary: Review Request: g-wrap - A tool for creating Scheme interfaces to C libraries
Keywords:
Status: CLOSED CURRENTRELEASE
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:
TreeView+ depends on / blocked
 
Reported: 2007-01-11 20:09 UTC by Bill Nottingham
Modified: 2014-03-17 03:04 UTC (History)
1 user (show)

Fixed In Version: 1.9.6-12
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2007-06-18 18:51:13 UTC
Type: ---
Embargoed:
j: fedora-review+
j: fedora-cvs+


Attachments (Terms of Use)

Description Bill Nottingham 2007-01-11 20:09:15 UTC
Spec URL: http://people.redhat.com/notting/review/g-wrap.spec
SRPM URL: http://people.redhat.com/notting/review/g-wrap-1.9.6-8.src.rpm
Description: scheme/C interface library used for GNUCash

Part of reviewing the GNUCash stack for the grand merger.

Only rpmlint complaints I know of is that g-wrap-devel has an include in /usr/{lib,lib64} - this is intentional as it's wordsize-specific and can't go in /usr/include.

Latest upstream version is 1.9.7, but as that has caused issues with gnucash, and gnucash is the only consumer of g-wrap... not updating.

Comment 2 Bill Nottingham 2007-01-18 17:23:58 UTC
Thanks, fixed. -9 uploaded.

Note: the currently-in-development version of gnucash moves from g-wrap to swig
for its guile bindings; when that version is added to the development tree,
g-wrap will be orphaned, as nothing else uses it. I'll still maintain it for
older trees that need it.

Comment 3 Jason Tibbitts 2007-06-08 20:50:45 UTC
rpmlint has some complaints:

This just looks like a typo in the changelog:
   W: g-wrap incoherent-version-in-changelog 1.9.6-10 1.9.6-9

I'm not really sure about these:
   W: g-wrap unused-direct-shlib-dependency 
    /usr/lib64/libgw-guile-standard.so.0.0.0 /lib64/libpthread.so.0
   W: g-wrap unused-direct-shlib-dependency 
    /usr/lib64/libgw-guile-gw-glib.so.0.0.0 /lib64/libglib-2.0.so.0
   W: g-wrap unused-direct-shlib-dependency 
    /usr/lib64/libgw-guile-gw-glib.so.0.0.0 /lib64/libpthread.so.0
I don't think they're especially problematic and I guess it's possible that
they're necessary.

I admit I find it odd to see /usr/lib64/g-wrap/include/ffi.h:
   E: g-wrap-devel only-non-binary-in-usr-lib
Why not /usr/share/g-wrap, or with the rest of the installed headers in
/usr/include/g-wrap?

Seems to me that the license is LGPL, not GPL.  The libffi license seems to be
MIT but that's compatible and doesn't change the overall license of the package.

I note that there is a test suite, but some notes about it being disabled on
x86_64.  I tried a quick "make check" on i386 rawhide and all the tests failed
without attempting to actually test anything, so I must be missing something.

Review:
* source files match upstream:
   ddb0e31d40581402d6d7045cce7cdc79e0bc0627831a4b12012f45703446d311  
   g-wrap-1.9.6.tar.gz
* package meets naming and versioning guidelines.
* specfile is properly named, is cleanly written and uses macros consistently.
* summary is OK.
* description is OK.
* build root is OK.
X license field matches the actual license.
* license is open source-compatible.
* license text included in package.
O latest version is 1.9.8, but there are issues preventing its use.
* 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:
  g-wrap-1.9.6-9.x86_64.rpm
   libgw-guile-gw-glib.so.0()(64bit)
   libgw-guile-standard.so.0()(64bit)
   libgwrap-core-runtime.so.0()(64bit)
   libgwrap-guile-runtime.so.0()(64bit)
   g-wrap = 1.9.6-9
  =
   /sbin/ldconfig
   guile
   libglib-2.0.so.0()(64bit)
   libguile.so.17()(64bit)
   libgw-guile-gw-glib.so.0()(64bit)
   libgw-guile-standard.so.0()(64bit)
   libgwrap-core-runtime.so.0()(64bit)
   libgwrap-guile-runtime.so.0()(64bit)
   libpthread.so.0()(64bit)
   libpthread.so.0(GLIBC_2.2.5)(64bit)

  g-wrap-devel-1.9.6-9.x86_64.rpm
   g-wrap-devel = 1.9.6-9
  =
   /bin/sh
   /sbin/install-info
   g-wrap = 1.9.6
   guile-devel
   libgw-guile-gw-glib.so.0()(64bit)
   libgw-guile-standard.so.0()(64bit)
   libgwrap-core-runtime.so.0()(64bit)
   libgwrap-guile-runtime.so.0()(64bit)
   pkgconfig

* %check is not present.  Test suite doesn't seem to do much.
* shared libraries present; ldconfig called as appropriate.
* unversioned .so files 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.
* scriptlets are OK (ldconfig, install-info).
* 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 subpackage.
* a pkgconfig file is present in -devel; pkgconfig dependency is there.
* no static libraries.
* no libtool .la files.


Comment 4 Bill Nottingham 2007-06-08 21:05:17 UTC
(In reply to comment #3)
> rpmlint has some complaints:
> I admit I find it odd to see /usr/lib64/g-wrap/include/ffi.h:
>    E: g-wrap-devel only-non-binary-in-usr-lib
> Why not /usr/share/g-wrap, or with the rest of the installed headers in
> /usr/include/g-wrap?

It's wordsize-specific - having it in /usr/share or /usr/include would cause
multilib conflicts.

> Seems to me that the license is LGPL, not GPL.  The libffi license seems to be
> MIT but that's compatible and doesn't change the overall license of the package.

Oops, fixed in CVS.

> I note that there is a test suite, but some notes about it being disabled on
> x86_64.  I tried a quick "make check" on i386 rawhide and all the tests failed
> without attempting to actually test anything, so I must be missing something.

The test suite has never seemed to work. I should have probably put more effort
into figuring out why.

So...

> * no libtool .la files.

These are re-added in the current package; see bug 238263. gnucash uses a
version of libtool_ltdl as a module loader, and it does not work without either
a) .la files b) .so files in the main package. As moving the .so files breaks
multilib development, it was simplest to put the .la files back.


Comment 5 Jason Tibbitts 2007-06-08 21:21:29 UTC
Oh, I was reviewing the -9 package since that one was the last one mentioned in
this ticket and the one the spec link above points to.  Which version is
current?  Should I just get it from the old core CVS?

Comment 6 Bill Nottingham 2007-06-08 21:37:21 UTC
Just grab the latest from /cvs/pkgs.

Comment 7 Jason Tibbitts 2007-06-08 22:54:40 UTC
OK, all that's really missing is some commentary in the spec as to why the .la
files are included and why those headers are under /usr/lib.  Heck, since this
has already been imported, I'll even commit some if you like.

But I don't see anything holding up this package, so

APPROVED

Comment 8 Bill Nottingham 2007-06-09 00:38:45 UTC
Comments added in CVS.

Package Change Request
======================
Package Name: g-wrap
New Branches: EL-4 EL-5

Comment 9 Jason Tibbitts 2007-06-09 00:53:35 UTC
CVS done.

Comment 10 Jason Tibbitts 2007-06-18 18:45:28 UTC
Can this ticket be closed now?

Comment 11 Bill Nottingham 2007-06-18 18:51:13 UTC
Sure.


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