Re: [PR] SLING-11906 Migrate to slf4j 2.x [sling-org-apache-sling-commons-log]
rombert commented on PR #18: URL: https://github.com/apache/sling-org-apache-sling-commons-log/pull/18#issuecomment-2107505437 > I'm not sure what you mean by a "large number" of source files? There are only 4 files under src/main/java/ch/qos/logback and only 1 of those files copies any code blocks from the original vendor. The other three extend the original classes and add to it or override to provide a different implementation. Oh, I misread your PR. I thought that the large number of changes was due to copying logback code; my mistake. > Anyways with that said, I am not interested in spending any more time on solving this in another way That is a shame, but I can understand your reasoning. Let's please keep this open for now and see if anyone else is willing to extend it and - indeed - review it. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] SLING-11906 Migrate to slf4j 2.x [sling-org-apache-sling-commons-log]
enapps-enorman commented on PR #18: URL: https://github.com/apache/sling-org-apache-sling-commons-log/pull/18#issuecomment-2099081961 > Regarding the slf4j-api - could we not also register the capability ourselves, since we provide it and bypass the serviceloader? It seems like that would interfere with other usages of the serviceloader so I am not inclined to spend any time on that. > At this point I am wary of importing a large number of source files in the Sling Git repository I'm not sure what you mean by a "large number" of source files? There are only 4 files under src/main/java/ch/qos/logback and only 1 of those files copies any code blocks from the original vendor. The other three extend the original classes and add to it or override to provide a different implementation. Anyways with that said, I am not interested in spending any more time on solving this in another way. If you want to do that work then please go ahead and make a proposal to prove your theory. I'm perfectly happy just managing and using my own fork at this point since I don't anticipate it needing much maintenance in the future. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] SLING-11906 Migrate to slf4j 2.x [sling-org-apache-sling-commons-log]
rombert commented on PR #18: URL: https://github.com/apache/sling-org-apache-sling-commons-log/pull/18#issuecomment-2098321478 @enapps-enorman Regarding the slf4j-api - could we not also register the capability ourselves, since we provide it and bypass the serviceloader? Regarding the logback contribution - I am not sure if/how we can expedite the logback contribution. At this point I am wary of importing a large number of source files in the Sling Git repository; vendoring is not that easy and you can see on https://github.com/apache/sling-org-apache-sling-commons-json/tree/maintenance how we need to make changes and track them. I fully understand your desire to not do more work unless really required. I can think of alternative ways of approaching the logback issue, in case someone has the desire to explore them. We ended up using reflection in the Sling XSS bundle when upstream was unresponsive ( https://github.com/OWASP/java-html-sanitizer/issues/275 ) but we could also consider using Weaving Hooks ( https://docs.osgi.org/specification/osgi.core/7.0.0/framework.weavinghooks.html ) . -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] SLING-11906 Migrate to slf4j 2.x [sling-org-apache-sling-commons-log]
enapps-enorman commented on PR #18: URL: https://github.com/apache/sling-org-apache-sling-commons-log/pull/18#issuecomment-2088735403 > slf4j.provider @rombert After further review, using slf4j-api without the service mediator as described in my last comment would also require changes to the slf4j-api manifest as it currently does not declare the serviceloader capabilities as optional. For example: `Require-Capability: osgi.extender;filter:="(&(osgi.extender=osgi.serviceloader.processor)(version>=1.0.0)(!(version>=2.0.0)))", osgi.serviceloader;filter:="(osgi.serviceloader=org.slf4j.spi.SLF4JServiceProvider)";osgi.serviceloader="org.slf4j.spi.SLF4JServiceProvider"` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] SLING-11906 Migrate to slf4j 2.x [sling-org-apache-sling-commons-log]
enapps-enorman commented on PR #18: URL: https://github.com/apache/sling-org-apache-sling-commons-log/pull/18#issuecomment-2077746890 > 1. ServiceLoder (+mediator) ... > Would it be possible to short-circuit the ServiceLoader in a similar manner to the work we do for Commons Johnzon? That would keep deployments lean and do less unneeded work. The ServiceLoader mediator availability isn't really a problem for me since I have to have it around anyway for the various jetty-* bundles that I use that require it. Also, the sling starter already includes the mediator bundles for groovy, so... In my opinion, trying to do that kind of ServiceLoader workaround is a losing battle. I think what you are suggesting is to repackage the ServiceLoader consumers and providers into the same bundle so there is no classloading visibility problems. The problem with that is that someone has to be paying attention whenever any of those third-party bundles has a new release and then do a new release of the o.a.sling.commons.log with the new bits. That didn't happen the last time which led to the embedded slf4j / logback classes in the o.a.sling.commons.log bundle drifting years out of date. With that said, I do see that there is one other option described in the slf4j FAQ that might work without the mediator: https://www.slf4j.org/faq.html#changesInVersion200 > SINCE 2.0.9 You can specify the provider class explicitly via the "slf4j.provider" system property. This bypasses the service loader mechanism for finding providers and may shorten SLF4J initialization. I believe the extra work to do that would be to configure the "slf4j.provider" framework property, export the "org.apache.sling.commons.log.logback.spi" package from the o.a.sling.commons.log bundle and then deploy a fragment bundle attached to the slf4j-api host bundle that augments the Import-Package instruction to include the "org.apache.sling.commons.log.logback.spi" package to make it visible to the slf4j-api classloader. If that works then the choice to use the ServiceLoader mediator or the framework-property + fragment bundle could be decided by whoever is constructing the application. Let me know if that would be acceptable to you. > 2. The classloading issues from Logback It has been 2 weeks since I opened the PR proposal to logback and there has been no response at all. Do you know anyone over there would could review it and give a yes or no answer? I'm not really interested in wasting more of my time on alternate ways to do the same thing. If you want to try some other approach then please provide a patch with the changes you think will work better. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] SLING-11906 Migrate to slf4j 2.x [sling-org-apache-sling-commons-log]
rombert commented on PR #18: URL: https://github.com/apache/sling-org-apache-sling-commons-log/pull/18#issuecomment-2076491663 > For your reference, I stashed my workaround fragment bundle project at: https://github.com/enapps-enorman/sling-org-apache-sling-slf4j1x-compatibility On that topic, I think the bundle may be useful in case we are ready with the switch before Oak is, thank you for preparing that. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] SLING-11906 Migrate to slf4j 2.x [sling-org-apache-sling-commons-log]
sonarcloud[bot] commented on PR #18: URL: https://github.com/apache/sling-org-apache-sling-commons-log/pull/18#issuecomment-2076055412 ## [](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-commons-log=18) **Quality Gate passed** Issues  [15 New issues](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-commons-log=18=false=true)  [0 Accepted issues](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-commons-log=18=new_accepted_issues=list) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-commons-log=18=false=true)  [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-commons-log=18=new_coverage=list)  [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-commons-log=18=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-commons-log=18) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] SLING-11906 Migrate to slf4j 2.x [sling-org-apache-sling-commons-log]
rombert commented on code in PR #18:
URL:
https://github.com/apache/sling-org-apache-sling-commons-log/pull/18#discussion_r1574507607
##
src/main/java/ch/qos/logback/classic/PatternLayoutOsgi.java:
##
@@ -0,0 +1,183 @@
+/**
+ * Logback: the reliable, generic, fast and flexible logging framework.
+ * Copyright (C) 1999-2015, QOS.ch. All rights reserved.
+ *
+ * This program and the accompanying materials are dual-licensed under
+ * either the terms of the Eclipse Public License v1.0 as published by
+ * the Eclipse Foundation
+ *
+ * or (per the licensee's choosing)
+ *
+ * under the terms of the GNU Lesser General Public License version 2.1
+ * as published by the Free Software Foundation.
+ */
+package ch.qos.logback.classic;
+
+import java.util.HashMap;
+import java.util.Map;
+
+import ch.qos.logback.classic.pattern.*;
Review Comment:
@rishabhdaim - thanks for reviewing this PR. IIUC, @enapps-enorman has
'vendored'/copied over code from a third party library. If we decide to go this
way I think we should keep it unchanged, deviations make it hard to compare
with upstream and resync with later versions, if needed.
##
src/main/java/ch/qos/logback/classic/PatternLayoutOsgi.java:
##
@@ -0,0 +1,183 @@
+/**
+ * Logback: the reliable, generic, fast and flexible logging framework.
+ * Copyright (C) 1999-2015, QOS.ch. All rights reserved.
+ *
+ * This program and the accompanying materials are dual-licensed under
+ * either the terms of the Eclipse Public License v1.0 as published by
+ * the Eclipse Foundation
+ *
+ * or (per the licensee's choosing)
+ *
+ * under the terms of the GNU Lesser General Public License version 2.1
+ * as published by the Free Software Foundation.
+ */
+package ch.qos.logback.classic;
+
+import java.util.HashMap;
+import java.util.Map;
+
+import ch.qos.logback.classic.pattern.*;
+import ch.qos.logback.classic.pattern.color.HighlightingCompositeConverter;
+import ch.qos.logback.classic.spi.ILoggingEvent;
+import ch.qos.logback.core.CoreConstants;
+import ch.qos.logback.core.pattern.PatternLayoutBaseOsgi;
+import ch.qos.logback.core.pattern.color.*;
+import ch.qos.logback.core.pattern.parser.Parser;
+
+/**
+ *
+ * A flexible layout configurable with pattern string. The goal of this class
is
+ * to {@link #format format} a {@link ILoggingEvent} and return the results in
a
+ * {#link String}. The format of the result depends on the conversion
+ * pattern.
+ *
+ * For more information about this layout, please refer to the online manual at
+ * http://logback.qos.ch/manual/layouts.html#PatternLayout
+ *
+ */
+
+public class PatternLayoutOsgi extends PatternLayoutBaseOsgi {
+
+public static final Map DEFAULT_CONVERTER_MAP = new
HashMap<>();
Review Comment:
Same as
https://github.com/apache/sling-org-apache-sling-commons-log/pull/18#discussion_r1574507607
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@sling.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] SLING-11906 Migrate to slf4j 2.x [sling-org-apache-sling-commons-log]
rishabhdaim commented on code in PR #18:
URL:
https://github.com/apache/sling-org-apache-sling-commons-log/pull/18#discussion_r1574314774
##
src/main/java/ch/qos/logback/classic/PatternLayoutOsgi.java:
##
@@ -0,0 +1,183 @@
+/**
+ * Logback: the reliable, generic, fast and flexible logging framework.
+ * Copyright (C) 1999-2015, QOS.ch. All rights reserved.
+ *
+ * This program and the accompanying materials are dual-licensed under
+ * either the terms of the Eclipse Public License v1.0 as published by
+ * the Eclipse Foundation
+ *
+ * or (per the licensee's choosing)
+ *
+ * under the terms of the GNU Lesser General Public License version 2.1
+ * as published by the Free Software Foundation.
+ */
+package ch.qos.logback.classic;
+
+import java.util.HashMap;
+import java.util.Map;
+
+import ch.qos.logback.classic.pattern.*;
+import ch.qos.logback.classic.pattern.color.HighlightingCompositeConverter;
+import ch.qos.logback.classic.spi.ILoggingEvent;
+import ch.qos.logback.core.CoreConstants;
+import ch.qos.logback.core.pattern.PatternLayoutBaseOsgi;
+import ch.qos.logback.core.pattern.color.*;
+import ch.qos.logback.core.pattern.parser.Parser;
+
+/**
+ *
+ * A flexible layout configurable with pattern string. The goal of this class
is
+ * to {@link #format format} a {@link ILoggingEvent} and return the results in
a
+ * {#link String}. The format of the result depends on the conversion
+ * pattern.
+ *
+ * For more information about this layout, please refer to the online manual at
+ * http://logback.qos.ch/manual/layouts.html#PatternLayout
+ *
+ */
+
+public class PatternLayoutOsgi extends PatternLayoutBaseOsgi {
+
+public static final Map DEFAULT_CONVERTER_MAP = new
HashMap<>();
Review Comment:
We should provide the estimated size to `HashMap` to avoid resizing.
##
src/main/java/ch/qos/logback/classic/PatternLayoutOsgi.java:
##
@@ -0,0 +1,183 @@
+/**
+ * Logback: the reliable, generic, fast and flexible logging framework.
+ * Copyright (C) 1999-2015, QOS.ch. All rights reserved.
+ *
+ * This program and the accompanying materials are dual-licensed under
+ * either the terms of the Eclipse Public License v1.0 as published by
+ * the Eclipse Foundation
+ *
+ * or (per the licensee's choosing)
+ *
+ * under the terms of the GNU Lesser General Public License version 2.1
+ * as published by the Free Software Foundation.
+ */
+package ch.qos.logback.classic;
+
+import java.util.HashMap;
+import java.util.Map;
+
+import ch.qos.logback.classic.pattern.*;
Review Comment:
I would avoid using `*` in imports
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@sling.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] SLING-11906 Migrate to slf4j 2.x [sling-org-apache-sling-commons-log]
sonarcloud[bot] commented on PR #18: URL: https://github.com/apache/sling-org-apache-sling-commons-log/pull/18#issuecomment-2065428082 ## [](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-commons-log=18) **Quality Gate passed** Issues  [15 New issues](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-commons-log=18=false=true)  [0 Accepted issues](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-commons-log=18=new_accepted_issues=list) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-commons-log=18=false=true)  [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-commons-log=18=new_coverage=list)  [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-commons-log=18=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-commons-log=18) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] SLING-11906 Migrate to slf4j 2.x [sling-org-apache-sling-commons-log]
sonarcloud[bot] commented on PR #18: URL: https://github.com/apache/sling-org-apache-sling-commons-log/pull/18#issuecomment-2064825334 ## [](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-commons-log=18) **Quality Gate passed** Issues  [16 New issues](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-commons-log=18=false=true)  [0 Accepted issues](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-commons-log=18=new_accepted_issues=list) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-commons-log=18=false=true)  [88.8% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-commons-log=18=new_coverage=list)  [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-commons-log=18=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-commons-log=18) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] SLING-11906 Migrate to slf4j 2.x [sling-org-apache-sling-commons-log]
rombert commented on PR #18: URL: https://github.com/apache/sling-org-apache-sling-commons-log/pull/18#issuecomment-2063484884 Thanks a lot for preparing this @enapps-enorman ! I'll try to take a look in the coming days. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] SLING-11906 Migrate to slf4j 2.x [sling-org-apache-sling-commons-log]
sonarcloud[bot] commented on PR #18: URL: https://github.com/apache/sling-org-apache-sling-commons-log/pull/18#issuecomment-2062945983 ## [](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-commons-log=18) **Quality Gate passed** Issues  [16 New issues](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-commons-log=18=false=true)  [0 Accepted issues](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-commons-log=18=new_accepted_issues=list) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-commons-log=18=false=true)  [88.8% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-commons-log=18=new_coverage=list)  [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-commons-log=18=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-commons-log=18) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] SLING-11906 Migrate to slf4j 2.x [sling-org-apache-sling-commons-log]
sonarcloud[bot] commented on PR #18: URL: https://github.com/apache/sling-org-apache-sling-commons-log/pull/18#issuecomment-2062939330 ## [](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-commons-log=18) **Quality Gate failed** Failed conditions  [2 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-commons-log=18=false=true) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-commons-log=18) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] SLING-11906 Migrate to slf4j 2.x [sling-org-apache-sling-commons-log]
sonarcloud[bot] commented on PR #18: URL: https://github.com/apache/sling-org-apache-sling-commons-log/pull/18#issuecomment-2062929582 ## [](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-commons-log=18) **Quality Gate failed** Failed conditions  [2 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-commons-log=18=false=true) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-commons-log=18) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] SLING-11906 Migrate to slf4j 2.x [sling-org-apache-sling-commons-log]
sonarcloud[bot] commented on PR #18: URL: https://github.com/apache/sling-org-apache-sling-commons-log/pull/18#issuecomment-2062856322 ## [](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-commons-log=18) **Quality Gate failed** Failed conditions  [2 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-commons-log=18=false=true)  [67.5% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-commons-log=18=new_coverage=list) (required ≥ 80%) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-commons-log=18) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] SLING-11906 Migrate to slf4j 2.x [sling-org-apache-sling-commons-log]
sonarcloud[bot] commented on PR #18: URL: https://github.com/apache/sling-org-apache-sling-commons-log/pull/18#issuecomment-2062848693 ## [](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-commons-log=18) **Quality Gate failed** Failed conditions  [2 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-commons-log=18=false=true)  [67.5% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-commons-log=18=new_coverage=list) (required ≥ 80%) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-commons-log=18) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] SLING-11906 Migrate to slf4j 2.x [sling-org-apache-sling-commons-log]
sonarcloud[bot] commented on PR #18: URL: https://github.com/apache/sling-org-apache-sling-commons-log/pull/18#issuecomment-2062813806 ## [](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-commons-log=18) **Quality Gate failed** Failed conditions  [2 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-commons-log=18=false=true)  [67.5% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-commons-log=18=new_coverage=list) (required ≥ 80%) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-commons-log=18) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] SLING-11906 Migrate to slf4j 2.x [sling-org-apache-sling-commons-log]
sonarcloud[bot] commented on PR #18: URL: https://github.com/apache/sling-org-apache-sling-commons-log/pull/18#issuecomment-2062017247 ## [](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-commons-log=18) **Quality Gate failed** Failed conditions  [2 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-commons-log=18=false=true)  [67.4% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-commons-log=18=new_coverage=list) (required ≥ 80%) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-commons-log=18) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[PR] SLING-11906 Migrate to slf4j 2.x [sling-org-apache-sling-commons-log]
enapps-enorman opened a new pull request, #18:
URL: https://github.com/apache/sling-org-apache-sling-commons-log/pull/18
Increasingly more libraries have been migrating to slf4j 2.x (for example
logback 1.3+, tika 2.5+ and jetty 10+)
To be compatible with those, the sling commons log bundle should migrate to
slf4j v2.x and logback v1.5.x
The changes proposed here is an attempt to do the refactoring to migrate to
the newer versions.
Here are some differences in the proposal:
-
1. Per: https://www.slf4j.org/faq.html#changesInVersion200
SLF4J 2.x has switched to use the java.util.ServiceLoader to locate the
logging implementation so we now require a service loader mediator
implementation in order for the slf4j-api to find the logback implementation.
2. I have tried to avoid embedding the slf4j and logback classes inside of
the org.apache.sling.commons.log bundle and instead just use the bundles that
were already provided by those other organizations. That approach mostly
worked, but I did encounter one classloading (visiblity) problem from our
MaskingMessageUtil customizations where the class names were being passed to
logback and loaded via reflection. I was also able to create a temporary
workaround by forking and updating the logback PatternLayout (and related)
classes to additionally pass a Supplier instead of a classname for that.
I proposed a change to logback that would make my forked workaround
unnecessary which is tracked at: https://github.com/qos-ch/logback/issues/803
3. There still exists a problem with some of the OAK bundles importing the
org.slf4j.event with an old version that is not exported in the SLF4j 2.x
implementation. I was able to workaround this by creating a fragment bundle
that is attached to slf4j-api and exports the 1.7.36 version of the
org.slf4j.event bundle. Hopefully OAK can be modified to remove the dependency
on org.slf4j.event?
I believe this is tracked as:
https://issues.apache.org/jira/browse/OAK-10339
The result of this problem is errors reported by the feature analyzer
like this:
[ERROR] [bundle-packages] org.apache.jackrabbit:oak-lucene:1.60.0:
Bundle is importing package org.slf4j.event;version=[1.7,2) with start order 15
but no bundle is exporting these for that start order in the required version
range.
[ERROR] [bundle-packages]
org.apache.jackrabbit:oak-store-document:1.60.0: Bundle is importing package
org.slf4j.event;version=[1.7,2) with start order 15 but no bundle is exporting
these for that start order in the required version range.
[ERROR] [bundle-packages] org.apache.jackrabbit:oak-commons:1.60.0:
Bundle is importing package org.slf4j.event;version=[1.7,2) with start order 15
but no bundle is exporting these for that start order in the required version
range.
[ERROR] [bundle-packages] org.apache.jackrabbit:oak-core:1.60.0: Bundle
is importing package org.slf4j.event;version=[1.7,2) with start order 15 but no
bundle is exporting these for that start order in the required version range.
[ERROR] [bundle-packages] org.apache.jackrabbit:oak-query-spi:1.60.0:
Bundle is importing package org.slf4j.event;version=[1.7,2) with start order 15
but no bundle is exporting these for that start order in the required version
range.
4. Integrating this into the starter (or other sling based application)
requires this set of bundles in the boot.json feature descriptor:
{
"id":"org.ow2.asm:asm-analysis:${asm.version}",
"start-order":"1"
},
{
"id":"org.ow2.asm:asm-commons:${asm.version}",
"start-order":"1"
},
{
"id":"org.ow2.asm:asm-tree:${asm.version}",
"start-order":"1"
},
{
"id":"org.ow2.asm:asm-util:${asm.version}",
"start-order":"1"
},
{
"id":"org.ow2.asm:asm:${asm.version}",
"start-order":"1"
},
{
"id":"org.apache.aries.spifly:org.apache.aries.spifly.dynamic.bundle:1.3.7",
"start-order":"1"
},
{
"id":"org.apache.sling:org.apache.sling.commons.log:6.0.0-SNAPSHOT",
"start-order":"1"
},
{
"id":"ch.qos.logback:logback-core:${logback.version}",
"start-order":"1"
},
{
"id":"ch.qos.logback:logback-classic:${logback.version}",
"start-order":"1"
},
{
"id":"org.slf4j:jul-to-slf4j:${slf4j.version}",
"start-order":"1"
},
{
"id":"org.slf4j:jcl-over-slf4j:${slf4j.version}",
"start-order":"1"
},
{
