[JENKINS-52282] Add isJavaWebStartSupported to JNLPLauncher #3766

MarkEWaite · 2018-11-17T17:08:04Z

See JENKINS-52282, Java 9 and later do not support Java Web Start.

No automated test because I don't have time to write a test for the existence of a button in the user interface.

Proposed changelog entries

Entry 1: Don't show the "Java Web Start" button if master is running on Java 11
...

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
For dependency updates: links to external changelogs and, if possible, full diffs

Desired reviewers

@jenkinsci/java11-support

oleg-nenashev

Thanks a lot @MarkEWaite! Some minor patches are needed IMHO, but I think we can land it in the weekly release on Nov 26

core/src/main/java/hudson/slaves/JNLPLauncher.java

oleg-nenashev · 2018-11-17T18:01:27Z

core/src/main/resources/hudson/slaves/JNLPLauncher/main.jelly

+                ${%Launch agent from browser}
+              </p>
+            </li>
+          </j:if>


else add some warning to the WebUI? Otherwise we may end up with users who report the issue after reading the docs and failing to fond the button

alecharp · 2018-11-17T18:02:11Z

core/src/main/resources/hudson/slaves/JNLPLauncher/main.jelly

-              ${%Launch agent from browser}
-            </p>
-          </li>
+          <j:if test="${it.launcher.javaWebStartSupported}">


I know it's jelly, but could we incorporate a link to a resource (wiki, blog, etc) to say why the web starter is not available. So having a <j:choose> rather than a <j:if> so we can have an else?

I'll need help from someone else to make those jelly changes. I'm not skilled with jelly and need to spend my weekends and nights working on the git plugin and git client plugin.

I think it's

<j:choose> <j:when test="${it.launcher.javaWebStartSupported}"> [...] </j:when> <j:otherwise> <div>There is no Web starter for the JVM version used to run Jenkins</div> </j:otherwise> </j:choose>

Same as https://github.com/jenkinsci/jenkins/pull/3766/files#r234416298 .
I believe we need to have a Wiki or jenkins.io page to list known Jenkins core incompatibilities.
There is https://jenkins.io/doc/administration/requirements/java/ , but probably it worth having a separate page.

Let's just add a redirect page on Jenkins.io? This way we can modify the final destination later?

Like https://jenkins.io/redirect/java-web-start ? Or a more general https://jenkins.io/redirect/java-11 ?

Let's do https://jenkins.io/redirect/java11-java-web-start or so. It can be finally linking to the anchor on a page, but it can just link to the JIRA ticket for now

I agree, +1 on redirect and it can be changed to wiki page in the future. @MarkEWaite do you want me to test / commit the jelly code I provided?

@alecharp that would be great.

I added the conditional text and redirect link as suggested.

alecharp · 2018-11-17T18:04:45Z

core/src/main/java/hudson/slaves/JNLPLauncher.java

+     * @since FIXME
+     */
+    public boolean isJavaWebStartSupported() {
+        return System.getProperty("java.version", "1.8").startsWith("1.8");


I was looking for https://docs.oracle.com/javase/10/docs/api/java/lang/Runtime.Version.html but it's only available since Java 9, sadly. So I guess this startWith is really all we can do here..

That's what I saw as well. I would have preferred a better check, but...

This is a hack we usually do. Probably we need to move these checks to a utility class

batmat · 2018-11-23T10:32:30Z

core/src/main/resources/hudson/slaves/JNLPLauncher/main.jelly

-              ${%Launch agent from browser}
-            </p>
-          </li>
+          <j:if test="${it.launcher.javaWebStartSupported}">


Let's just add a redirect page on Jenkins.io? This way we can modify the final destination later?

Like https://jenkins.io/redirect/java-web-start ? Or a more general https://jenkins.io/redirect/java-11 ?

Seems cleaner to use JNLPLauncher than to use Functions. Narrows the scope of the method so that callers know it is specific to JNLPLauncher.

MarkEWaite · 2018-11-23T21:05:31Z

@alecharp I placed your proposed changes (with a phrasing change from me) in as f33e5f8 . If that's not workable for you or if I've made a mistake, feel free to remove that commit or to do whatever else is needed to include the Java Web Start conditional in the next weekly build of Jenkins.

oleg-nenashev

better UI would be nice, but IMO it can be merged once the redirect link is actually created

Oracle announced in March 2018 that it will not include Java Web Start in Java SE 11. Jenkins provides a "Java Web Start" button to simplify launching of clients with Java Web Start. That simplified start technique is often used for Windows agents. Jenkins on Java 11 can't use that button because the underlying Java implementation does not support it. The communication protocol used by agent.jar continues to be fully supported and operational with Java 11. Only the Java Web Start button from the web page is not supported in Java 11. See [JENKINS-52282] and jenkinsci/jenkins#3766 for more details.

MarkEWaite · 2018-11-23T22:28:42Z

See the jenkins.io pull request for the proposed redirect to Wikipedia. The Wikipedia destination was chosen because it states the deprecation very simply and includes a link to the authoritative PDF file.

When we add documentation on agents to jenkins.io, we can include Java Web Start and its deprecation in that section. Alternately, it could be placed in a jenkins.io section dedicated to Java 11.

oleg-nenashev · 2018-11-24T20:22:10Z

Upstream redirect was integrated, merging

daniel-beck · 2018-11-26T12:36:01Z

core/src/main/java/hudson/slaves/JNLPLauncher.java

+     * This flag is checked in {@code config.jelly} before displaying the
+     * Java Web Start button.
+     * @return {@code true} if Java Web Start button should be displayed.
+     * @since FIXME


No need to @since something that's @Restricted

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

oleg-nenashev requested review from oleg-nenashev, batmat and alecharp November 17, 2018 17:57

oleg-nenashev reviewed Nov 17, 2018

View reviewed changes

alecharp suggested changes Nov 17, 2018

View reviewed changes

MarkEWaite force-pushed the hide-java-web-start-on-jdk-11 branch from 0351a49 to 867ccd5 Compare November 19, 2018 14:20

batmat approved these changes Nov 23, 2018

View reviewed changes

MarkEWaite added 2 commits November 23, 2018 14:03

[JENKINS-52282] Add isJavaWebStartSupported to JNLPLauncher

cae208a

Seems cleaner to use JNLPLauncher than to use Functions. Narrows the scope of the method so that callers know it is specific to JNLPLauncher.

Include hyperlink to jenkins.io Java Web Start redirect page

f33e5f8

MarkEWaite force-pushed the hide-java-web-start-on-jdk-11 branch from 867ccd5 to f33e5f8 Compare November 23, 2018 21:03

batmat approved these changes Nov 23, 2018

View reviewed changes

batmat requested a review from alecharp November 23, 2018 21:10

oleg-nenashev approved these changes Nov 23, 2018

View reviewed changes

oleg-nenashev added the on-hold This pull request depends on another event/release, and it cannot be merged right now label Nov 23, 2018

MarkEWaite mentioned this pull request Nov 23, 2018

Redirect to Wikipedia for Java 11 Web Start deprecation jenkins-infra/jenkins.io#1903

Merged

oleg-nenashev removed the on-hold This pull request depends on another event/release, and it cannot be merged right now label Nov 24, 2018

oleg-nenashev merged commit 217b293 into jenkinsci:master Nov 24, 2018

MarkEWaite deleted the hide-java-web-start-on-jdk-11 branch November 24, 2018 21:35

daniel-beck reviewed Nov 26, 2018

View reviewed changes

daniel-beck mentioned this pull request May 3, 2020

Update at-since up to May 3, 2020 #4705

Merged

jglick mentioned this pull request Feb 4, 2022

[JENKINS-67000] Restore setting of null security manager jenkinsci/remoting#507

Merged

jglick mentioned this pull request May 6, 2022

[JENKINS-51820] Removing Java Web Start support #6543

Merged

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[JENKINS-52282] Add isJavaWebStartSupported to JNLPLauncher #3766

[JENKINS-52282] Add isJavaWebStartSupported to JNLPLauncher #3766

MarkEWaite commented Nov 17, 2018 •

edited

Loading

oleg-nenashev left a comment

oleg-nenashev Nov 17, 2018

alecharp Nov 17, 2018

MarkEWaite Nov 17, 2018

alecharp Nov 17, 2018

oleg-nenashev Nov 21, 2018

batmat Nov 23, 2018

oleg-nenashev Nov 23, 2018

alecharp Nov 23, 2018

MarkEWaite Nov 23, 2018

MarkEWaite Nov 23, 2018

alecharp Nov 17, 2018

MarkEWaite Nov 17, 2018

oleg-nenashev Nov 21, 2018

batmat Nov 23, 2018

MarkEWaite commented Nov 23, 2018

oleg-nenashev left a comment

MarkEWaite commented Nov 23, 2018 •

edited

Loading

oleg-nenashev commented Nov 24, 2018

daniel-beck Nov 26, 2018

[JENKINS-52282] Add isJavaWebStartSupported to JNLPLauncher #3766

[JENKINS-52282] Add isJavaWebStartSupported to JNLPLauncher #3766

Conversation

MarkEWaite commented Nov 17, 2018 • edited Loading

Proposed changelog entries

Submitter checklist

Desired reviewers

oleg-nenashev left a comment

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

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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MarkEWaite commented Nov 23, 2018

oleg-nenashev left a comment

Choose a reason for hiding this comment

MarkEWaite commented Nov 23, 2018 • edited Loading

oleg-nenashev commented Nov 24, 2018

Choose a reason for hiding this comment

MarkEWaite commented Nov 17, 2018 •

edited

Loading

MarkEWaite commented Nov 23, 2018 •

edited

Loading