This is an automated email from the ASF dual-hosted git repository. pkarwasz pushed a commit to branch fix/main/osgi-tests in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git
commit 09a27b845e6e1c20dda87eef141a1061711a4202 Author: Piotr P. Karwasz <[email protected]> AuthorDate: Tue Jun 18 14:46:48 2024 +0200 Fix OSGi test failures OSGi tests are failing for multiple reasons: - the constructor of the activator of `log4j-core`, indirectly made a call to `ServiceLoader.load` to populate the `InstanceFactory`. Experimentally it seems that Service Loader Mediator is not ready at this moment and the instance factory is not initialized correctly. To fix this we moved the construction of `Log4jProvider` to the `start()` method. - the test classpath of `log4j-osgi-test` contains both Felix and Equinox, which share some classes, but one artifact is signed and the other is not. To solve the classpath problem we split tests into two runs `test-equinox` and `test-felix`. - the new `logging-parent` stores the `PluginService` for test plugins in a random package. This breaks tests, since the `PluginService` must also be declared in OSGi headers and therefore need to be deterministic. --- .../apache/logging/log4j/core/LoggerContext.java | 3 ++ .../log4j/core/config/AbstractConfiguration.java | 1 + .../log4j/core/impl/internal/Activator.java | 26 +++++++-- log4j-osgi-test/pom.xml | 61 +++++++++++++++++++++- .../logging/log4j/osgi/tests/CoreOsgiTest.java | 2 +- 5 files changed, 86 insertions(+), 7 deletions(-) diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/LoggerContext.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/LoggerContext.java index 6cb8995056..2eb829db49 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/LoggerContext.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/LoggerContext.java @@ -18,6 +18,8 @@ package org.apache.logging.log4j.core; import static org.apache.logging.log4j.core.util.ShutdownCallbackRegistry.SHUTDOWN_HOOK_MARKER; +import aQute.bnd.annotation.Cardinality; +import aQute.bnd.annotation.spi.ServiceConsumer; import java.io.File; import java.net.URI; import java.util.ArrayList; @@ -76,6 +78,7 @@ import org.jspecify.annotations.Nullable; * applications and a reference to the Configuration. The Configuration will contain the configured loggers, appenders, * filters, etc. and will be atomically updated whenever a reconfigure occurs. */ +@ServiceConsumer(value = ConfigurableInstanceFactoryPostProcessor.class, cardinality = Cardinality.MULTIPLE) public class LoggerContext extends AbstractLifeCycle implements org.apache.logging.log4j.spi.LoggerContext, AutoCloseable, diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/AbstractConfiguration.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/AbstractConfiguration.java index 26d4fd8db2..e6c1f4427c 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/AbstractConfiguration.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/AbstractConfiguration.java @@ -92,6 +92,7 @@ import org.jspecify.annotations.Nullable; */ @NullMarked @ServiceConsumer(value = ScriptManagerFactory.class, cardinality = Cardinality.SINGLE, resolution = Resolution.OPTIONAL) +@ServiceConsumer(value = ConfigurableInstanceFactoryPostProcessor.class, cardinality = Cardinality.MULTIPLE) public abstract class AbstractConfiguration extends AbstractFilterable implements Configuration { private static final List<String> EXPECTED_ELEMENTS = diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/internal/Activator.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/internal/Activator.java index 2d12905ce3..30ee8efccc 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/internal/Activator.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/internal/Activator.java @@ -16,15 +16,33 @@ */ package org.apache.logging.log4j.core.impl.internal; +import java.util.Hashtable; import org.apache.logging.log4j.core.impl.Log4jProvider; -import org.apache.logging.log4j.util.ProviderActivator; +import org.apache.logging.log4j.spi.Provider; import org.osgi.annotation.bundle.Header; +import org.osgi.framework.BundleActivator; +import org.osgi.framework.BundleContext; import org.osgi.framework.Constants; +import org.osgi.framework.ServiceRegistration; @Header(name = Constants.BUNDLE_ACTIVATOR, value = "${@class}") @Header(name = Constants.BUNDLE_ACTIVATIONPOLICY, value = Constants.ACTIVATION_LAZY) -public class Activator extends ProviderActivator { - public Activator() { - super(new Log4jProvider()); +public class Activator implements BundleActivator { + + private ServiceRegistration<Provider> providerRegistration = null; + + @Override + public void start(final BundleContext context) throws Exception { + final Provider provider = new Log4jProvider(); + final Hashtable<String, String> props = new Hashtable<>(); + props.put("APIVersion", provider.getVersions()); + this.providerRegistration = context.registerService(Provider.class, provider, props); + } + + @Override + public void stop(final BundleContext context) throws Exception { + if (this.providerRegistration != null) { + this.providerRegistration.unregister(); + } } } diff --git a/log4j-osgi-test/pom.xml b/log4j-osgi-test/pom.xml index 48e8f62fbd..45c538d262 100644 --- a/log4j-osgi-test/pom.xml +++ b/log4j-osgi-test/pom.xml @@ -27,7 +27,7 @@ </parent> <artifactId>log4j-osgi-test</artifactId> - <packaging>jar</packaging> + <name>Apache Log4j OSGi tests</name> <description>The Apache Log4j OSGi tests</description> @@ -41,6 +41,9 @@ <spotbugs.skip>true</spotbugs.skip> <spifly.version>1.3.7</spifly.version> + + <!-- Fix the name of the package with `PluginService`, since it is also used in the code --> + <log4jPluginPackageForTests>org.apache.logging.log4j.osgi_test</log4jPluginPackageForTests> </properties> <dependencies> @@ -150,6 +153,20 @@ <plugins> + <!-- Disable randomisation of the package used by `PluginService`, + since it also must be referenced in the test code. --> + <plugin> + <groupId>org.codehaus.mojo</groupId> + <artifactId>build-helper-maven-plugin</artifactId> + <version>3.5.0</version> + <executions> + <execution> + <id>define-log4jPluginPackageForTests</id> + <phase>none</phase> + </execution> + </executions> + </plugin> + <!-- ~ Unban Logback. --> @@ -198,16 +215,56 @@ <groupId>org.apache.maven.plugins</groupId> <artifactId>maven-surefire-plugin</artifactId> <configuration> - <classpathDependencyExcludes>org.osgi:org.osgi.framework</classpathDependencyExcludes> + <classpathDependencyExcludes> + <exclude>org.osgi:org.osgi.framework</exclude> + </classpathDependencyExcludes> <systemPropertyVariables> <!-- PAX logging has a copy of Log4j2 API--> <pax.exam.logging>false</pax.exam.logging> <java.protocol.handler.pkgs>org.ops4j.pax.url</java.protocol.handler.pkgs> + <!-- Used in `osgi.properties --> + <felix.cache.rootdir>${project.build.directory}</felix.cache.rootdir> </systemPropertyVariables> <!-- Illegal access is disabled by default in Java 16 due to JEP-396. We are relaxing it for tests. --> <argLine>--add-opens java.base/java.net=ALL-UNNAMED</argLine> </configuration> + <executions> + <!-- Split the Felix and Equinox tests to prevent classpath conflicts. + Both frameworks contain e.g. the `org.apache.felix.resolver` package. --> + <execution> + <id>default-test</id> + <phase>none</phase> + </execution> + <execution> + <id>test-equinox</id> + <goals> + <goal>test</goal> + </goals> + <configuration> + <classpathDependencyExcludes combine.children="append"> + <exclude>org.apache.felix:org.apache.felix.framework</exclude> + </classpathDependencyExcludes> + <excludes> + <exclude>org.apache.logging.log4j.osgi.tests.FelixLoadApiBundleTest</exclude> + </excludes> + </configuration> + </execution> + <execution> + <id>test-felix</id> + <goals> + <goal>test</goal> + </goals> + <configuration> + <classpathDependencyExcludes combine.children="append"> + <exclude>org.eclipse.platform:org.eclipse.osgi</exclude> + </classpathDependencyExcludes> + <excludes> + <exclude>org.apache.logging.log4j.osgi.tests.EquinoxLoadApiBundleTest</exclude> + </excludes> + </configuration> + </execution> + </executions> </plugin> </plugins> diff --git a/log4j-osgi-test/src/test/java/org/apache/logging/log4j/osgi/tests/CoreOsgiTest.java b/log4j-osgi-test/src/test/java/org/apache/logging/log4j/osgi/tests/CoreOsgiTest.java index 709c3fffa9..b27dfac08d 100644 --- a/log4j-osgi-test/src/test/java/org/apache/logging/log4j/osgi/tests/CoreOsgiTest.java +++ b/log4j-osgi-test/src/test/java/org/apache/logging/log4j/osgi/tests/CoreOsgiTest.java @@ -69,7 +69,7 @@ public class CoreOsgiTest { builder.setHeader( Constants.PROVIDE_CAPABILITY, "osgi.serviceloader;osgi.serviceloader=\"org.apache.logging.log4j.plugins.model.PluginService\";" - + "register:=\"org.apache.logging.log4j.osgi.tests.plugins.Log4jPlugins\""); + + "register:=\"org.apache.logging.log4j.osgi_test.plugins.Log4jPlugins\""); builder.setHeader( Constants.REQUIRE_CAPABILITY, "osgi.extender;filter:=\"(&(osgi.extender=osgi.serviceloader.registrar)(version>=1.0)(!(version>=2.0)))\"");
