Stop targeting JDK 8 #187

Stephan202 · 2022-01-04T09:45:09Z

❗ This PR is currently stacked on top of #186. ❗

Suggested commit message:

Stop targeting JDK 8 (#187)

ferdinand-swoboda · 2022-01-04T09:51:48Z

.travis.yml

@@ -1,12 +1,12 @@
 language: java
 matrix:
  include:
-    - jdk: openjdk8


Then we should also bump version.jdk, right?

@ferdinand-swoboda good point. Without this the PR would only about not supporting JDK 8 for building, but indeed my intention was to also raise the baseline. Pushed a commit.

rickie · 2022-01-04T10:12:32Z

pom.xml

@@ -100,7 +100,7 @@
        <version.error-prone-javac>9+181-r4173-1</version.error-prone-javac>
        <version.guava-beta-checker>1.0</version.guava-beta-checker>
        <version.immutables>2.8.8</version.immutables>
-        <version.jdk>1.8</version.jdk>
+        <version.jdk>1.11</version.jdk>


Shouldn't it be: <version.jdk>11</version.jdk> ?

I need a coffee.

(And this also shows we should prioritize #4 or some other solution that allows us to verify most/all changes in this repo.)

ferdinand-swoboda · 2022-01-04T12:36:39Z

There are 2 hits for "JDK 8". These are comments indicating some simplification might be possible now.

ferdinand-swoboda · 2022-01-04T12:38:48Z

.travis.yml

    - jdk: openjdk11
      # This is the primary target platform. In this build perform a SonarQube
      # analysis.
      script: ./mvnw install sonar:sonar
+    - jdk: openjdk17


In order to build with JDK 16+ some changes to the error-prone profile are required; at least when I mvn clean verify Jolo. -> Pushed some changes

ferdinand-swoboda · 2022-01-04T13:50:04Z

pom.xml

-                                    --> -XepAllErrorsAsWarnings <!--
-                                    We want to enable almost all Error
+                                    encountered. -->
+                                <arg>-XepAllErrorsAsWarnings</arg>


For some reason this causes error: invalid flag: -XepAllErrorsAsWarnings on Jolo. 😕

In PSM we don't wrap every flag in <arg>..</arg> so I tried it there and got the same error message as you mention here.
I found a message posted by @Stephan202 here with a small summary:

All -Xplugin:ErrorProne arguments must be specified inside the same . So on JDK 8 one either has to put them all on one line (like here) or hide the newlines inside XML comments, like so.

So I'd say that reverting the <arg> changes should fix the issue 😄.

Indeed. More generally: where possible we should match the setup of the Picnic-internal Maven parent.

ferdinand-swoboda · 2022-01-04T13:51:14Z

There are 2 hits for "JDK 8". These are comments indicating some simplification might be possible now.

Pushed 2 commits that drop JDK 8 workarounds. There might be more outdated comments that we can address in a separate PR.

Stephan202

I clearly should have tested this PR in downstream projects before opening it. Please don't spend more time on this for now.

Stephan202 · 2022-01-05T16:00:40Z

pom.xml

+                                <!-- Required to work with JDK 16+;
+                                see also https://errorprone.info/docs/installation#jdk-16. -->
+                                <arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED</arg>
+                                <arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED</arg>
+                                <arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.main=ALL-UNNAMED</arg>
+                                <arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.model=ALL-UNNAMED</arg>
+                                <arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.parser=ALL-UNNAMED</arg>
+                                <arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.processing=ALL-UNNAMED</arg>
+                                <arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED</arg>
+                                <arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED</arg>
+                                <arg>-J--add-opens=jdk.compiler/com.sun.tools.javac.code=ALL-UNNAMED</arg>
+                                <arg>-J--add-opens=jdk.compiler/com.sun.tools.javac.comp=ALL-UNNAMED</arg>


We should not do this, but instead define a .mvn/jvm.config file; that yields more performant builds (#2786) and also avoids issues such as #106. More generally, we should port changes from internal builds that are already compatible with JDK 17; that could has already been carefully reviewed and tested.

Fixed the link 😉

Stephan202 · 2022-01-05T16:03:58Z

pom.xml

-                                    --> -XepAllErrorsAsWarnings <!--
-                                    We want to enable almost all Error
+                                    encountered. -->
+                                <arg>-XepAllErrorsAsWarnings</arg>


Indeed. More generally: where possible we should match the setup of the Picnic-internal Maven parent.

ferdinand-swoboda · 2022-01-14T10:34:27Z

I filed #192 for JDK 17 support. We should keep this PR about dropping JDK 8 support.

sonarqubecloud · 2022-01-14T10:42:06Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication

…en-compiler-plugin

This reverts commit 84f9b0d.

This reverts commit a302b3b.

rickie

LGTM!

Stephan202

Updated suggested commit message:

Target JDK 11 by default, drop JDK 8 support (#187)

Stephan202 · 2022-04-16T11:42:04Z

pom.xml

+                                <path>
+                                    <groupId>com.google.errorprone</groupId>
+                                    <artifactId>error_prone_annotations</artifactId>
+                                    <version>${version.error-prone}</version>
+                                </path>


Just tested against reactive-support: it seems we can omit this. (Perhaps because I just merged #197, which does introduce an Error Prone/Guava dependency convergence error, but we'll fix that in #175.)

sonarqubecloud · 2022-04-16T11:43:52Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication

Stephan202 added this to the 0.0.4 milestone Jan 4, 2022

ferdinand-swoboda reviewed Jan 4, 2022

View reviewed changes

rickie reviewed Jan 4, 2022

View reviewed changes

ferdinand-swoboda approved these changes Jan 4, 2022

View reviewed changes

rickie approved these changes Jan 4, 2022

View reviewed changes

ferdinand-swoboda reviewed Jan 4, 2022

View reviewed changes

ferdinand-swoboda mentioned this pull request Jan 4, 2022

Upgrade com.coveo:fmt-maven-plugin 2.9.1 -> 2.13 #137

Merged

1 task

ferdinand-swoboda reviewed Jan 4, 2022

View reviewed changes

rickie self-requested a review January 5, 2022 08:05

Stephan202 commented Jan 5, 2022

View reviewed changes

ferdinand-swoboda mentioned this pull request Jan 14, 2022

Support JDK 17 #191

Closed

ferdinand-swoboda changed the title ~~Stop targeting JDK 8, add support for JDK 17~~ Stop targeting JDK 8 Jan 14, 2022

ferdinand-swoboda mentioned this pull request Jan 14, 2022

Support JDK 17 #192

Merged

Base automatically changed from sschroevers/drop-lint-maven-plugin to master April 16, 2022 11:32

Stephan202 and others added 10 commits April 16, 2022 13:35

Stop targeting JDK 8, add support for JDK 17

28013b3

Doh

3747809

Man man man

e730af3

Support building with JDK 17

4ce6f1b

Make InlineMeValidationDisabled error-prone annotation known to mav…

ef67dcc

…en-compiler-plugin

Drop jdk8 profile

b136411

Drop JDK 8 workaround for newline separated compiler args

b769049

Revert "Drop JDK 8 workaround for newline separated compiler args"

1e648f7

This reverts commit 84f9b0d.

Revert "Support building with JDK 17"

7e64e84

This reverts commit a302b3b.

Don't run this build on JDK 17 just yet

dc2d262

rickie approved these changes Apr 16, 2022

View reviewed changes

Stephan202 force-pushed the sschroevers/raise-build-baseline branch from 424c9fb to dc2d262 Compare April 16, 2022 11:35

Drop explicit error_prone_annotations entry

da7ee6e

Stephan202 commented Apr 16, 2022

View reviewed changes

Stephan202 merged commit 7fce171 into master Apr 16, 2022

Stephan202 deleted the sschroevers/raise-build-baseline branch April 16, 2022 11:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Stop targeting JDK 8 #187

Stop targeting JDK 8 #187

Stephan202 commented Jan 4, 2022 •

edited by ferdinand-swoboda

Loading

ferdinand-swoboda Jan 4, 2022

Stephan202 Jan 4, 2022

rickie Jan 4, 2022 •

edited

Loading

Stephan202 Jan 4, 2022

Stephan202 Jan 4, 2022

ferdinand-swoboda commented Jan 4, 2022

ferdinand-swoboda Jan 4, 2022 •

edited

Loading

ferdinand-swoboda Jan 4, 2022

rickie Jan 5, 2022

Stephan202 Jan 5, 2022

ferdinand-swoboda commented Jan 4, 2022

Stephan202 left a comment

Stephan202 Jan 5, 2022 •

edited by rickie

Loading

rickie Jan 6, 2022 •

edited

Loading

Stephan202 Jan 5, 2022

ferdinand-swoboda commented Jan 14, 2022 •

edited

Loading

sonarqubecloud bot commented Jan 14, 2022

rickie left a comment

Stephan202 left a comment

Stephan202 Apr 16, 2022

sonarqubecloud bot commented Apr 16, 2022

Stop targeting JDK 8 #187

Stop targeting JDK 8 #187

Conversation

Stephan202 commented Jan 4, 2022 • edited by ferdinand-swoboda Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rickie Jan 4, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ferdinand-swoboda commented Jan 4, 2022

ferdinand-swoboda Jan 4, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ferdinand-swoboda commented Jan 4, 2022

Stephan202 left a comment

Choose a reason for hiding this comment

Stephan202 Jan 5, 2022 • edited by rickie Loading

Choose a reason for hiding this comment

rickie Jan 6, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ferdinand-swoboda commented Jan 14, 2022 • edited Loading

sonarqubecloud bot commented Jan 14, 2022

rickie left a comment

Choose a reason for hiding this comment

Stephan202 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarqubecloud bot commented Apr 16, 2022

Stephan202 commented Jan 4, 2022 •

edited by ferdinand-swoboda

Loading

rickie Jan 4, 2022 •

edited

Loading

ferdinand-swoboda Jan 4, 2022 •

edited

Loading

Stephan202 Jan 5, 2022 •

edited by rickie

Loading

rickie Jan 6, 2022 •

edited

Loading

ferdinand-swoboda commented Jan 14, 2022 •

edited

Loading