-
Notifications
You must be signed in to change notification settings - Fork 19
feat(GH-188): Implement package type providers #223
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
base: master
Are you sure you want to change the base?
Conversation
This uses the Java Service Provider Interface via `ServiceLoader` to register new types, where each type gets its own class.
My 2 cents about the API changes:
|
1097c9e
to
ac200a8
Compare
@ppkarwasz I made most of the changes you suggested, but I need to know how to update the [WARNING] bnd.bnd [0:0]: jpms.jarname calculated module names from file name: [org.osgi.resource-1.0.0, org.osgi.service.serviceloader-1.0.0]
[ERROR] bnd.bnd [0:0]: Requirement osgi.extender resolution not correct
[ERROR] bnd.bnd [0:0]: Requirement osgi.serviceloader cardinality not correct
[ERROR] bnd.bnd [0:0]: Requirement osgi.serviceloader resolution not correct |
@ppkarwasz There seems to be duplicated classes between this and |
@ppkarwasz I think it's better to auto-generate this file if we can. |
value = PackageTypeProvider.class, | ||
resolution = Requirement.Resolution.MANDATORY, | ||
cardinality = Requirement.Cardinality.MULTIPLE) | ||
public final class PackageTypeFactory implements PackageTypeProvider { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It bothers me to have the factory class also implementing some type of provider, since it's not actually added to the set of providers.
This is trying to provider the default normalization that needs to happen before all type-specific normalization.
But, maybe there is a better way to do this (add it to the list in the first position?).
try { | ||
PackageURL packageURL = normalize(); | ||
namespace = packageURL.getNamespace(); | ||
name = packageURL.getName(); | ||
version = packageURL.getVersion(); | ||
qualifiers = packageURL.getQualifiers(); | ||
subpath = packageURL.getSubpath(); | ||
} catch (MalformedPackageURLException e) { | ||
throw new ValidationException("Normalization failed", e); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is kind of a hack right now.
@ppkarwasz I am stuck on the OSGi errors. I don't have experience creating a bnd file manually as opposed to plugins that generate the |
I'll check it out. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The BND errors are due to the wrong constants being used.
import org.osgi.annotation.bundle.Requirement.Cardinality; | ||
import org.osgi.annotation.bundle.Requirement.Resolution; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
import org.osgi.annotation.bundle.Requirement.Cardinality; | |
import org.osgi.annotation.bundle.Requirement.Resolution; | |
import aQute.bnd.annotation.Cardinality; | |
import aQute.bnd.annotation.Resolution; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! I was using something from aQute, but it said deprecated use the OSGi ones, but now I can't find it.
import java.util.Map; | ||
import org.jspecify.annotations.NonNull; | ||
import org.jspecify.annotations.Nullable; | ||
import org.osgi.annotation.bundle.Requirement.Resolution; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
import org.osgi.annotation.bundle.Requirement.Resolution; | |
import aQute.bnd.annotation.Resolution; |
What bothers me a little with the current proposal is that type-specific normalization and validation only occurs when
I deduce that the components should only and always be stored in normalized form (and be valid). So the |
This uses the Java Service Provider Interface via
ServiceLoader
to register new types, where each type gets its own class.