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 193896 - Review Request: libreadline-java - Java wrapper for the GNU-readline library
Summary: Review Request: libreadline-java - Java wrapper for the GNU-readline library
Keywords:
Status: CLOSED RAWHIDE
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 193898
TreeView+ depends on / blocked
 
Reported: 2006-06-02 19:15 UTC by Igor Foox
Modified: 2010-08-02 16:24 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2006-09-09 01:19:46 UTC
Type: ---
Embargoed:
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Igor Foox 2006-06-02 19:15:27 UTC
Spec URL: http://people.redhat.com/ifoox/extras/libreadline-java.spec
SRPM URL: http://people.redhat.com/ifoox/extras/libreadline-java-0.8.0-10jpp_1fc.src.rpm
Description: 
Java-Readline is a port of GNU Readline for Java.  Or, to be more
precise, it is a JNI-wrapper to Readline. It is distributed under
the LGPL.

I am submitting this package with several other packages (5 in total), and these are my first packages for Extras, so they may need a sponsor.

Comment 1 Jason Tibbitts 2006-06-22 21:56:51 UTC
Since Hans has offered to sponsor you once a few of your packages are in shape,
I thought I'd take a look at one.  I can't take this for review until you've
been sponsored but I can make some comments.

The package builds fine in mock (x86_64, development) with the reduced
buildroot.    The debuginfo package comes up a bit empty due to the usual rpm
bugs with java; adding the following to the end of the %build section helps, but
you'll want to macroize it to match the rest of the spec:

# Fix debuginfo generation
rm -f org test
ln -s src/org
ln -s src/test

rpmlint has this to say:
W: libreadline-java non-standard-group Development/Libraries/Java
W: libreadline-java no-soname /usr/lib64/libJavaReadline.so.0.8.0
W: libreadline-java devel-file-in-non-devel-package /usr/lib64/libJavaReadline.so
W: libreadline-java-javadoc non-standard-group Development/Java

I'm not sure what's happening with the package groups; Development/Libraries
would seem appropriate unless someone has officially added the Java subgroup
(which wasn't done for the other languages as far as I know.

The unversioned .so file cannot go in the main package; it must go in -devel.

I don't know what's causing the no-soname error; it looks like the upstream
Makefile doesn't call GCC with -Wl,-soname,blah, but I'm not sure if this is a
blocker in this situation.

About the spec:

The gcj_support thing makes things pretty nasty to read; I wonder if it's really
necessary.

Don't set Epoch unless you need it to be a nonzero value.

No need to use an Epoch on the readline Require.

I'm not sure why you need the java_readline and gnu.readline provides.

Don't use Distribution or Vendor.

You need:
Requires(post): /sbin/ldconfig
Requires(postun): /sbin/ldconfig

instead of Requires: /sbin/ldconfig.

Comment 2 Igor Foox 2006-06-23 21:18:56 UTC
Hi Jason, thanks for the comments. I've created a new SRPM and spec here:
http://people.redhat.com/ifoox/extras/libreadline-java.spec
http://people.redhat.com/ifoox/extras/libreadline-java-0.8.0-10jpp_2fc.src.rpm

(In reply to comment #1)
> Since Hans has offered to sponsor you once a few of your packages are in shape,
> I thought I'd take a look at one.  I can't take this for review until you've
> been sponsored but I can make some comments.
> 
> The package builds fine in mock (x86_64, development) with the reduced
> buildroot.    The debuginfo package comes up a bit empty due to the usual rpm
> bugs with java; adding the following to the end of the %build section helps, but
> you'll want to macroize it to match the rest of the spec:
> 
> # Fix debuginfo generation
> rm -f org test
> ln -s src/org
> ln -s src/test

Done.
 
> rpmlint has this to say:
> W: libreadline-java non-standard-group Development/Libraries/Java
> W: libreadline-java no-soname /usr/lib64/libJavaReadline.so.0.8.0
> W: libreadline-java devel-file-in-non-devel-package /usr/lib64/libJavaReadline.so
> W: libreadline-java-javadoc non-standard-group Development/Java
> 
> I'm not sure what's happening with the package groups; Development/Libraries
> would seem appropriate unless someone has officially added the Java subgroup
> (which wasn't done for the other languages as far as I know.

Done. 

> 
> The unversioned .so file cannot go in the main package; it must go in -devel.

Can you elaborate on why an unversioned .so cannot go into the main package?

> I don't know what's causing the no-soname error; it looks like the upstream
> Makefile doesn't call GCC with -Wl,-soname,blah, but I'm not sure if this is a
> blocker in this situation.

I'll look into this next week.
 
> About the spec:
> 
> The gcj_support thing makes things pretty nasty to read; I wonder if it's really
> necessary.

The main reason for that in Java packages is that we can build these packages
in RHEL using the same spec file as in Fedora. This really cuts down on
maintenance time, and is what we do for all eclipse-* packages, as well as other
java packages in Core. I realize that it makes it a bit ugly to look at, but
I think it's a neccessary evil.

> 
> Don't set Epoch unless you need it to be a nonzero value.
> 
> No need to use an Epoch on the readline Require.

All these are fixed.
 
> I'm not sure why you need the java_readline and gnu.readline provides.

I'm not entirely sure why that was there, it was from the JPackage  RPM.
It seems safe to remove it, so I did.

> Don't use Distribution or Vendor.
> 
> You need:
> Requires(post): /sbin/ldconfig
> Requires(postun): /sbin/ldconfig
> 
> instead of Requires: /sbin/ldconfig.

These are fixed as well.

Thanks again.


Comment 3 Jason Tibbitts 2006-06-23 22:29:37 UTC
(In reply to comment #2)

> Can you elaborate on why an unversioned .so cannot go into the main package?

The simple answer is that the guidelines demand it: see
http://fedoraproject.org/wiki/Packaging/ReviewGuidelines:

- MUST: If a package contains library files with a suffix (e.g. libfoo.so.1.1),
then library files that end in .so (without suffix) must go in a -devel package.

The more difficult answer is that I don't completely understand it myself; I
believe it has to do with the fact that applications must link against the
versioned .so, leaving the unversioned one unneeded for regular operation, along
with the question of what the unversioned .so would mean in the face of a future
potential compat-libreadlinejava080 package.

Comment 4 Igor Foox 2006-06-26 18:38:48 UTC
Updated files:
http://people.redhat.com/ifoox/extras/libreadline-java-0.8.0-10jpp_3fc.src.rpm
http://people.redhat.com/ifoox/extras/libreadline-java.spec

I've moved the unversioned .so file into a -devel package, and also changed the
Group for the -javadoc package to be Development/Libraries instead of
Development/Java.
The only remaining problem seems to be the no-soname error, which I'm not sure
what to do about.

Comment 5 Jason Tibbitts 2006-09-07 19:46:11 UTC
Somehow this slipped through the cracks, but I'll take it for review now.

The no-soname issue is, as far as I can tell, not a blocker.  The jpp stuff in
the release, however, is.  You should just use a a plain integer release (with
the appended dist tag).

The simplest way to do this and interleave cleanly with the jpackage release
number is to use, in this case, 0.8.0-11{?%dist} (i.e. drop the "jpp", increment
the release by one, and append the dist tag.)  Or you can divorce yourself from
the jpackage numbering altogether; it's your choice.

Comment 6 Jason Tibbitts 2006-09-08 04:07:20 UTC
Hmm, this builds but will not install on current rawhide:

Error: Missing Dependency: readline = 5.0 is needed by package libreadline-java

Rawhide has readline 5.1; I changed readline_ver to match and it seems to be OK.  

Also, there are a few rpmlint warnings; I think it now checks for more than it
did when I first looked at this package.

W: libreadline-java macro-in-%changelog _jnidir
W: libreadline-java macro-in-%changelog _libdir
   You need to double any percent symbols that appear in the changelog, lest
they be expanded in the final RPM.

E: libreadline-java no-cleaning-of-buildroot
   You need rpm -rf %{buildroot} at the beginning of %install.

W: libreadline-java mixed-use-of-spaces-and-tabs
   It's complaining about a couple of tabs after Group: (use "less -U" to see
them).  This isn't a blocker.

The debuginfo package is now missing the source again.  I'm at a loss as to why
this is continually a problem, but it's not really a blocker.  Perhaps some day
someone will actually understand what's going on here.

You need to include COPYING.LIB as %doc.

Shouldn't the jar file go in /usr/share/java?  If not, nothing in your
dependency chain owns /usr/lib/java.

* source files match upstream:
   501720ddded45eaedf429b7cc356107c  libreadline-java-0.8.0-src.tar.gz
X package meets naming and packaging guidelines (non-numeric release bits).
* specfile is properly named, is cleanly written and uses macros consistently.
X dist tag is present.
* build root is correct.
* license field matches the actual license.
* license is open source-compatible.
X license text upstream but not 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).
? debuginfo package looks complete.
X rpmlint has valid complaints
X final provides and requires are sane:
   libreadline-java = 0.8.0-10jpp_3fc
  =
X  readline = 5.0
   /sbin/ldconfig
   java-gcj-compat >= 1.0.31
   /bin/sh
* %check is not present; no test suite upstream.
* a shared library is present and ldconfig is called properly.
* package is not relocatable.
? owns the directories it creates.
* doesn't own any directories it shouldn't.
* no duplicates in %files.
* file permissions are appropriate.
* scriptlets OK (ldconfig, gcj-db)
* code, not content.
* javadocs split out into separate jackage.
* %docs are not necessary for the proper functioning of the package.
* no headers.
* no pkgconfig files.
* no libtool .la droppings.
* unversioned .so in -devel subpackage.

Comment 7 Igor Foox 2006-09-08 14:33:31 UTC
New files:
http://people.redhat.com/ifoox/extras/libreadline-java-0.8.0-11.src.rpm
http://people.redhat.com/ifoox/extras/libreadline-java.spec

(In reply to comment #6)
> Hmm, this builds but will not install on current rawhide:
> 
> Error: Missing Dependency: readline = 5.0 is needed by package libreadline-java
> 
> Rawhide has readline 5.1; I changed readline_ver to match and it seems to be OK.  

Fixed.

> Also, there are a few rpmlint warnings; I think it now checks for more than it
> did when I first looked at this package.
> 
> W: libreadline-java macro-in-%changelog _jnidir
> W: libreadline-java macro-in-%changelog _libdir
>    You need to double any percent symbols that appear in the changelog, lest
> they be expanded in the final RPM.

Fixed.

> E: libreadline-java no-cleaning-of-buildroot
>    You need rpm -rf %{buildroot} at the beginning of %install.

Fixed.
 
> W: libreadline-java mixed-use-of-spaces-and-tabs
>    It's complaining about a couple of tabs after Group: (use "less -U" to see
> them).  This isn't a blocker.

Fixed.

> The debuginfo package is now missing the source again.  I'm at a loss as to why
> this is continually a problem, but it's not really a blocker.  Perhaps some day
> someone will actually understand what's going on here.

Hopefully. :-)

> You need to include COPYING.LIB as %doc.

Fixed.
 
> Shouldn't the jar file go in /usr/share/java?  If not, nothing in your
> dependency chain owns /usr/lib/java.

You are right, I'm not sure why it was in /usr/lib/java. Fixed.
 
> * source files match upstream:
>    501720ddded45eaedf429b7cc356107c  libreadline-java-0.8.0-src.tar.gz
> X package meets naming and packaging guidelines (non-numeric release bits).
> * specfile is properly named, is cleanly written and uses macros consistently.
> X dist tag is present.
Fixed.
> * build root is correct.
> * license field matches the actual license.
> * license is open source-compatible.
> X license text upstream but not 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).
> ? debuginfo package looks complete.
> X rpmlint has valid complaints
> X final provides and requires are sane:
>    libreadline-java = 0.8.0-10jpp_3fc
>   =
> X  readline = 5.0
>    /sbin/ldconfig
>    java-gcj-compat >= 1.0.31
>    /bin/sh
> * %check is not present; no test suite upstream.
> * a shared library is present and ldconfig is called properly.
> * package is not relocatable.
> ? owns the directories it creates.
> * doesn't own any directories it shouldn't.
> * no duplicates in %files.
> * file permissions are appropriate.
> * scriptlets OK (ldconfig, gcj-db)
> * code, not content.
> * javadocs split out into separate jackage.
> * %docs are not necessary for the proper functioning of the package.
> * no headers.
> * no pkgconfig files.
> * no libtool .la droppings.
> * unversioned .so in -devel subpackage.



Comment 8 Jason Tibbitts 2006-09-08 21:43:33 UTC
OK, the package builds fine in devel with no problems.

rpmlint complains much less (just the two that are OK).

The other issues I had are fixed as well.

APPROVED


Comment 9 Igor Foox 2006-09-08 23:27:25 UTC
New files:
http://people.redhat.com/ifoox/extras/libreadline-java.spec
http://people.redhat.com/ifoox/extras/libreadline-java-0.8.0-12.src.rpm

I was looking at getting bug #193898 into shape, and saw that I need to switch
libreadline-java to use libedit instead of readline due to licensing issues with
Jython. I uploaded a new spec and srpm. There are no new rpmlint warnings, and
the  debuginfo warning has disappeared. Please have a quick look and let me
know, if this looks reasonable, and I will build it.

Comment 10 Jason Tibbitts 2006-09-09 00:55:19 UTC
Yes, this still builds fine and looks pretty much the same as the previous
version outside of a couple of name changes.

Comment 11 Igor Foox 2006-09-09 01:19:46 UTC
Built in development. Closing.

Comment 12 Ville Skyttä 2006-09-09 09:44:26 UTC
By the way, it would be great if the readline->libedit change would also make it
possible to enable history support in bsh.  OTOH, bsh is in core so a
buildrequires: libreadline-java wouldn't work, but perhaps it'd be possible to
accomplish it without the build dependency some way?

Comment 13 Ruediger Landmann 2010-08-02 04:53:55 UTC
Package Change Request
=======================
Package Name: libreadline-java
New Branches: EL-5
Owners: rlandmann
InitialCC: 

I have discussed this with Alexander Kurtakov, the current package owner, and he is happy for me to maintain this branch.

Comment 14 Kevin Fenzi 2010-08-02 16:24:21 UTC
GIT done (by process-git-requests).


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