[JENKINS-55048] Add support for plugin Java requirement metadata #3016

daniel-beck · 2017-09-11T00:47:01Z

Downstream of jenkinsci/maven-hpi-plugin#75 and jenkins-infra/update-center2#164.

Future enhancement might be to send the Java version with the request (or consider it already sent by the user agent header) and handle that on the server side, similar to the tiered update sites. But for now, a big red (newly bold) warning should at least get us started.

Here is how it renders:

(I used http://updates.jenkins.io/temporary-experimental-java11/ for testing.
Pipeline: Supporting APIs, aka workflow-support is the only one requiring Java 11+.
The others are red because they depend on it directly or transitively.
I used the following system groovy quick&dirty script to check these things:

def showDeps(plugin, level) {
  println ("  ".multiply(level)+"${plugin.name}:${plugin.version}")
  
  plugin.getNeededDependencies().each { dep ->
    def dep1 = dep
    showDeps(dep1, level + 1);
  }
}
showDeps(Jenkins.instance.updateCenter.getPlugin('throttle-concurrents'), 0)

).

Proposed changelog entries

Add support for plugins declaring a minimum Java version in manifest and update center, showing warnings and refusing to load plugins with unsatisfied dependencies.

Submitter checklist

JIRA issue is well described
Changelog entry appropriate for the audience affected by the change (users or developer, depending on the change). Examples
* Use the Internal: prefix if the change has no user-visible impact (API, test frameworks, etc.)
Appropriate autotests or explanation to why this change has no tests

Desired reviewers

@jenkinsci/code-reviewers

oleg-nenashev

Generally the PR looks good, added few comments.

I would also consider adding Maximum-Java-Version as a part of this story while you are around. E.g. I barely believe that Maven Integration Plugin can be made Java9-compatible easily (CC @aheritier)

core/src/main/java/hudson/PluginWrapper.java

core/src/main/java/hudson/model/UpdateSite.java

oleg-nenashev · 2017-09-11T18:19:55Z

core/src/main/resources/hudson/PluginManager/_table.css

@@ -8,3 +8,6 @@
    margin: 1em;
    text-align: right;
 }
+.compatWarning, .securityWarning {
+    font-weight: bold;
+}


jglick

Misuse of java.version, otherwise looks good.

core/src/main/java/hudson/PluginWrapper.java

core/src/main/java/hudson/model/UpdateSite.java

daniel-beck · 2018-05-14T13:18:29Z

Rebased to current master with no impl changes so far.

I'm not sure what the log message is supposed to be for, but other checks also log, so apply the same behavior for Java dependency.

…with-more-recent-jdk

Given this field is present in maven-hpi-plugin, update-center, etc. I think it does not hurt to keep the 'Minimum-Java-Version' name across various usages to help make it clear.

This way at least we know the backend methods should not be broken without noticing.

"Misuse of java.version, otherwise looks good." main reason addressed. Need another review.

batmat · 2019-01-07T14:43:17Z

test/src/test/resources/plugins/minJavaVersion-update-center.json

+            "title": "Dummy",
+            "url": "http://nowhere.net/dummy.hpi",
+            "version": "1.0",
+            "minimumJavaVersion": "25"


Hopefully, we shouldn't get too quickly to this as a "future" baseline we're not running in during tests (?). Or maybe I should even have gone bigger. (maybe I should see to mock the runtime Java version retrieval to isolate the test from such issue in the future)

batmat · 2019-01-07T14:50:50Z

@jenkinsci/java11-support please review this pull-request.

batmat · 2019-01-07T15:01:09Z

To test this, a simple way is the following:

# Build and run a Jenkins with this patch
docker run -ti -e ID=3016 -p 8080:8080 batmat/jenkins-pr-tester

Then open a browser on 8080, and change the update center URL to http://updates.jenkins.io/temporary-experimental-java11/update-center.json to check how it behaves.
In this UC, the workflow-support plugin in 3.0-java11-alpha-11 requires Java 11.

core/src/main/java/hudson/model/UpdateSite.java

MRamonLeon

Minor comments, but anyway it LGTM

core/src/main/java/hudson/model/UpdateSite.java

core/src/main/java/hudson/PluginWrapper.java

core/src/main/java/hudson/model/UpdateSite.java

core/src/main/java/hudson/PluginWrapper.java

core/src/main/java/hudson/model/UpdateSite.java

core/src/main/resources/hudson/PluginManager/table.properties

test/src/test/java/hudson/model/UpdateSiteTest.java

test/src/test/resources/plugins/minJavaVersion-update-center.json

@jglick

as proposed by @jglick

…s coming from the plugin's metadata

@jglick

using the proposal from @jglick in https://github.com/jenkinsci/jenkins/pull/3016/files#r246146054

batmat · 2019-01-09T17:13:53Z

@jglick I believe I addressed your feedback. Can you please take another quick look?

Thanks!

batmat · 2019-01-09T18:10:37Z

BTW, anyone willing to have another look is welcome.
The additions I made since approvals are very trivial, often in javadoc only but a few confirmed approvals would be better I suppose.

Thanks!

jglick · 2019-01-09T19:31:42Z

core/src/main/java/jenkins/util/java/JavaUtils.java

        return !javaVersion.startsWith("1.");
    }

+    /**
+     * Returns the JVM's current version as a {@link VersionNumber} instance.


Best to document this with examples. IIUC these would be

1.8

9

10

11

?

Yes. Basically JEP-223

Great, but please link to this so that people do not have to discover it by code inspection and independent research.

@jglick fix proposed in #3842

oleg-nenashev · 2019-01-14T12:34:50Z

It causes https://issues.jenkins-ci.org/browse/JENKINS-55562 , because there is no version normalization code in this pull request. Both 8 and 1.8 are valid version numbers, because pre-JEP-223 format stays valid for older versions

jglick · 2019-01-14T15:26:22Z

there is no version normalization code in this pull request

Nor should there be IMO—normalization should be performed on input, at build time.

Both 8 and 1.8 are valid version numbers, because pre-JEP-223 format stays valid for older versions

My reading of JEP-223 is that Java version numbers in the range [1.9,9) are invalid for all time.

daniel-beck added needs-more-reviews Complex change, which would benefit from more eyes on-hold This pull request depends on another event/release, and it cannot be merged right now labels Sep 11, 2017

This was referenced Sep 11, 2017

[JENKINS-20679] Add support for Minimum-Java-Version in manifest jenkins-infra/update-center2#164

Closed

[JENKINS-20679] Try to declare the minimum Java version in the manifest jenkinsci/maven-hpi-plugin#75

Merged

oleg-nenashev reviewed Sep 11, 2017

View reviewed changes

jglick previously requested changes Sep 12, 2017

View reviewed changes

jglick mentioned this pull request Oct 25, 2017

Drop support for 1.x by default in plugin-pom jenkinsci/plugin-pom#83

Merged

daniel-beck force-pushed the JENKINS-20679 branch from e5d4199 to cf723f6 Compare May 14, 2018 13:17

daniel-beck added 5 commits September 20, 2018 11:38

[JENKINS-20679] Add support for plugin Java requirement metadata

3fa7427

Rename 'minimumJavaVersion' to 'requiredJava'

e65ceca

Adapt tests to changed localizable strings

d1c972a

Update Javadocs and comments

725fb30

Add Java version check to setup wizard and installNecessaryPlugins

f55864f

I'm not sure what the log message is supposed to be for, but other checks also log, so apply the same behavior for Java dependency.

daniel-beck force-pushed the JENKINS-20679 branch from cf723f6 to f55864f Compare September 20, 2018 18:39

daniel-beck added 2 commits September 20, 2018 13:44

Use java.specification.version

5cdbf39

Adapt to newer core internals

e364a84

oleg-nenashev added the java11 Java 11 support in Jenkins label Nov 22, 2018

batmat self-assigned this Jan 3, 2019

batmat added 4 commits January 7, 2019 11:11

Merge branch 'master' into JENKINS-20679-filter-out-plugins-compiled-…

27c49bf

…with-more-recent-jdk

Rename to minimumJavaVersion to keep the name across tooling

29de9aa

Given this field is present in maven-hpi-plugin, update-center, etc. I think it does not hurt to keep the 'Minimum-Java-Version' name across various usages to help make it clear.

Add warning if minimumJavaVersion is not parseable

89d20ef

Add test covering methods for "is*ForNewerJava()"

246adab

This way at least we know the backend methods should not be broken without noticing.

batmat changed the title ~~[JENKINS-20679] Add support for plugin Java requirement metadata~~ [JENKINS-55048] Add support for plugin Java requirement metadata Jan 7, 2019

batmat removed the on-hold This pull request depends on another event/release, and it cannot be merged right now label Jan 7, 2019

Log warning in case of unparseable minimum Java version

8a666ce

batmat reviewed Jan 7, 2019

View reviewed changes

batmat requested a review from MRamonLeon January 8, 2019 13:29

alecharp reviewed Jan 8, 2019

View reviewed changes

core/src/main/java/hudson/model/UpdateSite.java Outdated Show resolved Hide resolved

MRamonLeon approved these changes Jan 8, 2019

View reviewed changes

core/src/main/java/hudson/model/UpdateSite.java Outdated Show resolved Hide resolved

core/src/main/java/hudson/model/UpdateSite.java Outdated Show resolved Hide resolved

Remove json comment

6fe6615

MarkEWaite approved these changes Jan 8, 2019

View reviewed changes

jglick approved these changes Jan 8, 2019

View reviewed changes

fcojfernandez approved these changes Jan 9, 2019

View reviewed changes

batmat added 10 commits January 9, 2019 17:37

Add missing space in javadoc

c61031d

Remove duplicated Javadoc

791253d

as proposed by @jglick

Add reference to related JIRA entry

ee04f92

Provide more information about where/how the Minimum-Java-Version i…

282e42b

…s coming from the plugin's metadata

Clarify what null means

94c114a

Remove apostrophes to avoid issues in string un/expansion

36d32ed

Remove spurious 'it' (and duplicate doc)

3b07b35

Make the phrasing less awkward

3ced8bb

using the proposal from @jglick in https://github.com/jenkinsci/jenkins/pull/3016/files#r246146054

Remove also param given minimumJavaVersion is a class member already

2225631

Introduce method to fetch java.specification.version

1afa724

batmat requested a review from jglick January 9, 2019 17:13

jglick approved these changes Jan 9, 2019

View reviewed changes

batmat added the ready-for-merge The PR is ready to go, and it will be merged soon if there is no negative feedback label Jan 9, 2019

batmat merged commit eb39d6b into jenkinsci:master Jan 10, 2019

batmat mentioned this pull request Jan 11, 2019

Give more details about the expected Java version format #3842

Merged

oleg-nenashev mentioned this pull request Jan 14, 2019

[JENKINS-55562] - Apply workaround in the version number jenkinsci/performance-signature-dynatrace-plugin#5

Merged

sghill mentioned this pull request Mar 26, 2019

Add Minimum-Java-Version to Manifest jenkinsci/gradle-jpi-plugin#111

Merged

jglick mentioned this pull request Mar 30, 2022

Deprecate java.level jenkinsci/plugin-pom#522

Merged

6 tasks

basil mentioned this pull request May 7, 2022

Revert JENKINS-20679 (support for plugin Java requirement metadata) #6549

Merged

11 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[JENKINS-55048] Add support for plugin Java requirement metadata #3016

[JENKINS-55048] Add support for plugin Java requirement metadata #3016

daniel-beck commented Sep 11, 2017 •

edited by batmat

Loading

oleg-nenashev left a comment

oleg-nenashev Sep 11, 2017

jglick left a comment

daniel-beck commented May 14, 2018

batmat Jan 7, 2019 •

edited

Loading

batmat commented Jan 7, 2019

batmat commented Jan 7, 2019 •

edited

Loading

MRamonLeon left a comment

batmat commented Jan 9, 2019

batmat commented Jan 9, 2019

jglick Jan 9, 2019

batmat Jan 9, 2019

jglick Jan 10, 2019

batmat Jan 11, 2019

oleg-nenashev commented Jan 14, 2019

jglick commented Jan 14, 2019

[JENKINS-55048] Add support for plugin Java requirement metadata #3016

[JENKINS-55048] Add support for plugin Java requirement metadata #3016

Conversation

daniel-beck commented Sep 11, 2017 • edited by batmat Loading

Proposed changelog entries

Submitter checklist

Desired reviewers

oleg-nenashev left a comment

Choose a reason for hiding this comment

oleg-nenashev Sep 11, 2017

Choose a reason for hiding this comment

jglick left a comment

Choose a reason for hiding this comment

daniel-beck commented May 14, 2018

batmat Jan 7, 2019 • edited Loading

Choose a reason for hiding this comment

batmat commented Jan 7, 2019

batmat commented Jan 7, 2019 • edited Loading

MRamonLeon left a comment

Choose a reason for hiding this comment

batmat commented Jan 9, 2019

batmat commented Jan 9, 2019

jglick Jan 9, 2019

Choose a reason for hiding this comment

batmat Jan 9, 2019

Choose a reason for hiding this comment

jglick Jan 10, 2019

Choose a reason for hiding this comment

batmat Jan 11, 2019

Choose a reason for hiding this comment

oleg-nenashev commented Jan 14, 2019

jglick commented Jan 14, 2019

daniel-beck commented Sep 11, 2017 •

edited by batmat

Loading

batmat Jan 7, 2019 •

edited

Loading

batmat commented Jan 7, 2019 •

edited

Loading