Closed pykereaper closed 1 year ago
The website states, that the minimum Java Runtime needed is 8
That is correct. If you want to run GpsPrune, then you need a runtime of at least version 8.
That does not mean that you can compile the master branch from github with a compiler of version 8.
Compiling and running are two different things. If you want to run GpsPrune using a version 8 runtime, you can use the special "_java8" jar from the website. This is not generated from the master branch code on github, but instead it is created by compiling the java8
branch using a version 8 JDK. On this branch there are none of the isBlank
or List.of
or other issues you mentioned.
In fact I think you should already be familiar with this java8
branch, because you have previously submitted separate pull requests for the master
and java8
branches not so long ago: see https://github.com/activityworkshop/GpsPrune/pull/78 .
So, to summarize:
master
branch uses features of Java 9 and 11, requires therefore a compiler of at least version 11, and the result requires a runtime of at least version 11 too, depending on the compiler settings. This produces the "regular" jar on the website suitable for most users.java8
branch patches out these problematic features of Java 9 and 11 in order to be compilable with a JDK version 8. The result can be run with a runtime of version 8 (or higher) and produces the "_java8" jar on the website. I'd like to get rid of this and concentrate on Java 11, but there are still users who aren't willing or able to upgrade their Java runtime to version 11, so for now this branch has to still be maintained. I tried compiling with JDK 11 specifying a target of Java 8, but this did not produce satisfactory results for me, so I decided to branch and patch.A pull request to replace String.isBlank and List.of and String.repeat is therefore not necessary, these can stay as they are. I don't know how your Gradle and/or Maven tools will complain on the master branch though (I'd expect the java8 branch to be fine).
Thanks for the explanation!
Just to clear things up: I didn't intend to provide a pull request that patches the 9+ code occurrences, but one that changes the Maven and Gradle build scripts.
The javac
compiler supports the flags -target
and -source
:
javac [options] [sourcefiles]
Options:
-source release - Specifies the version of source code accepted.
-target release - Generates class files for a specific VM version.
Those two flags can be configured via Maven and Gradle.
Setting target
to e.g. 1.8 allows to use a Java 11 SDK and still make the byte code 1.8-compatible.
Additionally, as far as i understood, that allows, that even the source code is 11+ (source
), one could define 8 as target
and make it backwards-compatible. But as it turns out, I was wrong. If methods are used, that only are available in newer versions, the minimum target SDK must be this version.
To be honest, I can't imagine a use case, where using different minimum Java version for compiling and running could be of use. But that's what one could do.
So.. As the main
branch needs at least Java 11 to compile and run, I'd change the setting in Maven and Gradle, as both currently define 1.8 and provide a pull request. The java8
branch can stay as is. Do you agree?
(Regarding the java8
branch: Only 7 years to go. :D )
As the main branch needs at least Java 11 to compile and run, I'd change the setting in Maven and Gradle, as both currently define 1.8 and provide a pull request.
If you wish. I'm curious why Gradle / Maven don't complain themselves, as it looks like it's been incorrectly defined for some time. Nobody has noticed before now?
The java8 branch can stay as is. Do you agree?
I agree - it looks like the compiler versions are correctly set on the java8
branch, both in the Gradle and the Maven files. The weather xml files issue is also present on the java8
branch too, but that will be automatically merged over for the next release.
As the main branch needs at least Java 11 to compile and run, I'd change the setting in Maven and Gradle, as both currently define 1.8 and provide a pull request.
If you wish. I'm curious why Gradle / Maven don't complain themselves, as it looks like it's been incorrectly defined for some time. Nobody has noticed before now?
I guess, all people compiling do already use a JDK version ≥ 11.
Thanks for staying in this big discussion, and being open to merge improvements/fixes. Really appreciate it!
Pull request incoming. :slightly_smiling_face:
The website states, that the minimum Java Runtime needed is 8:
Right now, Maven and Gradle build scripts define Java 8 for both target compatibility & source compatibility.
IntelliJ code inspection showed several usages of newer APIs than Java 8. Most of them 9+, but also some 11+. See following examples.
Am I correct, that the min Java SDK to compile has to be 11 and the Runtime 8? In that case, I'd provide a pull request.
Most of them are from the following two types:
isBlank()
is 11+. Used in several classes. e.g.: https://github.com/activityworkshop/GpsPrune/blob/06d9ff3e10971bd586137862d93103adb1a2a0f7/src/tim/prune/cmd/Command.java#L68List.of()
is 9+. It is the most common error, the inspection brought up. A lot of usages in several classes. e.g.: https://github.com/activityworkshop/GpsPrune/blob/06d9ff3e10971bd586137862d93103adb1a2a0f7/src/tim/prune/function/CutAndMoveFunction.java#L69But also other methods.
repeat
is 11+: https://github.com/activityworkshop/GpsPrune/blob/06d9ff3e10971bd586137862d93103adb1a2a0f7/src/tim/prune/data/CoordFormatters.java#L31