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

[JENKINS-68568] Require Java 11 or newer #478

Merged
merged 15 commits into from Nov 30, 2022
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions README.md
Expand Up @@ -46,7 +46,7 @@ In order to use the new POM:
```xml
<properties>
<jenkins.version>2.303.1</jenkins.version>
<java.level>8</java.level>
<java.level>11</java.level>
</properties>
```

Expand All @@ -61,7 +61,7 @@ If you had a `jar:test-jar` execution, delete it and add to `properties`:
The plugin POM is designed for plugin builds with JDK 8 or above,
but target `java.level` for a plugin may differ from a JDK version used for the build.
Starting from Plugin POM `3.44`, support of Java 7 targets in Plugin POM is deprecated and has been removed in `4.0`,
`java.level=8` and `jenkins.version>2.249.1` are expected to be used for most plugins.
`java.level=11` and `jenkins.version>2.249.1` are expected to be used for most plugins.


## Incrementals
Expand Down
74 changes: 5 additions & 69 deletions pom.xml
Expand Up @@ -103,7 +103,6 @@

<mockito.version>4.3.1</mockito.version>
<access-modifier-checker.version>1.27</access-modifier-checker.version>
<animal.sniffer.version>1.21</animal.sniffer.version>

<!-- To opt in to @Restricted(Beta.class) APIs, set to true: -->
<useBeta>false</useBeta>
Expand Down Expand Up @@ -166,17 +165,6 @@
<artifactId>junit</artifactId>
<version>4.13.2</version>
</dependency>
<dependency>
<groupId>org.codehaus.mojo</groupId>
<artifactId>animal-sniffer-annotations</artifactId>
<version>${animal.sniffer.version}</version>
</dependency>
<dependency>
<groupId>org.codehaus.mojo.signature</groupId>
<artifactId>java18</artifactId>
<version>1.0</version>
<type>signature</type>
</dependency>
<dependency>
<groupId>org.hamcrest</groupId>
<artifactId>hamcrest-core</artifactId>
Expand Down Expand Up @@ -235,14 +223,6 @@
<optional>true</optional>
</dependency>

<!-- for JRE requirement check annotation -->
<dependency>
<!-- no need to have this at runtime -->
<groupId>org.codehaus.mojo</groupId>
<artifactId>animal-sniffer-annotations</artifactId>
<scope>provided</scope>
<optional>true</optional>
</dependency>
<dependency>
<!-- to avoid https://www.slf4j.org/codes.html#release warning -->
<!-- TODO this would be better as a managed version of [0] -->
Expand Down Expand Up @@ -422,11 +402,6 @@
<artifactId>maven-hpi-plugin</artifactId>
<version>${hpi-plugin.version}</version>
</plugin>
<plugin>
<groupId>org.codehaus.mojo</groupId>
<artifactId>animal-sniffer-maven-plugin</artifactId>
<version>${animal.sniffer.version}</version>
</plugin>
<plugin>
<groupId>org.jvnet.localizer</groupId>
<artifactId>localizer-maven-plugin</artifactId>
Expand Down Expand Up @@ -515,7 +490,7 @@
</requirePluginVersions>
-->
<enforceBytecodeVersion>
<maxJdkVersion>1.${java.level}</maxJdkVersion>
<maxJdkVersion>${java.level}</maxJdkVersion>
Copy link
Member

Choose a reason for hiding this comment

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

This is going to break for java.level=8.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good thing this PR explicitly drops support for java.level=8, then, as stated in the PR description:

Drop support for Java 8

Copy link
Member

Choose a reason for hiding this comment

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

Yeah but we cannot just do that because then people cannot simply accept updates to the parent POM without also updating jenkins.version to something very new.

Copy link
Member

Choose a reason for hiding this comment

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

I mean, we could, but then we have plugin POM versions that work with older cores but are not well suited to newer cores, and POM versions that require newer cores, without any overlap. So this seems pretty drastic when we normally encourage people to update the parent even when the core version is a few LTS lines behind. For example, anyone staying with an older line would not be able to accept new POM versions for totally unrelated reasons (new tools or JTH).

If we really go with the flag day approach in this PR, then we should definitely get rid of the java.level property as it serves no purpose. Add a profile making it an error to set it, and hard-code 11 as the Java level. And make sure that the build fails early and clearly when your jenkins.version is too old.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think we can. We can always do a backport branch if we want to deliver plugin parent POM updates to plugins that do not want to upgrade jenkins.version to something very new.

Copy link
Member Author

Choose a reason for hiding this comment

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

So this seems pretty drastic

Yes it is drastic but nevertheless feasible without changing the current semantics of java.level being defined in a <properties> section in plugin pom.xml files. Other proposals like #133 are not feasible as far as I can tell so long as feedback like this remain unaddressed.

Instead figure out what Java level to pass to tools based on the jenkins.version (somehow).

That approach could be feasible (and less drastic!), but it would be a lot more work and (as of today) I am explicitly not volunteering to do that work.

Copy link
Member

Choose a reason for hiding this comment

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

Agree with Basil, because of how this was done you can't keep compat.

We can run a 4.x branch if we need to backport changes...

alternatively plugins that want to stay on old core could just set this to 1.8?

Copy link
Member

Choose a reason for hiding this comment

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

set this to 1.8

IIRC that will break other places which expect 8.

<ignoredScopes>
<ignoredScope>test</ignoredScope>
</ignoredScopes>
Expand Down Expand Up @@ -699,31 +674,10 @@
<plugin>
<artifactId>maven-compiler-plugin</artifactId>
<configuration>
<source>1.${java.level}</source>
<target>1.${java.level}</target>
<testSource>1.${java.level}</testSource>
<testTarget>1.${java.level}</testTarget>
<release>${java.level}</release>
<testRelease>${java.level}</testRelease>
</configuration>
</plugin>
<plugin>
<groupId>org.codehaus.mojo</groupId>
<artifactId>animal-sniffer-maven-plugin</artifactId>
<configuration>
<signature>
<groupId>org.codehaus.mojo.signature</groupId>
<artifactId>java1${java.level}</artifactId>
</signature>
</configuration>
<executions>
<execution>
<id>check</id>
<goals>
<goal>check</goal>
</goals>
<phase>test</phase>
</execution>
</executions>
</plugin>
<plugin>
<artifactId>maven-eclipse-plugin</artifactId>
</plugin>
Expand All @@ -736,7 +690,7 @@
<webApp>
<contextPath>/jenkins</contextPath>
</webApp>
<minimumJavaVersion>1.${java.level}</minimumJavaVersion>
<minimumJavaVersion>${java.level}</minimumJavaVersion>
Copy link
Member

Choose a reason for hiding this comment

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

We should just get rid of this in maven-hpi-plugin I think. Jenkins core does not really deal with plugins requiring a newer version of Java and we do not really want to offer such a system.

Copy link
Member Author

Choose a reason for hiding this comment

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

We should just get rid of this in maven-hpi-plugin I think.

I tend to agree, but I am not tackling that cleanup in this PR.

<systemProperties>
<hudson.Main.development>${hudson.Main.development}</hudson.Main.development>
</systemProperties>
Expand Down Expand Up @@ -950,26 +904,9 @@
</file>
</activation>
<properties>
<java.level>8</java.level>
<java.level>11</java.level>
Copy link
Member

Choose a reason for hiding this comment

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

No this must be reverted so long as

<jenkins.version>2.249</jenkins.version>

Copy link
Member Author

Choose a reason for hiding this comment

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

It cannot be reverted while also accomplishing the purpose of this PR, which is stated in the PR description:

Drop support for Java 8

The minimum Jenkins version cannot yet be updated because we do not know which version will drop support for Java 8, but the intention is that when we do we would update the minimum Jenkins version to that version.

Copy link
Member

Choose a reason for hiding this comment

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

Right, I misunderstood the intent of this PR.

</properties>
</profile>
<profile>
<id>jdk-above-9</id>
<activation>
<jdk>[1.9,)</jdk>
</activation>
<build>
<plugins>
<plugin>
<artifactId>maven-compiler-plugin</artifactId>
<configuration>
<release>${java.level}</release>
<testRelease>${java.level}</testRelease>
</configuration>
</plugin>
</plugins>
</build>
</profile>
<profile>
<id>jenkins-release</id>
<properties>
Expand Down Expand Up @@ -1519,7 +1456,6 @@
<spotbugs.skip>true</spotbugs.skip>
<enforcer.skip>true</enforcer.skip>
<access-modifier-checker.skip>true</access-modifier-checker.skip>
<animal.sniffer.skip>true</animal.sniffer.skip>
<invoker.skip>true</invoker.skip>
</properties>
</profile>
Expand Down
2 changes: 1 addition & 1 deletion src/it/benchmark/pom.xml
Expand Up @@ -14,7 +14,7 @@
<packaging>jar</packaging>
<properties>
<jenkins.version>2.249</jenkins.version>
<java.level>8</java.level>
<java.level>11</java.level>
Copy link
Member

Choose a reason for hiding this comment

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

Revert these unless also updating jenkins.version.

Copy link
Member Author

Choose a reason for hiding this comment

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

It cannot be reverted while also accomplishing the purpose of this PR, which is stated in the PR description:

Drop support for Java 8

The minimum Jenkins version cannot yet be updated because we do not know which version will drop support for Java 8, but the intention is that when we do we would update the minimum Jenkins version to that version.

</properties>
<repositories>
<repository>
Expand Down
2 changes: 1 addition & 1 deletion src/it/beta-fail/pom.xml
Expand Up @@ -14,7 +14,7 @@
<packaging>pom</packaging>
<properties>
<jenkins.version>2.277.4</jenkins.version>
<java.level>8</java.level>
<java.level>11</java.level>
<maven-hpi-plugin.disabledTestInjection>true</maven-hpi-plugin.disabledTestInjection>
</properties>
<repositories>
Expand Down
2 changes: 1 addition & 1 deletion src/it/beta-just-testing/pom.xml
Expand Up @@ -14,7 +14,7 @@
<packaging>pom</packaging>
<properties>
<jenkins.version>2.277.4</jenkins.version>
<java.level>8</java.level>
<java.level>11</java.level>
<maven-hpi-plugin.disabledTestInjection>true</maven-hpi-plugin.disabledTestInjection>
</properties>
<repositories>
Expand Down
2 changes: 1 addition & 1 deletion src/it/beta-pass/pom.xml
Expand Up @@ -14,7 +14,7 @@
<packaging>pom</packaging>
<properties>
<jenkins.version>2.277.4</jenkins.version>
<java.level>8</java.level>
<java.level>11</java.level>
<maven-hpi-plugin.disabledTestInjection>true</maven-hpi-plugin.disabledTestInjection>
</properties>
<repositories>
Expand Down
2 changes: 1 addition & 1 deletion src/it/incrementals-and-plugin-bom/pom.xml
Expand Up @@ -15,7 +15,7 @@
<revision>1.0</revision>
<changelist>-SNAPSHOT</changelist>
<jenkins.version>2.249</jenkins.version>
<java.level>8</java.level>
<java.level>11</java.level>
</properties>

<repositories>
Expand Down
2 changes: 1 addition & 1 deletion src/it/localizer/pom.xml
Expand Up @@ -14,7 +14,7 @@
<packaging>hpi</packaging>
<properties>
<jenkins.version>2.277.4</jenkins.version>
<java.level>8</java.level>
<java.level>11</java.level>
<maven-hpi-plugin.disabledTestInjection>true</maven-hpi-plugin.disabledTestInjection>
</properties>
<repositories>
Expand Down
2 changes: 1 addition & 1 deletion src/it/sample-plugin/pom.xml
Expand Up @@ -14,7 +14,7 @@
<packaging>hpi</packaging>
<properties>
<jenkins.version>2.249</jenkins.version>
<java.level>8</java.level>
<java.level>11</java.level>
</properties>
<repositories>
<repository>
Expand Down