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

Remove bundled Ant and Javadoc plugins #5338

Merged
merged 1 commit into from
Mar 12, 2021

Conversation

basil
Copy link
Member

@basil basil commented Mar 5, 2021

Previously: #4242 and #5102. Dealing with bundled plugins in Jenkins core is painful. The fewer of these we have, the better. Here we stop bundling the Ant and Javadoc plugins in the Jenkins WAR. As I explain below, we do not actually need to bundle these any longer, just like in #4242 and #5102.

Implementation notes

ClassicPluginStrategyTest#testDisabledDependencyClassLoader happened to be relying on the fact that Ant was a detached plugin in order to test JENKINS-18654. The test case was not specific to Ant and could work with any detached plugin, so I changed it to use JUnit instead. I stepped through it and the test case works the same way it did before. Since the change is buried in a .hpi file in a .zip file and isn't visible in GitHub, I am including it here:

diff -ruN old/META-INF/MANIFEST.MF new/META-INF/MANIFEST.MF
--- old/META-INF/MANIFEST.MF    2020-12-07 04:26:02.000000000 -0800
+++ new/META-INF/MANIFEST.MF    2021-03-05 12:20:48.000000000 -0800
@@ -11,6 +11,6 @@
 Url: https://hudson.dev.java.net/plugin/foo4
 Plugin-Version: 0.4
 Hudson-Version: 1.450
-Plugin-Dependencies: ant:1.0
+Plugin-Dependencies: junit:1.0
 Plugin-Developers: 

Testing Done

First, I verified that none of the other plugins we bundle in the WAR have a (non-implied) dependency on the Ant or Javadoc plugins. Only 20 plugins in the ecosystem explicitly depend on the Ant or Javadoc plugins, and none of them are bundled in the WAR. So we are good to go there.

Next, I did a realistic smoke test by starting up Jenkins with java -jar jenkins.war (after verifying the WAR no longer contained the Ant or Javadoc plugins in WEB-INF/detached-plugins) and running through the wizard. This installed Ant (which is a suggested plugin), but that was expected. The wizard completed successfully with no errors.

Finally I had to make sure this wouldn't break any plugins consuming the classes provided by the Ant and Javadoc plugins. The last core release still containing the Ant and Javadoc functionality was 1.430. Trawling through the graveyard I found 202 plugins whose required core was ≤ 1.430. Of those, 198 returned no results for these three grep(1) commands:

$ grep -ri hudson.tasks.ant .
$ grep -ri hudson.tasks._ant .
$ grep -ri hudson.tasks.javadoc .

I could not find the source code for these three plugins …

… but I checked the binaries with javap(1) and found no references to hudson.tasks.ant, hudson.tasks._ant, or hudson.tasks.javadoc there either.

The only positive result I got from my search was RAD Builder, a plugin with 87 installs whose required core is 1.375. RAD Builder uses hudson.tasks.Ant in hudson.plugins.radbuilder.RAD. On the off chance that one of these 87 users doesn't already have the Ant plugin installed (note that it is a suggested plugin in the wizard) they will need to install a recent release of the Ant plugin after this unbundling. I do not anticipate this being a major issue.

RAD Builder testing

I tested shutting Jenkins down after going through setup without installing anything, putting rad-builder.hpi in $JENKINS_HOME/plugins/, then starting Jenkins. Before this change, Ant and Javadoc get installedled; after, they don't, as expected, and RAD Builder fails to launch with:

2021-03-05 21:01:13.656+0000 [id=33]    WARNING h.ExtensionFinder$GuiceFinder$FaultTolerantScope$1#error: Failed to instantiate Key[type=hudson.plugins.radbuilder.RAD$DescriptorImpl, annotation=[none]]; skipping this component
java.lang.ClassNotFoundException: hudson.tasks._ant.AntConsoleAnnotator
        at jenkins.util.AntClassLoader.findClassInComponents(AntClassLoader.java:1392)
        at jenkins.util.AntClassLoader.findClass(AntClassLoader.java:1347)
        at jenkins.util.AntClassLoader.loadClass(AntClassLoader.java:1093)
        at java.lang.ClassLoader.loadClass(ClassLoader.java:351)

After I correct the problem by manually installing Ant, RAD Builder loads again.

Proposed changelog entries

  • Stop bundling the Ant and Javadoc plugins. Jenkins will no longer automatically install the Ant and Javadoc plugins on startup if a plugin depending on Jenkins (then Hudson) 1.430 or earlier is discovered. If you use such a plugin that also relies on the functionality provided by the Ant or Javadoc plugin (e.g., the RAD Builder plugin) and manage plugins outside Jenkins' plugin manager, you will now need to ensure that a recent release of the Ant or Javadoc plugin is installed. Jenkins will attempt to load such plugins but may fail at any time during startup or afterwards with ClassNotFoundException or similar.

Proposed upgrade guidelines

See above.

Submitter checklist

  • (If applicable) Jira issue is well described
  • Changelog entries and upgrade guidelines are appropriate for the audience affected by the change (users or developer, depending on the change). Examples
    • Fill-in the Proposed changelog entries section only if there are breaking changes or other changes which may require extra steps from users during the upgrade
  • 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

@daniel-beck

Maintainer checklist

Before the changes are marked as ready-for-merge:

  • There are at least 2 approvals for the pull request and no outstanding requests for change
  • Conversations in the pull request are over OR it is explicit that a reviewer does not block the change
  • Changelog entries in the PR title and/or Proposed changelog entries are correct
  • Proper changelog labels are set so that the changelog can be generated automatically
  • If the change needs additional upgrade steps from users, upgrade-guide-needed label is set and there is a Proposed upgrade guidelines section in the PR title. (example)
  • If it would make sense to backport the change to LTS, a Jira issue must exist, be a Bug or Improvement, and be labeled as lts-candidate to be considered (see query).

@daniel-beck daniel-beck self-requested a review March 5, 2021 23:13
@basil
Copy link
Member Author

basil commented Mar 5, 2021

FYI once this change is merged and released, I plan to unbundle the External Monitor Job Type, LDAP, and PAM Authentication plugins next. I've already verified that it is safe to do so.

@timja timja added the rfe For changelog: Minor enhancement. use `major-rfe` for changes to be highlighted label Mar 6, 2021
@oleg-nenashev oleg-nenashev added the upgrade-guide-needed This changes might be breaking in rare circumstances, an entry in the LTS upgrade guide is needed label Mar 8, 2021
Copy link
Member

@oleg-nenashev oleg-nenashev left a comment

Choose a reason for hiding this comment

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

+1 from me

@oleg-nenashev oleg-nenashev added the ready-for-merge The PR is ready to go, and it will be merged soon if there is no negative feedback label Mar 9, 2021
@oleg-nenashev
Copy link
Member

We may merge it in 24 hours if there is no negative feedback. Please see the merge process documentation for more information about the merge process

@oleg-nenashev oleg-nenashev requested a review from a team March 12, 2021 13:19
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.

Thanks for the diligent testing!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-for-merge The PR is ready to go, and it will be merged soon if there is no negative feedback rfe For changelog: Minor enhancement. use `major-rfe` for changes to be highlighted upgrade-guide-needed This changes might be breaking in rare circumstances, an entry in the LTS upgrade guide is needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants