[Bug 453119] Review Request: libvirt-java: Java bindings for the libvirt library

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: libvirt-java: Java bindings for the libvirt library


https://bugzilla.redhat.com/show_bug.cgi?id=453119





------- Additional Comments From veillard at redhat.com  2008-07-04 04:05 EST -------
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


-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.