Caching for Osgi-bundles.

[markehammons]

This is a modification I made to your plugin a couple of months back to speed up builds of my project. It makes the bundler regenerate a bundle if and only if the following have changed:

• The classpath
• The manifest
• The embedded jars
• The resources

If the bundle doesn't exist it will be generated no matter what.

[hseeberger]

I don't understand why this is necessary, because – at least by default – a bundle is only created when running publish. Is it really necessary to speed that up? I think caching is one of the most tricky things in programming and hence I need a good reason to include it. And good tests, of course ;-)

[markehammons]

My project must run publish-local every time a compile happens due to the way it's set up. Even if I managed to remove the publish-local requirement, it would still require osgi-bundle to be called on 40-45 subprojects each compile. That osgi-bundle step ends up costing me 30-40 seconds per rebuild, no matter if anything has changed. The caching I added reduces our minimum build time from 45s to 15s, so yes it's a pretty necessary speed up for our project.

I'm fairly certain it works since my project has been relying on this functionality for every build for about 2 months, but I can try to develop some tests to prove it's safe.

[harrah]

Caching tasks like this is a good idea. Maybe only publish creates a bundle by default, but it wouldn't be unreasonable for custom tasks to do something with it.

[reggert]

This PR conflicts significantly with the two other PR's I just merged (which conflicted with each other, but only by 1 line). From looking at the code, the conflict shouldn't be difficult to resolve (you just need to modify your code to account for the 1 character my PR removed, and the half-line the other PR added). Additionally, this change needs unit tests to verify that it works correctly. Please rebase your branch and add some tests.

[markehammons]

I'll get it done.

Would a unit test showing the inputs to the bundle function are properly converted to a Set[File] to be fed to FileFunction.cached be fine?

[reggert]

Good question. The existing unit tests for the plugin are woefully inadequate. Currently, nothing tests the bundleTask function at all. All of the existing tests just test the individual subroutines that bundleTask calls (seqToStrOpt, headersToProperties, includeResourceProperty, etc.). This will need to be remedied some time in the (hopefully near) future. In the meantime, it would probably be sufficient to break out as much of your code into separate functions as possible and then test those, unless you feel ambitious enough to take on the task of running an end-to-end test for bundleTask.

[reggert]

I've just submitted issue #11 to try to address this to some degree.

[markehammons]

Maybe on my vacation :)

On Sunday, November 17, 2013 09:42:55 AM Richard W. Eggert II wrote:

Good question. The existing unit tests for the plugin are woefully
inadequate. Currently, nothing tests the bundleTask function at all. All
of the existing tests just test the individual subroutines that
bundleTask calls (seqToStrOpt, headersToProperties,
includeResourceProperty, etc.). This will need to be remedied some time
in the (hopefully near) future. In the meantime, it would probably be
sufficient to break out as much of your code into separate functions as
possible and then test those, unless you feel ambitious enough to take on
the task of running an end-to-end test for bundleTask.

---

Reply to this email directly or view it on GitHub:
#6 (comment)

[markehammons]

I've fixed the merge conflicts and better documented my code. I can help split up the bundle function soon, cause it's getting really complex.

[romainreuillon]

Hi,

any new on this caching functionality being merged ?

Cheers,
Romain

[reggert]

I'm not really familiar enough with the sbt build process to understand the implications of this change to be comfortable with merging this. Maybe Mark or one of the Typesafe guys could take a look at it and verify that's it not going to cause the universe to implode or anything like that?

I'm mainly concerned that there may be edge cases where this would prevent the bundle from being rebuilt when it should be.

[ktoso]

I'm very sorry that this has been dropped like this. Truth is, no one has maintained this plugin over the last years. With the Akka team occassionally stepping in to fix bugs that impacted our build...

I have to close this, as it's been a number of years, I'm not comfortable merging it, and it is currently conflicting.

Please reopen if you'd like to give it another shot though.