From 41ba2643df6ad4d4174be30f3122bf82cc2ff270 Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Mon, 4 Oct 2021 10:12:36 +0200 Subject: [PATCH 01/16] Mojo Logging proposal This is a DRAFT to boost some discussion about Mojo logging. I believe Maven should not get in a way to Maven Plugin developers, but we still can offer some "convenience" helpers and make things right (like for example logging in ctor of Mojo). IMHO, we have 3 log-related use cases: * trivial to simple Maven Plugins (from one mojo plugin to more mojos + several "own" components plugins): here developer can easily choose (and usually choose Mojo Log) to do the job. Maven is helping here. * complex Maven plugins: plugins pulling in "foreign" frameworks/components/whatever, here logging is usually "given", but as Mojo class loader is "self first", it is really easy for developer to decide (and implement) which logging it wants (or must) to use, Maven should not interfere. One thing IMHO remains, to make Mojo Log interface more "slf4j Logger"-lookalike. --- .../internal/DefaultMavenPluginManager.java | 7 - .../{MojoLogWrapper.java => MojoLog.java} | 8 +- .../maven/plugin/internal/MojoLogFactory.java | 56 +++++ .../resources/META-INF/maven/extension.xml | 1 + .../org/apache/maven/plugin/AbstractMojo.java | 42 ++-- .../apache/maven/plugin/ContextEnabled.java | 4 +- .../java/org/apache/maven/plugin/Mojo.java | 23 -- .../org/apache/maven/plugin/logging/Log.java | 3 +- .../maven/plugin/logging/LogFactory.java | 62 ++++++ .../maven/plugin/logging/SystemStreamLog.java | 201 ------------------ .../plugin/logging/internal/ILogFactory.java | 41 ++++ 11 files changed, 181 insertions(+), 267 deletions(-) rename maven-core/src/main/java/org/apache/maven/plugin/internal/{MojoLogWrapper.java => MojoLog.java} (96%) create mode 100644 maven-core/src/main/java/org/apache/maven/plugin/internal/MojoLogFactory.java create mode 100644 maven-plugin-api/src/main/java/org/apache/maven/plugin/logging/LogFactory.java delete mode 100644 maven-plugin-api/src/main/java/org/apache/maven/plugin/logging/SystemStreamLog.java create mode 100644 maven-plugin-api/src/main/java/org/apache/maven/plugin/logging/internal/ILogFactory.java diff --git a/maven-core/src/main/java/org/apache/maven/plugin/internal/DefaultMavenPluginManager.java b/maven-core/src/main/java/org/apache/maven/plugin/internal/DefaultMavenPluginManager.java index ca1910b25faa..fda931517b20 100644 --- a/maven-core/src/main/java/org/apache/maven/plugin/internal/DefaultMavenPluginManager.java +++ b/maven-core/src/main/java/org/apache/maven/plugin/internal/DefaultMavenPluginManager.java @@ -30,7 +30,6 @@ import org.apache.maven.plugin.ExtensionRealmCache; import org.apache.maven.plugin.InvalidPluginDescriptorException; import org.apache.maven.plugin.MavenPluginManager; -import org.apache.maven.plugin.Mojo; import org.apache.maven.plugin.MojoExecution; import org.apache.maven.plugin.MojoNotFoundException; import org.apache.maven.plugin.PluginArtifactsCache; @@ -569,12 +568,6 @@ else if ( cause instanceof LinkageError ) } } - if ( mojo instanceof Mojo ) - { - Logger mojoLogger = LoggerFactory.getLogger( mojoDescriptor.getImplementation() ); - ( (Mojo) mojo ).setLog( new MojoLogWrapper( mojoLogger ) ); - } - Xpp3Dom dom = mojoExecution.getConfiguration(); PlexusConfiguration pomConfiguration; diff --git a/maven-core/src/main/java/org/apache/maven/plugin/internal/MojoLogWrapper.java b/maven-core/src/main/java/org/apache/maven/plugin/internal/MojoLog.java similarity index 96% rename from maven-core/src/main/java/org/apache/maven/plugin/internal/MojoLogWrapper.java rename to maven-core/src/main/java/org/apache/maven/plugin/internal/MojoLog.java index 060dffdbd8a0..33394889a97a 100644 --- a/maven-core/src/main/java/org/apache/maven/plugin/internal/MojoLogWrapper.java +++ b/maven-core/src/main/java/org/apache/maven/plugin/internal/MojoLog.java @@ -25,14 +25,16 @@ import static java.util.Objects.requireNonNull; /** - * @author jdcasey + * Mojo {@link Log} wrapper using Maven Core logging. + * + * @since TBD */ -public class MojoLogWrapper +public class MojoLog implements Log { private final Logger logger; - public MojoLogWrapper( Logger logger ) + public MojoLog( Logger logger ) { this.logger = requireNonNull( logger ); } diff --git a/maven-core/src/main/java/org/apache/maven/plugin/internal/MojoLogFactory.java b/maven-core/src/main/java/org/apache/maven/plugin/internal/MojoLogFactory.java new file mode 100644 index 000000000000..759eb5380a1c --- /dev/null +++ b/maven-core/src/main/java/org/apache/maven/plugin/internal/MojoLogFactory.java @@ -0,0 +1,56 @@ +package org.apache.maven.plugin.internal; + +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import javax.inject.Named; + +import org.apache.maven.plugin.logging.Log; +import org.apache.maven.plugin.logging.LogFactory; +import org.apache.maven.plugin.logging.internal.ILogFactory; +import org.eclipse.sisu.EagerSingleton; +import org.slf4j.LoggerFactory; + +/** + * Bridge implementation for Mojo {@link Log}. + * + * @since TBD + */ +@EagerSingleton +@Named +public class MojoLogFactory + implements ILogFactory +{ + public MojoLogFactory() + { + LogFactory.initLogFactory( this ); // init Mojo Logging + } + + @Override + public Log getLog( final Class clazz ) + { + return new MojoLog( LoggerFactory.getLogger( clazz ) ); + } + + @Override + public Log getLog( final String name ) + { + return new MojoLog( LoggerFactory.getLogger( name ) ); + } +} diff --git a/maven-core/src/main/resources/META-INF/maven/extension.xml b/maven-core/src/main/resources/META-INF/maven/extension.xml index a9e201c70825..d64bbeceb9aa 100644 --- a/maven-core/src/main/resources/META-INF/maven/extension.xml +++ b/maven-core/src/main/resources/META-INF/maven/extension.xml @@ -35,6 +35,7 @@ under the License. org.apache.maven.model org.apache.maven.monitor org.apache.maven.plugin + org.apache.maven.plugin.logging org.apache.maven.profiles org.apache.maven.project org.apache.maven.reporting diff --git a/maven-plugin-api/src/main/java/org/apache/maven/plugin/AbstractMojo.java b/maven-plugin-api/src/main/java/org/apache/maven/plugin/AbstractMojo.java index 4042f84dea62..c8a82badc93a 100644 --- a/maven-plugin-api/src/main/java/org/apache/maven/plugin/AbstractMojo.java +++ b/maven-plugin-api/src/main/java/org/apache/maven/plugin/AbstractMojo.java @@ -22,7 +22,7 @@ import java.util.Map; import org.apache.maven.plugin.logging.Log; -import org.apache.maven.plugin.logging.SystemStreamLog; +import org.apache.maven.plugin.logging.LogFactory; /** * Abstract class to provide most of the infrastructure required to implement a Mojo except for @@ -144,55 +144,39 @@ public abstract class AbstractMojo implements Mojo, ContextEnabled { /** Instance logger */ - private Log log; + private final Log log = LogFactory.getLog( getClass() ); /** Plugin container context */ - private Map pluginContext; + private Map pluginContext; /** - * @deprecated Use SLF4J directly + * Returns the {@link Log} instance this mojo uses (is public for backward compatibility), never returns + * {@code null}. */ - @Deprecated - @Override - public void setLog( Log log ) + public Log getLog() { - this.log = log; + return log; } /** - *

- * Returns the logger that has been injected into this mojo. If no logger has been setup yet, a - * SystemStreamLog logger will be created and returned. - *

- * Note: - * The logger returned by this method must not be cached in an instance field during the construction of the mojo. - * This would cause the mojo to use a wrongly configured default logger when being run by Maven. The proper logger - * gets injected by the Plexus container after the mojo has been constructed. Therefore, simply call this - * method directly whenever you need the logger, it is fast enough and needs no caching. + * Method in place only for backward binary compatibility, otherwise has no effect. * - * @see org.apache.maven.plugin.Mojo#getLog() - * @deprecated Use SLF4J directly + * @deprecated This method should not be used, as {@link #log} field is initialized at construction time. */ @Deprecated - @Override - public Log getLog() + public void setLog( Log log ) { - if ( log == null ) - { - log = new SystemStreamLog(); - } - - return log; + // nothing } @Override - public Map getPluginContext() + public Map getPluginContext() { return pluginContext; } @Override - public void setPluginContext( Map pluginContext ) + public void setPluginContext( Map pluginContext ) { this.pluginContext = pluginContext; } diff --git a/maven-plugin-api/src/main/java/org/apache/maven/plugin/ContextEnabled.java b/maven-plugin-api/src/main/java/org/apache/maven/plugin/ContextEnabled.java index 68516988b5d1..c07baed95d47 100644 --- a/maven-plugin-api/src/main/java/org/apache/maven/plugin/ContextEnabled.java +++ b/maven-plugin-api/src/main/java/org/apache/maven/plugin/ContextEnabled.java @@ -35,10 +35,10 @@ public interface ContextEnabled * * @param pluginContext a new Map */ - void setPluginContext( Map pluginContext ); + void setPluginContext( Map pluginContext ); /** * @return a Map stored in the plugin container's context. */ - Map getPluginContext(); + Map getPluginContext(); } diff --git a/maven-plugin-api/src/main/java/org/apache/maven/plugin/Mojo.java b/maven-plugin-api/src/main/java/org/apache/maven/plugin/Mojo.java index 0419176057a9..d025e6af3fb6 100644 --- a/maven-plugin-api/src/main/java/org/apache/maven/plugin/Mojo.java +++ b/maven-plugin-api/src/main/java/org/apache/maven/plugin/Mojo.java @@ -19,8 +19,6 @@ * under the License. */ -import org.apache.maven.plugin.logging.Log; - /** * This interface forms the contract required for Mojos to interact with the Maven * infrastructure.
@@ -48,25 +46,4 @@ public interface Mojo */ void execute() throws MojoExecutionException, MojoFailureException; - - /** - * Inject a standard Maven logging mechanism to allow this Mojo to communicate events - * and feedback to the user. - * - * @param log a new logger - * - * @deprecated Use SLF4J directly - */ - @Deprecated - void setLog( Log log ); - - /** - * Furnish access to the standard Maven logging mechanism which is managed in this base class. - * - * @return a log4j-like logger object which allows plugins to create messages at levels of "debug", - * "info", "warn", and "error". - * @deprecated Use SLF4J directly - */ - @Deprecated - Log getLog(); } diff --git a/maven-plugin-api/src/main/java/org/apache/maven/plugin/logging/Log.java b/maven-plugin-api/src/main/java/org/apache/maven/plugin/logging/Log.java index 04d85bd960cc..6973161081b1 100644 --- a/maven-plugin-api/src/main/java/org/apache/maven/plugin/logging/Log.java +++ b/maven-plugin-api/src/main/java/org/apache/maven/plugin/logging/Log.java @@ -29,9 +29,8 @@ * * @author jdcasey * - * @deprecated Use SLF4J directly + * TODO: Make this interface more SLF4J Logger-lookalike. */ -@Deprecated public interface Log { /** diff --git a/maven-plugin-api/src/main/java/org/apache/maven/plugin/logging/LogFactory.java b/maven-plugin-api/src/main/java/org/apache/maven/plugin/logging/LogFactory.java new file mode 100644 index 000000000000..af32937b066f --- /dev/null +++ b/maven-plugin-api/src/main/java/org/apache/maven/plugin/logging/LogFactory.java @@ -0,0 +1,62 @@ +package org.apache.maven.plugin.logging; + +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import org.apache.maven.plugin.logging.internal.ILogFactory; + +/** + * This interface supplies {@link Log} instances to Mojos. Plugin code may freely use this log factory to obtain + * loggers that will be abridged to Maven internal logging system. + * + * @since TBD + */ +public final class LogFactory +{ + private static ILogFactory bridge; + + private LogFactory() + { + // no instances of this can be created + } + + /** + * Initialized Mojo LogFactory with a bridge. + */ + public static void initLogFactory( ILogFactory bridge ) + { + LogFactory.bridge = bridge; + } + + /** + * Returns the {@link Log} instance for given class. + */ + public static Log getLog( Class clazz ) + { + return bridge.getLog( clazz ); + } + + /** + * Returns the {@link Log} instance for given name. + */ + public static Log getLog( String name ) + { + return bridge.getLog( name ); + } +} diff --git a/maven-plugin-api/src/main/java/org/apache/maven/plugin/logging/SystemStreamLog.java b/maven-plugin-api/src/main/java/org/apache/maven/plugin/logging/SystemStreamLog.java deleted file mode 100644 index f0fc6189d57f..000000000000 --- a/maven-plugin-api/src/main/java/org/apache/maven/plugin/logging/SystemStreamLog.java +++ /dev/null @@ -1,201 +0,0 @@ -package org.apache.maven.plugin.logging; - -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -import java.io.PrintWriter; -import java.io.StringWriter; - -/** - * Logger with "standard" output and error output stream. - * - * @author jdcasey - * - * @deprecated Use SLF4J directly - */ -@Deprecated -public class SystemStreamLog - implements Log -{ - /** - * @see org.apache.maven.plugin.logging.Log#debug(java.lang.CharSequence) - */ - public void debug( CharSequence content ) - { - print( "debug", content ); - } - - /** - * @see org.apache.maven.plugin.logging.Log#debug(java.lang.CharSequence, java.lang.Throwable) - */ - public void debug( CharSequence content, Throwable error ) - { - print( "debug", content, error ); - } - - /** - * @see org.apache.maven.plugin.logging.Log#debug(java.lang.Throwable) - */ - public void debug( Throwable error ) - { - print( "debug", error ); - } - - /** - * @see org.apache.maven.plugin.logging.Log#info(java.lang.CharSequence) - */ - public void info( CharSequence content ) - { - print( "info", content ); - } - - /** - * @see org.apache.maven.plugin.logging.Log#info(java.lang.CharSequence, java.lang.Throwable) - */ - public void info( CharSequence content, Throwable error ) - { - print( "info", content, error ); - } - - /** - * @see org.apache.maven.plugin.logging.Log#info(java.lang.Throwable) - */ - public void info( Throwable error ) - { - print( "info", error ); - } - - /** - * @see org.apache.maven.plugin.logging.Log#warn(java.lang.CharSequence) - */ - public void warn( CharSequence content ) - { - print( "warn", content ); - } - - /** - * @see org.apache.maven.plugin.logging.Log#warn(java.lang.CharSequence, java.lang.Throwable) - */ - public void warn( CharSequence content, Throwable error ) - { - print( "warn", content, error ); - } - - /** - * @see org.apache.maven.plugin.logging.Log#warn(java.lang.Throwable) - */ - public void warn( Throwable error ) - { - print( "warn", error ); - } - - /** - * @see org.apache.maven.plugin.logging.Log#error(java.lang.CharSequence) - */ - public void error( CharSequence content ) - { - System.err.println( "[error] " + content.toString() ); - } - - /** - * @see org.apache.maven.plugin.logging.Log#error(java.lang.CharSequence, java.lang.Throwable) - */ - public void error( CharSequence content, Throwable error ) - { - StringWriter sWriter = new StringWriter(); - PrintWriter pWriter = new PrintWriter( sWriter ); - - error.printStackTrace( pWriter ); - - System.err.println( "[error] " + content.toString() - + System.lineSeparator() + System.lineSeparator() + sWriter.toString() ); - } - - /** - * @see org.apache.maven.plugin.logging.Log#error(java.lang.Throwable) - */ - public void error( Throwable error ) - { - StringWriter sWriter = new StringWriter(); - PrintWriter pWriter = new PrintWriter( sWriter ); - - error.printStackTrace( pWriter ); - - System.err.println( "[error] " + sWriter.toString() ); - } - - /** - * @see org.apache.maven.plugin.logging.Log#isDebugEnabled() - */ - public boolean isDebugEnabled() - { - // TODO Not sure how best to set these for this implementation... - return false; - } - - /** - * @see org.apache.maven.plugin.logging.Log#isInfoEnabled() - */ - public boolean isInfoEnabled() - { - return true; - } - - /** - * @see org.apache.maven.plugin.logging.Log#isWarnEnabled() - */ - public boolean isWarnEnabled() - { - return true; - } - - /** - * @see org.apache.maven.plugin.logging.Log#isErrorEnabled() - */ - public boolean isErrorEnabled() - { - return true; - } - - private void print( String prefix, CharSequence content ) - { - System.out.println( "[" + prefix + "] " + content.toString() ); - } - - private void print( String prefix, Throwable error ) - { - StringWriter sWriter = new StringWriter(); - PrintWriter pWriter = new PrintWriter( sWriter ); - - error.printStackTrace( pWriter ); - - System.out.println( "[" + prefix + "] " + sWriter.toString() ); - } - - private void print( String prefix, CharSequence content, Throwable error ) - { - StringWriter sWriter = new StringWriter(); - PrintWriter pWriter = new PrintWriter( sWriter ); - - error.printStackTrace( pWriter ); - - System.out.println( "[" + prefix + "] " + content.toString() - + System.lineSeparator() + System.lineSeparator() + sWriter.toString() ); - } -} diff --git a/maven-plugin-api/src/main/java/org/apache/maven/plugin/logging/internal/ILogFactory.java b/maven-plugin-api/src/main/java/org/apache/maven/plugin/logging/internal/ILogFactory.java new file mode 100644 index 000000000000..b95421938ecb --- /dev/null +++ b/maven-plugin-api/src/main/java/org/apache/maven/plugin/logging/internal/ILogFactory.java @@ -0,0 +1,41 @@ +package org.apache.maven.plugin.logging.internal; + +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import org.apache.maven.plugin.logging.Log; +import org.apache.maven.plugin.logging.LogFactory; + +/** + * This internal interface bridges {@link LogFactory} with Maven internals. + * + * @since TBD + */ +public interface ILogFactory +{ + /** + * Returns the {@link Log} instance for given class. + */ + Log getLog( Class clazz ); + + /** + * Returns the {@link Log} instance for given name. + */ + Log getLog( String name ); +} From c7e11f5d98acfbb241cffcf02790147b144c3d96 Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Tue, 5 Oct 2021 21:45:19 +0200 Subject: [PATCH 02/16] Back to the roots --- .../internal/DefaultMavenPluginManager.java | 14 ++++- .../apache/maven/plugin/internal/MojoLog.java | 52 ++++++++++++------- .../maven/plugin/internal/MojoLogFactory.java | 18 +++---- .../org/apache/maven/plugin/AbstractMojo.java | 17 ++---- .../java/org/apache/maven/plugin/Mojo.java | 18 +++++++ .../maven/plugin/logging/LogFactory.java | 40 +++++--------- .../plugin/logging/internal/ILogFactory.java | 41 --------------- 7 files changed, 86 insertions(+), 114 deletions(-) delete mode 100644 maven-plugin-api/src/main/java/org/apache/maven/plugin/logging/internal/ILogFactory.java diff --git a/maven-core/src/main/java/org/apache/maven/plugin/internal/DefaultMavenPluginManager.java b/maven-core/src/main/java/org/apache/maven/plugin/internal/DefaultMavenPluginManager.java index fda931517b20..ab6e933ebb69 100644 --- a/maven-core/src/main/java/org/apache/maven/plugin/internal/DefaultMavenPluginManager.java +++ b/maven-core/src/main/java/org/apache/maven/plugin/internal/DefaultMavenPluginManager.java @@ -30,6 +30,7 @@ import org.apache.maven.plugin.ExtensionRealmCache; import org.apache.maven.plugin.InvalidPluginDescriptorException; import org.apache.maven.plugin.MavenPluginManager; +import org.apache.maven.plugin.Mojo; import org.apache.maven.plugin.MojoExecution; import org.apache.maven.plugin.MojoNotFoundException; import org.apache.maven.plugin.PluginArtifactsCache; @@ -47,6 +48,8 @@ import org.apache.maven.plugin.descriptor.Parameter; import org.apache.maven.plugin.descriptor.PluginDescriptor; import org.apache.maven.plugin.descriptor.PluginDescriptorBuilder; +import org.apache.maven.plugin.logging.Log; +import org.apache.maven.plugin.logging.LogFactory; import org.apache.maven.plugin.version.DefaultPluginVersionRequest; import org.apache.maven.plugin.version.PluginVersionRequest; import org.apache.maven.plugin.version.PluginVersionResolutionException; @@ -142,6 +145,7 @@ public class DefaultMavenPluginManager private PluginVersionResolver pluginVersionResolver; private PluginArtifactsCache pluginArtifactsCache; private MavenPluginValidator pluginValidator; + private LogFactory logFactory; private final ExtensionDescriptorBuilder extensionDescriptorBuilder = new ExtensionDescriptorBuilder(); private final PluginDescriptorBuilder builder = new PluginDescriptorBuilder(); @@ -157,7 +161,8 @@ public DefaultMavenPluginManager( ExtensionRealmCache extensionRealmCache, PluginVersionResolver pluginVersionResolver, PluginArtifactsCache pluginArtifactsCache, - MavenPluginValidator pluginValidator ) + MavenPluginValidator pluginValidator, + LogFactory logFactory ) { this.container = container; this.classRealmManager = classRealmManager; @@ -169,6 +174,7 @@ public DefaultMavenPluginManager( this.pluginVersionResolver = pluginVersionResolver; this.pluginArtifactsCache = pluginArtifactsCache; this.pluginValidator = pluginValidator; + this.logFactory = logFactory; } public synchronized PluginDescriptor getPluginDescriptor( Plugin plugin, List repositories, @@ -568,6 +574,12 @@ else if ( cause instanceof LinkageError ) } } + if ( mojo instanceof Mojo ) + { + Log mojoLog = logFactory.getLog( mojoDescriptor.getImplementation() ); + ( ( Mojo ) mojo ).setLog( mojoLog ); + } + Xpp3Dom dom = mojoExecution.getConfiguration(); PlexusConfiguration pomConfiguration; diff --git a/maven-core/src/main/java/org/apache/maven/plugin/internal/MojoLog.java b/maven-core/src/main/java/org/apache/maven/plugin/internal/MojoLog.java index 33394889a97a..7e450848a29f 100644 --- a/maven-core/src/main/java/org/apache/maven/plugin/internal/MojoLog.java +++ b/maven-core/src/main/java/org/apache/maven/plugin/internal/MojoLog.java @@ -22,6 +22,8 @@ import org.apache.maven.plugin.logging.Log; import org.slf4j.Logger; +import javax.inject.Provider; + import static java.util.Objects.requireNonNull; /** @@ -32,16 +34,28 @@ public class MojoLog implements Log { - private final Logger logger; + private final Provider loggerProvider; + + private volatile Logger logger; - public MojoLog( Logger logger ) + public MojoLog( Provider loggerProvider ) { - this.logger = requireNonNull( logger ); + this.loggerProvider = requireNonNull( loggerProvider ); + this.logger = null; + } + + private Logger getLogger() + { + if ( logger == null ) + { + logger = requireNonNull( loggerProvider.get() ); + } + return logger; } public void debug( CharSequence content ) { - logger.debug( toString( content ) ); + getLogger().debug( toString( content ) ); } private String toString( CharSequence content ) @@ -59,90 +73,90 @@ private String toString( CharSequence content ) @Override public void debug( CharSequence content, Throwable error ) { - logger.debug( toString( content ), error ); + getLogger().debug( toString( content ), error ); } @Override public void debug( Throwable error ) { - logger.debug( "", error ); + getLogger().debug( "", error ); } @Override public void info( CharSequence content ) { - logger.info( toString( content ) ); + getLogger().info( toString( content ) ); } @Override public void info( CharSequence content, Throwable error ) { - logger.info( toString( content ), error ); + getLogger().info( toString( content ), error ); } @Override public void info( Throwable error ) { - logger.info( "", error ); + getLogger().info( "", error ); } @Override public void warn( CharSequence content ) { - logger.warn( toString( content ) ); + getLogger().warn( toString( content ) ); } @Override public void warn( CharSequence content, Throwable error ) { - logger.warn( toString( content ), error ); + getLogger().warn( toString( content ), error ); } @Override public void warn( Throwable error ) { - logger.warn( "", error ); + getLogger().warn( "", error ); } @Override public void error( CharSequence content ) { - logger.error( toString( content ) ); + getLogger().error( toString( content ) ); } @Override public void error( CharSequence content, Throwable error ) { - logger.error( toString( content ), error ); + getLogger().error( toString( content ), error ); } @Override public void error( Throwable error ) { - logger.error( "", error ); + getLogger().error( "", error ); } @Override public boolean isDebugEnabled() { - return logger.isDebugEnabled(); + return getLogger().isDebugEnabled(); } @Override public boolean isInfoEnabled() { - return logger.isInfoEnabled(); + return getLogger().isInfoEnabled(); } @Override public boolean isWarnEnabled() { - return logger.isWarnEnabled(); + return getLogger().isWarnEnabled(); } @Override public boolean isErrorEnabled() { - return logger.isErrorEnabled(); + return getLogger().isErrorEnabled(); } } diff --git a/maven-core/src/main/java/org/apache/maven/plugin/internal/MojoLogFactory.java b/maven-core/src/main/java/org/apache/maven/plugin/internal/MojoLogFactory.java index 759eb5380a1c..b462d3976d3d 100644 --- a/maven-core/src/main/java/org/apache/maven/plugin/internal/MojoLogFactory.java +++ b/maven-core/src/main/java/org/apache/maven/plugin/internal/MojoLogFactory.java @@ -20,37 +20,31 @@ */ import javax.inject.Named; +import javax.inject.Singleton; import org.apache.maven.plugin.logging.Log; import org.apache.maven.plugin.logging.LogFactory; -import org.apache.maven.plugin.logging.internal.ILogFactory; -import org.eclipse.sisu.EagerSingleton; import org.slf4j.LoggerFactory; /** - * Bridge implementation for Mojo {@link Log}. + * Bridge implementation for Mojo {@link LogFactory}. * * @since TBD */ -@EagerSingleton +@Singleton @Named public class MojoLogFactory - implements ILogFactory + implements LogFactory { - public MojoLogFactory() - { - LogFactory.initLogFactory( this ); // init Mojo Logging - } - @Override public Log getLog( final Class clazz ) { - return new MojoLog( LoggerFactory.getLogger( clazz ) ); + return new MojoLog( () -> LoggerFactory.getLogger( clazz ) ); } @Override public Log getLog( final String name ) { - return new MojoLog( LoggerFactory.getLogger( name ) ); + return new MojoLog( () -> LoggerFactory.getLogger( name ) ); } } diff --git a/maven-plugin-api/src/main/java/org/apache/maven/plugin/AbstractMojo.java b/maven-plugin-api/src/main/java/org/apache/maven/plugin/AbstractMojo.java index c8a82badc93a..03345fd116f2 100644 --- a/maven-plugin-api/src/main/java/org/apache/maven/plugin/AbstractMojo.java +++ b/maven-plugin-api/src/main/java/org/apache/maven/plugin/AbstractMojo.java @@ -22,7 +22,6 @@ import java.util.Map; import org.apache.maven.plugin.logging.Log; -import org.apache.maven.plugin.logging.LogFactory; /** * Abstract class to provide most of the infrastructure required to implement a Mojo except for @@ -144,29 +143,21 @@ public abstract class AbstractMojo implements Mojo, ContextEnabled { /** Instance logger */ - private final Log log = LogFactory.getLog( getClass() ); + private Log log; /** Plugin container context */ private Map pluginContext; - /** - * Returns the {@link Log} instance this mojo uses (is public for backward compatibility), never returns - * {@code null}. - */ + @Override public Log getLog() { return log; } - /** - * Method in place only for backward binary compatibility, otherwise has no effect. - * - * @deprecated This method should not be used, as {@link #log} field is initialized at construction time. - */ - @Deprecated + @Override public void setLog( Log log ) { - // nothing + this.log = log; } @Override diff --git a/maven-plugin-api/src/main/java/org/apache/maven/plugin/Mojo.java b/maven-plugin-api/src/main/java/org/apache/maven/plugin/Mojo.java index d025e6af3fb6..97dcebfdb994 100644 --- a/maven-plugin-api/src/main/java/org/apache/maven/plugin/Mojo.java +++ b/maven-plugin-api/src/main/java/org/apache/maven/plugin/Mojo.java @@ -19,6 +19,8 @@ * under the License. */ +import org.apache.maven.plugin.logging.Log; + /** * This interface forms the contract required for Mojos to interact with the Maven * infrastructure.
@@ -46,4 +48,20 @@ public interface Mojo */ void execute() throws MojoExecutionException, MojoFailureException; + + /** + * Inject a standard Maven logging mechanism to allow this Mojo to communicate events + * and feedback to the user. + * + * @param log a new logger + */ + void setLog( Log log ); + + /** + * Furnish access to the standard Maven logging mechanism which is managed in this base class. + * + * @return a logger object which allows plugins to create messages at levels of "debug", + * "info", "warn", and "error". + */ + Log getLog(); } diff --git a/maven-plugin-api/src/main/java/org/apache/maven/plugin/logging/LogFactory.java b/maven-plugin-api/src/main/java/org/apache/maven/plugin/logging/LogFactory.java index af32937b066f..85f9ff343955 100644 --- a/maven-plugin-api/src/main/java/org/apache/maven/plugin/logging/LogFactory.java +++ b/maven-plugin-api/src/main/java/org/apache/maven/plugin/logging/LogFactory.java @@ -19,44 +19,28 @@ * under the License. */ -import org.apache.maven.plugin.logging.internal.ILogFactory; - /** - * This interface supplies {@link Log} instances to Mojos. Plugin code may freely use this log factory to obtain - * loggers that will be abridged to Maven internal logging system. + * This component supplies {@link Log} instances. You can require this component to be injected into your own components + * and use it for provisioning logger instances. Simplest is to inject it via constructor injection (recommended way) + * and just get a {@link Log} instance: + * 
+ *   Log log = logFactory.getLog( getClass() );
+ *
+ * + * {@link Log} instance for mojo implementations are provided by this component as well, but to not push constructor + * signature down to implementations, it is injected using setter method. * * @since TBD */ -public final class LogFactory +public interface LogFactory { - private static ILogFactory bridge; - - private LogFactory() - { - // no instances of this can be created - } - - /** - * Initialized Mojo LogFactory with a bridge. - */ - public static void initLogFactory( ILogFactory bridge ) - { - LogFactory.bridge = bridge; - } - /** * Returns the {@link Log} instance for given class. */ - public static Log getLog( Class clazz ) - { - return bridge.getLog( clazz ); - } + Log getLog( Class clazz ); /** * Returns the {@link Log} instance for given name. */ - public static Log getLog( String name ) - { - return bridge.getLog( name ); - } + Log getLog( String name ); } diff --git a/maven-plugin-api/src/main/java/org/apache/maven/plugin/logging/internal/ILogFactory.java b/maven-plugin-api/src/main/java/org/apache/maven/plugin/logging/internal/ILogFactory.java deleted file mode 100644 index b95421938ecb..000000000000 --- a/maven-plugin-api/src/main/java/org/apache/maven/plugin/logging/internal/ILogFactory.java +++ /dev/null @@ -1,41 +0,0 @@ -package org.apache.maven.plugin.logging.internal; - -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -import org.apache.maven.plugin.logging.Log; -import org.apache.maven.plugin.logging.LogFactory; - -/** - * This internal interface bridges {@link LogFactory} with Maven internals. - * - * @since TBD - */ -public interface ILogFactory -{ - /** - * Returns the {@link Log} instance for given class. - */ - Log getLog( Class clazz ); - - /** - * Returns the {@link Log} instance for given name. - */ - Log getLog( String name ); -} From affa287d94156c1525242bb44747a98fe5819731 Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Tue, 5 Oct 2021 21:59:39 +0200 Subject: [PATCH 03/16] Returnd javadoc, mostly still stands. --- .../org/apache/maven/plugin/AbstractMojo.java | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/maven-plugin-api/src/main/java/org/apache/maven/plugin/AbstractMojo.java b/maven-plugin-api/src/main/java/org/apache/maven/plugin/AbstractMojo.java index 03345fd116f2..9e544ca25627 100644 --- a/maven-plugin-api/src/main/java/org/apache/maven/plugin/AbstractMojo.java +++ b/maven-plugin-api/src/main/java/org/apache/maven/plugin/AbstractMojo.java @@ -149,15 +149,26 @@ public abstract class AbstractMojo private Map pluginContext; @Override - public Log getLog() + public void setLog( Log log ) { - return log; + this.log = log; } + /** + *

+ * Returns the logger that has been injected into this mojo. Injection happens AFTER instance is constructed. + *

+ * Note: + * The logger returned by this method must not be cached in an instance field during the construction of the mojo, + * as it is not yet injected. The proper logger gets injected by Maven after the mojo has been constructed. + * Therefore, simply call this method directly whenever you need the logger, it is fast enough and needs no caching. + * + * @see org.apache.maven.plugin.Mojo#getLog() + */ @Override - public void setLog( Log log ) + public Log getLog() { - this.log = log; + return log; } @Override From 64c91f420176d2406776d50dbfb622a40a3ccc7a Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Tue, 5 Oct 2021 22:13:20 +0200 Subject: [PATCH 04/16] Improve javadoc --- .../maven/plugin/internal/DefaultMavenPluginManager.java | 4 +--- .../main/java/org/apache/maven/plugin/internal/MojoLog.java | 2 +- .../java/org/apache/maven/plugin/internal/MojoLogFactory.java | 2 +- 3 files changed, 3 insertions(+), 5 deletions(-) diff --git a/maven-core/src/main/java/org/apache/maven/plugin/internal/DefaultMavenPluginManager.java b/maven-core/src/main/java/org/apache/maven/plugin/internal/DefaultMavenPluginManager.java index ab6e933ebb69..0ecd3c95abcc 100644 --- a/maven-core/src/main/java/org/apache/maven/plugin/internal/DefaultMavenPluginManager.java +++ b/maven-core/src/main/java/org/apache/maven/plugin/internal/DefaultMavenPluginManager.java @@ -48,7 +48,6 @@ import org.apache.maven.plugin.descriptor.Parameter; import org.apache.maven.plugin.descriptor.PluginDescriptor; import org.apache.maven.plugin.descriptor.PluginDescriptorBuilder; -import org.apache.maven.plugin.logging.Log; import org.apache.maven.plugin.logging.LogFactory; import org.apache.maven.plugin.version.DefaultPluginVersionRequest; import org.apache.maven.plugin.version.PluginVersionRequest; @@ -576,8 +575,7 @@ else if ( cause instanceof LinkageError ) if ( mojo instanceof Mojo ) { - Log mojoLog = logFactory.getLog( mojoDescriptor.getImplementation() ); - ( ( Mojo ) mojo ).setLog( mojoLog ); + ( ( Mojo ) mojo ).setLog( logFactory.getLog( mojoDescriptor.getImplementation() ) ); } Xpp3Dom dom = mojoExecution.getConfiguration(); diff --git a/maven-core/src/main/java/org/apache/maven/plugin/internal/MojoLog.java b/maven-core/src/main/java/org/apache/maven/plugin/internal/MojoLog.java index 7e450848a29f..f5afd55560ae 100644 --- a/maven-core/src/main/java/org/apache/maven/plugin/internal/MojoLog.java +++ b/maven-core/src/main/java/org/apache/maven/plugin/internal/MojoLog.java @@ -27,7 +27,7 @@ import static java.util.Objects.requireNonNull; /** - * Mojo {@link Log} wrapper using Maven Core logging. + * Mojo {@link Log} implementation that lazily creates {@link Logger} instances. * * @since TBD */ diff --git a/maven-core/src/main/java/org/apache/maven/plugin/internal/MojoLogFactory.java b/maven-core/src/main/java/org/apache/maven/plugin/internal/MojoLogFactory.java index b462d3976d3d..00d5dbd4c7ad 100644 --- a/maven-core/src/main/java/org/apache/maven/plugin/internal/MojoLogFactory.java +++ b/maven-core/src/main/java/org/apache/maven/plugin/internal/MojoLogFactory.java @@ -27,7 +27,7 @@ import org.slf4j.LoggerFactory; /** - * Bridge implementation for Mojo {@link LogFactory}. + * Implementation of Mojo {@link LogFactory} backed by Maven internal logging (slf4j). * * @since TBD */ From 1683b4563a126ed1682bfecde0196a3fef33e4a5 Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Tue, 5 Oct 2021 22:39:38 +0200 Subject: [PATCH 05/16] Make Log DEBUG level like SLF4j to showcase the idea --- .../apache/maven/plugin/internal/MojoLog.java | 25 ++++++++++++++----- .../org/apache/maven/plugin/logging/Log.java | 10 ++++++++ 2 files changed, 29 insertions(+), 6 deletions(-) diff --git a/maven-core/src/main/java/org/apache/maven/plugin/internal/MojoLog.java b/maven-core/src/main/java/org/apache/maven/plugin/internal/MojoLog.java index f5afd55560ae..51409518d91b 100644 --- a/maven-core/src/main/java/org/apache/maven/plugin/internal/MojoLog.java +++ b/maven-core/src/main/java/org/apache/maven/plugin/internal/MojoLog.java @@ -53,11 +53,6 @@ private Logger getLogger() return logger; } - public void debug( CharSequence content ) - { - getLogger().debug( toString( content ) ); - } - private String toString( CharSequence content ) { if ( content == null ) @@ -70,10 +65,28 @@ private String toString( CharSequence content ) } } + @Override + public void debug( CharSequence content ) + { + getLogger().debug( toString( content ) ); + } + + @Override + public void debug( CharSequence format, Object... arguments ) + { + if ( getLogger().isDebugEnabled() ) + { + getLogger().debug( toString( format ), arguments ); + } + } + @Override public void debug( CharSequence content, Throwable error ) { - getLogger().debug( toString( content ), error ); + if ( getLogger().isDebugEnabled() ) + { + getLogger().debug( toString( content ), error ); + } } @Override diff --git a/maven-plugin-api/src/main/java/org/apache/maven/plugin/logging/Log.java b/maven-plugin-api/src/main/java/org/apache/maven/plugin/logging/Log.java index 6973161081b1..3968fda5fc39 100644 --- a/maven-plugin-api/src/main/java/org/apache/maven/plugin/logging/Log.java +++ b/maven-plugin-api/src/main/java/org/apache/maven/plugin/logging/Log.java @@ -45,6 +45,16 @@ public interface Log */ void debug( CharSequence content ); + /** + * Send a message to the user in the debug error level using given format and arguments. In format use + * {@code {}} as placeholders for objects passed in as arguments. No argument is converted to string is + * given level is not enabled. + * + * @param format + * @param arguments + */ + void debug( CharSequence format, Object... arguments ); + /** * Send a message (and accompanying exception) to the user in the debug error level.
* The error's stacktrace will be output when this error level is enabled. From 97043af65c124d69afce49d3d0ce8a9d033d209a Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Thu, 7 Oct 2021 12:21:43 +0200 Subject: [PATCH 06/16] Make clear on MavenSessiob that plugin context is ConcurrentMap --- .../java/org/apache/maven/execution/MavenSession.java | 11 +++++++---- .../plugin/internal/DefaultMavenPluginManager.java | 3 ++- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/maven-core/src/main/java/org/apache/maven/execution/MavenSession.java b/maven-core/src/main/java/org/apache/maven/execution/MavenSession.java index f8dbb035704f..d609ab8ecd96 100644 --- a/maven-core/src/main/java/org/apache/maven/execution/MavenSession.java +++ b/maven-core/src/main/java/org/apache/maven/execution/MavenSession.java @@ -26,6 +26,7 @@ import java.util.Map; import java.util.Properties; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentMap; import org.apache.maven.artifact.repository.ArtifactRepository; import org.apache.maven.artifact.repository.RepositoryCache; @@ -75,7 +76,8 @@ public class MavenSession private boolean parallel; - private final Map>> pluginContextsByProjectAndPluginKey = + @SuppressWarnings( "checkstyle:linelength" ) + private final ConcurrentMap>> pluginContextsByProjectAndPluginKey = new ConcurrentHashMap<>(); @@ -192,11 +194,12 @@ public MavenExecutionResult getResult() // Backward compat - public Map getPluginContext( PluginDescriptor plugin, MavenProject project ) + public ConcurrentMap getPluginContext( PluginDescriptor plugin, MavenProject project ) { String projectKey = project.getId(); - Map> pluginContextsByKey = pluginContextsByProjectAndPluginKey.get( projectKey ); + ConcurrentMap> pluginContextsByKey = + pluginContextsByProjectAndPluginKey.get( projectKey ); if ( pluginContextsByKey == null ) { @@ -207,7 +210,7 @@ public Map getPluginContext( PluginDescriptor plugin, MavenProje String pluginKey = plugin.getPluginLookupKey(); - Map pluginContext = pluginContextsByKey.get( pluginKey ); + ConcurrentMap pluginContext = pluginContextsByKey.get( pluginKey ); if ( pluginContext == null ) { diff --git a/maven-core/src/main/java/org/apache/maven/plugin/internal/DefaultMavenPluginManager.java b/maven-core/src/main/java/org/apache/maven/plugin/internal/DefaultMavenPluginManager.java index 0ecd3c95abcc..94337eff905c 100644 --- a/maven-core/src/main/java/org/apache/maven/plugin/internal/DefaultMavenPluginManager.java +++ b/maven-core/src/main/java/org/apache/maven/plugin/internal/DefaultMavenPluginManager.java @@ -100,6 +100,7 @@ import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.concurrent.ConcurrentMap; import java.util.jar.JarFile; import java.util.zip.ZipEntry; @@ -561,7 +562,7 @@ else if ( cause instanceof LinkageError ) { MavenProject project = session.getCurrentProject(); - Map pluginContext = session.getPluginContext( pluginDescriptor, project ); + ConcurrentMap pluginContext = session.getPluginContext( pluginDescriptor, project ); if ( pluginContext != null ) { From b496948f580b92259b501168a1dae0c56e0cc0b4 Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Thu, 7 Oct 2021 13:21:47 +0200 Subject: [PATCH 07/16] Tidy up and javadoc clarifications. --- .../java/org/apache/maven/plugin/AbstractMojo.java | 12 +++++++----- .../org/apache/maven/plugin/ContextEnabled.java | 13 +++++++++---- .../src/main/java/org/apache/maven/plugin/Mojo.java | 6 +++++- 3 files changed, 21 insertions(+), 10 deletions(-) diff --git a/maven-plugin-api/src/main/java/org/apache/maven/plugin/AbstractMojo.java b/maven-plugin-api/src/main/java/org/apache/maven/plugin/AbstractMojo.java index 9e544ca25627..835ab32e16ca 100644 --- a/maven-plugin-api/src/main/java/org/apache/maven/plugin/AbstractMojo.java +++ b/maven-plugin-api/src/main/java/org/apache/maven/plugin/AbstractMojo.java @@ -23,6 +23,8 @@ import org.apache.maven.plugin.logging.Log; +import static java.util.Objects.requireNonNull; + /** * Abstract class to provide most of the infrastructure required to implement a Mojo except for * the execute method.
@@ -151,7 +153,7 @@ public abstract class AbstractMojo @Override public void setLog( Log log ) { - this.log = log; + this.log = requireNonNull( log ); } /** @@ -172,14 +174,14 @@ public Log getLog() } @Override - public Map getPluginContext() + public void setPluginContext( Map pluginContext ) { - return pluginContext; + this.pluginContext = requireNonNull( pluginContext ); } @Override - public void setPluginContext( Map pluginContext ) + public Map getPluginContext() { - this.pluginContext = pluginContext; + return pluginContext; } } diff --git a/maven-plugin-api/src/main/java/org/apache/maven/plugin/ContextEnabled.java b/maven-plugin-api/src/main/java/org/apache/maven/plugin/ContextEnabled.java index c07baed95d47..d3a1820b5497 100644 --- a/maven-plugin-api/src/main/java/org/apache/maven/plugin/ContextEnabled.java +++ b/maven-plugin-api/src/main/java/org/apache/maven/plugin/ContextEnabled.java @@ -23,10 +23,13 @@ /** * Interface to allow Mojos to communicate with each others Mojos, other than - * project's source root and project's attachment.
- * The plugin manager would pull the context out of the plugin container context, and populate it into the Mojo. - * - * @author jdcasey + * project's source root and project's attachment. The plugin manager populates it into the Mojo implementing this + * interface, before executing it. Mojos also may access other contexts using Maven Session API.
+ * Word of warning: Mojos live in their own classloader that is usually destroyed after Mojo is executed, and even + * same Mojo within two different projects will different classloader. That said, (mis) using the context to store + * anything that was loaded up by Mojo classloader is wrong to do and will lead to errors.
+ * Regarding concurrency: best practice is to "write once" (for example invoked Mojo writes to its own context), + * while other Mojos MAY get (via Maven Session API) and inspect (read) other Mojos contexts. */ public interface ContextEnabled { @@ -38,6 +41,8 @@ public interface ContextEnabled void setPluginContext( Map pluginContext ); /** + * Gets the shared context of the mojo. + * * @return a Map stored in the plugin container's context. */ Map getPluginContext(); diff --git a/maven-plugin-api/src/main/java/org/apache/maven/plugin/Mojo.java b/maven-plugin-api/src/main/java/org/apache/maven/plugin/Mojo.java index 97dcebfdb994..0b51d8a3b13c 100644 --- a/maven-plugin-api/src/main/java/org/apache/maven/plugin/Mojo.java +++ b/maven-plugin-api/src/main/java/org/apache/maven/plugin/Mojo.java @@ -33,7 +33,11 @@ */ public interface Mojo { - /** The component role hint for Plexus container */ + /** The component role hint for Plexus container + * + * @deprecated do not use legacy Plexus API to look up by string. + */ + @Deprecated String ROLE = Mojo.class.getName(); /** From ac7286926ad4c5bbfc215ec1c4183830aa206cda Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Fri, 8 Oct 2021 16:03:07 +0200 Subject: [PATCH 08/16] Make session plugin context back to Map --- .../java/org/apache/maven/execution/MavenSession.java | 10 +++++++++- .../plugin/internal/DefaultMavenPluginManager.java | 3 +-- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/maven-core/src/main/java/org/apache/maven/execution/MavenSession.java b/maven-core/src/main/java/org/apache/maven/execution/MavenSession.java index d609ab8ecd96..8dea0ea590c0 100644 --- a/maven-core/src/main/java/org/apache/maven/execution/MavenSession.java +++ b/maven-core/src/main/java/org/apache/maven/execution/MavenSession.java @@ -194,7 +194,15 @@ public MavenExecutionResult getResult() // Backward compat - public ConcurrentMap getPluginContext( PluginDescriptor plugin, MavenProject project ) + /** + * Returns the plugin context for given key ({@link PluginDescriptor#getPluginLookupKey()} and + * {@link MavenProject}, never returns {@code null} as if context not present, creates it. + * + * Implementation note: while this method return type is {@link Map}, the returned map instance + * implements {@link ConcurrentMap} as well. + * + */ + public Map getPluginContext( PluginDescriptor plugin, MavenProject project ) { String projectKey = project.getId(); diff --git a/maven-core/src/main/java/org/apache/maven/plugin/internal/DefaultMavenPluginManager.java b/maven-core/src/main/java/org/apache/maven/plugin/internal/DefaultMavenPluginManager.java index 94337eff905c..0ecd3c95abcc 100644 --- a/maven-core/src/main/java/org/apache/maven/plugin/internal/DefaultMavenPluginManager.java +++ b/maven-core/src/main/java/org/apache/maven/plugin/internal/DefaultMavenPluginManager.java @@ -100,7 +100,6 @@ import java.util.List; import java.util.Map; import java.util.Objects; -import java.util.concurrent.ConcurrentMap; import java.util.jar.JarFile; import java.util.zip.ZipEntry; @@ -562,7 +561,7 @@ else if ( cause instanceof LinkageError ) { MavenProject project = session.getCurrentProject(); - ConcurrentMap pluginContext = session.getPluginContext( pluginDescriptor, project ); + Map pluginContext = session.getPluginContext( pluginDescriptor, project ); if ( pluginContext != null ) { From ea7511cbadea0132f88efed0f202244ab5637496 Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Fri, 8 Oct 2021 16:05:00 +0200 Subject: [PATCH 09/16] Extend deprecation message with "what to do instead" --- .../src/main/java/org/apache/maven/plugin/Mojo.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/maven-plugin-api/src/main/java/org/apache/maven/plugin/Mojo.java b/maven-plugin-api/src/main/java/org/apache/maven/plugin/Mojo.java index 0b51d8a3b13c..c1d089e8972b 100644 --- a/maven-plugin-api/src/main/java/org/apache/maven/plugin/Mojo.java +++ b/maven-plugin-api/src/main/java/org/apache/maven/plugin/Mojo.java @@ -35,7 +35,8 @@ public interface Mojo { /** The component role hint for Plexus container * - * @deprecated do not use legacy Plexus API to look up by string. + * @deprecated do not use legacy Plexus API to look up by string, use JSR330 instead (or if must, use legacy + * Plexus lookup by class instead). */ @Deprecated String ROLE = Mojo.class.getName(); From c0f830ccc29d4a5eaac467a91c9ac421af38afdb Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Fri, 8 Oct 2021 16:59:17 +0200 Subject: [PATCH 10/16] Memoize logger --- .../apache/maven/plugin/internal/MojoLog.java | 48 +++++++++++++++---- 1 file changed, 38 insertions(+), 10 deletions(-) diff --git a/maven-core/src/main/java/org/apache/maven/plugin/internal/MojoLog.java b/maven-core/src/main/java/org/apache/maven/plugin/internal/MojoLog.java index 51409518d91b..73ddc4ab373c 100644 --- a/maven-core/src/main/java/org/apache/maven/plugin/internal/MojoLog.java +++ b/maven-core/src/main/java/org/apache/maven/plugin/internal/MojoLog.java @@ -27,7 +27,9 @@ import static java.util.Objects.requireNonNull; /** - * Mojo {@link Log} implementation that lazily creates {@link Logger} instances. + * Mojo {@link Log} implementation that lazily creates {@link Logger} instances. To achieve "lazyness" it uses + * {@link Provider} for logger, but guarantees that {@link Provider#get()} is invoked only once, and once got + * Logger instance is reused. * * @since TBD */ @@ -36,21 +38,14 @@ public class MojoLog { private final Provider loggerProvider; - private volatile Logger logger; - public MojoLog( Provider loggerProvider ) { - this.loggerProvider = requireNonNull( loggerProvider ); - this.logger = null; + this.loggerProvider = memoize( loggerProvider ); } private Logger getLogger() { - if ( logger == null ) - { - logger = requireNonNull( loggerProvider.get() ); - } - return logger; + return loggerProvider.get(); } private String toString( CharSequence content ) @@ -172,4 +167,37 @@ public boolean isErrorEnabled() { return getLogger().isErrorEnabled(); } + + /** + * A helper bit to make users of this class easier: it wraps {@link Provider} into "memoizing" provider: the + * wrapped (delegate) provider will be invoked only once, and the returned value will be stored for subsequent + * invocations of {@link Provider#get()} on wrapper. + */ + private static Provider memoize( Provider provider ) + { + requireNonNull( provider ); + return new Provider() + { + Provider delegate = this::memoize; + + volatile boolean memoized = false; + + @Override + public T get() + { + return delegate.get(); + } + + private synchronized T memoize() + { + if ( !memoized ) + { + T value = provider.get(); + delegate = () -> value; + memoized = true; + } + return delegate.get(); + } + }; + } } From 26ca30739ad105a857617fc0eb13c0cd2fc6c6d5 Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Fri, 8 Oct 2021 17:02:55 +0200 Subject: [PATCH 11/16] Explain this nested map --- .../main/java/org/apache/maven/execution/MavenSession.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/maven-core/src/main/java/org/apache/maven/execution/MavenSession.java b/maven-core/src/main/java/org/apache/maven/execution/MavenSession.java index 8dea0ea590c0..61e69bf8b947 100644 --- a/maven-core/src/main/java/org/apache/maven/execution/MavenSession.java +++ b/maven-core/src/main/java/org/apache/maven/execution/MavenSession.java @@ -76,6 +76,11 @@ public class MavenSession private boolean parallel; + /** + * Plugin context keyed by project ({@link MavenProject#getId()}) and by plugin lookup key + * ({@link PluginDescriptor#getPluginLookupKey()}). Plugin contexts itself are mappings of {@link String} keys to + * {@link Object} values. + */ @SuppressWarnings( "checkstyle:linelength" ) private final ConcurrentMap>> pluginContextsByProjectAndPluginKey = new ConcurrentHashMap<>(); From e29ce6fb629d9620e6b7a85ed594fb3117faf3b4 Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Sat, 9 Oct 2021 11:04:38 +0200 Subject: [PATCH 12/16] Remove distracting and unrealted changes --- .../apache/maven/execution/MavenSession.java | 22 +++---------------- 1 file changed, 3 insertions(+), 19 deletions(-) diff --git a/maven-core/src/main/java/org/apache/maven/execution/MavenSession.java b/maven-core/src/main/java/org/apache/maven/execution/MavenSession.java index 61e69bf8b947..f8dbb035704f 100644 --- a/maven-core/src/main/java/org/apache/maven/execution/MavenSession.java +++ b/maven-core/src/main/java/org/apache/maven/execution/MavenSession.java @@ -26,7 +26,6 @@ import java.util.Map; import java.util.Properties; import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.ConcurrentMap; import org.apache.maven.artifact.repository.ArtifactRepository; import org.apache.maven.artifact.repository.RepositoryCache; @@ -76,13 +75,7 @@ public class MavenSession private boolean parallel; - /** - * Plugin context keyed by project ({@link MavenProject#getId()}) and by plugin lookup key - * ({@link PluginDescriptor#getPluginLookupKey()}). Plugin contexts itself are mappings of {@link String} keys to - * {@link Object} values. - */ - @SuppressWarnings( "checkstyle:linelength" ) - private final ConcurrentMap>> pluginContextsByProjectAndPluginKey = + private final Map>> pluginContextsByProjectAndPluginKey = new ConcurrentHashMap<>(); @@ -199,20 +192,11 @@ public MavenExecutionResult getResult() // Backward compat - /** - * Returns the plugin context for given key ({@link PluginDescriptor#getPluginLookupKey()} and - * {@link MavenProject}, never returns {@code null} as if context not present, creates it. - * - * Implementation note: while this method return type is {@link Map}, the returned map instance - * implements {@link ConcurrentMap} as well. - * - */ public Map getPluginContext( PluginDescriptor plugin, MavenProject project ) { String projectKey = project.getId(); - ConcurrentMap> pluginContextsByKey = - pluginContextsByProjectAndPluginKey.get( projectKey ); + Map> pluginContextsByKey = pluginContextsByProjectAndPluginKey.get( projectKey ); if ( pluginContextsByKey == null ) { @@ -223,7 +207,7 @@ public Map getPluginContext( PluginDescriptor plugin, MavenProje String pluginKey = plugin.getPluginLookupKey(); - ConcurrentMap pluginContext = pluginContextsByKey.get( pluginKey ); + Map pluginContext = pluginContextsByKey.get( pluginKey ); if ( pluginContext == null ) { From 60d49b50f2990008f4abfc6ce8a5ecea279c5200 Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Sat, 9 Oct 2021 11:58:55 +0200 Subject: [PATCH 13/16] Add isXXXEnabled check in consistent manner One method that does not have message just Throwable does not need to protect toString() invocation. --- .../main/java/org/apache/maven/plugin/internal/MojoLog.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/maven-core/src/main/java/org/apache/maven/plugin/internal/MojoLog.java b/maven-core/src/main/java/org/apache/maven/plugin/internal/MojoLog.java index 73ddc4ab373c..c01ad1f67e17 100644 --- a/maven-core/src/main/java/org/apache/maven/plugin/internal/MojoLog.java +++ b/maven-core/src/main/java/org/apache/maven/plugin/internal/MojoLog.java @@ -63,7 +63,10 @@ private String toString( CharSequence content ) @Override public void debug( CharSequence content ) { - getLogger().debug( toString( content ) ); + if ( getLogger().isDebugEnabled() ) + { + getLogger().debug(toString(content)); + } } @Override From 8d38a7a5a47098d6c24a0fa04fad189b42705a7b Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Sat, 9 Oct 2021 12:04:54 +0200 Subject: [PATCH 14/16] Formatting, no code change --- .../src/main/java/org/apache/maven/plugin/internal/MojoLog.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/maven-core/src/main/java/org/apache/maven/plugin/internal/MojoLog.java b/maven-core/src/main/java/org/apache/maven/plugin/internal/MojoLog.java index c01ad1f67e17..e5492af9ed10 100644 --- a/maven-core/src/main/java/org/apache/maven/plugin/internal/MojoLog.java +++ b/maven-core/src/main/java/org/apache/maven/plugin/internal/MojoLog.java @@ -65,7 +65,7 @@ public void debug( CharSequence content ) { if ( getLogger().isDebugEnabled() ) { - getLogger().debug(toString(content)); + getLogger().debug( toString( content ) ); } } From 3e2ccb025b5e521ee9fbf695e01e86cfc22630c6 Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Fri, 15 Oct 2021 17:02:42 +0200 Subject: [PATCH 15/16] Status quo: remove LogFactory, leave all as it was for now --- .../internal/DefaultMavenPluginManager.java | 5 +- .../maven/plugin/internal/MojoLogFactory.java | 6 +-- .../maven/plugin/logging/LogFactory.java | 46 ------------------- 3 files changed, 3 insertions(+), 54 deletions(-) delete mode 100644 maven-plugin-api/src/main/java/org/apache/maven/plugin/logging/LogFactory.java diff --git a/maven-core/src/main/java/org/apache/maven/plugin/internal/DefaultMavenPluginManager.java b/maven-core/src/main/java/org/apache/maven/plugin/internal/DefaultMavenPluginManager.java index 053f66094481..6ecf4bda15cd 100644 --- a/maven-core/src/main/java/org/apache/maven/plugin/internal/DefaultMavenPluginManager.java +++ b/maven-core/src/main/java/org/apache/maven/plugin/internal/DefaultMavenPluginManager.java @@ -48,7 +48,6 @@ import org.apache.maven.plugin.descriptor.Parameter; import org.apache.maven.plugin.descriptor.PluginDescriptor; import org.apache.maven.plugin.descriptor.PluginDescriptorBuilder; -import org.apache.maven.plugin.logging.LogFactory; import org.apache.maven.plugin.version.DefaultPluginVersionRequest; import org.apache.maven.plugin.version.PluginVersionRequest; import org.apache.maven.plugin.version.PluginVersionResolutionException; @@ -144,7 +143,7 @@ public class DefaultMavenPluginManager private PluginVersionResolver pluginVersionResolver; private PluginArtifactsCache pluginArtifactsCache; private MavenPluginValidator pluginValidator; - private LogFactory logFactory; + private MojoLogFactory logFactory; private final ExtensionDescriptorBuilder extensionDescriptorBuilder = new ExtensionDescriptorBuilder(); private final PluginDescriptorBuilder builder = new PluginDescriptorBuilder(); @@ -161,7 +160,7 @@ public DefaultMavenPluginManager( PluginVersionResolver pluginVersionResolver, PluginArtifactsCache pluginArtifactsCache, MavenPluginValidator pluginValidator, - LogFactory logFactory ) + MojoLogFactory logFactory ) { this.container = container; this.classRealmManager = classRealmManager; diff --git a/maven-core/src/main/java/org/apache/maven/plugin/internal/MojoLogFactory.java b/maven-core/src/main/java/org/apache/maven/plugin/internal/MojoLogFactory.java index 00d5dbd4c7ad..d8d64bd0d5e7 100644 --- a/maven-core/src/main/java/org/apache/maven/plugin/internal/MojoLogFactory.java +++ b/maven-core/src/main/java/org/apache/maven/plugin/internal/MojoLogFactory.java @@ -23,26 +23,22 @@ import javax.inject.Singleton; import org.apache.maven.plugin.logging.Log; -import org.apache.maven.plugin.logging.LogFactory; import org.slf4j.LoggerFactory; /** - * Implementation of Mojo {@link LogFactory} backed by Maven internal logging (slf4j). + * Implementation of Mojo {@link Log} factory backed by Maven internal logging (slf4j). * * @since TBD */ @Singleton @Named public class MojoLogFactory - implements LogFactory { - @Override public Log getLog( final Class clazz ) { return new MojoLog( () -> LoggerFactory.getLogger( clazz ) ); } - @Override public Log getLog( final String name ) { return new MojoLog( () -> LoggerFactory.getLogger( name ) ); diff --git a/maven-plugin-api/src/main/java/org/apache/maven/plugin/logging/LogFactory.java b/maven-plugin-api/src/main/java/org/apache/maven/plugin/logging/LogFactory.java deleted file mode 100644 index 85f9ff343955..000000000000 --- a/maven-plugin-api/src/main/java/org/apache/maven/plugin/logging/LogFactory.java +++ /dev/null @@ -1,46 +0,0 @@ -package org.apache.maven.plugin.logging; - -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -/** - * This component supplies {@link Log} instances. You can require this component to be injected into your own components - * and use it for provisioning logger instances. Simplest is to inject it via constructor injection (recommended way) - * and just get a {@link Log} instance: - * 
- *   Log log = logFactory.getLog( getClass() );
- *
- * - * {@link Log} instance for mojo implementations are provided by this component as well, but to not push constructor - * signature down to implementations, it is injected using setter method. - * - * @since TBD - */ -public interface LogFactory -{ - /** - * Returns the {@link Log} instance for given class. - */ - Log getLog( Class clazz ); - - /** - * Returns the {@link Log} instance for given name. - */ - Log getLog( String name ); -} From 4a291d4a4f125ddb70a24daf1715c48e14c81432 Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Fri, 15 Oct 2021 18:28:42 +0200 Subject: [PATCH 16/16] Undo export --- maven-core/src/main/resources/META-INF/maven/extension.xml | 1 - 1 file changed, 1 deletion(-) diff --git a/maven-core/src/main/resources/META-INF/maven/extension.xml b/maven-core/src/main/resources/META-INF/maven/extension.xml index d64bbeceb9aa..a9e201c70825 100644 --- a/maven-core/src/main/resources/META-INF/maven/extension.xml +++ b/maven-core/src/main/resources/META-INF/maven/extension.xml @@ -35,7 +35,6 @@ under the License. org.apache.maven.model org.apache.maven.monitor org.apache.maven.plugin - org.apache.maven.plugin.logging org.apache.maven.profiles org.apache.maven.project org.apache.maven.reporting