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

Allow Indexing with Java 8 #236

Closed
wants to merge 2 commits into from
Closed

Conversation

gselzer
Copy link
Member

@gselzer gselzer commented Jun 24, 2024

Should enable seamless Op indexing in ImgLib2 projects.

@hinerm - do you see any pure-POM way to enable this? I had to make a lot of unfortunate changes to the code to make it Java-8 compliant, however I feel like that shouldn't be necessary, if we instead set maven.compiler.source/maven.compiler.target. Do you know what I did wrong?

@gselzer gselzer requested review from ctrueden and hinerm June 24, 2024 22:29
@hinerm
Copy link
Member

hinerm commented Jun 24, 2024

@gselzer I don't think there's a way to use newer Java language features in older Java runtimes. What you did seems appropriate. I am curious why it was necessary though?

@gselzer
Copy link
Member Author

gselzer commented Jun 25, 2024

I am curious why it was necessary though?

My goal is to enable Op indexing within projects that may not be ready to transition to Java 11 (see imglib/imglib2-mesh#10, imglib/imglib2-algorithm#94). If you attempt to use org.scijava:scijava-ops-indexer:1.0.0 compiling with Java 8, you'll receive compilation errors:

[INFO] -------------------------------------------------------------
[ERROR] COMPILATION ERROR :
[INFO] -------------------------------------------------------------
[ERROR] error: Could not load processor class file due to 'org/scijava/ops/indexer/OpImplNoteParser has been compiled by a more recent version of the Java Runtime (class file version 55.0), this version of the Java Runtime only recognizes class file versions up to 52.0'.
[INFO] 1 error
[INFO] -------------------------------------------------------------
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  2.523 s
[INFO] Finished at: 2024-06-25T08:52:41-05:00
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.8.1:compile (default-compile) on project
 imglib2-mesh: Compilation failure
[ERROR] error: Could not load processor class file due to 'org/scijava/ops/indexer/OpImplNoteParser has been compiled by a more recent version of the Java Runtime (class file version 55.0), this version of the Java Runtime only recognizes class file versions up to 52.0'.
[ERROR]
[ERROR] -> [Help 1]

@gselzer
Copy link
Member Author

gselzer commented Jun 25, 2024

However many of the changes (specifically deleting the module-info.java) I find to be a step backwards

Copy link
Member

@ctrueden ctrueden left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am torn on this. For the module-info.java specifically, we could maybe keep it but move it to the META-INF/versions tree. But the code changes to regress the source compatibility back to 8 from 11+ feels bad, indeed. OTOH, it's not very many changes—maintenance-wise, probably pretty easy to keep working for quite some time still. Since @gselzer already did the work, I guess the main thing is just to test whether META-INF/versions works for the module-info.java?

@gselzer gselzer force-pushed the scijava-ops-indexer/allow-java-8 branch 2 times, most recently from 3006c60 to 7852ef7 Compare July 15, 2024 14:48
Should enable seamless Op indexing in ImgLib2 projects
@gselzer gselzer force-pushed the scijava-ops-indexer/allow-java-8 branch from 7852ef7 to 67b5ba7 Compare July 15, 2024 17:45
@gselzer gselzer force-pushed the scijava-ops-indexer/allow-java-8 branch from 86059da to bc13092 Compare July 15, 2024 18:50
@ctrueden
Copy link
Member

ctrueden commented Aug 7, 2024

Current plan is to change Java-8-dependent projects to have a Java-11+ build time requirement, but still target Java 8 bytecode via the --release flag. Then those projects can depend on the scijava-ops-indexer as is, even though it's built targeting Java 11 bytecode. And we won't have to devolve the code back to Java 8 syntax here. 🙈

@ctrueden ctrueden closed this Aug 26, 2024
@ctrueden ctrueden deleted the scijava-ops-indexer/allow-java-8 branch August 26, 2024 16:05
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.

3 participants