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

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



Am 05.10.2017 um 13:08 schrieb Carnë Draug:
[...]
> I still don't understand the connection between the build system of a
> project and how the end binary should be used afterwards.  These are
> not Maven projects.  They are Java projects where maven was the build
> system.  The build system should only affect how that project is
> built, not how it is used by the end-user.

Carnë I would really appreciate it if we could be more efficient with
this review. I have noticed this before but you seem to be in the habit
of questioning statements over and over again which are not
controversial at all and this will needlessly delay the review.

I only stated that your upstream packages are all Maven projects and
they are intended to be used in other Maven projects. If you take a look
at the README.md file of jlargearrays you can see for instance how
upstream thinks this library should be used. It is a Maven project and
that affects the end-user. Period.

>> I can't find any hint
>> of support for other build systems in the original tarballs.
> 
> That's not what I am trying to do. The java library is already built.
> I am only trying to configure it properly for usage in a Debian
> system.  Both fairsim and JTransforms provide a jar file with all
> dependencies bundled to be used so one can use them fine without
> maven.

Yes. Now take a look at my next two sentences.

> What you
>> are doing is adding support for other use cases by installing the jar
>> files into /usr/share/java. That's absolutely fine and the historical
>> way how Java projects were initially packaged for Debian.

Sounds familiar? My point was upstream obviously does not support other
build systems than Maven but you added support for it and that's great!

 For a pure
>> Maven project adding the Class-Path attribute to the manifest file would
>> be unnecessary because the files in /usr/share/java are not used by Maven.
>>>
>>>>>> 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.
>>>>
>>>
>>> Why is that bad practice?
>>
>> Can you guarantee that your relative path is always unambiguous even if
>> you copy the jar file into another project or install the jar file into
>> a completely different directory?
> 
> If the file is not copied, which should not, then yes, it will
> unambiguous.  Other Debian packages should create symlinks not copy
> the file in which case the path will be resolved correctly.  Why
> should Debian packages have to cover the case where a file is copied
> somewhere else?

Why should Debian not cover this use case?

>> I have been working for the past five
>> years in this team and I can tell you a lot of things can go wrong when
>> it comes to class loading. It really helps if all packages behave
>> identical hence you will find many, many packages in our team that use
>> absolute paths when exporting the CLASSPATH variable or when modifying
>> the Class-Path attribute.
>>
> 
> I believe you but I don't know what those situations are because I
> never faced them and I am only starting with packaging java libraries.

Here is a pro tip. If you are not a regular contributor to a project and
even a newcomer then it is usually advantageous to listen to what the
current members have to say and do not assume that everything they did
is totally wrong. Try to get the hang of packaging Java software for
Debian first. After you have studied, updated and fixed around 50
packages you will get the bigger picture and then it's time to criticize
them and to tell those fools how it should be done.

> My reasoning is that if the location of a jar file changes, it would
> be more likely because the location for jar files in general changed.
> So a relative path would continue to work while an absolute path would
> break.  But I can set absolute paths no problem.
> 
>> [...]
>>>>
>>>> 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/...).
>>>
>>> Because he doesn't use or like maven.  It's there only so that other
>>> people can make use of it but he uses the Makefile for himself.
>>
>> That is not an excuse for writing bad pom files.
> 
> That is very unfair.  It is not a bad pom file because the project
> builds fine.  Maven supports using alternative file structure and
> that's what upstream is doing.  What is bad about it?  Seems fair to
> me that a project should not have to worry with issues involving the
> addition of foreign files to the tree.

This thread is proof that this pom file is less than optimal. Use the
Maven conventions whenever you can, especially for smaller projects like
fairsim. This will make life for people like us tremendously easier. You
believe using an alternative file structure is a completely valid
solution? Absolutely! But then you must also figure out another way to
solve your problem.

>>>> 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.
>>>>
>>>
>>> I have tried to change the build system already but I can't figure out
>>> the right maven magic to make it look only inside ".org" either.  I
>>> think it may have to do with how debian-maven-helper is used.  Note
>>> how I had to manually remove a java file from the source tree in
>>> `rules` [2] even though `pom.xml` should clearly exclude it [3].
>>
>> This is not an issue with maven-debian-helper because it simply invokes
>> Maven which rightfully complains about duplicated classes. The pom.xml
>> and the directory layout should be changed.
>>
> 
> A patch that moves all the files in 'org' to 'src/main/java/org' will
> cause conflicts with each update.  Wouldn't be much less disruptive to
> instead patch the pom.xml file to ignore a '.pc' directory?

Yes, that is a valid solution/workaround.

> Maven documentation suggests it is possible to exclude en entire
> directory but for some reason the exclude options seem to be ignored
> during the build.  Note that it even ignores the exclude options set
> in the original pom.xml file which is why I think the issue is on
> debian-maven-helper or what else is doing the compilation.

When I remove the override for dh_auto_build I can't find any trace of
the JTransformsForkConnector class. Seems to work as intended unless I
missed something else.


Steps to get your packages uploaded:

1. Fix the Standards-Version
2. Use absolute paths
3. (optional) apply your patch for fairsim and fix the build failure as
you see fit*


* Must be Policy compliant though

Thanks,

Markus

Attachment: signature.asc
Description: OpenPGP digital signature


Reply to: