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

Move Aries SPIFly bundle after simpleconfigurator to start it afterwards #1252

Merged
merged 1 commit into from
Aug 15, 2023

Conversation

HannesWell
Copy link
Member

Fixes eclipse-equinox/equinox#302 (hopefully).

Copy link
Contributor

@laeubi laeubi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is sufficient. Relying on the implementation to write it in the order of the product (and Equinox reading and starting/install it in that order) is waiting for problems to happen. Beside this, it would not be possible to update spifly as far as I know.

Why not move spyfly to level 2 as its done with ds, event and alike?

@HannesWell
Copy link
Member Author

Changing the order in the configuration section didn't Change the generated config.ini.

Why not move spyfly to level 2 as its done with ds, event and alike?

Yes that's the only releiable thing left. We have to hope that then all bundels that need the Mediator are activated in a later level.

To migitate that we should switch to a Framework-Extension in the next release, ideally one that does not need asm. :)

Since we already missed the deadline we need Project-lead or PMC approval.
@iloveeclipse, @akurtakov are you fine with this?

Please note that this does not affect any bundle.

@iloveeclipse
Copy link
Member

Is this a problem with some new feature?
Could you please provide a link to original change so one can understand what we are talking about here.

@HannesWell
Copy link
Member Author

Is this a problem with some new feature?

Not directly a feature but with a new configuration necessary for slf4j-2 and IIRC now also others that use ServiceLoader.

Could you please provide a link to original change so one can understand what we are talking about here.

Of course, it was #1195

@iloveeclipse
Copy link
Member

iloveeclipse commented Aug 15, 2023

Does the #1195 actually work?
I see following printed to the console if starting SDK latest with egit that uses slf4j:

Aug 15, 2023 1:13:57 PM org.apache.aries.spifly.BaseActivator log
INFO: Registered provider org.slf4j.simple.SimpleServiceProvider of service org.slf4j.spi.SLF4JServiceProvider in bundle slf4j.simple
SLF4J: No SLF4J providers were found.
SLF4J: Defaulting to no-operation (NOP) logger implementation
SLF4J: See https://www.slf4j.org/codes.html#noProviders for further details.

(note: I've set -Dorg.slf4j.simpleLogger.defaultLogLevel=on in eclipse.ini for testing, but see no difference to the "off")

Copy link
Member

@iloveeclipse iloveeclipse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If that fixes the error, let's merge.

Copy link
Contributor

@laeubi laeubi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

@HannesWell
Copy link
Member Author

HannesWell commented Aug 15, 2023

Does the #1195 actually work?
I see following printed to the console if starting SDK latest with egit that uses slf4j:

Aug 15, 2023 1:13:57 PM org.apache.aries.spifly.BaseActivator log
INFO: Registered provider org.slf4j.simple.SimpleServiceProvider of service org.slf4j.spi.SLF4JServiceProvider in bundle slf4j.simple
SLF4J: No SLF4J providers were found.
SLF4J: Defaulting to no-operation (NOP) logger implementation
SLF4J: See https://www.slf4j.org/codes.html#noProviders for further details.

The INFO: Registered provider org.slf4j.simple.SimpleServiceProvider of service org.slf4j.spi.SLF4JServiceProvider in bundle slf4j.simple print-out tells that at least aries was successfully started and slf4j.simple was registered.
But you shouldn't see SLF4J: No SLF4J providers were found.
Currently I'm on a train for a short vacation till thursday. If that remains I can look into that when back.

(note: I've set -Dorg.slf4j.simpleLogger.defaultLogLevel=on in eclipse.ini for testing, but see no difference to the "off")

On is not a valid log-level. I have described a possible configuration and linked the slf4j-doc in #1195 (comment), which
also uses jgit as an example and that worked in my Testing.

@HannesWell HannesWell merged commit bb805d3 into eclipse-platform:master Aug 15, 2023
1 of 2 checks passed
@HannesWell HannesWell deleted the moveAries branch August 15, 2023 12:06
@iloveeclipse
Copy link
Member

I have described a possible configuration and linked the slf4j-doc in #1195 (comment), which
also uses jgit as an example and that worked in my Testing.

It would be good to add this to N&N section.

@jcompagner
Copy link

The INFO: Registered provider org.slf4j.simple.SimpleServiceProvider of service org.slf4j.spi.SLF4JServiceProvider in bundle slf4j.simple print-out tells that at least aries was successfully started and slf4j.simple was registered.
But you shouldn't see SLF4J: No SLF4J providers were found.
Currently I'm on a train for a short vacation till thursday. If that remains I can look into that when back.

Is by accident also not slf4j 1.7 installed (so both?) I think you will see this double output

@jcompagner
Copy link

.

Why not move spyfly to level 2 as its done with ds, event and alike?

We use in our product even lvl 3 already for years for this, but I guess it greatly depends on where slf4j is used in more core stuff.

What I do wonder why Aries needs asm.. not for stuf where we really use it for right?

I have a feeling that this service loader thing should really just be done in the core implementation of osgi.. (so not even needing Aries)

@HannesWell
Copy link
Member Author

.

Why not move spyfly to level 2 as its done with ds, event and alike?

We use in our product even lvl 3 already for years for this, but I guess it greatly depends on where slf4j is used in more core stuff.

The Eclipse SDK neither uses SLF4J loggers nor service loaders but the earlier it is started the better, just to be sure.

What I do wonder why Aries needs asm.. not for stuf where we really use it for right?

Aries transforms the class-files of classes that use the ServiceLoader in Order to inject special classloaders that consider the OSGi environment.

I have a feeling that this service loader thing should really just be done in the core implementation of osgi.. (so not even needing Aries)

Me too. You might be interested in eclipse-equinox/equinox#295

@HannesWell
Copy link
Member Author

I have described a possible configuration and linked the slf4j-doc in #1195 (comment), which
also uses jgit as an example and that worked in my Testing.

It would be good to add this to N&N section.

SLF4J-simple is not used in all packages, many use logback through m2e.logback, therefore I'm not sure that's necessary respectivly could be confusing.

@iloveeclipse
Copy link
Member

But how the users of SLF4J-simple should know about the way to configure SDK or more general: that they can now configure SLF4J-simple in the SDK? Or do I miss something and it was always possible but then I don't understand the change anymore?

@HannesWell
Copy link
Member Author

But how the users of SLF4J-simple should know about the way to configure SDK or more general: that they can now configure SLF4J-simple in the SDK? Or do I miss something and it was always possible but then I don't understand the change anymore?

I don't know for sure how it was with SLF4J from Orbit was, but from a quick look at https://download.eclipse.org/eclipse/updates/4.25/R-4.25-202208311800/ there was no binding at all. Since slf4j from Maven there was only the noop Binding/Provider present. So no it was'nt possible.

For the SDK it would probably be good yes, but I hestitate a bit because in some EPP packages the logback logging backend supplied by m2e.logback is used, which supplied a logback configuration.
Altough it is actually a implementation-detail many rely on it. And if we add doc for the SDK we should tell the difference. But I don't want to make the M2E.logback config permanent.
So that should be made clear.

buchen added a commit to portfolio-performance/portfolio that referenced this pull request Oct 18, 2024
Otherwise requirements (such as org.objectweb.asm) are not available.

See eclipse-platform/eclipse.platform.releng.aggregator#1252
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 this pull request may close these issues.

Could not resolve module: org.apache.aries.spifly.dynamic.bundle
4 participants