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

Use OSGi Annotations to produce felix.log manifest and clean up. #359

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

wilx
Copy link
Contributor

@wilx wilx commented Dec 11, 2024

While I was working on reviving felix.logback I was going through the code and I wondered if it could be modernized a little bit. This is the result. I am not sure if it is useful for the project as I have no idea what minimum platform and JVM version it targets.

@stbischof
Copy link
Contributor

stbischof commented Dec 12, 2024

Just to validate.
Could you show Manifest before and after here as comments.

Updating parent pom seems okay.
Annotations are made for that. So also good.

Need to look deeper but think this is fine.

@stbischof stbischof changed the title Use BND to produce felix.log manifest and clean up. Use OSGi Annotations to produce felix.log manifest and clean up. Dec 12, 2024
log/pom.xml Outdated Show resolved Hide resolved
@wilx
Copy link
Contributor Author

wilx commented Dec 12, 2024

Could you show Manifest before and after here as comments.

It is slightly different. I will provide the comparison later today.

@laeubi
Copy link

laeubi commented Dec 12, 2024

While I was working on reviving felix.logback I was going through the code and I wondered if it could be modernized a little bit.

There is also now some github validation so you should enable that for your module as well, just duplicate the line with your module.

- name: Felix SCR

Copy link
Contributor

@rotty3000 rotty3000 left a comment

Choose a reason for hiding this comment

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

Just a few nits.

Besides that, thank you @wilx for breathing some life into this bundle. :)

@wilx wilx force-pushed the tiny-fixes2 branch 4 times, most recently from c7a0dae to 3362c1f Compare December 13, 2024 10:38
@wilx
Copy link
Contributor Author

wilx commented Dec 13, 2024

I have pushed some changes.

This is the difference between the 1.3.0 version and the branch snapshot version in MANIFEST.MF after those changes. I have made manually concatenated the MANIFEST.MF lines for easier comparison. There are differences in uses for the Provide-Capability.

--- scratch_1.MF        2024-12-13 11:35:27.643249474 +0100
+++ scratch_2.MF        2024-12-13 11:35:55.859447078 +0100
@@ -1,7 +1,5 @@
 Manifest-Version: 1.0
-Bnd-LastModified: 1684133251156
-Build-Jdk: 11.0.19
-Built-By: jbonofre
+Build-Jdk-Spec: 17
 Bundle-Activator: org.apache.felix.log.Activator
 Bundle-Description: A simple implementation of the OSGi R8 Log service
  .
@@ -11,8 +9,8 @@
 Bundle-Name: Apache Felix Log Service
 Bundle-SymbolicName: org.apache.felix.log
 Bundle-Vendor: The Apache Software Foundation
-Bundle-Version: 1.3.0
-Created-By: Apache Maven Bundle Plugin
+Bundle-Version: 1.3.1.SNAPSHOT
+Created-By: Apache Maven Bundle Plugin 5.1.9
 Export-Package: org.osgi.service.log;version="1.5";uses:="org.osgi.fra
  mework",org.osgi.service.log.admin;version="1.0";uses:="org.osgi.serv
  ice.log"
@@ -20,8 +18,9 @@
  k.wiring;version="[1.2,2)",org.osgi.service.log;version="[1.5,1.6)",o
  rg.osgi.service.log.admin;version="[1.0,1.1)",org.osgi.util.tracker;v
  ersion="[1.5,2)"
-Provide-Capability: osgi.service;objectClass:List<String>="org.osgi.service.log.LogReaderService";uses:="org.osgi.service.log,org.osgi.service.log.admin",
- osgi.service;objectClass:List<String>="org.osgi.service.log.LogService,org.osgi.service.log.LoggerFactory";uses:="org.osgi.service.log,org.osgi.service.log.admin",
- osgi.service;objectClass:List<String>="org.osgi.service.log.admin.LoggerAdmin";uses:="org.osgi.service.log,org.osgi.service.log.admin"
+Provide-Capability: osgi.service;objectClass:List<String>="org.osgi.service.log.LogReaderService";uses:="org.apache.felix.log,org.osgi.service.log",
+ osgi.service;objectClass:List<String>="org.osgi.service.log.LogService,org.osgi.service.log.LoggerFactory";uses:="org.apache.felix.log,org.osgi.service.log,org.osgi.service.log.admin",
+ osgi.service;objectClass:List<String>="org.osgi.service.log.admin.LoggerAdmin";uses:="org.apache.felix.log,org.osgi.service.log.admin"
 Require-Capability: osgi.service;filter:="(objectClass=org.osgi.service.cm.ConfigurationAdmin)";effective:=active,osgi.ee;filter:="(&(osgi.ee=JavaSE)(version=1.8))"
-Tool: Bnd-3.5.0.201709291849
+Tool: Bnd-6.3.1.202206071316
+

log/pom.xml Show resolved Hide resolved
Copy link
Contributor

@rotty3000 rotty3000 left a comment

Choose a reason for hiding this comment

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

LGMT (though I do wish we would use provided with the "build time" dependencies.)

@stbischof
Copy link
Contributor

stbischof commented Dec 17, 2024

@laeubi could you look on the ci?

@wilx i think just run pr against master is enough

Would be also greate to have a setup that runs also the original tck.

Copy link

@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 also think the branch spec should only be master at the moment, the rest of the CI change looks fine

.github/workflows/maven.yml Outdated Show resolved Hide resolved
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.

4 participants