Improve detection of conflicting artifact versions

[stelfrich]

Currently, there is a filename/regex-based check for determining if a file in scijava.app.directory conflicts with the artifact to be installed. While this check works, it has hardcoded patterns and is thus not extensible w.r.t. new architectures, etc.

We (@ctrueden and I) have come up with the following scheme to extract the version and qualifier of an artifact:

1. Look in Jar for version information:
   1. pom.properties in META-INF/maven/<groupId>/<artifactId>/ (if found, return)
   2. pom.xml in META-INF/maven/<groupId>/<artifactId>/ (if found, return)
   3. MANIFEST.mf (Implementation-Version)
2. If a version is found in the filename suffix, the rest is a qualifier [If not: ij.jar or inconsistent: warning]
3. If nothing is in the filename
   1. use regex (dash+digit: substring)
   2. if there is no version, assume there is a qualifier or suffix

[LauLauThom]

Example of case when the regex results in having 2 distinct dependency artefacts erroneously overwritten for IJ-OpenCV, namely opencv and opencv-platform.

[stelfrich]

Some results of a recent investigation:

---

An exemplary pom.properties contains:

#Created by Apache Maven 3.0.5
version=3.4.2-1.4.2
groupId=org.bytedeco.javacpp-presets
artifactId=opencv

The issue with that is that qualifiers are not contained in here.

---

The pom.xml that is packaged into the JAR files at META-INF/maven/<groupId>/<artifactId>/ is not an Effective POM but still contains variables and some such. Hence, the POMs for different architectures are exactly the same.

---

The MANIFEST.MF of the native JARs doesn't contain a Implementation-Version.

[ctrueden]

I'd forgotten all about this issue, sorry. For now, let's just add to the regex to cover the IJ-OpenCV use case. @LauLauThom What are the exact filenames that are separate classifiers, and scijava-maven-plugin should detect as such?

The logic to change lives in scijava-common in org.scijava.util.FileUtils. Edit: That logic affects the ImageJ Updater, but not this plugin here. We should find a way to unify the reasoning.

Here is a recent commit after which our commit should be modeled:
scijava/scijava-common@32c4098

It's too bad the Java ecosystem has not unified around fewer native JAR wrapper naming schemes. But adding to the regex is straightforward—especially if you also add a unit test ensuring it's working as desired.

[imagejan]

We also might need to special-case classifiers such as -swing in miglayout-3.7.4-swing.jar see this discussion on gitter.

[rimadoma]

After I excluded com.miglayout.miglayout from both net.imagej.imagej and net.imagej.imagej-ops I noticed that the issue also affects the artefacts org.scala-lang.modules:scala-xml_2.12:jar:1.0.6 and com.squareup.okhttp3:okhttp:jar:3.6.0 due similarly awkward naming schemes. The former is a dependency of org.scijava:scripting-scala and the latter org.scijava:scijava-io-http.

Furthermore, excluding com.miglayout.miglayout is not really a viable workaround, because it breaks imageJ = new ImageJ(); imagej.launch(), which makes development awkward.

[ctrueden]

@rimadoma We are in the process of switching to the newer miglayout-swing 5.2 which does not use swing as a classifier anymore—see scijava/pom-scijava#97. So this issue of scijava-maven-plugin not working well with miglayout specifically will be moot. I'm working on pom-scijava 29 now, which will include this update.

In the meantime, in addition to excluding the old miglayout artifact, try also adding com.miglayout:miglayout-swing:5.2 as a dependency. Happy to help if you are stuck.

P.S. About your GitHub tagline "L'ordinateur a dit 'Non'": are you quoting Little Britain‽ 😆 🏆

[rimadoma]

@ctrueden Adding com.miglayout:miglayout-swing:5.2 on top of excluding the old does the trick! Thanks very much! The other necessary exclusions I mentioned above don't seem to cause any problems for us.

P.S. I am. Just having fun with my pidgin French. TBH I don't really know the show, but as a programmer that quote speaks to me.

[ctrueden]

@LauLauThom Could you please test pom-scijava 29.0.0-beta-1—which uses scijava-maven-plugin 2.0.0—and report back whether there are still problems with IJ-OpenCV? With cca73a7 it might be fixed. And if it's not fixed, I'm guessing that fixing it now will be much more trivial.

[LauLauThom]

@ctrueden Yeah that works ! Thanks a lot ! And sorry for the late reply, yesterday was off 😎

Just the updater recognizes some artifacts as conflicting. But I guess it is relying on the same logic than Maven no ? so could it be just a matter of updating it too ?
And I just found out I can specify the OS platform as a maven property to copy only the correct OS dependencies, which actually prevents the error, and for miglayout I think it was mentioned as a work in progress on gitter, right?