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

Produce correct module-info class for org.eclipse.osgi using BND #27

Closed
akurtakov opened this issue Jun 16, 2022 · 23 comments · Fixed by #64
Closed

Produce correct module-info class for org.eclipse.osgi using BND #27

akurtakov opened this issue Jun 16, 2022 · 23 comments · Fixed by #64
Labels
in:framework Issues related to the OSGi Framework implementation

Comments

@akurtakov
Copy link
Member

Log is:

Comparator differences from current build
	/home/jenkins/agent/workspace/I-build-4.25/eclipse.platform.releng.aggregator/eclipse.platform.releng.aggregator/cje-production/siteDir/eclipse/downloads/drops4/I20220616-0910
compared to reference repo at 
	https://download.eclipse.org/eclipse/updates/4.25-I-builds

1.  equinox/bundles/org.eclipse.osgi
   no-classifier: different
      module-info.class: present in baseline only
    The main artifact has been replaced with the baseline version.
    The following attached artifacts have been replaced with the baseline version: [sources]

at https://download.eclipse.org/eclipse/downloads/drops4/I20220616-0910/buildlogs/comparatorlogs/buildtimeComparatorUnanticipated.log.txt

@akurtakov
Copy link
Member Author

@tjwatson is module-info.class in o.e.osgi needed ? what could have caused it to no longer be generated/added

@sravanlakkimsetti
Copy link
Member

module-info definition is provided in pom.xml

open module org.eclipse.osgi {
. This is not getting generated after equinox merge.

@sravanlakkimsetti
Copy link
Member

There are extra maven tasks after merge. These extra maven tasks are causing the trouble.
Here ate the task lists
Old : log_old.txt
New : log_new.txt

@sravanlakkimsetti
Copy link
Member

specifically there are two tasks that are running extra now

[INFO] --- tycho-source-plugin:3.0.0-SNAPSHOT:feature-source (feature-source) @ org.eclipse.osgi ---
[INFO] 
[INFO] --- tycho-p2-plugin:3.0.0-SNAPSHOT:p2-metadata (attach-p2-metadata) @ org.eclipse.osgi ---
[INFO] 

they are running just before add-module-info task.

@laeubi
Copy link
Member

laeubi commented Jun 17, 2022

they are running just before add-module-info task.

these should be independet from any other task generating sources, the more interesting would be if the moditec is running and if it issues any warning/errors.

@laeubi
Copy link
Member

laeubi commented Jun 17, 2022

If I build it local I see a module-info.class included...

grafik

@akurtakov
Copy link
Member Author

If I understood correctly @sravanlakkimsetti he sees p2-metadata task ran before add-module-info task and
comparator runs during p2-metadata task .

akurtakov added a commit to akurtakov/equinox that referenced this issue Jun 17, 2022
Provoked by eclipse-equinox#27 so
build uses the latest plugin version.
@sravanlakkimsetti
Copy link
Member

they are running just before add-module-info task.

these should be independet from any other task generating sources, the more interesting would be if the moditec is running and if it issues any warning/errors.

you can see the log here

please look for section add-module-info. As far as I see there is no error/warnings

@laeubi
Copy link
Member

laeubi commented Jun 17, 2022

he sees p2-metadata task ran before add-module-info task and
comparator runs during p2-metadata task .

Then this seems like a false positive here, the p2-metadata task is required to update the meta-data after generating the source bundle.

@laeubi
Copy link
Member

laeubi commented Jun 17, 2022

So maybe one can disable the baseline compare for the extra metadata task?

akurtakov added a commit to akurtakov/equinox that referenced this issue Jun 17, 2022
Provoked by eclipse-equinox#27 so
build uses the latest plugin version.
akurtakov added a commit that referenced this issue Jun 17, 2022
Provoked by #27 so
build uses the latest plugin version.
@laeubi
Copy link
Member

laeubi commented Jun 17, 2022

I think the problem is that moditec is currently bound to <phase>package</phase> while it actually should be <phase>process-classes</phase>

@akurtakov
Copy link
Member Author

Sounds correct, if you're not doing the change I'll do it right now.

@akurtakov
Copy link
Member Author

Unfortunately:

Couldn't find file /home/akurtakov/git/equinox/bundles/org.eclipse.osgi/target/org.eclipse.osgi-3.18.0-SNAPSHOT.jar. Run this goal for the project's JAR only after the maven-jar-plugin. -> [Help 1]

@laeubi
Copy link
Member

laeubi commented Jun 17, 2022

Stupid moditec... or baseline-compare ;-)

I'll propose a patch using BND for the JPMS stuff...

@akurtakov
Copy link
Member Author

Thanks, @laeubi !

@sravanlakkimsetti
Copy link
Member

here are the decompiled class files

Module-info from baseline : module-info.txt
Module-info from pr : module-info-frompr.txt

The differences:
Missing

provides org.osgi.framework.launch.FrameworkFactory with org.eclipse.osgi.launch.EquinoxFactory;
provides org.osgi.framework.connect.ConnectFrameworkFactory with org.eclipse.osgi.launch.EquinoxFactory;
uses org.osgi.framework.connect.FrameworkUtilHelper;

Added extra

org.osgi.service.condition
org.eclipse.osgi.storage
org.eclipse.osgi.storage.bundlefile
org.eclipse.osgi.storage.url
org.eclipse.osgi.storage.url.bundleentry
org.eclipse.osgi.storage.url.bundleresource
org.eclipse.osgi.storage.url.reference
org.eclipse.osgi.framework.util
org.eclipse.osgi.internal.cds
org.eclipse.osgi.internal.connect
org.eclipse.osgi.internal.container
org.eclipse.osgi.internal.debug
org.eclipse.osgi.internal.framework
org.eclipse.osgi.internal.framework.legacy
org.eclipse.osgi.internal.hookregistry
org.eclipse.osgi.internal.hooks
org.eclipse.osgi.internal.loader
org.eclipse.osgi.internal.loader.buddy
org.eclipse.osgi.internal.loader.classpath
org.eclipse.osgi.internal.loader.sources
org.eclipse.osgi.internal.location
org.eclipse.osgi.internal.log
org.eclipse.osgi.internal.messages
org.eclipse.osgi.internal.permadmin
org.eclipse.osgi.internal.provisional.service.security
org.eclipse.osgi.internal.provisional.verifier
org.eclipse.osgi.internal.service.security
org.eclipse.osgi.internal.serviceregistry
org.eclipse.osgi.internal.signedcontent
org.eclipse.osgi.internal.url
org.eclipse.osgi.internal.util
org.eclipse.osgi.internal.weaving
org.eclipse.osgi.framework.internal.reliablefile
org.apache.felix.resolver
org.apache.felix.resolver.reason
org.apache.felix.resolver.util
org.eclipse.core.runtime.adaptor
org.eclipse.core.runtime.internal.adaptor

Added

requires java.management

@laeubi
Copy link
Member

laeubi commented Jun 17, 2022

Missing:

uses org.osgi.framework.connect.FrameworkUtilHelper;

I'm not an JPMS expert, but as the interface is embedded in the OSGi jar it seems strange to declare its uses? Please let me know why this should be included and I can report this to the BND project.

provides org.osgi.framework.launch.FrameworkFactory with org.eclipse.osgi.launch.EquinoxFactory;
provides org.osgi.framework.connect.ConnectFrameworkFactory with org.eclipse.osgi.launch.EquinoxFactory;

reported here: bndtools/bnd#5288

Added requires java.management

Could it be a transitive one?

Added extra

can you explain which one are unexpected so we can report an issue to BND? Will it harm to have those added?

@tjwatson
Copy link
Contributor

tjwatson commented Jun 17, 2022

For anyone asking why the module-info is needed at all. It is to allow the framework impl to be placed on the module path along with a launcher without it being treated as a JPMS automatic module. In particular it is important for something called OSGi connect (from OSGi R8 core) that allows the framework and a set of bundles to have alternative sources of content from the typical JAR that is supported by default by all framework implementations.

The Apache Felix Atomos project (https://github.com/apache/felix-atomos) provides such a launcher and connect implementation and can be used with Equinox or Felix frameworks to load the launcher, framework and set of bundles contained a JPMS module layer as modules. Once that can be done that that set of modules can then be jlink'ed into a jlink image that launches the framework and set of bundles with the minimal set of modules from the JVM.

uses org.osgi.framework.connect.FrameworkUtilHelper;

I'm not an JPMS expert, but as the interface is embedded in the OSGi jar it seems strange to declare its uses? Please let me know why this should be included and I can report this to the BND project.

Because the framework uses the Java ServiceLoader to discover FrameworkUtilHelper providers available on the classpath of the framework (or module Layer for JPMS). In the case of JPMS, the module system will not allow ServiceLoader to discover providers from other modules unless the using module declares that it uses the service interface/class.

Added requires java.management

Could it be a transitive one?

I'm not aware of anything the framework needs out of java.management. We may need help from BND to figure out what it thinks needs that.

org.osgi.service.condition

This is an oversight in the current pom.xml. An argument for automating the generation. It should actually be included.

org.eclipse.osgi.storage
org.eclipse.osgi.storage.bundlefile
org.eclipse.osgi.storage.url
org.eclipse.osgi.storage.url.bundleentry
org.eclipse.osgi.storage.url.bundleresource
org.eclipse.osgi.storage.url.reference
org.eclipse.osgi.framework.util
org.eclipse.osgi.internal.cds
org.eclipse.osgi.internal.connect
org.eclipse.osgi.internal.container
org.eclipse.osgi.internal.debug
org.eclipse.osgi.internal.framework
org.eclipse.osgi.internal.framework.legacy
org.eclipse.osgi.internal.hookregistry
org.eclipse.osgi.internal.hooks
org.eclipse.osgi.internal.loader
org.eclipse.osgi.internal.loader.buddy
org.eclipse.osgi.internal.loader.classpath
org.eclipse.osgi.internal.loader.sources
org.eclipse.osgi.internal.location
org.eclipse.osgi.internal.log
org.eclipse.osgi.internal.messages
org.eclipse.osgi.internal.permadmin
org.eclipse.osgi.internal.provisional.service.security
org.eclipse.osgi.internal.provisional.verifier
org.eclipse.osgi.internal.service.security
org.eclipse.osgi.internal.serviceregistry
org.eclipse.osgi.internal.signedcontent
org.eclipse.osgi.internal.url
org.eclipse.osgi.internal.util
org.eclipse.osgi.internal.weaving
org.eclipse.osgi.framework.internal.reliablefile
org.apache.felix.resolver
org.apache.felix.resolver.reason
org.apache.felix.resolver.util
org.eclipse.core.runtime.adaptor
org.eclipse.core.runtime.internal.adaptor

The rest look to be not exported at all or exported as x-internal. I intentionally left private and x-internal packages out. I wish I could just not export the x-internal packages at all. But it seems weird that BND is specifying the packages that are not even exported (e.g. org.apache.felix.resolver)

@laeubi
Copy link
Member

laeubi commented Jun 17, 2022

The BND part is described here, also how to customize this, I have enabled "edit by maintainer", so one could simply switch to file tab of the PR and edit the file, because my knowledge to JPMS is to limited here.

Any obstacle should be reported to the BND people so this could be improved, I think it is much more useful to generate this stuff instead of us editing it manually.

@tjwatson
Copy link
Contributor

I have little time to focus on this today with any urgency. Can we not just accept the module-info is in a bad state for now and allow the I-Build to continue? If you are waiting for me it may not be for more than a week.

@laeubi
Copy link
Member

laeubi commented Jun 17, 2022

Can we not just accept the module-info is in a bad state for now

I'm fine with that, so if you are... please approve the PR then and I'll merge it as soon as the build passes.

@tjwatson tjwatson changed the title Comparator error in https://download.eclipse.org/eclipse/downloads/drops4/I20220616-0910/ Produce correct module-info class for org.eclipse.osgi using BND Jun 20, 2022
@tjwatson
Copy link
Contributor

I've repurposed this issue to confirm the output of the module-info class is correct. In particular we need to ensure the output can still be used for such things as Apache Felix Atomos.

@tjwatson tjwatson added the in:framework Issues related to the OSGi Framework implementation label Jun 20, 2022
@tjwatson
Copy link
Contributor

We need output of jar --file=<path to org.eclipse.osgi jar> --describe-module:

jar --file=org.eclipse.osgi_3.18.0.v20220516-2155.jar --describe-module
org.eclipse.osgi@3.18.0-SNAPSHOT jar:file:///home/tjwatson/dev/tools/eclipse/plugins/org.eclipse.osgi_3.18.0.v20220516-2155.jar/!module-info.class open
exports org.eclipse.equinox.log
exports org.eclipse.osgi.container
exports org.eclipse.osgi.container.builders
exports org.eclipse.osgi.container.namespaces
exports org.eclipse.osgi.framework.console
exports org.eclipse.osgi.framework.eventmgr
exports org.eclipse.osgi.framework.log
exports org.eclipse.osgi.launch
exports org.eclipse.osgi.report.resolution
exports org.eclipse.osgi.service.datalocation
exports org.eclipse.osgi.service.debug
exports org.eclipse.osgi.service.environment
exports org.eclipse.osgi.service.localization
exports org.eclipse.osgi.service.pluginconversion
exports org.eclipse.osgi.service.resolver
exports org.eclipse.osgi.service.runnable
exports org.eclipse.osgi.service.security
exports org.eclipse.osgi.service.urlconversion
exports org.eclipse.osgi.signedcontent
exports org.eclipse.osgi.storagemanager
exports org.eclipse.osgi.util
exports org.osgi.dto
exports org.osgi.framework
exports org.osgi.framework.connect
exports org.osgi.framework.dto
exports org.osgi.framework.hooks.bundle
exports org.osgi.framework.hooks.resolver
exports org.osgi.framework.hooks.service
exports org.osgi.framework.hooks.weaving
exports org.osgi.framework.launch
exports org.osgi.framework.namespace
exports org.osgi.framework.startlevel
exports org.osgi.framework.startlevel.dto
exports org.osgi.framework.wiring
exports org.osgi.framework.wiring.dto
exports org.osgi.resource
exports org.osgi.resource.dto
exports org.osgi.service.condpermadmin
exports org.osgi.service.log
exports org.osgi.service.log.admin
exports org.osgi.service.packageadmin
exports org.osgi.service.permissionadmin
exports org.osgi.service.resolver
exports org.osgi.service.startlevel
exports org.osgi.service.url
exports org.osgi.util.tracker
requires java.base mandated
requires java.xml
requires jdk.unsupported static
uses org.osgi.framework.connect.FrameworkUtilHelper
provides org.osgi.framework.connect.ConnectFrameworkFactory with org.eclipse.osgi.launch.EquinoxFactory
provides org.osgi.framework.launch.FrameworkFactory with org.eclipse.osgi.launch.EquinoxFactory

Currently it looks like this:

jar --file=modules/org.eclipse.osgi-3.18.100.v20220620-1437.jar --describe-module
org.eclipse.osgi@3.18.100 jar:file:///home/tjwatson/dev/apache/issues/atomos/60/modules/org.eclipse.osgi-3.18.100.v20220620-1437.jar/!module-info.class open
requires java.base
requires java.xml
requires jdk.unsupported
contains org.apache.felix.resolver
contains org.apache.felix.resolver.reason
contains org.apache.felix.resolver.util
contains org.eclipse.core.runtime.adaptor
contains org.eclipse.core.runtime.internal.adaptor
contains org.eclipse.equinox.log
contains org.eclipse.osgi.container
contains org.eclipse.osgi.container.builders
contains org.eclipse.osgi.container.namespaces
contains org.eclipse.osgi.framework.console
contains org.eclipse.osgi.framework.eventmgr
contains org.eclipse.osgi.framework.internal.reliablefile
contains org.eclipse.osgi.framework.log
contains org.eclipse.osgi.framework.util
contains org.eclipse.osgi.internal.cds
contains org.eclipse.osgi.internal.connect
contains org.eclipse.osgi.internal.container
contains org.eclipse.osgi.internal.debug
contains org.eclipse.osgi.internal.framework
contains org.eclipse.osgi.internal.framework.legacy
contains org.eclipse.osgi.internal.hookregistry
contains org.eclipse.osgi.internal.hooks
contains org.eclipse.osgi.internal.loader
contains org.eclipse.osgi.internal.loader.buddy
contains org.eclipse.osgi.internal.loader.classpath
contains org.eclipse.osgi.internal.loader.sources
contains org.eclipse.osgi.internal.location
contains org.eclipse.osgi.internal.log
contains org.eclipse.osgi.internal.messages
contains org.eclipse.osgi.internal.permadmin
contains org.eclipse.osgi.internal.provisional.service.security
contains org.eclipse.osgi.internal.provisional.verifier
contains org.eclipse.osgi.internal.service.security
contains org.eclipse.osgi.internal.serviceregistry
contains org.eclipse.osgi.internal.signedcontent
contains org.eclipse.osgi.internal.url
contains org.eclipse.osgi.internal.util
contains org.eclipse.osgi.internal.weaving
contains org.eclipse.osgi.launch
contains org.eclipse.osgi.report.resolution
contains org.eclipse.osgi.service.datalocation
contains org.eclipse.osgi.service.debug
contains org.eclipse.osgi.service.environment
contains org.eclipse.osgi.service.localization
contains org.eclipse.osgi.service.pluginconversion
contains org.eclipse.osgi.service.resolver
contains org.eclipse.osgi.service.runnable
contains org.eclipse.osgi.service.security
contains org.eclipse.osgi.service.urlconversion
contains org.eclipse.osgi.signedcontent
contains org.eclipse.osgi.storage
contains org.eclipse.osgi.storage.bundlefile
contains org.eclipse.osgi.storage.url
contains org.eclipse.osgi.storage.url.bundleentry
contains org.eclipse.osgi.storage.url.bundleresource
contains org.eclipse.osgi.storage.url.reference
contains org.eclipse.osgi.storagemanager
contains org.eclipse.osgi.util
contains org.osgi.dto
contains org.osgi.framework
contains org.osgi.framework.connect
contains org.osgi.framework.dto
contains org.osgi.framework.hooks.bundle
contains org.osgi.framework.hooks.resolver
contains org.osgi.framework.hooks.service
contains org.osgi.framework.hooks.weaving
contains org.osgi.framework.launch
contains org.osgi.framework.namespace
contains org.osgi.framework.startlevel
contains org.osgi.framework.startlevel.dto
contains org.osgi.framework.wiring
contains org.osgi.framework.wiring.dto
contains org.osgi.resource
contains org.osgi.resource.dto
contains org.osgi.service.condition
contains org.osgi.service.condpermadmin
contains org.osgi.service.log
contains org.osgi.service.log.admin
contains org.osgi.service.packageadmin
contains org.osgi.service.permissionadmin
contains org.osgi.service.resolver
contains org.osgi.service.startlevel
contains org.osgi.service.url
contains org.osgi.util.tracker

It exports nothing, not sure why it has the contains stuff, but probably ok (but not necessary), it doesn't have the uses or provides stuff. I'll look at how to get BND to produce the correct stuff here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in:framework Issues related to the OSGi Framework implementation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants