Remove Stapler's Guava dependency

[basil]

Motivation

This is a step along the way to possibly being able to shade the Guava dependency in Jenkins core as described in jenkinsci/jenkins#5059 (comment). I'm not yet sure that effort will be feasible, but this change fell out of that effort and seems to have independent value on its own. At best, this change might be helpful to the effort to modernize Guava in Jenkins core; at worst, this change slims down Stapler's dependency tree, which isn't bad in and of itself.

Guava replacements

I dealt with the three remaining Guava dependencies in Stapler as follows:

• For usages of Guava's cache, I replaced them with usages of Caffeine or Map#computeIfAbsent if the needs were not exotic as suggested in stapler/stapler#206 (comment). Mercifully, I didn't have to use Caffeine in the jelly/ submodule, so I only had to introduce a Caffeine dependency for the core/ Stapler module.
• For usages of Iterators#limit I simply inlined the corresponding Guava code (with the appropriate license).

Shading Caffeine

The above having been done, I was concerned about exposing Caffeine as a new dependency. I'm not sure how stable its API is, and including it as a transitive dependency of Jenkins core implies that we are committing to supporting that API for all plugins in the future. It seems safer to avoid making that commitment unless it is truly necessary. So I tried to figure out how to shade the Caffeine dependency so that its usage remains internal to Stapler.

Unfortunately, using the Maven Shade Plugin in reactor builds (including both Stapler and Jenkins core) is a bit of a mess due to MNG-5899. In particular, the so-called "dependency reduced POM" doesn't quite work right in reactor builds. After spending quite a bit of time trying to understand how this works (or doesn't) and scratching my head, I decided to dispense with this unreliable "magic" and set createDependencyReducedPom to false. While this does have some downsides, the upside is that at least one doesn't have to deal with the pathological behavior of MNG-5899. With createDependencyReducedPom set to false, we produce a Stapler JAR with the shaded Caffeine classes included (good), without a drastically rewritten POM file (good in the sense that it avoids the risk of a regression), predictable behavior in reactor builds (good), and a dependency on Caffeine in the POM file (bad in the sense that the classes are already shaded so this is unnecessary). The main downside is the unnecessary dependency on Caffeine in the POM. That can be excluded on the consumer side, which is ugly, but it works.

Overall this is the simplest approach I could come up with that actually works. I tried a lot of other approaches that didn't actually work. I recognize this isn't necessarily the most elegant solution, but I don't think there really are any elegant ways of using the Maven Shade Plugin in reactor builds. If anyone can provide a proof of concept for a different way of shading Caffeine that actually works and is more elegant, I would be happy to go with that instead.

Testing done

I ran the Jenkins core test suite against this change in jenkinsci/jenkins#5422. Note that there I am excluding the Caffeine dependency, which is declared in the POM file (because createDependencyReducedPom is false) but not actually needed (because the Caffeine classes are shaded in the core Stapler JAR file).

[daniel-beck]

Shading is configured but unused, or what am I missing? Or is both the library and Stapler being rewritten afterwards?

[basil]

I wrote a description of the change that hopefully answers your questions. Feel free to let me know if it doesn't or if there's anything else I can clarify.

[daniel-beck]

@basil Thanks, now I think I see the relation between your description and the code. The only shading that seemed to really work in the past is adding a new module that just creates a shaded library, then using that, and that'll be reflected in downstream source code, which is what I expected to see here.

[jglick]

IIUC Caffeine is really needed here just for a couple of things using softValues. I wonder whether we want to keep this logic.

[jglick]

I doubt this ever worked to begin with. See https://github.com/jglick/biweak-class-map

[basil]

I doubt this ever worked to begin with

This would not have worked in the case of a class with a fromStapler method, because then the value would be a MethodFunction whose Method m has a Class<?> clazz that refers to the key of the map.

But it would have worked in the case of a class without a fromStapler method, because then the value would be a StaticFunction whose Method m has a Class<?> clazz that refers to org.kohsuke.stapler.Function rather than the key of the map.

[jglick]

@jtnord PTAL

[jtnord]

So Caffeine within major lines has good backwards compatibility. Version 3.x breaks 2.x API compatibility and does so using the same artifactId and same package names - so if we do not shade we will have stored an issue in the future (although the breakage is much less than the guava API bump breakage).

I have been thinking about how to deal with Guava and the cache in config-as-code plugin (actually more generally in Jenkins and this probably deserves a dump of my thoughts into the Jenkins core dev list).

Anyway - I am not sure why we would go to the effort to replace one library with another and shade it, the point of shading is so you can have multiple versions of the library. In other words why don't we just bump Guava here to the latest and shade that? is this just because google/guava#4011 has no fix at all (which is a perfectly acceptable answer - but then what about Jenkins core?) That said it is highly likely I would use exactly the same approach in casc - so call me out on this one!

Or not shade and make sure we are not using any of the known API breakages in Guava. (I have a list I need to share that too!).
(Itterators, and LoadingCache.getUnchecked -> LoadingCache.get here from a glance at what has changed)

Anyway - Jenkins needs to be able to have library dependencies that are not exposed to plugins, and that is what I will be looking at shortly.

Having said all this - the POM really should not expose the original non shaded libraries - that means plugins and the like will continue to get auto completion for things and more deps will sneak into plugins that really required, and causes things that will work at build time and blow up on deployment.

[jglick]

Given the level of risk with either Guava or Caffeine here, can we just get rid of both? For the two cache cases that are not bogus, use [Concurrent]HashMap<KeyType, SoftReference<ValueType>> and replace the computeIfAbsent with a couple lines of logic to check for either a null map value or a null referent. A bit more verbose but way less technical debt than the alternatives IMO.

[ben-manes]

Caffeine 2.x and 3.x are nearly identical, except for removal of deprecations. The major bump was to increment the JDK for removing Unsafe and some very minor API changes (e.g. returning a future for LoadingCache.refresh instead of void). If you use the latest 2.x and avoid deprecations, it should be forward compatible.