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

Make compatible starting at Java 1.8 #53

Closed
wants to merge 1 commit into from
Closed

Conversation

wviana
Copy link

@wviana wviana commented Jul 11, 2024

Hi there,

We are using this in a project at work.

One thing that I missed was to be able to run it on older Java versions. We do use Java 17

So I've tried changing the build.gradle, set it to JAVA_1_8, compiled it using gradlew and java 21.

./gradlew releaseJar

So I did changed to java 1.8, I do use sdkman to menage multiple java versions.

$ sdk use java 8.0.412-tem
Using java version 8.0.412-tem in this shell.
$ java -version
openjdk version "1.8.0_412"
OpenJDK Runtime Environment (Temurin)(build 1.8.0_412-b08)
OpenJDK 64-Bit Server VM (Temurin)(build 25.412-b08, mixed mode)
$  java -jar build/releases/adr-j.jar list
0001-record-architecture-decisions.md
0005-help-comments.md
0008-use-java-lts-versions.md
0006-use-command-line-processing-package.md
0004-markdown-format.md
0007-use-specific-environment-variables-for-editors.md
0002-implement-as-Java.md
0009-plain-text-format.md
0003-single-command-with-subcommands.md

That's it, I was able to use it with Java 1.8.

Other than it later I may take some time to make list sub-command to list adr sorted.
Also thought about having it as a gradle plugin.
Maybe in future.

@adoble
Copy link
Owner

adoble commented Jul 11, 2024

Many thanks for looking into this and getting it to work with a previous version of Java.

I'm unsure what to do with the PR now. ADR 8 states that the tool is compiled using the last LTS version of Java, which kind of rules out using earlier versions such as 1.8. It doesn't mean that adr-j can not be compiled with earlier versions, just that it's not guaranteed to work.

My understanding of the PR is that the gradle script will just compile using the older version (here I must admit that I'm not really an expert on Gradle so maybe I'm wrong). This runs counter to ADR 8 and won't suit all users of the tool.

You have, however, demonstrated that there is maybe a need to use older versions of Java. Perhaps this is more of a documentation issue, which shows where changes can be made to the gradle script like you have.

I'm afraid it's a mystery to me why the order of the listed ADRs changes when using Java 1.8. I suspect that this indicates something wrong with the sorting algo.

What are your thoughts on this?

@wviana
Copy link
Author

wviana commented Jul 11, 2024

It doesn't mean that adr-j can not be compiled with earlier versions, just that it's not guaranteed to work.

Ok, it was not actually compiled using Java 8. I did compiled using Java 21 LTS.
Only changed the target and source compatibility version.
My intent is only make it possible to use it with versions as old as possible.
Big enterprise projects using ADRs are probably not on the cutting edge java version.

My understanding of the PR is that the gradle script will just compile using the older version (here I must admit that I'm not really an expert on Gradle so maybe I'm wrong). This runs counter to ADR 8 and won't suit all users of the tool.

Actually not, it will keep compiling with recent versions of java. I actually already tried to compile with java 8 and it isn't possible. Here I'm changing just which JVM would be necessary to run the jar. It would not impact which java features to use.

I was digging more about source and target compatibility stackoverflow question
And I probably should be keeping source at 21 and making just target 1.8

As one answer cites java doc

-source Specifies the version of source code accepted.
-target Generate class files that target a specified version of the VM. Class files will run on the specified target and on later versions, but not on earlier versions of the VM.

I think the confusion here is that I didn't compiled it using older java version, as I stated in PR message

So I've tried changing the build.gradle, set it to JAVA_1_8, compiled it using gradlew and java 21.

Just changed to say to javac 21 to make bytecode that java 1.8 would be able to execute.

I'm afraid it's a mystery to me why the order of the listed ADRs changes when using Java 1.8. I suspect that this indicates something wrong with the sorting algo.

This is something I thought we didn't have. I've noticed that here in CommandList it should be printing it ordered.

So right now I'm going to test changing source back to 21 and only target to 1.8.

I'll also try to figure out why when using list in java 1.8 it didn't sort the ADRs.

@wviana
Copy link
Author

wviana commented Jul 11, 2024

About list command ordering.

I did compiled exactly as it is in your main right now, using Java 21

$ sdk use java 21.0.3-tem
Using java version 21.0.3-tem in this shell.

$ java --version
openjdk 21.0.3 2024-04-16 LTS
OpenJDK Runtime Environment Temurin-21.0.3+9 (build 21.0.3+9-LTS)
OpenJDK 64-Bit Server VM Temurin-21.0.3+9 (build 21.0.3+9-LTS, mixed mode)

$ ./gradlew clean releaseJar

Deprecated Gradle features were used in this build, making it incompatible with Gradle 9.0.

You can use '--warning-mode all' to show the individual deprecation warnings and determine if they come from your own scripts or plugins.

For more on this, please refer to https://docs.gradle.org/8.2/userguide/command_line_interface.html#sec:command_line_warnings in the Gradle documentation.

BUILD SUCCESSFUL in 1s
5 actionable tasks: 5 executed

$ java -jar build/releases/adr-j.jar list
0001-record-architecture-decisions.md
0005-help-comments.md
0008-use-java-lts-versions.md
0006-use-command-line-processing-package.md
0004-markdown-format.md
0007-use-specific-environment-variables-for-editors.md
0002-implement-as-Java.md
0009-plain-text-format.md
0003-single-command-with-subcommands.md

Notice it's listing out of order.
I may be doing something wrong. Please tell me if you couldn't reproduce same result.

@wviana
Copy link
Author

wviana commented Jul 11, 2024

Also tried to make source 21 and target 1.8, but it makes gradle not able to compile it. I confess I don't deeply understand all the implications of target and source.

Here is my attempt, just to document the process here in this PR thread.

$ grep -i javaversion build.gradle
    sourceCompatibility = JavaVersion.VERSION_21
    targetCompatibility = JavaVersion.VERSION_1_8

$ java --version
openjdk 21.0.3 2024-04-16 LTS
OpenJDK Runtime Environment Temurin-21.0.3+9 (build 21.0.3+9-LTS)
OpenJDK 64-Bit Server VM Temurin-21.0.3+9 (build 21.0.3+9-LTS, mixed mode)

$ ./gradlew clean releaseJar
> Task :compileJava FAILED

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':compileJava'.
> warning: source release 21 requires target release 21

* Try:
> Run with --stacktrace option to get the stack trace.
> Run with --info or --debug option to get more log output.
> Run with --scan to get full insights.
> Get more help at https://help.gradle.org.

Deprecated Gradle features were used in this build, making it incompatible with Gradle 9.0.

You can use '--warning-mode all' to show the individual deprecation warnings and determine if they come from your own scripts or plugins.

For more on this, please refer to https://docs.gradle.org/8.2/userguide/command_line_interface.html#sec:command_line_warnings in the Gradle documentation.

BUILD FAILED in 744ms
2 actionable tasks: 2 executed

@wviana
Copy link
Author

wviana commented Jul 11, 2024

First problem I found so far by setting both source and target to 1_8 was when running the tests.

$ ./gradlew test

> Task :compileJava
warning: [options] source value 8 is obsolete and will be removed in a future release
warning: [options] target value 8 is obsolete and will be removed in a future release
warning: [options] To suppress warnings about obsolete options, use -Xlint:-options.
3 warnings

> Task :compileTestJava FAILED
warning: [options] source value 8 is obsolete and will be removed in a future release
warning: [options] target value 8 is obsolete and will be removed in a future release
warning: [options] To suppress warnings about obsolete options, use -Xlint:-options.
/Users/wviana/repos/adr-j/src/test/java/org/doble/adr/RecordTest.java:359: error: text blocks are not supported in -source 8
                String expectedContents = """
                                          ^
  (use -source 15 or higher to enable text blocks)
1 error
3 warnings

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':compileTestJava'.
> Compilation failed; see the compiler error output for details.

* Try:
> Run with --info option to get more log output.
> Run with --scan to get full insights.

Deprecated Gradle features were used in this build, making it incompatible with Gradle 9.0.

You can use '--warning-mode all' to show the individual deprecation warnings and determine if they come from your own scripts or plugins.

For more on this, please refer to https://docs.gradle.org/8.2/userguide/command_line_interface.html#sec:command_line_warnings in the Gradle documentation.

BUILD FAILED in 12s
3 actionable tasks: 2 executed, 1 up-to-date

I was expecting this for other things.

Now I'm trying to better understand it by reading some of gradle docs
https://docs.gradle.org/8.2/userguide/building_java_projects.html

Also found this snippet here that I was thinking about having to be able to also run the tests against older versions

task('testsOn17', type: Test) {
    javaLauncher = javaToolchains.launcherFor {
        languageVersion = JavaLanguageVersion.of(17)
    }
}

@adoble
Copy link
Owner

adoble commented Jul 12, 2024

First things first, the problem you had with the incorrect ordering of the adr list entries remains a mystery, but I have raised a new issue #54 to track this.

@adoble
Copy link
Owner

adoble commented Jul 12, 2024

I think you're doing a better job of tracing this problem down than I could. I hadn't, at first, understood your idea of just addressing target compatibility rather then source code compatibility.

Although I am unsure if it would help with the Java target/source version problem, maybe the gradle build script should be updated anyway to make it compatible with Gradle version 9.0.

Please let me know of you make any progress with this. The idea of compatibility with older Java versions is attractive.

I also should warn you that I will not be able to do anything from my side for the next 2 weeks (is there a way in GitHub to set an "out of office"?).

@sdavids
Copy link
Contributor

sdavids commented Aug 20, 2024

A little late to the party …

Gradle-DSL

java {
  toolchain {
    languageVersion = JavaLanguageVersion.of(21)
  }
}

tasks.withType(JavaCompile).configureEach {
  options.release = 8
}

https://docs.gradle.org/current/userguide/toolchains.html

https://docs.gradle.org/current/dsl/org.gradle.api.tasks.compile.JavaCompile.html

https://docs.gradle.org/current/javadoc/org/gradle/api/tasks/compile/CompileOptions.html#getRelease()

https://docs.oracle.com/en/java/javase/21/docs/specs/man/javac.html#option-release


You might want to consider migrating to Kotlin-DSL—the current build file is straightforward to migrate.

Kotlin-DSL

java {
  toolchain {
    languageVersion = JavaLanguageVersion.of(21)
  }
}

tasks.withType<JavaCompile>().configureEach {
  options.apply {
    release = 8
  }
}

@adoble
Copy link
Owner

adoble commented Aug 27, 2024

Will this solve the problem? I noticed the following answer in SO:

No, you cannot compile Java 11 source to Java 8 binaries.

In javac terms, the -source parameter can't be greater than the -target parameter.

So, if you want to produce Java 8 binaries, your sources should be written in java 8 (or earlier). If you don't use any Java 11 language features, your sources basically already are in Java 8 so this shouldn't be too big a problem.

I also noticed that this has not been included in #56 (or am I reading it wrong?).

I am concerned that target compatibility for ADR-J may prove to be unobtainable.

@sdavids
Copy link
Contributor

sdavids commented Aug 27, 2024

#57 makes this PR obsolete.

@adoble
Copy link
Owner

adoble commented Sep 3, 2024

As such I am closing this PR.

Many thanks for identifying this as an issue.

@adoble adoble closed this Sep 3, 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.

3 participants