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 08b758dd11c227ef8e39a516a8c2f7e0ba4593cd
Author: Andreas Dangel <adan...@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() {
