search

imagej / imagej-launcher

The ImageJ native launcher
https://imagej.net/learn/launcher
BSD 2-Clause "Simplified" License
21 stars 18 forks source link

Investigate missing jvm.dll with Java10+ on Windows #58

Closed stelfrich closed 2 years ago

stelfrich commented 6 years ago

As reported by @imagejan in https://github.com/imagej/imagej-launcher/pull/57#issuecomment-424319338, there seems to be an issue with the launcher when it's used with Java 10 (and/or very likely also with Java 11) on Windows.

@imagejan: Could you please clarify if you encounter the issue with an OpenJDK or Oracle JDK?

imagejan commented 6 years ago

Thanks @stelfrich for opening the issue here. I was using Oracle JDK 10.0.1 on Windows 7, and the latest launcher build indeed picks up this Java version, but not before first showing a dialog:

image

(Strangely, the previous launcher I tested with this installation - 5.0.0-20171212 - picks up Java 1.8.0_172 from my system, while JAVA_HOME points to the path of Java 10.0.1... but that is likely some peculiarity of my system here, I didn't investigate further.)

stelfrich commented 6 years ago

I have checked this morning with Oracle JDK 10.0.2 on a fresh Windows 10 and couldn't reproduce the warning that you are showing, @imagejan. I'll check with Windows 7 and Oracle JDK 10.0.2 when I find some time next week..

stelfrich commented 6 years ago

I was able to reproduce the issue on Windows 7 with Oracle JDK 10.0.2. The reason seems to be related to libsplashscreen, which I couldn't find in the local JDK anymore. #53 fixed the warning for me...

So I'd suggest focussing our efforts on #53 while it's still unlikely that people end up with the combination of Windows 7 and Oracle JDK 10+. Thoughts, @ctrueden, @imagejan? How often have you seen this combination in the wild?

ctrueden commented 6 years ago

I commented on #53. I'd like to merge and release 5.1.0, if you agree.

It has been years since we have pushed out a new launcher for Windows via the Updater, but we will need to do it in order to address this and other issues. We should probably have a testing period for the new ImageJ Launcher release before uploading it to the core update site. Sound good?

stelfrich commented 6 years ago

We should probably have a testing period for the new ImageJ Launcher release before uploading it to the core update site. Sound good?

I can do some more testing until the end of the week, after merging #53. Then we could revive https://forum.image.sc/t/please-test-the-new-imagej-launcher/8238/26 to ask for more feedback and or start a new thread..

stelfrich commented 6 years ago

Then we could revive https://forum.image.sc/t/please-test-the-new-imagej-launcher/8238/26 to ask for more feedback and or start a new thread..

With #60 merged and working properly in my tests (Ubuntu 18.04, macOS 10.12.6, Windows 10 with various Java combinations), I would suggest we go ahead and either revive the mentioned thread or start a new one. I would suggest starting a new one since the instructions will change to also install imagej-launcher-5.0.1-20181108.194258-66.jar (for which they have to download imagej-launcher-5.0.1-20181108.194258-66.nar and rename it from *.nar to *.jar (why the f*** is this packaged as NAR...)).

What do you think @ctrueden?

ctrueden commented 6 years ago

Great. Could you branch a new topic from the old topic, @stelfrich? That may be a little more visible than appending to the old one. And we should announce on Gitter as well, no?

But regarding the NAR packaging: we need to get to the bottom of that! I think we should do that first before announcing.

stelfrich commented 6 years ago

But regarding the NAR packaging: we need to get to the bottom of that! I think we should do that first before announcing.

Oh well. It has always been like this: sorry for the false alarm, although we should still look into it!

I think this is expected behavior, at least according to http://maven-nar.github.io/#install:

-.jar, containing a property file describing the behaviour of the library and other nar files available. For a description see NAR dependencies. **This jar file is renamed to nar when installed or deployed.**

What do we make of that now, @ctrueden?

ctrueden commented 6 years ago

Oh, it's still actually using NAR! I forgot. I thought we had fully switched away from the nar-maven-plugin. Should we do that now? Or are there still obstacles?

stelfrich commented 6 years ago

Should we do that now? Or are there still obstacles?

You mean completely getting rid of it, @ctrueden? I quickly tried to at least change the packaging to jar and add the maven-nar-plugin explicitly: https://github.com/imagej/imagej-launcher/commit/4dd15f8e79f8b49434104b40186365c6f941d4a1. It's not ideal but everything seems to still work in my tests (and we end up with a imagej-launcher-5.0.1-XXX.jar).

I wouldn't want to go any further and e.g. do the compiling via CMake. Or was that your idea?

ctrueden commented 6 years ago

I wouldn't want to go any further and e.g. do the compiling via CMake. Or was that your idea?

Why not? Too big a change?

If you are confident your hack is working, then maybe it is OK to proceed that way, since we never want to deploy any NAR artifacts anymore. Right? And then we could switch to CMake at a later time?

ctrueden commented 2 years ago

I believe this issue has been resolved now with the launcher 6.x series. Unfortunately, we aren't shipping that version of the native executable with Fiji yet, due to launch breakages on certain systems, but the latest mainline code in this repository does support newer Java versions including Java 11. Also, no more NAR dependency.