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 db1275c8c7497a7321d142bf5490c6d74ce21504
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() {

Reply via email to