Make SNT available in scijava-pom

[tferr]

Since this commit SNT is no longer available in scijava. The direct consequence of this is that SNT can only be accessed from pyimagej using a local Fiji install. The problem relies on sciview dependencies that are not under our control. A way to handle this would be to make SNTSciview an independent module.

[ctrueden]

Here is an example of dependency resolution failure:

Failed to collect dependencies at sc.fiji:TrakEM2_:jar:1.3.6 -> org.morphonets:SNT:jar:3.1.112 -> org.jgrapht:jgrapht-ext:jar:1.3.0 -> org.tinyjee.jgraphx:jgraphx:jar:4.1.0

There is also an issue with scenery's dependence on biojava, because biojava has invalid dependency tree.

FWIW, I am planning to work on addressing this tangle soon, as part of the pom-scijava 32.0.0 release work in 2nd half May and 1st half June.

[carshadi]

@tferr, @ctrueden , Is there any way I can help with this?

I'm going to need to be able to run versions > 4.0.3 through pyimagej + maven endpoints very soon, and be able to package it and share with others without having them download a local Fiji.

From the discussion at https://gitter.im/fiji/fiji?at=6217cf210909252318f6f3df, my understanding is that the issue is with the dependencies scenery pulls in. It's not clear to me that we actually need scenery as a compile-time dependency, since the current sciview functionality is optional and requires subscribing to the sciview update site anyways.

Turning SciViewSNT into an independent module makes sense to me. Let me know what steps are necessary to make it happen and I can start on it.

[tferr]

@carshadi, the issue is restricted to SciViewSNT.java. Namely, these imports: https://github.com/morphonets/SNT/blob/7744c94662d23c4f0a8cf60ae3b446feda41d746/src/main/java/sc/fiji/snt/SciViewSNT.java#L25-L27

That forces us to declare scenery in the pom: https://github.com/morphonets/SNT/blob/7744c94662d23c4f0a8cf60ae3b446feda41d746/pom.xml#L232-L250

Which in turn forces maven to import all the sciview dependency stack. So - for this sole reason - scenery is required at compile time.

The reason why we do not need at runtime (and thus we do not upload scenery to the neuroanatomy update site), is because when users press the "Sciview" button - the only entry point to SciViewSNT - we use reflection to check that sciview is indeed available:

https://github.com/morphonets/SNT/blob/7744c94662d23c4f0a8cf60ae3b446feda41d746/src/main/java/sc/fiji/snt/gui/cmds/EnableSciViewUpdateSiteCmd.java#L81-L83

The rationale behind this reflection was to solve the same problem: uploading the full sciview dependency stack to the neuroanatomy update site was always causing problems to users. (NB: currently compiling SNT and uploading SNT to the update site are distinct tasks, that's why this workaround works).

To make SciViewSNT a module, we probably need to create a new project (SNTsciview!?), and then use the reflection trick above to look for it when users click on the sciview button. Alternatively, I guess we could patch SciViewSNT so that all the scenery code goes into a (groovy!?) script. Then SciViewSNT could just call ScriptService to execute a script at run time

Of course, without the ability to compile we would lose the ability to know if sciview integration remains functional, but that seems to more bearable than the current situation?

Let me try something. I'll try to report back soon

[tferr]

@carshadi, have a look at the sciview-fix branch.

The way this would work is as follows:

1. Compile the new SNTsciview project and place it in your Fiji.app/jars. This project contains the old SNTSciview class. scenery and sciview are defined in its pom. We would upload this independently to the Neuroanatomy update site.
2. The new sciview-fix branch uses reflection (SciviewRefectionBridge) to access the SNTsciview project. It no longer declares any sciview dependency.

The major immediate issue with this is that SNTService has a getSciView() method that now has to return an Object rather than the actual sciview instance. Scripts would have to use casting somehow. Also, we now need to declare explicitly in the SNT pom dependencies that we are using but were being silently brought in by maven when we were declaring scenery as a dependency. This includes: imglib2, net.imagej, etc.

I did not have time to work any further. It is all in a crude state.

Could you please have a look and see if any of this makes sense on your end ?

[ctrueden]

I want to help with this as well—it's been on my list for a while to address in conjunction with other pom-scijava work—but as always it's tough to find the time... so @carshadi if you have bandwidth to work on it, that's great.

@tferr One immediate thing to note is that you can put <optional>true</optional> on your sciview dependency to make it required at compile time, but not at runtime. (This is how ImageJ2 handles its ImageJ 1.x dependency.) Since you are guarding against the case where sciview is missing anyway, I would highly recommend this. Then any downstream project depending on SNT does not inherit sciview automatically, but rather has to opt into it.

[tferr]

Wow. That is way simpler!!!! So to clarify, with the optional flag on, would the biojava et. al errors go away?

[ctrueden]

with the optional flag on, would the biojava et. al errors go away?

Not for SNT itself, I don't think, because Maven still needs to drag in the sciview dependency tree when compiling the SNT project. But downstream projects shouldn't have problems, because their dependency trees won't include sciview. But this is off the top of my head—I did not test it at the moment.

[ctrueden]

To stop biojava from being dragged in at SNT compile time, you could add an exclusion to the sciview dependency declaration as well. As long as the compilation of SNT does not rely on biojava classes being present.

[tferr]

@ctrueden, thanks for all the pointers. could you have see if https://github.com/morphonets/SNT/commit/827c2695477d732b3215eb7fcc1840160a09366c (in master) fixes things? It makes sciview/scenery optional and excludes biojava (that we do not use).

[tferr]

@carshadi, I pushed a couple of changes to the sciview-fix branch. It seems to work here. If the changes in master do not fix the issue, we can fall back to that approach. Somehow, i think that would be the preferred way, in the sense that is the least likely to cause problems downstream of SNT.

[carshadi]

@tferr https://github.com/morphonets/SNT/commit/827c2695477d732b3215eb7fcc1840160a09366c seems to work. The sciview splash screen hangs, but that was also the case before these changes. Is this all that's necessary to re-introduce SNT into the scijava BOM?

I also tried out the sciview-fix + https://github.com/morphonets/SNTsciview

I placed the SNTsciview jar into my Fiji.app and then built sciview-fix into Fiji.app via: mvn -Dscijava.app.directory=Fiji.app -Ddelete.other.versions=true clean install

I then enabled the sciview-buttercup update site and tried to open sciview from SNT, but got the error

Exception in thread "Thread-8" java.lang.NoClassDefFoundError: SciViewSNT is not available. Please see https://imagej.net/plugins/snt/#sciview for details
    at sc.fiji.snt.SciviewReflectionBridge.error(SciviewReflectionBridge.java:120)
    at sc.fiji.snt.SciviewReflectionBridge.<init>(SciviewReflectionBridge.java:69)
    at sc.fiji.snt.SNTUI.lambda$null$38(SNTUI.java:1914)
    at java.lang.Thread.run(Thread.java:748)

[carshadi]

we specify net.imagej:imagej in our POM, and it looks like net.imagej:imagej-common and net.imagej:imagej-ops bring in all the imglib2 dependencies we need, including imglib2, imglib2-cache, imglib2-algorithm, imglib2-roi. Might not be a bad idea to declare them explicitly though.

[imagejan]

Might not be a bad idea to declare them explicitly though.

mvn dependency:analyze will tell you about any issues with your dependency declarations (i.e. used but undeclared, unused but declared, etc.)

After https://github.com/morphonets/SNT/commit/827c2695477d732b3215eb7fcc1840160a09366c, we still have the following warnings:

[WARNING] Used undeclared dependencies found:
[WARNING]    net.imglib2:imglib2-ij:jar:2.0.0-beta-46:compile
[WARNING]    org.scijava:j3dcore:jar:1.6.0-scijava-2:compile
[WARNING]    org.jzy3d:jzy3d-native-jogl-awt:jar:2.0.0:compile
[WARNING]    org.jogamp.jogl:jogl-all:jar:2.3.2:compile
[WARNING]    com.github.haifengl:smile-math:jar:2.6.0:compile
[WARNING]    net.imglib2:imglib2:jar:5.12.0:compile
[WARNING]    net.imagej:imagej-common:jar:0.34.1:compile
[WARNING]    org.scijava:scijava-common:jar:2.87.0:compile
[WARNING]    sc.fiji:pal-optimization:jar:2.0.1:compile
[WARNING]    com.github.vlsi.mxgraph:jgraphx:jar:4.1.0:compile
[WARNING]    org.slf4j:slf4j-api:jar:1.7.30:compile
[WARNING]    io.scif:scifio:jar:0.42.0:compile
[WARNING]    org.scijava:scijava-ui-awt:jar:0.1.7:compile
[WARNING]    net.imagej:imagej-ops:jar:0.45.8:compile
[WARNING]    org.joml:joml:jar:1.9.25:compile
[WARNING]    org.jetbrains.kotlin:kotlin-stdlib:jar:1.4.21:compile
[WARNING]    net.imglib2:imglib2-cache:jar:1.0.0-beta-16:compile
[WARNING]    org.jzy3d:jzy3d-core:jar:2.0.0:compile
[WARNING]    org.jzy3d:jzy3d-native-jogl-core:jar:2.0.0:compile
[WARNING]    net.imagej:ij:jar:1.53f:compile
[WARNING]    org.apache.commons:commons-lang3:jar:3.10:compile
[WARNING]    net.imagej:imagej-mesh:jar:0.8.0:compile
[WARNING]    org.jzy3d:jzy3d-core-awt:jar:2.0.0:compile
[WARNING]    org.scijava:vecmath:jar:1.6.0-scijava-2:compile
[WARNING]    net.imglib2:imglib2-roi:jar:0.11.0:compile
[WARNING]    sc.fiji:VIB-lib:jar:2.2.0:compile
[WARNING]    org.scijava:script-editor:jar:0.5.9:compile
[WARNING]    net.imglib2:imglib2-algorithm:jar:0.12.0:compile
[WARNING]    org.jogamp.gluegen:gluegen-rt:jar:2.3.2:compile
[WARNING]    net.imagej:imagej-legacy:jar:0.38.0:compile
[WARNING]    org.jetbrains:annotations:jar:13.0:compile
[WARNING] Unused declared dependencies found:
[WARNING]    com.formdev:flatlaf-jide-oss:jar:2.3:compile
[WARNING]    org.webjars:font-awesome:jar:6.1.1:compile
[WARNING]    ch.qos.reload4j:reload4j:jar:1.2.19:runtime
[WARNING] Non-test scoped test only dependencies found:
[WARNING]    org.scijava:scijava-common:jar:2.87.0:compile
[WARNING]    org.slf4j:slf4j-api:jar:1.7.30:compile
[WARNING]    org.jgrapht:jgrapht-core:jar:1.4.0:compile
[WARNING]    net.imglib2:imglib2-algorithm:jar:0.12.0:compile

[tferr]

@imagejan, thanks! I did not know (or forgot) about mvn dependency:analyze. So, what is the recommendation? Should we explicitly declare everything in that undeclared dependencies list?

BTW, there are some false hits on that list, e.g.

[WARNING] Unused declared dependencies found:
[WARNING]    com.formdev:flatlaf-jide-oss:jar:2.3:compile
[WARNING]    org.webjars:font-awesome:jar:6.1.1:compile

Even if there are no formal import declarations in the project from these, both are used. The first patches the L&F of jide-oss (that we declare), the second provides all the icons we use in the GUI. Is there a way to tag these entries as valid?

Also, hoow can we find out which libraries are requesting:

[WARNING]    org.jetbrains.kotlin:kotlin-stdlib:jar:1.4.21:compile
[WARNING]    org.jetbrains:annotations:jar:13.0:compile

AFAIK, we do not use them at all!? (@carshadi, agree?)

@carshadi:

Is this all that's necessary to re-introduce SNT into the scijava BOM?

I doubt. I think there were also additional issues (including with TrakEM2) that Curtis had found. But, hopefully it will make us closer to have everything resolved. That's why it would be useful to be able to use the option in the sciview-fix branch as backup. On that: SciviewReflectionBridge was masking the original exception, that's why that stack trace was not that useful. It looks as if this constructor fails to initialize.

I don't know why. The original stack trace should now be printed so hopefully we'll get more insights. I think I found the issue. Please pull the recent changes on both SNT/sciview-fix and SNTsciview. Let me know what you find. I will try to have a look this evening.

[carshadi]

unfortunately I still get an error, but it's different this time

java.lang.NoSuchMethodException: sc.fiji.snt.SciViewSNT.<init>(sc.fiji.snt.SNT)
    at java.lang.Class.getConstructor0(Class.java:3082)
    at java.lang.Class.getConstructor(Class.java:1825)
    at sc.fiji.snt.SciviewReflectionBridge.<init>(SciviewReflectionBridge.java:65)
    at sc.fiji.snt.SNTUI.lambda$null$38(SNTUI.java:1914)
    at java.lang.Thread.run(Thread.java:750)
Exception in thread "Thread-9" java.lang.NullPointerException
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:498)
    at sc.fiji.snt.SciviewReflectionBridge.getSciView(SciviewReflectionBridge.java:93)
    at sc.fiji.snt.SNTUI.lambda$null$38(SNTUI.java:1914)
    at java.lang.Thread.run(Thread.java:750)

This time with a fresh Fiji subscribed to Neuroanatomy and sciview-buttercup. I copied the jars from both the sciview-fix branch and the new SNTsciview. How do you run it? I am on Windows 10, if that makes any difference.

@tferr we do actually use Jetbrains Annotations org.jetbrains:annotations:jar:13.0:compile since IntelliJ always recommends things and I tend to just hit accept without thinking. For an example see https://github.com/morphonets/SNT/blob/827c2695477d732b3215eb7fcc1840160a09366c/src/main/java/sc/fiji/snt/tracing/image/MapSearchImage.java#L53

For the kotlin stuff,

 mvn dependency:tree -Dincludes=org.jetbrains.kotlin:kotlin-stdlib

outputs

[INFO] --- maven-dependency-plugin:3.2.0:tree (default-cli) @ SNT ---
[INFO] org.morphonets:SNT:jar:4.1.2-SNAPSHOT
[INFO] \- sc.iview:sciview:jar:e3510f4:compile (optional)
[INFO]    \- org.jetbrains.kotlin:kotlin-stdlib:jar:1.4.21:compile

[tferr]

@carshadi,

the sciview-fix + SNTsciview hack seems to be working now (please test it on your end):

As per the approach of cleaning up the dependencies in master. I must say I am a bit lost. kotlin-stdlib is indeed a dependency of sc.iview:sciview:jar:, but if I add it to the sciview exclusion list, then it becomes listed as a dependency of okhttp:

[INFO] \- com.squareup.okhttp3:okhttp:jar:4.9.3:compile
[INFO]    \- org.jetbrains.kotlin:kotlin-stdlib:jar:1.4.21:compile

I ran out of time on this, would be great if you could peruse the pom and:

1. Add exclusions for sciview/scenery dependencies that are never used by us
2. Add the optional flag to everything that is not required at run time
3. Mark test-only dependencies

---

@ctrueden, TLDR; we now have two tracks to tackle this:

1. Remove conflicting dependencies from the pom (and @carshadi will try to identify anything else spurious there). This is in `master'
2. A separate project to handle sciview, so that SNT does not need to declare it in the pom. This is in the scivew-fix branch.

Could you please have a look to see if any of these fixes things?

[carshadi]

@tferr ,

The new sciview-fix works on my end!

I added the used but undeclared dependencies in https://github.com/morphonets/SNT/pull/117, using the same versions that were output by mvn dependency:analyze

Now if I run

mvn dependency:analyze -DignoreNonCompile

We get

[INFO] --- maven-dependency-plugin:3.2.0:analyze (default-cli) @ SNT ---
[WARNING] Unused declared dependencies found:
[WARNING]    org.webjars:font-awesome:jar:6.1.1:compile
[WARNING]    com.formdev:flatlaf-jide-oss:jar:2.3:compile
[WARNING] Non-test scoped test only dependencies found:
[WARNING]    org.slf4j:slf4j-api:jar:1.7.36:compile
[WARNING]    org.scijava:scijava-common:jar:2.87.0:compile
[WARNING]    org.jgrapht:jgrapht-core:jar:1.4.0:compile
[WARNING]    net.imglib2:imglib2-algorithm:jar:0.12.0:compile

I think the warnings about test-only dependencies are wrong. We use org.jgrapht:jgrapht-core (DirectedWeightedGraph) and net.imglib2:imglib2-algorithm (Frangi, Tubeness, etc) all over the place, so I don't understand why they show up as test-only.

On Add exclusions for sciview/scenery dependencies that are never used by us , is it really necessary to exclude everything, or just the ones that result in issues like biojava? They bring in a lot of dependencies, and it would probably take me a week to add every exclusion, in addition to blowing up the POM.

On Add the optional flag to everything that is not required at run time , is there an easy way to find dependencies required at compile time but not at run time?

Mark test-only dependencies I believe junit is the only true test-only dependency, and it is marked as test.

[tferr]

The new sciview-fix works on my end!

Awesome. What do you think? Should we adopt it as the official way to handle sciview? I just checked, and the demo script seems to be working too. If you feel strongly about it, then we could go ahead and implement. My understanding about all this was that the problem stems only from sciview dependencies, thus we would have this fixed. But it uses reflection, and reflection is controversial.

Maybe we should ask for @kephale and @skalarproduktraum opinion (i.e., what do you guys think of moving SciViewSNT to its own project)?

is it really necessary to exclude everything, or just the ones that result in issues like biojava?

Just the problematic ones. I think you covered pretty much everything. I think Curtis has a series of scripts he uses to assess how different projects interact with each other. I think our pom was throwing so many errors, he had no other option than to ban SNT from the scijava pom. The hope is that being more tidy in our end, we are making things validate faster on his end.

[ctrueden]

@tferr I'm on vacation this week, but will try to come back to this after July 10th. The timing may be good with the upcoming pom-scijava 32.0.0 release planned for later this month.

[skalarproduktraum]

@carshadi Happy to help you sort this out! I've just realised we've not propagated the exclusions (of some broken Biojava stuff) to the POM we publish. I'm just working on that now.

[ctrueden]

@tferr @carshadi I did an initial melting pot testing just now with SNT 4.1.2 + the latest pom-scijava. Findings:

• The jgraphx dependency is good now: groupId changed from org.tinyjee.jgraphx to com.github.vlsi.mxgraph.

• When rebuilding SNT 4.1.2 with BOM-pinned versions, there is a clash between org.bytedeco.javacpp-presets:ffmpeg:4.1-1.4.4 (dependency of org.janelia:H5J_Loader_Plugin:1.1.3) and org.bytedeco:ffmpeg:4.2.1-1.5.2 (dependency of graphics.scenery:scenery:4d07a30). I am attempting to update H5J_Loader_Plugin to ffmpeg 4.2.1-1.5.2 now.

[ctrueden]

SNT is now back in the SciJava BOM. 🎉 See scijava/pom-scijava@fe9065a4a536c4b665fac7a64c55c3ef9f1253fb.

The issue with the ffmpeg version is a real version clash, but I have updated sc.fiji:H5J_Loader_Plugin (fiji/H5J_Loader_Plugin@ed78e51aa20a40e94105a3118f9127fecf265911), and will do a release soon bringing everything into alignment.

The next step for me will be to look at the TrakEM2 side of things, its dependency on SNT, figure out whether it can be optional, or will need to restructure something.

[carshadi]

amazing, thank you @ctrueden !!!