bndtools / bnd

Bnd/Bndtools. Tooling to build OSGi bundles including Eclipse, Maven, and Gradle plugins.
https://bndtools.org
Other
526 stars 304 forks source link

Bnd can cause a .jar to contain the wrong modules’ classes #6089

Closed swankjesse closed 2 months ago

swankjesse commented 2 months ago

To reproduce

Check out the OkHttp at the parent-5.0.0-alpha.13 tag, and build the jar file for the okhttp-coroutines module.

git clone git@github.com:square/okhttp.git
cd okhttp
git checkout parent-5.0.0-alpha.13
./gradlew clean okhttp-coroutines:assemble
jar tvf okhttp-coroutines/build/libs/okhttp-coroutines-5.0.0-alpha.13.jar

Note that the .jar file doesn’t contain the classes from the okhttp-coroutines/build/classes directory. Instead, it contains the classes from the okhttp/build/classes directory.

Does Bnd Cause this?

I’m not sure if it’s Bnd’s fault, or an interaction between Bnd and another part of my build. This bug goes away when I remove Bnd from my build by commenting out this section in okhttp-coroutines/build.gradle.kts:

//project.applyOsgi(
//  "Export-Package: okhttp3.coroutines",
//  "Automatic-Module-Name: okhttp3.coroutines",
//  "Bundle-SymbolicName: com.squareup.okhttp3.coroutines"
//)

Theory of the bug

OkHttp has many modules, but only okhttp-coroutines is impacted by this Bnd bug. What makes okhttp-coroutines special? Well as of 5.0.0-alpha.13, that module’s only code was in the okhttp3 package. The problem also goes away if we move the code to another package, as implemented in this PR.

I assume there’s a cache that’s keyed by the package name instead of the package contents.

pkriens commented 2 months ago

In the ./okhttp/okhttp-coroutines project directory there is no source in the okhttp.coroutines package but there are sources in the okhttp package. In the whole repository, I can't find a single kotlin file that declares the okhttp.coroutines package?

okhttp/okhttp-coroutines ((parent-5.0.0-alpha.13) *)$ ls -R src
src/main/kotlin/okhttp3:
JvmCallExtensions.kt
src/test/kotlin/okhttp3:
SuspendCallTest.kt
okhttp/okhttp-coroutines ((parent-5.0.0-alpha.13) *)$ cd ..
okhttp ((parent-5.0.0-alpha.13) *)$ find . -name "*.kt" -exec grep okhttp.coroutines {} \;
okhttp ((parent-5.0.0-alpha.13) *)

This does not seem to look right, does it?

swankjesse commented 2 months ago

Yep. We’ve since moved the code to that and package.

pkriens commented 2 months ago

The problem seems to be that if you do split packages, bnd will turn merge them.

The parent-5.0.0-alpha.13 branch has 1 class in the okhttp package: src/main/kotlin/okhttp3/JvmCallExtensions.kt. This will then include ALL classes from the class path in that package since split packages don't work in OSGi without a lot of work and they are evil anyway :-)

If I remove the src/main/kotlin/okhttp3/JvmCallExtensions.kt I get an empty bundle, as expected.

The fact that you do not get an error on the superfluous Export-Package is that sometimes you need to export a package even if you don't have it. When you'd use wildcards it would've warned you.

So I do not think this is a bug. Do not have packages in your source tree that overlap with packages on the build path and you should be ok?

swankjesse commented 2 months ago

It’s a surprising result but it’s not causing us problems any more.

pkriens commented 2 months ago

In bnd, the default always was the classpath, not the project's sources. This made it easy to assemble bundles with their APIs and subsets. However, in this case we have some unexpected interaction. Although I guess it made you find a real problem :-)