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

Generate automatically a marker file for a maven profile depending on java level to use #578

Merged

Conversation

Vlatombe
Copy link
Member

@Vlatombe Vlatombe commented Feb 7, 2024

(This idea came up as a discussion during the Jenkins contributor summit, based on the IDE problems experienced during the transition between Java 8 and 11 support)

This effectively reverts #403, reinstating #323, as well as adding a new mechanism for better IDE support.

The logic of setting maven.compiler.release dynamically works fine for the Maven CLI. However IDE support (at least IntelliJ) doesn't work well with this and happily ignores what the plugin set dynamically. Indeed, it has its own logic to parse the pom and apply profiles, but doesn't call any maven lifecycle doing so.

Instead, it picks up the default value set through the plugin pom that defaults to the minimum java version supported (currently 11), instead of the version inferred from Java bytecode version obtained from Jenkins core.

Generating markerfiles allows to enable maven profiles that will set the java release to the correct version when loaded in IDE, as long as they are able to read activated profiles.

Testing done

Subject: [PATCH] Add java level profiles
---
Index: pom.xml
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/pom.xml b/pom.xml
--- a/pom.xml	(revision 6949f8af159d30e2722fcf773b397bc870e47359)
+++ b/pom.xml	(revision 28d462c55325e673d6f0c1710ac703300abae7b1)
@@ -1453,5 +1453,27 @@
         <yarn.lint.skip>true</yarn.lint.skip>
       </properties>
     </profile>
+    <profile>
+      <id>java-level-17</id>
+      <activation>
+        <file>
+          <exists>target/java-level/17</exists>
+        </file>
+      </activation>
+      <properties>
+        <maven.compiler.release>17</maven.compiler.release>
+      </properties>
+    </profile>
+    <profile>
+      <id>java-level-21</id>
+      <activation>
+        <file>
+          <exists>target/java-level/21</exists>
+        </file>
+      </activation>
+      <properties>
+        <maven.compiler.release>21</maven.compiler.release>
+      </properties>
+    </profile>
   </profiles>
 </project>
  • built a jenkins snapshot with -Dmaven.compiler.release=17

Then in a plugin (I used parallel-test-executor as an example)

  • Reference maven-hpi-plugin snapshot
  • Reference plugin-pom snapshot
  • Imported the project in IntelliJ (2024.1 EAP, they seem to have reworked the maven projects import, so it could matter)
  • Check module settings -> Language level is set to 11 (expected)
  • Update jenkins.version property to the snapshot built above
  • Call mvn validate
  • In IntelliJ, use Maven > Reload project
  • Check module settings -> See the language level changing to 17

Submitter checklist

Edit tasklist title
Beta Give feedback Tasklist Submitter checklist, more options

Delete tasklist

Delete tasklist block?
Are you sure? All relationships in this tasklist will be removed.
  1. Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
    Options
  2. Ensure that the pull request title represents the desired changelog entry
    Options
  3. Please describe what you did
    Options
  4. Link to relevant issues in GitHub or Jira
    Options
  5. Link to relevant pull requests, esp. upstream and downstream changes
    Options
  6. Ensure you have provided tests - that demonstrates feature works or fixes the issue
    Options

…n java level to use

The current logic of setting `maven.compiler.release` dynamically works fine for the Maven CLI. However IDE support (at least IntelliJ) doesn't work well with this and happily ignores what the plugin set dynamically.

Instead, it picks up the default value set through the plugin pom that defaults to the minimum java version supported (currently [11](https://github.com/jenkinsci/plugin-pom/blob/6949f8af159d30e2722fcf773b397bc870e47359/pom.xml#L61)), instead of the version inferred from Java bytecode version obtained from Jenkins core.

Generating markerfiles allow to enable maven profiles that will set the java release to the correct version when loaded in IDE.
This reverts commit 13db53e.
@Vlatombe Vlatombe marked this pull request as ready for review February 7, 2024 08:43
@Vlatombe Vlatombe requested a review from basil February 7, 2024 08:43
@Vlatombe Vlatombe changed the title Generates automatically a marker file for a maven profile depending on java level to use Generate automatically a marker file for a maven profile depending on java level to use Feb 7, 2024
@Vlatombe Vlatombe requested a review from jglick February 8, 2024 09:12
Copy link
Member

@jglick jglick left a comment

Choose a reason for hiding this comment

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

I am not an IDEA user so I have no strong opinion. Has someone at least checked whether the IDEA bug is fixable in some reasonable way?

import org.apache.maven.plugins.annotations.Mojo;

/**
* Configure Maven for the desired version of Java.
Copy link
Member

Choose a reason for hiding this comment

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

Better to update to reflect its new more limited purpose.

@MarkEWaite MarkEWaite merged commit d45cb78 into jenkinsci:master Feb 9, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants