Merge ~vpa1977/ubuntu/+source/libcommons-logging-java:disable-slf4j into ubuntu/+source/libcommons-logging-java:ubuntu/devel

Proposed by Vladimir Petko
Status: Merged
Merged at revision: c19d503778b27274bf571ddf213590db11057efa
Proposed branch: ~vpa1977/ubuntu/+source/libcommons-logging-java:disable-slf4j
Merge into: ubuntu/+source/libcommons-logging-java:ubuntu/devel
Diff against target: 531 lines (+497/-1)
4 files modified
debian/changelog (+7/-0)
debian/control (+2/-1)
debian/patches/09_disable_slf4j.patch (+487/-0)
debian/patches/series (+1/-0)
Reviewer Review Type Date Requested Status
Pushkar Kulkarni (community) Approve
git-ubuntu import Pending
Review via email: mp+461847@code.launchpad.net

Description of the change

This MP resolves circular dependency between libcommons-logging-java and slf4j.

PPA: ppa:vpa1977/plusone [1]

Changes:
 - Remove slf4j logging configuration provider introduced in libcommons-logging-java 1.3

Testing:
 - rebuild httpcomonents-client[2] and libslf4j-java[3]
 - piuparts test [3]

[1] https://launchpad.net/~vpa1977/+archive/ubuntu/plusone
[2] https://launchpad.net/~vpa1977/+archive/ubuntu/plusone-rebuild/+sourcepub/15840974/+listing-archive-extra
[3] https://launchpad.net/~vpa1977/+archive/ubuntu/plusone-rebuild/+sourcepub/15840973/+listing-archive-extra
[4] https://bugs.launchpad.net/debian/+source/httpcomponents-client/+bug/2055845/comments/1

To post a comment you must log in.
Revision history for this message
Pushkar Kulkarni (pushkarnk) wrote :

LGTM

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/debian/changelog b/debian/changelog
2index 8aee41e..1d94aa3 100644
3--- a/debian/changelog
4+++ b/debian/changelog
5@@ -1,3 +1,10 @@
6+libcommons-logging-java (1.3.0-1ubuntu1) noble; urgency=medium
7+
8+ * d/p/09_disable_slf4j.patch: disable commons-logging slf4j
9+ backend (LP: #2055845).
10+
11+ -- Vladimir Petko <vladimir.petko@canonical.com> Wed, 06 Mar 2024 10:36:48 +1300
12+
13 libcommons-logging-java (1.3.0-1) unstable; urgency=medium
14
15 * New upstream release
16diff --git a/debian/control b/debian/control
17index 8067e25..dcdfa13 100644
18--- a/debian/control
19+++ b/debian/control
20@@ -1,7 +1,8 @@
21 Source: libcommons-logging-java
22 Section: java
23 Priority: optional
24-Maintainer: Debian Java Maintainers <pkg-java-maintainers@lists.alioth.debian.org>
25+Maintainer: Ubuntu Developers <ubuntu-devel-discuss@lists.ubuntu.com>
26+XSBC-Original-Maintainer: Debian Java Maintainers <pkg-java-maintainers@lists.alioth.debian.org>
27 Uploaders:
28 Varun Hiremath <varun@debian.org>,
29 Emmanuel Bourg <ebourg@apache.org>,
30diff --git a/debian/patches/09_disable_slf4j.patch b/debian/patches/09_disable_slf4j.patch
31new file mode 100644
32index 0000000..c4f4441
33--- /dev/null
34+++ b/debian/patches/09_disable_slf4j.patch
35@@ -0,0 +1,487 @@
36+Description: disable slf4j logging backend
37+ commons-logging added support for slf4j backend that is incompatible
38+ with slf4j 1.7.x. Both logging subsystems try to get logger from
39+ each other, causing "IllegalState: Recursive Update" exception.
40+Author: Vladimir Petko <vladimir.petko@canonical.com>
41+Bug: https://lists.apache.org/thread/vgj87y00m5svz9ydv5grt3zp2z9hx2zx
42+Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/+source/libcommons-logging-java/+bug/2055845
43+Bug-Debian: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1061025
44+Forwarded: not-needed
45+Last-Update: 2024-03-06
46+
47+--- a/src/main/java/org/apache/commons/logging/impl/Slf4jLogFactory.java
48++++ /dev/null
49+@@ -1,322 +0,0 @@
50+-/*
51+- * Licensed to the Apache Software Foundation (ASF) under one or more
52+- * contributor license agreements. See the NOTICE file distributed with
53+- * this work for additional information regarding copyright ownership.
54+- * The ASF licenses this file to You under the Apache License, Version 2.0
55+- * (the "License"); you may not use this file except in compliance with
56+- * the License. You may obtain a copy of the License at
57+- *
58+- * http://www.apache.org/licenses/LICENSE-2.0
59+- *
60+- * Unless required by applicable law or agreed to in writing, software
61+- * distributed under the License is distributed on an "AS IS" BASIS,
62+- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
63+- * See the License for the specific language governing permissions and
64+- * limitations under the License.
65+- */
66+-package org.apache.commons.logging.impl;
67+-
68+-import static org.slf4j.spi.LocationAwareLogger.DEBUG_INT;
69+-import static org.slf4j.spi.LocationAwareLogger.ERROR_INT;
70+-import static org.slf4j.spi.LocationAwareLogger.INFO_INT;
71+-import static org.slf4j.spi.LocationAwareLogger.TRACE_INT;
72+-import static org.slf4j.spi.LocationAwareLogger.WARN_INT;
73+-
74+-import java.util.concurrent.ConcurrentHashMap;
75+-import java.util.concurrent.ConcurrentMap;
76+-
77+-import org.apache.commons.logging.Log;
78+-import org.apache.commons.logging.LogConfigurationException;
79+-import org.apache.commons.logging.LogFactory;
80+-import org.slf4j.ILoggerFactory;
81+-import org.slf4j.Logger;
82+-import org.slf4j.LoggerFactory;
83+-import org.slf4j.Marker;
84+-import org.slf4j.MarkerFactory;
85+-import org.slf4j.spi.LocationAwareLogger;
86+-
87+-/**
88+- * Logger factory hardcoded to send everything to SLF4J.
89+- *
90+- * @since 1.3.0
91+- */
92+-public final class Slf4jLogFactory extends LogFactory {
93+-
94+- private static final class Slf4jLocationAwareLog implements Log {
95+-
96+- private static final String FQCN = Slf4jLocationAwareLog.class.getName();
97+-
98+- private final LocationAwareLogger logger;
99+-
100+- public Slf4jLocationAwareLog(final LocationAwareLogger logger) {
101+- this.logger = logger;
102+- }
103+-
104+- @Override
105+- public void debug(Object message) {
106+- log(DEBUG_INT, message, null);
107+- }
108+-
109+- @Override
110+- public void debug(Object message, Throwable t) {
111+- log(DEBUG_INT, message, t);
112+- }
113+-
114+- @Override
115+- public void error(Object message) {
116+- log(ERROR_INT, message, null);
117+- }
118+-
119+- @Override
120+- public void error(Object message, Throwable t) {
121+- log(ERROR_INT, message, t);
122+- }
123+-
124+- @Override
125+- public void fatal(Object message) {
126+- error(message);
127+- }
128+-
129+- @Override
130+- public void fatal(Object message, Throwable t) {
131+- error(message, t);
132+- }
133+-
134+- @Override
135+- public void info(Object message) {
136+- log(INFO_INT, message, null);
137+- }
138+-
139+- @Override
140+- public void info(Object message, Throwable t) {
141+- log(INFO_INT, message, t);
142+- }
143+-
144+- @Override
145+- public boolean isDebugEnabled() {
146+- return logger.isDebugEnabled(MARKER);
147+- }
148+-
149+- @Override
150+- public boolean isErrorEnabled() {
151+- return logger.isErrorEnabled(MARKER);
152+- }
153+-
154+- @Override
155+- public boolean isFatalEnabled() {
156+- return isErrorEnabled();
157+- }
158+-
159+- @Override
160+- public boolean isInfoEnabled() {
161+- return logger.isInfoEnabled(MARKER);
162+- }
163+-
164+- @Override
165+- public boolean isTraceEnabled() {
166+- return logger.isTraceEnabled(MARKER);
167+- }
168+-
169+- @Override
170+- public boolean isWarnEnabled() {
171+- return logger.isWarnEnabled(MARKER);
172+- }
173+-
174+- private void log(final int level, final Object message, final Throwable t) {
175+- logger.log(MARKER, FQCN, level, String.valueOf(message), EMPTY_OBJECT_ARRAY, t);
176+- }
177+-
178+- @Override
179+- public void trace(Object message) {
180+- log(TRACE_INT, message, null);
181+- }
182+-
183+- @Override
184+- public void trace(Object message, Throwable t) {
185+- log(TRACE_INT, message, t);
186+- }
187+-
188+- @Override
189+- public void warn(Object message) {
190+- log(WARN_INT, message, null);
191+- }
192+-
193+- @Override
194+- public void warn(Object message, Throwable t) {
195+- log(WARN_INT, message, t);
196+- }
197+- }
198+- private static class Slf4jLog implements Log {
199+-
200+- private final Logger logger;
201+-
202+- public Slf4jLog(final Logger logger) {
203+- this.logger = logger;
204+- }
205+-
206+- @Override
207+- public void debug(Object message) {
208+- logger.debug(MARKER, String.valueOf(message));
209+- }
210+-
211+- @Override
212+- public void debug(Object message, Throwable t) {
213+- logger.debug(MARKER, String.valueOf(message), t);
214+- }
215+-
216+- @Override
217+- public void error(Object message) {
218+- logger.error(MARKER, String.valueOf(message));
219+- }
220+-
221+- @Override
222+- public void error(Object message, Throwable t) {
223+- logger.debug(MARKER, String.valueOf(message), t);
224+- }
225+-
226+- @Override
227+- public void fatal(Object message) {
228+- error(message);
229+- }
230+-
231+- @Override
232+- public void fatal(Object message, Throwable t) {
233+- error(message, t);
234+- }
235+-
236+- @Override
237+- public void info(Object message) {
238+- logger.info(MARKER, String.valueOf(message));
239+- }
240+-
241+- @Override
242+- public void info(Object message, Throwable t) {
243+- logger.info(MARKER, String.valueOf(message), t);
244+- }
245+-
246+- @Override
247+- public boolean isDebugEnabled() {
248+- return logger.isDebugEnabled(MARKER);
249+- }
250+-
251+- @Override
252+- public boolean isErrorEnabled() {
253+- return logger.isErrorEnabled(MARKER);
254+- }
255+-
256+- @Override
257+- public boolean isFatalEnabled() {
258+- return isErrorEnabled();
259+- }
260+-
261+- @Override
262+- public boolean isInfoEnabled() {
263+- return logger.isInfoEnabled(MARKER);
264+- }
265+-
266+- @Override
267+- public boolean isTraceEnabled() {
268+- return logger.isTraceEnabled(MARKER);
269+- }
270+-
271+- @Override
272+- public boolean isWarnEnabled() {
273+- return logger.isWarnEnabled(MARKER);
274+- }
275+-
276+- @Override
277+- public void trace(Object message) {
278+- logger.trace(MARKER, String.valueOf(message));
279+- }
280+-
281+- @Override
282+- public void trace(Object message, Throwable t) {
283+- logger.trace(MARKER, String.valueOf(message), t);
284+- }
285+-
286+- @Override
287+- public void warn(Object message) {
288+- logger.warn(MARKER, String.valueOf(message));
289+- }
290+-
291+- @Override
292+- public void warn(Object message, Throwable t) {
293+- logger.warn(MARKER, String.valueOf(message), t);
294+- }
295+- }
296+-
297+- private static final Object[] EMPTY_OBJECT_ARRAY = {};
298+-
299+- private static final String[] EMPTY_STRING_ARRAY = {};
300+-
301+- /**
302+- * Marker used by all messages coming from Apache Commons Logging.
303+- */
304+- private static final Marker MARKER = MarkerFactory.getMarker("COMMONS-LOGGING");
305+-
306+- /**
307+- * Caches Log instances.
308+- * <p>
309+- * The SLF4J reference implementation (Logback) has a single logger context, so each call to
310+- * {@link #getInstance(String)}
311+- * should give the same result.
312+- * </p>
313+- */
314+- private final ConcurrentMap<String, Log> loggers = new ConcurrentHashMap<>();
315+-
316+- private final ConcurrentMap<String, Object> attributes = new ConcurrentHashMap<>();
317+-
318+- @Override
319+- public Object getAttribute(final String name) {
320+- return attributes.get(name);
321+- }
322+-
323+- @Override
324+- public String[] getAttributeNames() {
325+- return attributes.keySet().toArray(EMPTY_STRING_ARRAY);
326+- }
327+-
328+- @Override
329+- public Log getInstance(final Class<?> clazz) throws LogConfigurationException {
330+- return getInstance(clazz.getName());
331+- }
332+-
333+- @Override
334+- public Log getInstance(final String name) {
335+- return loggers.computeIfAbsent(name, n -> {
336+- final Logger logger = LoggerFactory.getLogger(n);
337+- return logger instanceof LocationAwareLogger ? new Slf4jLocationAwareLog((LocationAwareLogger) logger) : new Slf4jLog(
338+- logger);
339+- });
340+- }
341+-
342+- /**
343+- * This method is supposed to clear all loggers.
344+- * <p>
345+- * In this implementation it calls a "stop" method if the logger factory supports it. This is the case of
346+- * Logback.
347+- * </p>
348+- */
349+- @Override
350+- public void release() {
351+- final ILoggerFactory factory = LoggerFactory.getILoggerFactory();
352+- try {
353+- factory.getClass().getMethod("stop").invoke(factory);
354+- } catch (final ReflectiveOperationException ignored) {
355+- }
356+- }
357+-
358+- @Override
359+- public void removeAttribute(final String name) {
360+- attributes.remove(name);
361+- }
362+-
363+- @Override
364+- public void setAttribute(final String name, final Object value) {
365+- if (value != null) {
366+- attributes.put(name, value);
367+- } else {
368+- removeAttribute(name);
369+- }
370+- }
371+-}
372+--- a/src/main/java/org/apache/commons/logging/LogFactory.java
373++++ b/src/main/java/org/apache/commons/logging/LogFactory.java
374+@@ -94,9 +94,7 @@
375+
376+ private static final String FACTORY_LOG4J_API = "org.apache.commons.logging.impl.Log4jApiLogFactory";
377+ private static final String LOG4J_API_LOGGER = "org.apache.logging.log4j.Logger";
378+- private static final String LOG4J_TO_SLF4J_BRIDGE = "org.apache.logging.slf4j.SLF4JProvider";
379+
380+- private static final String FACTORY_SLF4J = "org.apache.commons.logging.impl.Slf4jLogFactory";
381+ private static final String SLF4J_API_LOGGER = "org.slf4j.Logger";
382+
383+ /**
384+@@ -918,21 +916,10 @@
385+ try {
386+ // We prefer Log4j API, since it does not stringify objects.
387+ if (factory == null && isClassAvailable(LOG4J_API_LOGGER, baseClassLoader)) {
388+- // If the Log4j API is redirected to SLF4J, we use SLF4J directly.
389+- if (isClassAvailable(LOG4J_TO_SLF4J_BRIDGE, baseClassLoader)) {
390+- logDiagnostic(
391+- "[LOOKUP] Log4j API to SLF4J redirection detected. Loading the SLF4J LogFactory implementation '" + FACTORY_SLF4J + "'.");
392+- factory = newFactory(FACTORY_SLF4J, baseClassLoader, contextClassLoader);
393+- } else {
394+- logDiagnostic("[LOOKUP] Log4j API detected. Loading the Log4j API LogFactory implementation '" + FACTORY_LOG4J_API + "'.");
395+- factory = newFactory(FACTORY_LOG4J_API, baseClassLoader, contextClassLoader);
396+- }
397++ logDiagnostic("[LOOKUP] Log4j API detected. Loading the Log4j API LogFactory implementation '" + FACTORY_LOG4J_API + "'.");
398++ factory = newFactory(FACTORY_LOG4J_API, baseClassLoader, contextClassLoader);
399+ }
400+
401+- if (factory == null && isClassAvailable(SLF4J_API_LOGGER, baseClassLoader)) {
402+- logDiagnostic("[LOOKUP] SLF4J detected. Loading the SLF4J LogFactory implementation '" + FACTORY_SLF4J + "'.");
403+- factory = newFactory(FACTORY_SLF4J, baseClassLoader, contextClassLoader);
404+- }
405+ } catch (final Exception e) {
406+ logDiagnostic("[LOOKUP] An exception occurred while creating LogFactory: " + e.getMessage());
407+ }
408+--- a/src/test/java/org/apache/commons/logging/slf4j/CallerInformationTestCase.java
409++++ /dev/null
410+@@ -1,112 +0,0 @@
411+-/*
412+- * Licensed to the Apache Software Foundation (ASF) under one or more
413+- * contributor license agreements. See the NOTICE file distributed with
414+- * this work for additional information regarding copyright ownership.
415+- * The ASF licenses this file to You under the Apache License, Version 2.0
416+- * (the "License"); you may not use this file except in compliance with
417+- * the License. You may obtain a copy of the License at
418+- *
419+- * http://www.apache.org/licenses/LICENSE-2.0
420+- *
421+- * Unless required by applicable law or agreed to in writing, software
422+- * distributed under the License is distributed on an "AS IS" BASIS,
423+- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
424+- * See the License for the specific language governing permissions and
425+- * limitations under the License.
426+- */
427+-package org.apache.commons.logging.slf4j;
428+-
429+-import java.util.ArrayList;
430+-import java.util.Collections;
431+-import java.util.List;
432+-
433+-import org.apache.commons.logging.Log;
434+-import org.apache.commons.logging.LogFactory;
435+-import org.apache.commons.logging.impl.Slf4jLogFactory;
436+-import org.slf4j.LoggerFactory;
437+-import org.slf4j.Marker;
438+-import org.slf4j.MarkerFactory;
439+-
440+-import ch.qos.logback.classic.Level;
441+-import ch.qos.logback.classic.Logger;
442+-import ch.qos.logback.classic.LoggerContext;
443+-import ch.qos.logback.classic.spi.ILoggingEvent;
444+-import ch.qos.logback.classic.spi.ThrowableProxy;
445+-import ch.qos.logback.core.filter.Filter;
446+-import ch.qos.logback.core.read.ListAppender;
447+-import ch.qos.logback.core.spi.FilterReply;
448+-import junit.framework.TestCase;
449+-
450+-public class CallerInformationTestCase extends TestCase {
451+-
452+- private static final String STRING = "String";
453+- private static final Throwable T = new RuntimeException();
454+- private static final List<Marker> MARKERS = Collections.singletonList(MarkerFactory.getMarker("COMMONS-LOGGING"));
455+-
456+- private static final Level[] levels = {Level.ERROR, // SLF4J has no FATAL level
457+- Level.ERROR, Level.WARN, Level.INFO, Level.DEBUG, Level.TRACE};
458+-
459+- private LogFactory factory;
460+- private Log log;
461+- private ListAppender<ILoggingEvent> appender;
462+-
463+- @Override
464+- public void setUp() {
465+- factory = LogFactory.getFactory();
466+- log = factory.getInstance(getClass());
467+- final LoggerContext context = (LoggerContext) LoggerFactory.getILoggerFactory();
468+- final Logger logger = context.getLogger(Logger.ROOT_LOGGER_NAME);
469+- appender = (ListAppender) logger.getAppender("LIST");
470+- appender.clearAllFilters();
471+- appender.addFilter(new Filter<ILoggingEvent>() {
472+- @Override
473+- public FilterReply decide(ILoggingEvent event) {
474+- // Force the registration of caller data
475+- event.getCallerData();
476+- return FilterReply.NEUTRAL;
477+- }
478+- });
479+- }
480+-
481+- public void testFactoryClassName() {
482+- assertEquals(Slf4jLogFactory.class, factory.getClass());
483+- }
484+-
485+- public void testLocationInfo() {
486+- appender.list.clear();
487+- // The following value must match the line number
488+- final int currentLineNumber = 78;
489+- log.fatal(STRING);
490+- log.fatal(STRING, T);
491+- log.error(STRING);
492+- log.error(STRING, T);
493+- log.warn(STRING);
494+- log.warn(STRING, T);
495+- log.info(STRING);
496+- log.info(STRING, T);
497+- log.debug(STRING);
498+- log.debug(STRING, T);
499+- log.trace(STRING);
500+- log.trace(STRING, T);
501+- final List<ILoggingEvent> events = new ArrayList<>(appender.list);
502+- assertEquals("All events received.", levels.length * 2, events.size());
503+- for (int lev = 0; lev < levels.length; lev++) {
504+- for (int hasThrowable = 0; hasThrowable <= 1; hasThrowable++) {
505+- final ILoggingEvent event = events.get(2 * lev + hasThrowable);
506+- assertEquals("Correct message.", STRING, event.getMessage());
507+- assertEquals("Level matches.", levels[lev], event.getLevel());
508+- final StackTraceElement[] callerData = event.getCallerData();
509+- assertTrue("Has location", callerData != null && callerData.length > 0);
510+- final StackTraceElement location = callerData[0];
511+- assertEquals("Correct location class.", getClass().getName(), location.getClassName());
512+- assertEquals("Correct location line.",
513+- currentLineNumber + 2 * lev + hasThrowable + 1,
514+- location.getLineNumber());
515+- final ThrowableProxy throwableProxy = (ThrowableProxy) event.getThrowableProxy();
516+- assertEquals("Correct exception",
517+- hasThrowable > 0 ? T : null,
518+- throwableProxy != null ? throwableProxy.getThrowable() : null);
519+- }
520+- }
521+- }
522+-}
523diff --git a/debian/patches/series b/debian/patches/series
524index ff0bb39..e700797 100644
525--- a/debian/patches/series
526+++ b/debian/patches/series
527@@ -1,3 +1,4 @@
528 03_disable_securityallow_test.patch
529 07_disable_log4j2_tests.patch
530 08_logback_compatibility.patch
531+09_disable_slf4j.patch

Subscribers

People subscribed via source and target branches