Requiring only Java 17 and 21 doesn't make sense

[bartekpacia]

Synopsis

This doesn't make sense:

Relevant code:

patrol/packages/patrol_cli/lib/src/compatibility_checker/compatibility_checker.dart

Lines 157 to 168 in a2bc620

	javaVersion = await javaCompleterVersion.future;
	if (javaVersion == null) {
	throwToolExit(
	'Failed to read Java version. Make sure you have Java installed and added to PATH',
	);
	} else if (javaVersion.major != 17 && javaVersion.major != 21) {
	logger.warn(
	'You are using Java $javaVersion which can cause issues on Android.\n'
	'If you encounter any issues, try changing your Java version to 17 or 21.',
	);
	}

This condition should be simplified to javaVersion >= 17 (or 21, when the Android Gradle Plugin drops support for 17 and starts requiring at least 21 – which will happen at some point in the future).

(Admittedly, I approved the PR that introduced this code, but I didn't know it didn't make sense back then. Now I know and would like to fix it).

More info

2 great blogposts by Jake Wharton worth reading to understand there's no harm in using latest (even non-LTS) JDK:

• Build on latest Java, test through lowest Java
• Gradle toolchains are rarely a good idea (includes a lot of info why using latest JDK is great).

You can still target and test on old JVM versions but develop with the latest and greatest. Java and the JDK are literally built for this.

Java is strongly backward compatible.

Many users install Java on macOS using brew install openjdk (see the openjdk formula), which defaults to the latest stable Java (not necessarily LTS. Currently latest stable Java is 23). But it's fine; non-lts releases are still very stable and simply work. There's no need to force users to use either 17 or 21, and it can create a false belief among users that e.g Java 17 or Java 21 are "stable", which is not true in fact. They're just LTS versions.

[pdenert]

Thanks for the issue and your thoughts!

Requiring only Java 17 and 21 doesn't make sense

Agree! But it's not what we're doing here and you're know that. I thought about that while implementing this. So, why I left this like that?

1. First and most important reason is fact, that we're allow to use newer java, but we display warning, that we don't know if it will work.
2. Gradle prints similar warning for AGP (informing about tested compatibility).
   Here's example how gradle is doing this:

: WARNING:We recommend using a newer Android Gradle plugin to use compileSdk = 34
: 
: This Android Gradle plugin (7.2.1) was tested up to compileSdk = 32
: 
: This warning can be suppressed by adding
:     android.suppressUnsupportedCompileSdk=34
: to this project's gradle.properties
: 
: The build will continue, but you are strongly encouraged to update your project to
: use a newer Android Gradle Plugin that has been tested with compileSdk = 34

3. Based on our experience from stationary workshops - most of the flutter developers uses java from Android Studio.
4. Android always adopting new java with some delay, and so flutter.

Flutter built on top of Android, java and gradle is not so easy to debug and Patrol adds another layer for that. As our resources to develop and maintain Patrol are limited, we prefer to suggest users with issues, to switch to java that we know that patrol is compatible with.

I also have read the Jake Wharton article, you linked to issue, about testing compatibility with different java version and would like to have this in Patrol, but there are other priorities right now.

[bartekpacia]

The only incompatiblity that I think can occur is Java being too new for Gradle to run; but this is not a Patrol problem and Patrol shouldn't care about it, i.e. if you try to flutter build apk with too new Java for your Gradle, it will crash the same way it crashes with patrol build (and should be pretty obvious when someone complains and pastes the logs).

That said while I don't agree with your decision I understand the "why" and yeah it makes sense.

Closing, thx!