sbt / sbt-osgi

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

Use all dependencies as JARs to force proper manifests (fixes #60) #61

Closed lefou closed 5 years ago

lefou commented 6 years ago

This fixed #60.

After merge, a new release would be great.

lefou commented 6 years ago

I also added a test case.

Besides the test case we also depend on this PR in a large (60+) multi-project setup.

lefou commented 6 years ago

PS. To make the tests work, I had to explicitly set the project version to a not-yet used version number (0.9.5-SNAPSHOT). I assume, this ensures that the scripted tests really use the newly locally built plugin.

lefou commented 6 years ago

Don't know, why the Java 10 test fails. Any hint would be very welcome.

johanandren commented 6 years ago

Missing entries in the manifest?

[info] [error] java.lang.RuntimeException: Expected 'Export-Package: ' and 'proj1;' and 'version="1.2.3"' in manifest!
[info] [error] But was:
[info] [error] Manifest-Version: 1.0
[info] [error] Specification-Title: proj1
[info] [error] Specification-Version: 1.2.3
[info] [error] Specification-Vendor: com.typesafe.sbt
[info] [error] Implementation-Title: proj1
[info] [error] Implementation-Version: 1.2.3
[info] [error] Implementation-Vendor: com.typesafe.sbt
[info] [error] Implementation-Vendor-Id: com.typesafe.sbt
[info] [error] Implementation-URL: https://github.com/woq-blended/blended
lefou commented 6 years ago

I'm aware of this error message, but that is not the real issue. The real issue is, that the produced JAR has no proper OSGi manifest at all. But the same test works for Java 8. It is either a sporadic failure or something I can't explaint, because all the other sbt-test's ran successfully. Would be great, if someone with permissions could re-run the test.

johanandren commented 6 years ago

Oh. Weird, I see, I thought maybe you had missed that output.

I'll try to get access and trigger a rebuild.

lefou commented 6 years ago

Ok, this is now definitely strange. Same last commit runs successfully on Travis on my personal account: https://travis-ci.org/lefou/sbt-osgi/builds/433302843

You are seeing a false negative here. So, I assume, there must be some caching issue on your Travis setup. Invalidating caches might help.

johanandren commented 6 years ago

This travis build doesn't use caching, so there is something else that is different from your personal travis build (or the failure is intermittent and you got lucky?)

lefou commented 6 years ago

The only difference is, that my travis build just build's the branch, whereas this build first merges the branch into master and then builds the master branch.

johanandren commented 6 years ago

With a few repeated builds it seems quite random how it fails :/

lefou commented 6 years ago

Might be more related to Java 10 or to the way I wrote the test case then to my newly introduced feature.

If you think, this PR is OK, a merge and a release would be very great.

lefou commented 6 years ago

Looks like, my added test case is the only test case providing a multi-project setup. Could also be an sbt issue or an issue with the test runner. Unfortunately, I can't reproduce it locally.

johanandren commented 6 years ago

I'm somewhat cautious to merge before we figure out why it is sometimes failing - whether it is an actual problem or something travis-related, but I also have no clue to what is making it randomly fail, nothing obvious to investigate and a bit too few cycles to spare.

lefou commented 6 years ago

Are there any log outputs, which we can use to investigate the sbt behavoiur? I'm not very familar with sbt yet. Could you protocol how often you restarted travis, with and without success. As I said, I have no idea how to reproduce, event on travis itself I can't reproduce. So it's hard to provide more data.

I perfectly understand you point of view. But not fixing this essential, I would say, bug in sbt-osgi would be a shame too. We currently use a binary build from this PR in our project to get proper OSGi manifests, and we never saw cases, in which proper OSGi Manifests were missing.

Do you know other users of this plugin, who could stability-test this PR? Also, a review and/or rewrite of the test case by some "sbt expert" might help to stabilize the build and identify the issue.

johanandren commented 6 years ago

If we could run sbt with -debug only in that scripted test somehow (that creates crazy amounts of logs though).

johanandren commented 6 years ago

Eugene gave me the hint that you could either put -debug in the scriptedLaunchOpts or to put > debug in the scripted test file.

lefou commented 6 years ago

As the integration tests succeeded recently and this PR essentially fixes a bug in the plugin logic, can we merge it?

johanandren commented 5 years ago

Merged this but had to revert as the test still fails. (The build additionally fails on JDK 10 being deprecated on Travis but I'm afraid I don't have a single spare cycle to work on this)

lefou commented 5 years ago

So, what's the best solution to this? I could split the fix from the integration test. The fix itself works for us reliable. I have no idea, why the integration test on travis fails, it always works locally. Unfortunatelly, my sbt integration test skills aren't as good as my OSGi skills. WDYT?

johanandren commented 5 years ago

Hmm, yes. Maybe that is the best path.