[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: Review of fairsim, libjtransforms-java and libjlargearray-java



Am 03.10.2017 um 17:30 schrieb Carnë Draug:
> On 2 October 2017 at 00:21, Markus Koschany <apo@debian.org> wrote:
>> Hi,
>>
>> I think all three packages are in a good shape but there are some issues.
>>
>> Standards-Version is 4.1.1 now.
>>
>> I suggest to remove the -doc packages because I don't believe those
>> libraries are significant enough to warrant the maintenance of
>> additional documentation packages but I leave the decision to you. That
>> would also simplify the packaging a little.
> 
> I actually like and use the docs packages so I'd rather keep them.

Fair enough.

>> You don't have to add the classpath to the library. The Lintian check
>> that warned about this issue has recently been removed.
> 
> Without the classpath, then users of the jar files need to figure out
> the dependencies themselves which does not work.  As someone that
> routinely uses java libraries via Python and Octave, I find it
> essential.

It really depends on the build system. Since this is a Maven project
dependencies are declared in pom.xml files. What you are referring to is
Debian's way of installing jar files into /usr/share/java. This is meant
for build systems like Ant where you can easily add a symlink to your
package and reference the versionless jar file. That's fine if you want
to support this use case but it is not essential to get another package
working and it might even introduce unwanted side effects like [1].

>> If you do it and
>> use jh_classpath or the *.classpath file then you must specify the
>> absolute path to the libraries otherwise they won't be found.
> 
> Not true.  The path can be relative to the directory of the jar file
> that contains the manifest file.

Let me rephrase this. It is possible but bad practice. Don't assume that
your jar file is always in the same directory as your dependencies. The
only way to ensure that is to use an absolute path.

>> you rarely need both javahelper and maven-debian-helper in one package.
>> I believe maven-debian-helper would suffice here and you can remove the
>> build-dependency on javahelper and the related substvars.
> 
> I thought that d/package-name.classpath file was used by jh_classpath
> and that is provided by javahelper.  Or how can I set the classpath on
> the manifest without it?

Indeed we use javahelper as a tool to modify the classpath in manifest
files. There are other ways but using jh_classpath or *.classpath is the
recommended and most like easiest way to achieve it. If you want to
support this you must depend on javahelper.

>> I also suggest to remove the --has-package-version flag from the *.poms
>> files. There was a recent change in maven-debian-helper that
>> automatically adds a versioned dependency to reverse-dependencies if one
>> of their build-dependencies uses this flag. In my opinion in most cases
>> this is too strict and not what you probably wanted.
>>
>> Regarding your failing patch I'm not sure. It doesn't sound like it is
>> Java specific. You can send me your patch and I can take a look though.
>>
> 
> Here is the patch [1].  When I build with this patch, quilt will make
> a copy of the original org.fairsim.fiji.FairSim_ImageJplugin package
> in `.pc/set-git-build-id.patch/org/fairsim/fiji/FairSim_ImageJplugin.java`
> This causes a duplicate class error because both the original and copy
> are found.

I can reproduce your issue. This is related to upstream's unusual choice
of options in the pom.xml file. First of all I'm not sure why he won't
simply stick to the default Maven directory layout (src/main/...). This
would solve your problem immediately. The issue here is that he uses
<sourceDirectory>./</sourceDirectory> which tells Maven to look into the
root of your build directory and then it detects the patched class in
the ~/.pc directory and the original class under org and fails.

I would try to fix the build system (the pom.xml) but I don't have a
patch at hand at the moment. Perhaps someone else on the list knows a
better solution.

Regards,

Markus

[1] https://lists.debian.org/debian-java/2017/09/msg00062.html

Attachment: signature.asc
Description: OpenPGP digital signature


Reply to: