Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ENH] Sanitize broken manifest in source bundles #249

Closed
emweber opened this issue Mar 30, 2021 · 7 comments · Fixed by #266
Closed

[ENH] Sanitize broken manifest in source bundles #249

emweber opened this issue Mar 30, 2021 · 7 comments · Fixed by #266
Milestone

Comments

@emweber
Copy link
Contributor

emweber commented Mar 30, 2021

Some artifacts on central are already OSGI-bundles, but their source-bundle mainfest is broken; i.e the source bundle claims to export the same packages as the bundle. This causes p2 to resolve to the source-bundle instead of the real bundle jar.

I found that the source bundle of org.jboss.spec.javax.annotation:jboss-annotations-api_1.3_spec:2.0.1.Final is broken.
Specifying the <override> option did not help to fix the source-bundle manifest.
It would be useful if the <override> option would also process the source bundle or if there is a separate option to force manifest re-generation for source bundles only.

@sparsick
Copy link
Collaborator

sparsick commented Apr 8, 2021

Hi @emweber,

IMHO the manifest of the source jar and the "normal" jar should be identical. Therefore, I would prefer the <override> option should override both manifest files (in source jar and in the normal jar).

Do you want to create a PR for solving this issue?

@emweber
Copy link
Contributor Author

emweber commented Apr 9, 2021

Well, I forgot to explicitly mention that in case of org.jboss.spec.javax.annotation:jboss-annotations-api_1.3_spec:2.0.1.Final
the bundle jar has perfect OSGI-headers, so I do not want to override them.
Only the manifest of the source bundle is broken and needs to be re-written.

What if p2-maven-plugin would always automatically detect broken source-bundles and re-writes the manifest then?
WDYT?

@sparsick
Copy link
Collaborator

sparsick commented May 4, 2021

Hi @emweber,

your idea that p2 maven plugin could detect broken source-bundle is good. P2 Maven plugin could log a warning if it finds it. But I struggle with the rewriting the manifest automatically, because it fights only a symptom and fighting symptoms should be a conscious decision by the developer to rewrite the manifest. Nevertheless, I would prefer the <override> option should override both manifest files (in source jar and in the normal jar). Yes, in this case the bundle jar is fine, but from consistency reason the <override> option should always override both manifest (in source-jar and normal jar). In your case it would mean, that you have to "copy" the bnd instruction from the original one.

@emweber
Copy link
Contributor Author

emweber commented May 4, 2021

Hi Sandra,
the pull requests as a POC simply removes the Export-Package, Export-Service and Provide-Capability headers from an existing source-bundle manifest before the Eclipse-SourceBundle headers are written. No logging yet since I'm unsure what the message should look like.
While this does not fix the issue, it solves my use-case: Fix broken source bundle manifests.
Maybe I should update the issue title?

@sparsick
Copy link
Collaborator

sparsick commented May 8, 2021

Hi @emweber, yes... please update the issue title.

@emweber emweber changed the title [ENH] Allow to force manifest re-generation for source bundles [ENH] Sanitize broken manifest in source bundles May 19, 2021
@sparsick sparsick linked a pull request May 23, 2021 that will close this issue
sparsick pushed a commit that referenced this issue May 23, 2021
* Issue #249: sanitize existing source bundle manifest

Change-Id: Ie8d16185942228b730b0e143ea551918ed2614b6
@sparsick sparsick added this to the 1.7.0 milestone May 23, 2021
sparsick added a commit that referenced this issue May 23, 2021
@sparsick
Copy link
Collaborator

@emweber Thank you for your contribution. I released a new version 1.7.0. with your PR. So in the next hours it should be available on Maven Central.

@castortech
Copy link

Hi @emweber,

This is great and I just had a similar case, but wondering if there are any reasons why only removing the exports?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants