Bug 193896 - Review Request: libreadline-java - Java wrapper for the GNU-readline library
Review Request: libreadline-java - Java wrapper for the GNU-readline library
Status: CLOSED RAWHIDE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Jason Tibbitts
Fedora Package Reviews List
:
Depends On:
Blocks: FE-ACCEPT 193898
  Show dependency treegraph
 
Reported: 2006-06-02 15:15 EDT by Igor Foox
Modified: 2010-08-02 12:24 EDT (History)
2 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-09-08 21:19:46 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Igor Foox 2006-06-02 15:15:27 EDT
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 17:56:51 EDT
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 17:18:56 EDT
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 18:29:37 EDT
(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 14:38:48 EDT
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 15:46:11 EDT
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 00:07:20 EDT
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 10:33:31 EDT
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 17:43:33 EDT
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 19:27:25 EDT
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-08 20:55:19 EDT
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-08 21:19:46 EDT
Built in development. Closing.
Comment 12 Ville Skyttä 2006-09-09 05:44:26 EDT
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 00:53:55 EDT
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 12:24:21 EDT
GIT done (by process-git-requests).

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