-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
#1367 Added missing activation policy #1429
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
Conversation
|
@HannesWell this is the issue we talked about, where the activation policies are missing for the automatic bundle start |
Thanks for the link. This change looks good to me. This really simplifies using log4j-2 in OSGi/Eclipse-based applications. 👍🏽 Additionally the activation policy should also be added to the log4j-to-slf4j bundle: logging-log4j2/log4j-to-slf4j/pom.xml Lines 98 to 102 in 83bba1b
@ppkarwasz could you please have a look at this one? @N1k145 what might be missing is a change-log entry. I think if you create one like in #1375, this should speed-up the review. |
|
@HannesWell thanks for the hint, I added the changelog entry. |
|
@HannesWell and @N1k145, I don't work with OSGI on a daily basis, so I need time to setup a testing environment. Since Hannes confirms that this solution works, I guess I could skip this step. What happens if someone puts |
|
We do have OSGi tests in the log4j-osgi module. Would it be possible to add a test there? |
In theory you do not need to modify the start levels. With this change Equinox should activate the bundle when it is accessed and the exception should no longer be thrown as the bundle context is there and the So no need for any manual startlevel or autostart configuration and OSGi just has to do it's magic. But feel free to test it, I only tested it in our quite large setup, maybe it behaves differently in other cases. |
I have not yet tried out this explicit change, but I remember from that the past that I tried something similar (if not the same) already for the same reason.
As Niklas said, this should not cause harm. If you are interested in the details, chapter 4.4.6.2 Additionally as far as I know, Equinox has the specialty (I don't know how other implementations like Felix behave in this regard) to put bundles that have the |
|
After some tests it appears that as long as Moreover:
Anyway the PR looks good to me, thanks. |
|
Just tested the recent snapshots that include the fix (I had less time than expected to test this before it was merged) and everything looks good to me as well. Maybe it would be worth to think about an approach that only relies on lazy activation so that one would not need to start any bundle explicitly, in another issue/PR. But this change already simplifies the setup. Thanks @N1k145 and @ppkarwasz for the review! |
|
@ppkarwasz or @rgoers can you estimate when the next release for the 2-major will happen? |
Added the missing activation policy for #1367, this is required to start the bundle and get the bundle context in an eclipse OSGI environment.
Checklist
2.xbranch if you are targeting Log4j 2; usemainotherwise./mvnw verifysucceeds (if it fails due to code formatting issues reported by Spotless, simply run./mvnw spotless:applyand retry)src/changelog/.2.x.xdirectory