Bug 453119 - Review Request: libvirt-java: Java bindings for the libvirt library
Summary: Review Request: libvirt-java: Java bindings for the libvirt library
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
low
medium
Target Milestone: ---
Assignee: Jason Tibbitts
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2008-06-27 12:37 UTC by Daniel Veillard
Modified: 2008-08-10 11:54 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2008-08-10 11:54:38 UTC
Type: ---
Embargoed:
j: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Daniel Veillard 2008-06-27 12:37:09 UTC
Description of problem:


Version-Release number of selected component (if applicable):


How reproducible:


Steps to Reproduce:
1.
2.
3.
  
Actual results:


Expected results:


Additional info:

Comment 1 Daniel Veillard 2008-06-27 12:54:28 UTC
Oops apparenty I pressed enter before finishing to complete the bug ...

SRPMS: ftp://libvirt.org/libvirt/java/libvirt-java-0.1.0-1.fc9.src.rpm
spec: ftp://libvirt.org/libvirt/java/libvirt-java.spec

result of rpmlint:

wei:~ -> rpmlint rpms/SRPMS/libvirt-java-0.1.0-1.fc9.src.rpm 
libvirt-java.src:88: E: files-attr-not-set
libvirt-java.src: W: invalid-license LGPL
libvirt-java.src: E: unknown-key GPG#de95bc1f
libvirt-java.src: W: strange-permission libvirt-java.spec 0600
1 packages and 0 specfiles checked; 2 errors, 2 warnings.
wei:~ -> 

  the strange thing is that if I check the spec file I don't get
the same ...
wei:~/libvirt-java -> rpmlint libvirt-java.spec
0 packages and 1 specfiles checked; 0 errors, 0 warnings.
wei:~/libvirt-java -> 

  I think I found the error on line 88, 
%files javadoc
%defattr(-,root,root)  <- that was missing

  For the invalid licence I'm a bit puzzled... We include a
LGPL 2.1 COPYING.LIB and reference it in the source code.
I guess then it should be labelled LGPLv2 instead of LGPL
as the spec file now says.

  I have tried to adhere to the jpackage-utils-policy text
and follow some of the few examples of existing java bindings
using JNI around libraries, but there isn't that much in 
Fedora (the Java gnome bindings are a bit complex to follow
directly), so I had to make a few guesses when generating the
spec file. I hope it's okay ...

Daniel

Daniel


Comment 2 Jason Tibbitts 2008-06-27 19:24:20 UTC
Unfortunately it looks like libvirt has broken dependencies in rawhide:

Error: Missing Dependency: libgnutls.so.13()(64bit) is needed by package libvirt

so this doesn't build at the moment.

Comment 3 Daniel Veillard 2008-06-27 20:34:34 UTC
I built libvirt-0.4.4 when I released it in CVS on Wednesday,
it first broke because qemu broke, but I have rebuilt it as libvirt-0.4.4-1.fc10
'libvirt-0.4.4-1.fc10 Tag: dist-f10 Status: complete Built'

usually this mean the build suceeded, and unless rawhide didn't picked it
up that build from 2 days ago , or libgnutls released again an incompatible
version, you should not see the dependancy problem, really ...

I woudl have looked at the build in bodhi but ssl errors prevents me from
loggin in ...

Daniel



Comment 4 Jason Tibbitts 2008-06-27 20:39:21 UTC
It's quite possible that my mirror hasn't caught up yet; I'll keep trying.

Comment 5 Jason Tibbitts 2008-06-28 17:14:02 UTC
OK, it looks like my mirror has updated and the dependencies install correctly,
but the package doesn't build:

In file included from org_libvirt_VirNetwork.c:1:
org_libvirt_VirNetwork.h:2:17: error: jni.h: No such file or directory
In file included from org_libvirt_VirNetwork.c:1:
org_libvirt_VirNetwork.h:15: error: expected '=', ',', ';', 'asm' or
'__attribute__' before 'jstring'

and a bunch of similar errors follow.  Many packages seem to provide a jni.h so
I'm not really sure what's missing.

Comment 6 Daniel Veillard 2008-07-01 14:21:08 UTC
Okay, finding the right include paths for JNI stuff is fairly messy,
and well gcj-devel has a bug plugging them by default in gcc include
path (fun see #453572).
I think I have ironed out the problems, regenerated a new 0.1.1 release

SRPMS: ftp://libvirt.org/libvirt/java/libvirt-java-0.1.0-1.fc9.src.rpm
spec: ftp://libvirt.org/libvirt/java/libvirt-java.spec

wei:~/libvirt-java -> rpmlint
/u/veillard/rpms/SRPMS/libvirt-java-0.1.1-1.fc9.src.rpm
libvirt-java.src: E: unknown-key GPG#de95bc1f
libvirt-java.src: W: strange-permission libvirt-java.spec 0600
1 packages and 0 specfiles checked; 1 errors, 1 warnings.
wei:~/libvirt-java -> 

  I think if one ignore the fact that rpmlint doesn't know my PGP key
this looks decent.

  For the spec file I used the following:

%define java	java
and
Requires:	%{java} >= 1.5.0
BuildRequires:	%{java}-devel >= 1.5.0

because some users may want to define more precisely which java environment
they want for development. Ideally this should remain that flexible (the src
RPM rebuit even on a RHEL4 with Java SDK in /usr/lib/jvm/java-1.5.0-ibm-1.5.0.7)

I hope it's okay now,

Daniel

Comment 7 Daniel Veillard 2008-07-01 14:22:25 UTC
Forgot to update the new SRPM URL, it is at: 
SRPMS: ftp://libvirt.org/libvirt/java/libvirt-java-0.1.1-1.fc9.src.rpm


Daniel

Comment 8 Jason Tibbitts 2008-07-02 22:22:46 UTC
Now the build is failing elsewhere.  Unfortunately there are several hundred
lines of errors, so I just sent the src.rpm to koji:
http://koji.fedoraproject.org/koji/taskinfo?taskID=693438
A failing log:
http://koji.fedoraproject.org/koji/getfile?taskID=693440&name=build.log

It's trivial to do; I just ran:
  koji build --scratch dist-f10 libvirt-java-0.1.1-1.fc9.src.rpm



Comment 9 Daniel Veillard 2008-07-03 07:54:45 UTC
yeah it's picking gcj package to compile, which unfortunately defaults to
a very old java 1.4 version by default, and then miscompiles the enums
in the java binding code...
the good point is that it's already solved in CVS, the bad point is that
I will have to make a new release again...
The near complete lack of instructions for packaging JNI source code 
makes things way more difficult than they should, hopefully that will be
ironed out and the Fedora Java packaging page will be updated, c.f.:
 https://www.redhat.com/archives/fedora-devel-java-list/2008-July/msg00000.html
and
 https://www.redhat.com/archives/fedora-devel-java-list/2008-July/msg00027.html

Sorry for the mess, a bit too many iteration, but I think it's not really my 
fault :-\

I will update this bug as soon as I have something updated accordingly and
which builds in koji

Daniel

Comment 10 Daniel Veillard 2008-07-03 15:36:30 UTC
Okay I think I have most things ironed out now.
First I revamped the configure and spec file to work accordingly to suggestions
from David Walluck.
https://www.redhat.com/archives/fedora-devel-java-list/2008-July/msg00029.html

Then I made sure the thing would still build in kodji
http://koji.fedoraproject.org/koji/taskinfo?taskID=694617
I also tested in F9 and F8 with the rpm just before 0.1.2 release:
http://koji.fedoraproject.org/koji/taskinfo?taskID=694524
http://koji.fedoraproject.org/koji/taskinfo?taskID=694547

Then made a new release 0.1.2 and checked with rpmlint:

wei:~/libvirt-java -> rpmlint
/u/veillard/rpms/SRPMS/libvirt-java-0.1.2-1.fc9.src.rpm
libvirt-java.src: E: unknown-key GPG#de95bc1f
libvirt-java.src: W: strange-permission libvirt-java.spec 0600
1 packages and 0 specfiles checked; 1 errors, 1 warnings.
wei:~/libvirt-java -> 

SRPMS: ftp://libvirt.org/libvirt/java/libvirt-java-0.1.2-1.fc9.src.rpm
spec: ftp://libvirt.org/libvirt/java/libvirt-java.spec

  Hopefully that time it's the good one :-)

Daniel

Comment 11 Jason Tibbitts 2008-07-04 01:42:16 UTC
Yes, this builds.  In addition to the strange-permission complaint above (which
isn't a problem) rpmlint says:

  libvirt-java-javadoc.x86_64: W: non-standard-group Development/Documentation
which is bogus; we don't care what goes in Group:

  libvirt-java.x86_64: E: zero-length /usr/share/doc/libvirt-java-0.1.2/NEWS
There's generally no point in shipping an empty file.  Is it needed for something?

 libvirt-java.x86_64: W: unused-direct-shlib-dependency
  /usr/lib64/libvirt_jni.so.0.1.2 /usr/lib64/libxenstore.so.3.0
This just means that libvirt_jni links against libxenstore but doesn't call any
functions in it.  This is pretty common with pkgconfig files since they
maximally specify link flags; generally easy to correct by linking with
"-Wl,--as-needed" but not really a big issue as long as it doesn't result in
expanded dependencies, which it doesn't here.

The Source: tag should contain a URL to the sources:
  Source: http://libvirt.org/sources/java/libvirt-java-%{version}.tar.gz

Your BuildRoot must contain at least
%{release}.http://fedoraproject.org/wiki/Packaging/Guidelines#BuildRoot_tag

The license looks to be LGPLv2+ to me.  None of the source files have the proper
license blocks, and so the license in the COPYING.LIB files allows us to choose
any version of the LGPL that has ever or will ever exist.

The use of gcj is only a strong suggestion but not a requirement, so I'm
certainly not going to block on it here.  My understanding is that we still have
platforms where gcj is needed, but I guess that's between you and the (rest of)
the java team.

The javadoc subpackage needs dependencies on the main package and jpackage-utils.

I'm pretty sure that test.java file isn't really useful unless it has an
existing virtual machine to talk to.  Please correct me if I'm wrong.

I have to ask: what is the point of the libvert-src zipfile?  Why would a Java
package need to include its source code?

* source files match upstream:
   c746b1f8a76bdf16c0b9eb6751fa7998712ba07788a3ab5a6b4c0842a4bb7cb4  
   libvirt-java-0.1.2.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.
* dist tag is present.
X build root needs at least %{release}.
X license field does not match the actual license.
* license is open source-compatible.
* license text included in package.
* latest version is being packaged.
* BuildRequires are proper.
* %clean is present.
* package builds in mock (rawhide, x86_64).
* package installs properly.
* debuginfo package looks complete (no java files since gcj isn't called).
X rpmlint has a valid complaint (zero-length NEWS file).
X final provides and requires:
  libvirt-java-0.1.2-1.fc10.x86_64.rpm
   libvirt_jni.so.0()(64bit)
   libvirt-java = 0.1.2-1.fc10
  =
   /sbin/ldconfig
   java >= 1.5.0
   jpackage-utils
   libvirt >= 0.4.0
   libvirt.so.0()(64bit)
   libvirt_jni.so.0()(64bit)
   libxenstore.so.3.0()(64bit)

  libvirt-java-devel-0.1.2-1.fc10.x86_64.rpm
   libvirt-java-devel = 0.1.2-1.fc10
  =
   libvirt-devel >= 0.4.0
   libvirt-java = 0.1.2-1.fc10
   libvirt_jni.so.0()(64bit)
   pkgconfig

  libvirt-java-javadoc-0.1.2-1.fc10.x86_64.rpm
   libvirt-java-javadoc = 0.1.2-1.fc10
  =
X no dependencies on jpackage-utils or main package.

* %check is not present; test code can't be run in the buildsys.
* shared libraries installed:
   ldconfig is called properly.
   unversioned .so links 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 OK (ldconifg).
* code, not content.
* %docs are not necessary for the proper functioning of the package.
* no headers.
* pkgconfig files in the -devel package; pkgconfig dependency included.
* no static libraries.
* no libtool .la files.
* no pre-built jars
* single jar, named after the package
* jarfiles are under _javadir.
* javadocs are under _javadocdir.

Comment 12 Daniel Veillard 2008-07-04 08:05:41 UTC
Okay, thanks a lot of the review, let's look at things in order:

> libvirt-java-javadoc.x86_64: W: non-standard-group Development/Documentation
> which is bogus; we don't care what goes in Group:

 Well actually, I do as rpmfind maintainer. Seems that Development/Documentation
is a classic for javadoc rpms, see:
  http://rpmfind.net/linux/RPM/Development_Documentation.html
I would rather stick to this unless there is a better Group value suggestion...

> libvirt-java.x86_64: E: zero-length /usr/share/doc/libvirt-java-0.1.2/NEWS
> There's generally no point in shipping an empty file.  Is it needed for
> something?

  Oh it's empty now, but I intend to put in place a News section on the 
web site and fill it automatically, it's just not there yet. It won't stay
zero-lenght, just a TODO upstream

> libvirt-java.x86_64: W: unused-direct-shlib-dependency
>  /usr/lib64/libvirt_jni.so.0.1.2 /usr/lib64/libxenstore.so.3.0
> This just means that libvirt_jni links against libxenstore but doesn't call any
> functions in it.  This is pretty common with pkgconfig files since they
> maximally specify link flags; generally easy to correct by linking with
> "-Wl,--as-needed" but not really a big issue as long as it doesn't result in
> expanded dependencies, which it doesn't here.

  yes that's a libvirt dependancy, in that setup. if libvirt is doesn't have
Xen support it won't be used. I will look at adding the flag, i'm just a bit
vary of adding compiler/linker specifics commands to keep compatibility with
other platforms like solaris without gcc.

> The Source: tag should contain a URL to the sources:
>  Source: http://libvirt.org/sources/java/libvirt-java-%{version}.tar.gz

trivial, fixed
 
> Your BuildRoot must contain at least %{release}.
> http://fedoraproject.org/wiki/Packaging/Guidelines#BuildRoot_tag

trivial, fixed ... surprizingly I'm sure i had done that initially, but
I think I did some copy from libvirt spec file. I'm fixing libvirt/libxml2
and libxslt spec files accordingly, dohh...

>The license looks to be LGPLv2+ to me. None of the source files have the proper
>license blocks, and so the license in the COPYING.LIB files allows us to choose
>any version of the LGPL that has ever or will ever exist.

The intent is certainly to be usable under LGPLv2, and keep the maximum
flexibility from there, I'm fine with that, spec file
fixed accordingly.

>The use of gcj is only a strong suggestion but not a requirement, so I'm
>certainly not going to block on it here.  My understanding is that we still have
>platforms where gcj is needed, but I guess that's between you and the (rest of)
the java team.

  Apparently the Java team don't want to specify the JDK used in the spec file
have only the dependency on the java and java-devel (as well as JPackage) I
followed that:
http://fedoraproject.org/wiki/Packaging/Java#BuildRequires_and_Requires

the comment explaining how to switch to a different JDK is if a user has 
a specific need and want to do a manual rebuild.
So I guess the current main Requires/BuildRequires are fine.

> The javadoc subpackage needs dependencies on the main package and
> jpackage-utils.

hum, here we have conflicting informations. The rule to follow the JPackage
policy is different

---------- http://fedoraproject.org/wiki/Packaging/Java ----------
We include relevant sections of the JPackage guidelines here but caution that
the canonical document will always reside upstream: JPackage Guidelines . Over
time, we would like to remove any divergences in these documents, but where they
are different, these Fedora guidelines will take precedence for Fedora packages. 
--------------------

if you look at /usr/share/doc/jpackage-utils-1.7.5/jpackage-utils-policy
(from jpackage-utils) you will see

--------------------
Each Javadoc tree must be provided as a separate -javadoc subpackage,
with no special dependency to the main package.
--------------------

so JPackage viewpoint is that documentation should be freely available even
if the main package is installed. This makes some sense as it's only HTML
files and has absolutely no dependency on the main package.
http://fedoraproject.org/wiki/Packaging/Java
don't explicitely state that the javadoc must depend on the main package
only the examples show this.

So to me it's unclear what should really be done as dependancies for javadoc
though I don't have strong opinions here. But the divergence need to be
pointed out to whoever makes the decision.

I added the dependancies in the spec file.

> I'm pretty sure that test.java file isn't really useful unless it has an
> existing virtual machine to talk to.  Please correct me if I'm wrong.

wrong, it uses the test driver of libvirt which simulates the existence of 
virtual machines :-)

> I have to ask: what is the point of the libvert-src zipfile?  Why would a Java
> package need to include its source code?

I have seen it done in another package, and I think I saw instructions about it
but can't find them anymore. So I removed this ! If this is needed again, it's
trivial to add back.

I updated the spec file at:
ftp://libvirt.org/libvirt/java/libvirt-java.spec

I think the only issue left is about the NEWS file which is really a temporary
problem which will be fixed in libvirt-java next release.

Daniel


Comment 13 Jason Tibbitts 2008-07-04 16:19:11 UTC
(In reply to comment #12)
>  Well actually, I do as rpmfind maintainer. Seems that 
> Development/Documentation is a classic for javadoc rpms, see:

I think you misunderstood; it's not your choice of Group: that's bogus, it's
rpmlint's complaint about it.  Fedora doesn't care what goes in Group:.  You
could put "flatulent monkeys" there; we don't care.  What you have there is
perfectly fine.

> It won't stay zero-lenght, just a TODO upstream

OK.

> I will look at adding the flag, i'm just a bit vary of adding compiler/linker 
> specifics commands to keep compatibility with other platforms like solaris 
> without gcc.

You can if you like; it's not a big deal.

> Apparently the Java team don't want to specify the JDK used in the spec file
> have only the dependency on the java and java-devel (as well as JPackage) I
> followed that:
> http://fedoraproject.org/wiki/Packaging/Java#BuildRequires_and_Requires

I was referring to  to http://fedoraproject.org/wiki/Packaging/GCJGuidelines
which is referred to in the GCJ section of those Java guidelines.

> hum, here we have conflicting informations. The rule to follow the JPackage
> policy is different

Unfortunately the jpackage policy doesn't always work for us, but in this case I
honestly don't know what's up.  However, I can say that if you don't have a 
dependency on jpackage-utils you leave /usr/share/javadoc unowned, so that one's
pretty obvious.  The dependency on the main package is less clear, and I'll
simply leave that up to you.

> wrong, it uses the test driver of libvirt which simulates the existence of 
> virtual machines :-)

OK, then the next question is whether it can be called in a %check section. 
It's a good idea to always call available test suites if possible.  I have no
idea how you'd do that in this case, though, and I'm not inclined to block on it
because I'm not sure it can be done at build time in this case, but please
consider calling the test suite if you happen to know how.

> I updated the spec file at:
> ftp://libvirt.org/libvirt/java/libvirt-java.spec

Thanks.

APPROVED

I'll ask the question on the javadoc dependency on the main package over on the
proper lists.

Comment 14 Daniel Veillard 2008-07-07 09:39:26 UTC
Thanks a lot Jason !

So what's the next step now ? require fedora-cvs flag ?

Daniel

Comment 15 Daniel Veillard 2008-07-07 09:44:54 UTC
New Package CVS Request
=======================
Package Name: libvirt-java
Short Description: Java bindings for the libvirt virtualization API
Owners: veillard
Branches: F-9 
InitialCC: 
Cvsextras Commits: yes


Comment 16 Kevin Fenzi 2008-07-07 19:00:44 UTC
cvs done.

Comment 17 Jason Tibbitts 2008-08-09 15:11:34 UTC
It seems as if this package is in rawhide and F-9 now; any reason not to close this ticket?

Comment 18 Daniel Veillard 2008-08-10 11:54:38 UTC
Oh, right, forgot to close.

  thanks for your help Jason !

Daniel


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