sbt / sbt-osgi

sbt plugin for creating OSGi bundles
Apache License 2.0
47 stars 43 forks source link

`Compile / sbt.Keys.packageBin := bundle.value` can cause infinite recursive loop #104

Open mdedetrich opened 8 months ago

mdedetrich commented 8 months ago

As a consequence of https://github.com/sbt/sbt-osgi/pull/95, its possible to create an infinite recursive loop which means that sbt never ends up fully starting.

To reproduce this, simply change https://github.com/sbt/sbt-osgi/blob/main/src/main/scala/com/typesafe/sbt/osgi/SbtOsgi.scala#L47 to (fullClasspath in Compile).value.map(_.data) and then navigate into src/sbt-test/sbt-osgi/test-05-exportJars/ to run the test (i.e. run sbt osgiBundle).

You will notice that sbt stalls forever, failing to start. My suspicion is that there is an interdependency between

Which causes the sbt task graph to not complete when evaluated. Apparently the intention of https://github.com/sbt/sbt-osgi/pull/95 was to prevent asynchronicity issues (see https://github.com/sbt/sbt-osgi/pull/79#issue-1884046058) by forcefully evaluating Compile / sbt.Keys.packageBin.

@romainreuillon Maybe there is a better way to solve this otherwise I think I would have to revert the change since you can easily prevent sbt from even starting.

eed3si9n commented 8 months ago

I am not really familiar with how sbt-osgi is used, but do all sbt-osgi treat bundle to be same as packageBin, or only some? If people do not make the distinction between the regular JAR and OSGi bundle JAR, then maybe the plugin should reimplement packageBin instead of adding bundle? That might reduce the overall confusion around how the plugin is expected to behave with regard to publishing or interaction with other plugins. On the other hand, the whole point of OSGi iirc is to create a module system on top of the JAR ecosystem by allowing private import etc, so I don't understand how it's useful to treat raw JAR as your bundle. Maybe it's a special case only for open source libraries that are expected to be part of module participant, but not fully take advantage of it? Either case, if we want to support Compile / sbt.Keys.packageBin := bundle.value, then bundle can't transitively call packageBin. fullClasspath depends on exportedProducts, which depends on packageBin when exportJars is true.

mdedetrich commented 8 months ago

I am not really familiar with how sbt-osgi is used, but do all sbt-osgi treat bundle to be same as packageBin, or only some?

I am also familiarizing myself with the sbt-osgi as well, but from what I can tell users basically treat bundle in the same way as packageBin which is why PR's like https://github.com/sbt/sbt-osgi/pull/32 exist.

If people do not make the distinction between the regular JAR and OSGi bundle JAR, then maybe the plugin should reimplement packageBin instead of adding bundle? That might reduce the overall confusion around how the plugin is expected to behave with regard to publishing or interaction with other plugins.

From what I can tell this is definitely the ideal case, but there appears to be asynchronicity issues which is whats adding the complications here. Specifically sbt-osgi's bundle needs to be run either before or after packageBin if thats not the case then some data ends up missing which sbt-osgi needs to populate version information in the metadata, see response here https://github.com/sbt/sbt-osgi/issues/102#issuecomment-1882581926, specifically

There seems to be no API in sbt, that ensures that the result of tasks, once they are done, stays stable. I have no idea (and also did not receive any helpful idea from others) how this can be fixed.

On the other hand, the whole point of OSGi iirc is to create a module system on top of the JAR ecosystem by allowing private import etc, so I don't understand how it's useful to treat raw JAR as your bundle. Maybe it's a special case only for open source libraries that are expected to be part of module participant, but not fully take advantage of it?

So in my specific case, the original problem I hit is that since sbt-osgi was changed to use (dependencyClasspathAsJars in Compile).value.map(_.data) ++ (products in Compile).value to detect which jar's to automatically include in the bundle, it no longer worked with edge cases such as sbt-assembly. I ended up solving this issue by just setting the explodedJars setting (i.e. https://github.com/apache/incubator-pekko/blob/main/project/OSGi.scala#L85-L90) which solved the problem, but I guess can be argued isn't entirely clean?

Either case, if we want to support Compile / sbt.Keys.packageBin := bundle.value, then bundle can't transitively call packageBin. fullClasspath depends on exportedProducts, which depends on packageBin when exportJars is true.

I am still somewhat unsure whether this is an actual problem or not, the reason behind wanting to use fullClasspath was to fix an issue in Pekko where we had a bespoke case of creating fatjar using sbt-assembly that had zero dependencies and because that jar had zero dependencies it wasn't contained within dependencyClasspathAsJars (but is still contained within fullClasspath). So one can make an argument that this really is an exceptional case and hence the usage of explodedJars is appropriate.

On the other hand, its not the best user experience because if you don't do this, you basically create broken jar's which you only figure out at runtime in another project that includes your jar's as a standard dependency.

In short from what I can understand, ideally we would just use fullClasspath since that contains all the same jar's (generated by packageBin) that you would ordinarily have, but this doesn't work because of a technical limitation in sbt (the necessary dependency information is missing at that point in time) so instead we are using (dependencyClasspathAsJars in Compile).value.map(_.data) ++ (products in Compile).value.