You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@maven.apache.org by ad...@apache.org on 2023/05/20 08:57:32 UTC
[maven-pmd-plugin] 02/03: Finish logging implementation (fix it MPMD-244-logging)
This is an automated email from the ASF dual-hosted git repository.
adangel pushed a commit to branch pmd7
in repository https://gitbox.apache.org/repos/asf/maven-pmd-plugin.git
commit 72969c7f9dc21c21613592ed00e9ad130b7f008d
Author: Andreas Dangel <ad...@apache.org>
AuthorDate: Sat May 20 10:30:55 2023 +0200
Finish logging implementation (fix it MPMD-244-logging)
---
src/it/MPMD-244-logging/invoker.properties | 1 -
src/it/MPMD-244-logging/logging-disabled/pom.xml | 9 ---
.../src/main/java/App.java} | 7 +--
src/it/MPMD-244-logging/logging-enabled/pom.xml | 9 ---
.../src/main/java/App.java} | 7 +--
src/it/MPMD-244-logging/verify.groovy | 26 +--------
.../maven/plugins/pmd/AbstractPmdReport.java | 15 ++++-
.../apache/maven/plugins/pmd/exec/Executor.java | 65 +++++++---------------
.../apache/maven/plugins/pmd/exec/PmdRequest.java | 2 +-
9 files changed, 39 insertions(+), 102 deletions(-)
diff --git a/src/it/MPMD-244-logging/invoker.properties b/src/it/MPMD-244-logging/invoker.properties
index 38b6d74..d57dce3 100644
--- a/src/it/MPMD-244-logging/invoker.properties
+++ b/src/it/MPMD-244-logging/invoker.properties
@@ -16,6 +16,5 @@
# under the License.
invoker.goals = clean pmd:check
-invoker.goals.2 = pmd:check --projects logging-enabled --log-file build2.log
invoker.maven.version = 3.1.0+
invoker.debug = true
diff --git a/src/it/MPMD-244-logging/logging-disabled/pom.xml b/src/it/MPMD-244-logging/logging-disabled/pom.xml
index 46f0e2b..429d707 100644
--- a/src/it/MPMD-244-logging/logging-disabled/pom.xml
+++ b/src/it/MPMD-244-logging/logging-disabled/pom.xml
@@ -44,17 +44,8 @@ under the License.
<artifactId>@project.artifactId@</artifactId>
<version>@project.version@</version>
<configuration>
- <skipPmdError>true</skipPmdError>
<showPmdLog>false</showPmdLog>
</configuration>
- <executions>
- <execution>
- <phase>test-compile</phase>
- <goals>
- <goal>check</goal>
- </goals>
- </execution>
- </executions>
</plugin>
</plugins>
</build>
diff --git a/src/it/MPMD-244-logging/logging-enabled/src/main/java/BrokenFile.java b/src/it/MPMD-244-logging/logging-disabled/src/main/java/App.java
similarity index 88%
rename from src/it/MPMD-244-logging/logging-enabled/src/main/java/BrokenFile.java
rename to src/it/MPMD-244-logging/logging-disabled/src/main/java/App.java
index 1c5335a..64889d2 100644
--- a/src/it/MPMD-244-logging/logging-enabled/src/main/java/BrokenFile.java
+++ b/src/it/MPMD-244-logging/logging-disabled/src/main/java/App.java
@@ -17,10 +17,5 @@
* under the License.
*/
-public class BrokenFile
-{
-
- // This file has a parse error, so PMD will fail to parse it
- broken!!
-
+public class App {
}
diff --git a/src/it/MPMD-244-logging/logging-enabled/pom.xml b/src/it/MPMD-244-logging/logging-enabled/pom.xml
index 355992c..a7fe73b 100644
--- a/src/it/MPMD-244-logging/logging-enabled/pom.xml
+++ b/src/it/MPMD-244-logging/logging-enabled/pom.xml
@@ -44,17 +44,8 @@ under the License.
<artifactId>@project.artifactId@</artifactId>
<version>@project.version@</version>
<configuration>
- <skipPmdError>true</skipPmdError>
<showPmdLog>true</showPmdLog>
</configuration>
- <executions>
- <execution>
- <phase>test-compile</phase>
- <goals>
- <goal>check</goal>
- </goals>
- </execution>
- </executions>
</plugin>
</plugins>
</build>
diff --git a/src/it/MPMD-244-logging/logging-disabled/src/main/java/BrokenFile.java b/src/it/MPMD-244-logging/logging-enabled/src/main/java/App.java
similarity index 88%
rename from src/it/MPMD-244-logging/logging-disabled/src/main/java/BrokenFile.java
rename to src/it/MPMD-244-logging/logging-enabled/src/main/java/App.java
index 1c5335a..64889d2 100644
--- a/src/it/MPMD-244-logging/logging-disabled/src/main/java/BrokenFile.java
+++ b/src/it/MPMD-244-logging/logging-enabled/src/main/java/App.java
@@ -17,10 +17,5 @@
* under the License.
*/
-public class BrokenFile
-{
-
- // This file has a parse error, so PMD will fail to parse it
- broken!!
-
+public class App {
}
diff --git a/src/it/MPMD-244-logging/verify.groovy b/src/it/MPMD-244-logging/verify.groovy
index b3d0d7e..7360fd0 100644
--- a/src/it/MPMD-244-logging/verify.groovy
+++ b/src/it/MPMD-244-logging/verify.groovy
@@ -19,27 +19,7 @@
File buildLog = new File( basedir, 'build.log' )
assert buildLog.exists()
-assert buildLog.text.contains( "PMD processing errors" )
-assert buildLog.text.contains( "ParseException" )
-String disabledPath = new File( basedir, 'logging-disabled/src/main/java/BrokenFile.java' ).getCanonicalPath()
-String enabledPath = new File( basedir, 'logging-enabled/src/main/java/BrokenFile.java' ).getCanonicalPath()
-
-// logging disabled: the pmd exception is only output through the processing error reporting (since MPMD-246)
-assert 1 == buildLog.text.count( "${disabledPath}: ParseException: Parse exception in file" )
-
-// TODO: with PMD 7, the parse exception is not logged through PMD's logging anymore, it is only added as a processing error
-// in the report. is this correct?
-// logging enabled: the pmd exception is output twice: through the processing error reporting (since MPMD-246) and through PMD's own logging
-// assert 2 == buildLog.text.count( "${enabledPath}: ParseException: Parse exception in file" )
-assert 1 == buildLog.text.count( "${enabledPath}: ParseException: Parse exception in file" )
-
-// logging disabled module is executed first, which disables the logging
-// even when logging-enabled is executed afterwards in the same JVM, the logger are not reinitialized
-// everywhere, so logging is most likely still disabled.
-assert 0 == buildLog.text.count( "[DEBUG] Rules loaded from" )
-
-// only in the second invoker run, when only logging-enabled is executed, the logs from PMD are visible
-File build2Log = new File( basedir, 'build2.log' )
-assert build2Log.exists()
-assert 1 == build2Log.text.count( "[DEBUG] Rules loaded from" )
+// build.log contains the logging from the two PMD executions
+// only one execution has logging enabled, so we expect only one log output
+assert 1 == buildLog.text.count( "[DEBUG] Rules loaded from" )
diff --git a/src/main/java/org/apache/maven/plugins/pmd/AbstractPmdReport.java b/src/main/java/org/apache/maven/plugins/pmd/AbstractPmdReport.java
index d44028f..ff6ff90 100644
--- a/src/main/java/org/apache/maven/plugins/pmd/AbstractPmdReport.java
+++ b/src/main/java/org/apache/maven/plugins/pmd/AbstractPmdReport.java
@@ -49,6 +49,8 @@ import org.apache.maven.toolchain.ToolchainManager;
import org.codehaus.plexus.util.FileUtils;
import org.codehaus.plexus.util.PathTool;
import org.codehaus.plexus.util.StringUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
/**
* Base class for the PMD reports.
@@ -484,8 +486,17 @@ public abstract class AbstractPmdReport extends AbstractMavenReport {
logLevel = System.getProperty("maven.logging.root.level");
}
if (logLevel == null) {
- // TODO: logback level
- logLevel = "info";
+ Logger rootLogger = LoggerFactory.getLogger(Logger.ROOT_LOGGER_NAME);
+ if (rootLogger.isErrorEnabled()) {
+ logLevel = "error";
+ }
+ if (rootLogger.isInfoEnabled() || rootLogger.isWarnEnabled()) {
+ logLevel = "info";
+ }
+ if (rootLogger.isTraceEnabled() || rootLogger.isDebugEnabled()) {
+ logLevel = "debug";
+ }
+ logLevel = "info"; // default
}
return logLevel;
}
diff --git a/src/main/java/org/apache/maven/plugins/pmd/exec/Executor.java b/src/main/java/org/apache/maven/plugins/pmd/exec/Executor.java
index 6bcf115..6ef13f2 100644
--- a/src/main/java/org/apache/maven/plugins/pmd/exec/Executor.java
+++ b/src/main/java/org/apache/maven/plugins/pmd/exec/Executor.java
@@ -29,77 +29,52 @@ import java.net.URL;
import java.net.URLClassLoader;
import java.net.URLDecoder;
import java.nio.charset.StandardCharsets;
-import java.util.logging.Handler;
-import java.util.logging.Level;
-import java.util.logging.SimpleFormatter;
+import net.sourceforge.pmd.internal.Slf4jSimpleConfiguration;
import org.apache.maven.cli.logging.Slf4jConfiguration;
import org.apache.maven.cli.logging.Slf4jConfigurationFactory;
import org.codehaus.plexus.logging.console.ConsoleLogger;
import org.slf4j.ILoggerFactory;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-import org.slf4j.bridge.SLF4JBridgeHandler;
abstract class Executor {
private static final Logger LOG = LoggerFactory.getLogger(Executor.class);
/**
- * This holds a strong reference in case we configured the logger to
- * redirect to slf4j. See {@link #showPmdLog}. Without a strong reference,
- * the logger might be garbage collected and the redirect to slf4j is gone.
+ * Configures the appropriate log levels for PMD or disables
+ * PMD logging.
+ *
+ * @param showPmdLog whether the PMD logs should appear in the maven log output
+ * @param logLevel the maven log level, e.g. "info" or "debug".
*/
- private java.util.logging.Logger julLogger;
-
protected void setupPmdLogging(boolean showPmdLog, String logLevel) {
-
- // TODO: enabling/disabling the log doesn't work reliably, because
- // the log level is cached at each logger and the logger instances
- // are usually static.
if (!showPmdLog) {
System.setProperty("org.slf4j.simpleLogger.log.net.sourceforge.pmd", "off");
} else {
System.clearProperty("org.slf4j.simpleLogger.log.net.sourceforge.pmd");
}
- ILoggerFactory slf4jLoggerFactory = LoggerFactory.getILoggerFactory();
- Slf4jConfiguration slf4jConfiguration = Slf4jConfigurationFactory.getConfiguration(slf4jLoggerFactory);
- slf4jConfiguration.activate();
-
- if (!showPmdLog) {
- return;
- }
- java.util.logging.Logger logger = java.util.logging.Logger.getLogger("net.sourceforge.pmd");
-
- boolean slf4jBridgeAlreadyAdded = false;
- for (Handler handler : logger.getHandlers()) {
- if (handler instanceof SLF4JBridgeHandler) {
- slf4jBridgeAlreadyAdded = true;
- break;
- }
- }
-
- if (slf4jBridgeAlreadyAdded) {
- return;
- }
-
- SLF4JBridgeHandler handler = new SLF4JBridgeHandler();
- SimpleFormatter formatter = new SimpleFormatter();
- handler.setFormatter(formatter);
- logger.setUseParentHandlers(false);
- logger.addHandler(handler);
- handler.setLevel(Level.ALL);
- logger.setLevel(Level.ALL);
- julLogger = logger;
- julLogger.fine("Configured jul-to-slf4j bridge for " + logger.getName());
+ // When slf4j-simple is in use, the log level is cached at each logger and the logger
+ // instances are usually static. So they don't see any configuration changes at runtime
+ // normally. This call will go through each logger and reinitialize these with the
+ // freshly determined log level from the configuration.
+ // Note: This only works for slf4j-simple.
+ Slf4jSimpleConfiguration.reconfigureDefaultLogLevel(null);
}
+ /**
+ * Initializes the maven logging system. This is only needed when toolchain is in use. In that
+ * case we fork a new Java process to run PMD.
+ *
+ * @param logLevel the desired log level, e.g. "debug" or "info".
+ */
protected void setupLogLevel(String logLevel) {
ILoggerFactory slf4jLoggerFactory = LoggerFactory.getILoggerFactory();
Slf4jConfiguration slf4jConfiguration = Slf4jConfigurationFactory.getConfiguration(slf4jLoggerFactory);
- if ("debug".equals(logLevel)) {
+ if ("debug".equalsIgnoreCase(logLevel)) {
slf4jConfiguration.setRootLoggerLevel(Slf4jConfiguration.Level.DEBUG);
- } else if ("info".equals(logLevel)) {
+ } else if ("info".equalsIgnoreCase(logLevel)) {
slf4jConfiguration.setRootLoggerLevel(Slf4jConfiguration.Level.INFO);
} else {
slf4jConfiguration.setRootLoggerLevel(Slf4jConfiguration.Level.ERROR);
diff --git a/src/main/java/org/apache/maven/plugins/pmd/exec/PmdRequest.java b/src/main/java/org/apache/maven/plugins/pmd/exec/PmdRequest.java
index d1a8099..150eef6 100644
--- a/src/main/java/org/apache/maven/plugins/pmd/exec/PmdRequest.java
+++ b/src/main/java/org/apache/maven/plugins/pmd/exec/PmdRequest.java
@@ -215,7 +215,7 @@ public class PmdRequest implements Serializable {
}
public boolean isDebugEnabled() {
- return "debug".equals(logLevel);
+ return "debug".equalsIgnoreCase(logLevel);
}
public boolean isSkipPmdError() {