Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add mechanism for updating Java #93

Draft
wants to merge 6 commits into
base: master
Choose a base branch
from
Draft

Add mechanism for updating Java #93

wants to merge 6 commits into from

Conversation

hinerm
Copy link
Member

@hinerm hinerm commented May 14, 2024

‼️ NB: requires SJC release before merging ‼️

@hinerm hinerm requested a review from ctrueden May 14, 2024 21:28
@hinerm hinerm force-pushed the download-java branch 2 times, most recently from 2ee0eda to d9c0a1a Compare May 20, 2024 19:10
We can now control the java version per-platform using
https://downloads.imagej.net/java/jdk-urls.txt, which contains URLs for
JDK downloads. These are cached locally and any time a more-recent
version is available, it is downloaded to the appropriate /java
sub-directory.

If jaunch is being used, an app.cfg file is created to tell the launcher
which JDK to use on subsequent launches.
@hinerm hinerm self-assigned this Jul 19, 2024
@hinerm hinerm marked this pull request as draft September 13, 2024 19:05
@hinerm
Copy link
Member Author

hinerm commented Sep 13, 2024

This logic should be moved to Jaunch or ClassLauncher instead

@hinerm
Copy link
Member Author

hinerm commented Sep 13, 2024

Improvement to the logic is needed in that we're only really considering managed versions of Java right now.

  • The message that "a newer Java is needed" doesn't make sense if the user is manually running Java 21 (although I think this can only happen with a manually Jaunch'd installation?)
  • After declining an upgrade, we keep asking. There needs to be an ignore option, or only ask again if there's a version change (along with a way to manually trigger the upgrade, ideally, if someone declined and then changes their mind)
  • The path to the JDK should be relative instead of absolute, to preserve integrity if the app dir is moved

@ctrueden
Copy link
Member

ctrueden commented Sep 13, 2024

the user is manually running Java 21 (although I think this can only happen with a manually Jaunch'd installation?)

There are lots of ways users can run Fiji with a different OpenJDK 21 than the one we download and unpack, without having set up Jaunch manually. Assuming the user has installed SciJava Ops by enabling the SciJava Ops update site etc., they could then:

  • Manually download a different OpenJDK 21 bundle and unpack it into their Fiji.app/java/<platform> folder
  • Launch with ./fiji-linux-x64 --java-home /path/to/my/openjdk-21.
  • Rename their java folder to java.disabled and have OpenJDK 21 installed in some other fashion, such as system-wide, via conda, cjdk, or sdkman.

@hinerm
Copy link
Member Author

hinerm commented Sep 19, 2024

When comparing java versions we need to know if the current Java is a bundled Java or custom.
If the java is same or newer then we don't care.
If the Java version is older, we have to look if it's bundled or not. If so, we feel confident in updating. If not, notify the user?

We can use the FileDownloader logic for actually doing the downloads.

For the update checking comparison timing, instead of doing it pre-emptively, we can do it when the application fails to launch because of Java versions. But we probably still want a mechanism tied to the updater, which could mean adding a dependency on imagej-launcher.

Ideally this update-time check would be migrated to the ImageJ-Updater component

@hinerm
Copy link
Member Author

hinerm commented Sep 19, 2024

Other consideration: how can we get the Java updating to work with the command line updater?

In the launcher we could have a boolean updateHeadless that defaults true and just does the update logic if headless by default.

For the command line updater, could we easily add an update-java target?

ctrueden added a commit that referenced this pull request Oct 20, 2024
ctrueden added a commit that referenced this pull request Oct 20, 2024
ctrueden added a commit that referenced this pull request Oct 20, 2024
ctrueden added a commit that referenced this pull request Oct 20, 2024
ctrueden added a commit that referenced this pull request Oct 20, 2024
ctrueden added a commit that referenced this pull request Oct 20, 2024
ctrueden added a commit that referenced this pull request Oct 21, 2024
ctrueden added a commit that referenced this pull request Oct 21, 2024
ctrueden added a commit that referenced this pull request Nov 19, 2024
ctrueden added a commit that referenced this pull request Nov 19, 2024
ctrueden added a commit that referenced this pull request Nov 20, 2024
ctrueden added a commit that referenced this pull request Nov 20, 2024
ctrueden added a commit that referenced this pull request Nov 21, 2024
ctrueden added a commit that referenced this pull request Nov 21, 2024
hinerm pushed a commit that referenced this pull request Nov 21, 2024
ctrueden added a commit that referenced this pull request Nov 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants