ben-manes / caffeine

A high performance caching library for Java
Apache License 2.0
15.62k stars 1.58k forks source link

OSGi version range for Guava is too strict #1160

Closed guw closed 11 months ago

guw commented 11 months ago

In Eclipse we are moving away from having too strict version on package imports for Guava. It caused a quite a few issues over the years. It would be great if the OSGi metadata can be relaxed to only have a lower bound on the Guava package imports.

See https://github.com/eclipse-equinox/equinox/issues/304#issuecomment-1701157875 for more details.

ben-manes commented 11 months ago

I'm not really sure what to do here because this is automatically done by the bnd plugin.

There is common configuration like descriptions, https://github.com/ben-manes/caffeine/blob/586fe8c931a8d1da4f0532a8c59772fe9d71cd74/gradle/plugins/src/main/kotlin/lifecycle/java-library-caffeine-conventions.gradle.kts#L64-L77

and project-specific like package exports, https://github.com/ben-manes/caffeine/blob/586fe8c931a8d1da4f0532a8c59772fe9d71cd74/guava/build.gradle.kts#L37-L47

The plugin is the one setting the version based on the dependencies, e.g.

Export-Package: com.github.benmanes.caffeine.guava;uses:="com.github.b
 enmanes.caffeine.cache,com.google.common.cache";version="3.1.9"
Import-Package: com.google.common.cache;version="[32.1,33)",com.google
 .common.util.concurrent;version="[32.1,33)",com.github.benmanes.caffe
 ine.cache;version="[3.1,4)",com.github.benmanes.caffeine.cache.stats;
 version="[3.1,4)"

The closest I see their instruction -nodefaultversion to "not add a default version to exported packages when no version is present." I don't know if that's desired or if it would cause more pains?

This package could use a very old version of Guava, but I also think keeping the dependency up to date is worthwhile. I guess if we add the version tag ourselves then it won't be computed and we could use an old version as a minimum, like ;version=20.0. wdyt?

I am not very familiar with osgi and rely on bnd + pax-exam tests to be compliant. Therefore any changes here relies on experts as I'll be ignorant to whether a suggested change is good or bad, which makes me hesitant to muck around too much.

HannesWell commented 11 months ago

I guess if we add the version tag ourselves then it won't be computed and we could use an old version as a minimum, like ;version=20.0. wdyt?

Yes that should work. Furthermore you don't have to list all packages you need explicitly. You could do something like: com.google.common.*;version="20.0" and the bnd-lib will compute the actually used bundles for you automatically. To be sure I suggest you inspect the generated manifest from a local/CI build (at least that's what I'm usually doing in such cases).

guw commented 4 months ago

@ben-manes Any chance you can push a new release with the updated jars? We are being hit by the issue again (https://github.com/salesforce/bazel-vscode-java/issues/109)