Upgrade Guava from 11.0.1 to latest

[dbreheret]

Proposed changelog entries

• Upgrade guava to v30.1-jre

Proposed upgrade guidelines

N/A

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

@mention

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).

[res0nance]

Changing this is no trivial matter as multiple plugins use guava's libraries and there are likely API changes that will break existing plugins

[jvz]

A wide-reaching change like this will require changes in plugins or some sort of compatibility code. It would be useful to coordinate changes through a JEP.

[dbreheret]

The main change here is in the file PermalinkProjectAction.java, to have all the tests green.
The other changes are for javadoc that was failing.

[res0nance]

The main change here is in the file PermalinkProjectAction.java, to have all the tests green.
The other changes are for javadoc that was failing.

Its not the effect to this single repository, its the effect on the entire plugin ecosystem. Plugins may use a feature of guava that had API breaking changes. Due to this possibility its not as simple as ensuring this single repository compiles.

[jvz]

See also https://www.jenkins.io/blog/2020/11/10/spring-xstream/ for examples of two very recently upgraded libraries in Jenkins Core that were done for similar reasons. If upgrading Guava is that important, then this blog post should help get you pointed in the right direction on how it can be done.

[dbreheret]

Thank you Matt Sicker (aka jvz)
I have created a JEP, based on the xstream one you have pointed out:
jenkinsci/jep#336

[jglick]

Please revert any changes not directly related to Guava.

This will need to stay in draft for a long time, until there is some real plan for making plugins compatible. This is a major undertaking. Time permitting, CloudBees may be able to help by offering a list of known test failures (PCT/ATH) across a selected set of plugins.

[jglick]

Please keep the diff minimal.

[dbreheret]

Some javadoc changes are needed otherwise the build fail.

[daniel-beck]

The Javadoc fixes are nice but have no reason for being in the same PR as Guava changes. Could you separate these out so we can apply them independently?

[oleg-nenashev]

Thanks to @dbreheret for starting this pull request and the discussion! It is an important topic we need to look at as a community.

Indeed there is no easy way to update Guava, same as for other dependencies including the recent Acegi Security and XStream stories. Nevertheless, old Guava version remains a part of our technical debt that needs to be addressed. We already have many plugins depending on new Guava versions. It is a matter of time until we get into a major binary conflict.

It is not clear whether the community has bandwidth to address the plugin regressions right now. We already have 3 major changes requiring attention and fixes in plugins (tables to divs, XStream, Acegi). Core maintainers are heavily involved in helping to stabilize the ecosystem, and adding more stories could exceed the bandwidth. We would need more contributors to commit their time to maintenance.

Let's keep discussing it in the JEP and mailing lists.

[jvz]

Indeed, I agree that it'd be useful to upgrade Guava (even though I don't use it, I know many people do), though it likely won't be able to merge into a release for at least a couple months or longer while the rest of the plug-in system is checked for any potential compatibility issues.

[oleg-nenashev]

What if we shade Guava and start using the new version in the core only? While we shade it, we could probably restrict it so that it does not become public API. Example of Guava shading: https://github.com/datastax/java-driver-shaded-guava/blob/master/pom.xml

…

On Sun, Nov 22, 2020, 18:11 Matt Sicker ***@***.***> wrote: Indeed, I agree that it'd be useful to upgrade Guava (even though I don't use it, I know many people do), though it likely won't be able to merge into a release for at least a couple months or longer while the rest of the plug-in system is checked for any potential compatibility issues. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#5059 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAW4RIBMPYOKKQTZSKPFUZTSRFA2NANCNFSM4TYF7ZWQ> .

[jglick]

What if we shade Guava and start using the new version in the core only?

We could, but what does this buy us? The technical debt of Guava 11 in the classpath would remain, compounded by the complexity of bundling a library which is not even used.

What we could do is include a shaded new Guava in core (or simply rewrite the uses of Guava in core to use other idioms); then split Guava 11 to a detached plugin and deprecate it. Those plugins which currently refer to Guava types but have no particular reason to need one version or another would remain working as before (I hope), whereas those which actually wish to use newer Guava (e.g., artifact-manager-s3) could simply bump their core dep, decline to add a dep on the split plugin, and bundle whatever newer version they like. We could also create a new wrapper plugin for new versions of Guava if we believe the Guava team has been doing a better job of retaining backward compatibility lately than they evidently did in the past. At our leisure we would go through all plugins using Guava and bump their core dep, at which point we could remove the deprecated 11 plugin from the detached list.

[oleg-nenashev]

What we could do is include a shaded new Guava in core (or simply rewrite the uses of Guava in core to use other idioms); then split Guava 11 to a detached plugin and deprecate it.

Yes. If we have a shaded version in the core we can detach guava 11 and introduce API plugins for newer versions. It will take a while to get rid of the detached plugin, but it is doable.

Also note that some Guava classes are in the JEP-200 serialization permit list: https://github.com/jenkinsci/jenkins/blob/master/core/src/main/resources/jenkins/security/whitelisted-classes.txt . It means that they might be serialized to the disk or to agents in some of the plugins (and the core?). It might require additional migration logic

[jglick]

JEP-200 issues should not be a problem—if we shade Guava, we can simply add the shaded variants, if they are even used at all in core.

[basil]

What we could do

If we were to include a shaded new Guava in core and split Guava 11 to a detached plugin, how would you propose preserving compatibility with existing public APIs that reference Guava types? We have three such APIs:

1. hudson.util.RunList#filter(com.google.common.base.Predicate), consumed by com.smartcodeltd.jenkinsci.plugins.buildmonitor.viewmodel.JobView
2. jenkins.model.PeepholePermalink (which implements com.google.common.base.Predicate), consumed by io.jenkins.plugins.build_symlink.RunListenerImpl, org.jvnet.hudson.plugins.m2release.LastReleasePermalink, and com.itemis.jenkins.plugins.unleash.permalinks.LastSuccessfulReleasePermalink) (and, apparently, cloudbees-long-running-build)
3. jenkins.util.InterceptingExecutorService, which extends com.google.common.util.concurrent.ForwardingExecutorService (consumed by org.jenkinsci.plugins.workflow.cps.CpsVmExecutorService)

[basil]

Another concern with this approach: Jenkins core depends on Stapler and Guice, both of which depend on Guava. Yes, shading the Guava classes in Jenkins core would keep the Guava classes accessible to Jenkins core classes (albeit under relocated package names) and hide them from plugins (which would only have access to Guava classes via a Guava API plugin). But shading the Guava classes in Jenkins core would also hide the Guava classes from Stapler and Guice, direct dependencies of Jenkins core which were compiled against the non-relocated package names.

[jglick]

how would you propose preserving compatibility with existing public APIs that reference Guava types?

These APIs would need to be modified, and plugins adapted in advance.

Stapler is likely not a big problem; likely we could just remove the Guava dep. Guice could however be a problem. I would love to remove the Guice dep (it adds very little value to the extension loader system and introduces a lot of complexity) but that may be a bigger project.

It is probably simplest to just update the dep, as here, after preparing all plugins which would be broken. As a wrote above, bundling an older version of Guava does not really remove the technical debt, just shifts it around.

[basil]

Guice could however be a problem.

What if we shade Guava and Guice but only relocate the packages for Guava? This would hide our dependency on Guava but keep our dependency on Guice visible in the public API (to preserve compatibility with existing public APIs with Guice types). The primary purpose of this would be to allow for the shading and package relocation of Guava. The shaded version of Guice would be rewritten to reference the relocated Guava packages.

[olamy]

Thanks a lot for the pr. BTW we are now guava 30.1-jre :) I will fix description and title.

[dbreheret]

@jglick, I had to make some changes bom/pom.xml to force the errorprone version to 2.5.1, the version coming from guava 30.1.1-jre.
I have tried with version 2.4.0, but guava does not like it.

[timja]

Accidentally closed, continued in #5646