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

Propsal to remove the "wrapped." prefix in the symbolic-name of automatically wrapped Maven target bundles #330

Closed
HannesWell opened this issue Sep 18, 2021 · 7 comments
Labels
wontfix This will not be worked on

Comments

@HannesWell
Copy link
Contributor

When a Maven target dependency is not yet a OSGi bundle (i.e. provides the required OSGi metadata in its MANIFEST.MF) it is wrapped automatically using BND-tools.
The default BND-instructions for m2e Maven targets use the following symbolic name instruction:

Bundle-SymbolicName:   wrapped.${mvnGroupId}.${mvnArtifactId}

So the symbolic name of all automatically wrapped bundles start with the prefix wrapped..
Personally I find this naming convention not ideal because IMHO it is odd to start a bundle symbolic name with wrapped. Also for example when you have a list of bundle names this groups all wrapped bundles together instead of those with similar root names.

Therefore want to suggest to remove the wrapped. prefix, either without replacement or, if you insist to indicate the automatic wrapping in the bundle name, to replace it with a corresponding suffix instead.
The line above from the bnd-default.properties file would then be replaced by one of the following lines:

Bundle-SymbolicName:   ${mvnGroupId}.${mvnArtifactId}
Bundle-SymbolicName:   ${mvnGroupId}.${mvnArtifactId}.wrapped

Just for clarification: Did you indicate the wrapping so users are 'notified' that a Maven dependency became a OSGi bundle by the unresolved bundle errors (in case one used Require-Bundle)?

This would break the Manifest of users that reference a wrapped bundle via Require-Bundle, but since we change the format of the Maven target with the next release of m2e I think now would be the least worse time to do it.

@laeubi and @mickaelistria what do you think? Would this be a better generated symbolic-name? And if yes would it be worth the effort (especially for users)?

@laeubi
Copy link
Member

laeubi commented Sep 18, 2021

This would break the Manifest of users that reference a wrapped bundle via Require-Bundle,

Referencing "unofficial" OSGi bundles via Require-Bundle is bad anyway and I won't mind actually about that case. The more actual impact would be feature files where there is no alternative to mention the name directly, also we should take into consideration that this would require a change in tycho also.

Instead of changing the default, another approach would be to allow a new "auto naming strategy" attribute that could have value of "none", "prefix" (default) or "suffix" that could change the behavior if no explicit configuration is given.

About the prefix, this was an explicit requirement of @mickaelistria to make this feature happen (my initial approach simply do not add any prefix/suffix at all) so I'd like him to decide / discuss this.

@HannesWell
Copy link
Contributor Author

Thanks for your remarks.

This would break the Manifest of users that reference a wrapped bundle via Require-Bundle,

Referencing "unofficial" OSGi bundles via Require-Bundle is bad anyway and I won't mind actually about that case. The more actual impact would be feature files where there is no alternative to mention the name directly, also we should take into consideration that this would require a change in tycho also.

Yes, I thought using Require-Bundle is not recommended in general (regardless if the bundle is officially crafted as OSGi bundle or not) even though I have the impression that in the Eclipse/PDE world this information is not very popular. Anyway, the features are indeed a point, so changing the default generated name will require manual adjustments for users.

Yes, of course Tycho would have to be modified as well. I have not yet looked into the details but I assume that it would be a similarly small change as for m2e.

Instead of changing the default, another approach would be to allow a new "auto naming strategy" attribute that could have value of "none", "prefix" (default) or "suffix" that could change the behavior if no explicit configuration is given.

I had an idea that goes in the same direction: At the moment a user can either define no BND-instructions at all and take m2e's default instructions or a user can specify its own instructions and none of the default instructions is used. So if you only want to change one header (e.g. the Bundle-SymbolicName) one has to provide the full set of instructions. I think it could be handy if it where possible to override the default instructions, i.e. for example if one only provides the Bundle-SymbolicName instruction all other instructions are taken from m2e's defaults.
Or is this problematic because a user could also want to take the BND default for an instruction?
Are the m2e default instructions all required by BND? I have to admit I'm not very familiar with BND yet.

This would be more general and would not add another attribute, but a user would have to write more (and it is actually out of scope of this issue).
However I think the default should be a good one. And at least I would prefer adding wrapped as a suffix.

About the prefix, this was an explicit requirement of @mickaelistria to make this feature happen (my initial approach simply do not add any prefix/suffix at all) so I'd like him to decide / discuss this.

Understand that, so I think no prefix/suffix, at least by default, won't be an option.

@mickaelistria
Copy link
Contributor

However I think the default should be a good one. And at least I would prefer adding wrapped as a suffix.

One benefit with "wrapped" as prefix is that OSGi bundles are often understood as being provided by the prefix ( org.apache => comes from apache, org.eclipse => comes from eclipse, com.redhat => comes from Red Hat). This understanding is deeply rooted in how community read names. By having "wrapped" as prefix, we make readers confused about where this bundle comes from, and that's a good thing they're going to wonder. If we move it to suffix, then the understanding of prefix == provider would remain and people will make false assumption.
That's why I believe that prefix is to be preferred by default; because it prevent consumers from making false assumptions.

Understand that, so I think no prefix/suffix, at least by default, won't be an option.

IMO yes. However, I would be fine giving an option to specify the bundle-name in the .target (it could easily fit in BND instructions maybe), so the provider can chose the name they want; ideally adding themselves as prefix to explicit they're the provider. Eg Red Hat starts including some org.acme.lib in their product using this technology, they'll be able to call the OSGi bundle com.redhat.org.acme.lib to clarify they're the provider for this artifact that's derived from com.acme.lib.

IMO, we're better enforcing usage of a prefix; wrapped is neutral; but an option to replace wrapped by org.eclipse or com.redhat would be the best of both worlds IMO.

@HannesWell
Copy link
Contributor Author

However I think the default should be a good one. And at least I would prefer adding wrapped as a suffix.

One benefit with "wrapped" as prefix is that OSGi bundles are often understood as being provided by the prefix ( org.apache => comes from apache, org.eclipse => comes from eclipse, com.redhat => comes from Red Hat). This understanding is deeply rooted in how community read names. By having "wrapped" as prefix, we make readers confused about where this bundle comes from, and that's a good thing they're going to wonder. If we move it to suffix, then the understanding of prefix == provider would remain and people will make false assumption.
That's why I believe that prefix is to be preferred by default; because it prevent consumers from making false assumptions.

I have to admit I never thought about it this way. But you convinced me, using wrapped as prefix is actually a good choice that makes sense.

Understand that, so I think no prefix/suffix, at least by default, won't be an option.

IMO yes. However, I would be fine giving an option to specify the bundle-name in the .target (it could easily fit in BND instructions maybe), so the provider can chose the name they want; ideally adding themselves as prefix to explicit they're the provider. Eg Red Hat starts including some org.acme.lib in their product using this technology, they'll be able to call the OSGi bundle com.redhat.org.acme.lib to clarify they're the provider for this artifact that's derived from com.acme.lib.

IMO, we're better enforcing usage of a prefix; wrapped is neutral; but an option to replace wrapped by org.eclipse or com.redhat would be the best of both worlds IMO.

It is already possible to override the default naming schema by specifying your own BND-instructions and using a Bundle-Symbolic name instruction as described above or adjusted to your case. So I don't think we need another attribute for that. Instead I suggest to make it possible to override only certain instructions from the default set, if it would be feasible.

@laeubi
Copy link
Member

laeubi commented Sep 20, 2021

Overriding / Extending BND properties is possible but the problem is you can't really remove properties from the default, so if we want something like this we would need a similar approach like in maven where you can tell if the configuration should extend or replace the existing one.

Instead I suggest to make it possible to override only certain instructions from the default set, if it would be feasible.

Given that one can group multiple dependencies now in one location you can define your own defaults for that group, so I wonder if it really hurts much to (before it was a bit messy as one has to edit the instructions for each added dependency).

The defaults are just a very broad definition that make "things work" in most cases but are far from optimal in the specific case, so I would expect that often people want to specific their own anyways for complex setups and I don't think it would be a good idea to try to define an own way to only override single properties, that would result in just another way to write BND files. If we really like to help the user, we could better try to integrate the Source-Editor (last screenshot) from BND tools to make editing files more convenient, but I haven't check how separated this part of code is to be reused.

Another thing that might be useful instead here: We can have a property in the instructions that references a bnd file next to the target, so instructions

  1. can be reused
  2. do not blow up the XML to much

To summarize:

  • We shouldn't complicate things to much (too much configuration options and corresponding XML attributes/representations)
  • We should clearly define what is the problem (e.g. is only the naming scheme an issue and should we provide some more configuration for that particular problem but not for more)

@mickaelistria mickaelistria added the wontfix This will not be worked on label Oct 11, 2021
@mickaelistria
Copy link
Contributor

Let's close this one and track further improvements about how to "guess" better values in the target editor into dedicated issues.

@laeubi
Copy link
Member

laeubi commented Oct 12, 2021

I'd like to add one more comment here: The Wrapping Bundles feature is more meant to get something working fast, but the ultimate solution should be to encourage the consumed project to add OSGi metadata in the first place so it could officially be supported and wrapping is uneccesary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

3 participants