Skip to content

Commit ae39206

Browse files
ctruedenctrueden
committed
WIP: Attempt to JPMSify the scijava-ops-opencv tests
It does not quite work as written. Here are some notes I wrote as I was developing this patch: Adding a scijava-types dependency to main causes a weird problem where the unit tests then fail to execute, due to a sudden inability to locate classes from org.scijava.ops.api. To fix the issue, I made a separate module-info.java for the tests, to require the org.scijava.ops.api because the tests *do* need it. Aside: one thing that's weird is we weren't depending on the jupiter engine. So I just tried adding that. But the failures are the same. The maven-surefire-plugin documentation has a page on running unit tests for JPMS-enabled projects: https://maven.apache.org/surefire/maven-surefire-plugin/examples/jpms.html The examples given there use a distinct module name for the test module-info.java, and also require the main module. So I tried changing the test module-info.java of this patch to be structured this way, with its own module name (I appended .test), and moved the TestOpenCV class into a test subpackage accordingly to avoid package clash across multiple modules... but I get the same failures afterward. So I changed it back to the same module declaration as the main module-info.java, since that seems less broken. I think the issue with kotlin is that scijava-io-http depends on okhttp which depends on okio which depends on kotlin stuff (because okio is a KMP project now), and scijava-io-http is an unnamed module without a module-info.java. So we are stuck guessing how the module config should look for that piece of our dependency tree, I suppose. My guess is that when there is no module-info.java in the src/test, it runs the tests outside JPMS, i.e. with a plain classpath. Whereas once you have a test-side module-info, the surefire plugin thinks "ah, you want to test this as a JPMS module, I should use a module path." Which suggests to me that we are going to be in big trouble as soon as anything downstream tries to use e.g. scijava-ops-opencv as a JPMS module... The fix might be a simple as just adding all the test scope dependencies to the test module-info.java, as I should have but didn't. I was able to get much closer to a working JPMS configuration for scijava-ops-opencv, but the openblas native library dependency is not something I know how to overcome. The other crazy thing I had to do was exclude okio and explicitly add only okio-jvm. Otherwise, JPMS gets confused about which JAR has the okio module, since they both purport to have it (but really, only okio-jvm has the needed classes). I think the lesson here is: JPMS projects can't fully depend on non-JPMSified projects that do unusual things, such as wrap native libraries into JAR files and extract them. Or even non-JPMS-friendly things, like having multiple JAR files with the same package prefix and/or module name. I say "fully" depend because yeah, you can depend on them, but if they don't work in a modular context, you'll be stuck using your code only in a non-modular (i.e. classpath) style. This is probably worth a forum post in Development category on the forum, and a heads up to Tobias that JPMSifying ImgLib2 is going to cause these headaches for any projects that have third party dependencies. Getting things like BigWarp and SNT into JPMS is not going to be feasible any time soon. I noticed that okio 3.8.0 does not have this double-module-declaration issue anymore. But upgrading to 3.8.0 still does not avoid the `NoClassDefFound okio/Buffer`, for reasons I don't understand. I thought it was happening because okio.jar had a clashing `Automatic-Module-Info` line with okio-jvm, but nope, apparently that's not why. Excluding the former JAR does fix the error though, so I dunno. I guess we could exclude it in scijava-io-http and release a new version of that, so that JPMS projects built on scijava-ops can use scijava-io-http more easily. I found an SO post about exactly this JavaCPP + JPMS problem: https://stackoverflow.com/a/71385521/1207769. However, I think the solution detailed there boils down to passing `--add-modules ALL-MODULE-PATH` to the test execution (if I'm reading it correctly), which seems like not a proper solution. Giving up for now.
1 parent db58cef commit ae39206

4 files changed

Lines changed: 94 additions & 0 deletions

File tree

‎scijava-ops-opencv/pom.xml‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,18 @@
125125
<artifactId>opencv</artifactId>
126126
</dependency>
127127

128+
<!--
129+
NB: Added to expose a bug with classpath-based testing,
130+
where tests complain that org.scijava.ops.api is inaccessible
131+
when org.scijava.types is in the main module-info.java,
132+
but tests work fine without it.
133+
-->
134+
<dependency>
135+
<groupId>org.scijava</groupId>
136+
<artifactId>scijava-types</artifactId>
137+
<version>${project.version}</version>
138+
</dependency>
139+
128140
<!-- OpenCV runtime native dependencies -->
129141
<dependency>
130142
<groupId>org.bytedeco</groupId>
@@ -149,6 +161,11 @@
149161
<artifactId>junit-jupiter-api</artifactId>
150162
<scope>test</scope>
151163
</dependency>
164+
<dependency>
165+
<groupId>org.junit.jupiter</groupId>
166+
<artifactId>junit-jupiter-engine</artifactId>
167+
<scope>test</scope>
168+
</dependency>
152169
<dependency>
153170
<groupId>org.scijava</groupId>
154171
<artifactId>scijava-ops-api</artifactId>
@@ -171,6 +188,19 @@
171188
<groupId>org.scijava</groupId>
172189
<artifactId>scijava-io-http</artifactId>
173190
<scope>test</scope>
191+
<!-- HACK: Avoid JPMS issue with okio + okio-jvm. -->
192+
<exclusions>
193+
<exclusion>
194+
<groupId>com.squareup.okio</groupId>
195+
<artifactId>okio</artifactId>
196+
</exclusion>
197+
</exclusions>
198+
</dependency>
199+
<dependency>
200+
<groupId>com.squareup.okio</groupId>
201+
<artifactId>okio-jvm</artifactId>
202+
<version>${okio.version}</version>
203+
<scope>test</scope>
174204
</dependency>
175205
<dependency>
176206
<groupId>io.scif</groupId>

‎scijava-ops-opencv/src/main/java/module-info.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,4 +34,5 @@
3434
// FIXME: these module names derive from filenames and are thus unstable
3535
requires org.bytedeco.opencv;
3636

37+
requires org.scijava.types;
3738
}

‎scijava-ops-opencv/src/main/java/org/scijava/ops/opencv/MatCopier.java‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,12 @@
44
import org.bytedeco.opencv.opencv_core.GpuMat;
55
import org.bytedeco.opencv.opencv_core.Mat;
66
import org.bytedeco.opencv.opencv_core.UMat;
7+
import org.scijava.types.Any;
78

89
public final class MatCopier {
910

11+
public static final Class<?> TEMP = Any.class;
12+
1013
public static void copy(Mat source, Mat dest) {
1114
source.copyTo(dest);
1215
}

‎scijava-ops-opencv/src/test/java/module-info.java‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
/*-
2+
* #%L
3+
* SciJava Ops OpenCV: OpenCV configuration for ops
4+
* %%
5+
* Copyright (C) 2023 - 2024 SciJava developers.
6+
* %%
7+
* Redistribution and use in source and binary forms, with or without
8+
* modification, are permitted provided that the following conditions are met:
9+
*
10+
* 1. Redistributions of source code must retain the above copyright notice,
11+
* this list of conditions and the following disclaimer.
12+
* 2. Redistributions in binary form must reproduce the above copyright notice,
13+
* this list of conditions and the following disclaimer in the documentation
14+
* and/or other materials provided with the distribution.
15+
*
16+
* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
17+
* AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
18+
* IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
19+
* ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDERS OR CONTRIBUTORS BE
20+
* LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
21+
* CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
22+
* SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
23+
* INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
24+
* CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
25+
* ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
26+
* POSSIBILITY OF SUCH DAMAGE.
27+
* #L%
28+
*/
29+
module org.scijava.ops.opencv {
30+
exports org.scijava.ops.opencv;
31+
32+
requires java.scripting;
33+
34+
// FIXME: these module names derive from filenames and are thus unstable
35+
requires org.bytedeco.opencv;
36+
37+
// HACK: Because org.scijava.io.http is not JPMSified yet.
38+
requires kotlin.stdlib;
39+
requires okhttp3;
40+
requires okio;
41+
42+
// HACK: Because org.bytedeco.opencv is not JPMSified yet.
43+
requires org.bytedeco.openblas;
44+
requires org.bytedeco.javacpp;
45+
46+
// Automatic modules.
47+
requires io.scif;
48+
requires net.imagej.opencv;
49+
requires net.imglib2;
50+
requires org.scijava.io.http;
51+
52+
// JPMS modules.
53+
requires org.scijava.ops.api;
54+
requires org.scijava.ops.engine;
55+
requires org.scijava.ops.image;
56+
requires org.scijava.types;
57+
58+
requires transitive org.junit.jupiter.api;
59+
requires transitive org.junit.jupiter.engine;
60+
}

0 commit comments

Comments
 (0)