Skip to content

fix: build maven project using jdk11#145

Merged
octonato merged 2 commits intomainfrom
rgc/fix-maven-release
Jul 15, 2021
Merged

fix: build maven project using jdk11#145
octonato merged 2 commits intomainfrom
rgc/fix-maven-release

Conversation

@octonato
Copy link
Copy Markdown
Member

For unknown reason, we were building the maven projects using jdk8. It turns out that since we first need to publish the codegen locally and the sbt is configured with --release flag, we get the following error:

-release is only supported on Java 9 and higher

(https://app.circleci.com/pipelines/github/lightbend/akkaserverless-java-sdk/503/workflows/fcbd35cb-21e1-44a9-a9fe-a36ef028d7f2/jobs/3088)

For the record, when I say for unknown reason I mean, I don't know why the old maven-java project was using jdk8. What we have now here is a copy-n-paste from that previous project. Anyway, my understanding is that we only build with jdk11 (not only on this project, but every akka serverless project) therefore align with jdk11 is the way to go.

Copy link
Copy Markdown
Member

@pvlugter pvlugter left a comment

Choose a reason for hiding this comment

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

Do we have a -release 8 setting for the maven-java build as well? We still want to target and support JDK 8, even if we're building on JDK 11.

@johanandren
Copy link
Copy Markdown
Contributor

johanandren commented Jul 15, 2021

Isn't it safer to compile using Java 8 if that is what we want to support? (Thinking of the risk that we use JDK 11 only stdlib features for example)

@octonato
Copy link
Copy Markdown
Member Author

The maven-java build itself is not using the flag, but the job is configured to use jdk8 and when it comes to build the codegen and the sdk through publishM2, it fails because of the flag.

However, the maven build is configured to target jdk8. (see https://github.com/lightbend/akkaserverless-java-sdk/blob/main/maven-java/pom.xml#L30-L35)

I agree with Johan that if the goal is to support JDK8, then we should build with it (as well).

In any case, I would rather unlock this for now and add proper JDK8 support in another PR.

Copy link
Copy Markdown
Contributor

@raboof raboof left a comment

Choose a reason for hiding this comment

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

I'm OK with this if we make sure to also set maven.compiler.release for the maven builds - otherwise we might run into problems because we build against the jdk11 bootclasspath even if we generate java8 bytecode.

I think it's hard to say what is "safer": running on jdk8 when building the release avoids forgetting setting the release flag, on the other hand setting the release flag avoids forgetting to run on jdk8 when building :). Perhaps setting the release flag is slightly nicer since it makes building locally more similar to building on CI.

Comment thread maven-java/pom.xml
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
<maven.compiler.source>1.8</maven.compiler.source>
<maven.compiler.target>1.8</maven.compiler.target>
<maven.compiler.release>8</maven.compiler.release>
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Not clear if we should keep source and target. I didn't find clear documentation about it. I may be that release is sufficient. In any case, I don't want to fiddle with this right now.

@octonato octonato merged commit 9706ba4 into main Jul 15, 2021
@octonato octonato deleted the rgc/fix-maven-release branch July 15, 2021 21:25
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.

4 participants