[MNG-8421] Move all of logging setup to LookupInvoker; mvnenc IT (#1964)

Currently mvnenc is unable to log to file (-l) as logging setup is incomplete, move all of this logic to LookupInvoker. Also, create prompt in mvnenc only when needed. Finally, implement needed changes to support mvnenc ITs and add mvnenc IT. Other changes: * get rid of `distributionFileName` dirty hack, is remnant from old ITs * fix CI re removal of that above and use of site that is brain-dead --- https://issues.apache.org/jira/browse/MNG-8421
apache · Dec 12, 2024 · 5b7a6de · 5b7a6de
1 parent 9c77221
commit 5b7a6de
Show file tree

Hide file tree

Showing 23 changed files with 336 additions and 126 deletions.
diff --git a/.github/workflows/maven.yml b/.github/workflows/maven.yml
@@ -56,7 +56,7 @@ jobs:
 
       - name: Build Maven distributions
         shell: bash
-        run: ./mvnw verify -e -B -V -DdistributionFileName=apache-maven -Dmaven.repo.local=$HOME/.m2/repository/cached
+        run: ./mvnw verify -e -B -V -Dmaven.repo.local=$HOME/.m2/repository/cached
 
       - name: List contents of target directory
         shell: bash
@@ -117,9 +117,14 @@ jobs:
         run: |
           mkdir -p maven-local
           if [ "${{ runner.os }}" = "Windows" ]; then
-            unzip maven-dist/apache-maven-bin.zip -d maven-local
+            unzip maven-dist/apache-maven-*-bin.zip -d maven-local
+            # Get the name of the extracted directory
+            MAVEN_DIR=$(ls maven-local)
+            # Move contents up one level
+            mv "maven-local/$MAVEN_DIR"/* maven-local/
+            rm -r "maven-local/$MAVEN_DIR"
           else
-            tar xzf maven-dist/apache-maven-bin.tar.gz -C maven-local --strip-components 1
+            tar xzf maven-dist/apache-maven-*-bin.tar.gz -C maven-local --strip-components 1
           fi
           echo "MAVEN_HOME=$PWD/maven-local" >> $GITHUB_ENV
           echo "$PWD/maven-local/bin" >> $GITHUB_PATH
@@ -133,9 +138,13 @@ jobs:
             maven-${{ runner.os }}-full-
             maven-${{ runner.os }}-
 
+      - name: Build with downloaded Maven
+        shell: bash
+        run: mvn verify -e -B -V -Dmaven.repo.local=$HOME/.m2/repository/cached
+
       - name: Build site with downloaded Maven
         shell: bash
-        run: mvn verify site -e -B -V -Preporting -Dmaven.repo.local=$HOME/.m2/repository/cached
+        run: mvn site -e -B -V -Preporting -Dmaven.repo.local=$HOME/.m2/repository/cached
 
   integration-tests:
     needs: initial-build
@@ -172,14 +181,14 @@ jobs:
         run: |
           mkdir -p maven-local
           if [ "${{ runner.os }}" = "Windows" ]; then
-            unzip maven-dist/apache-maven-bin.zip -d maven-local
+            unzip maven-dist/apache-maven-*-bin.zip -d maven-local
             # Get the name of the extracted directory
             MAVEN_DIR=$(ls maven-local)
             # Move contents up one level
             mv "maven-local/$MAVEN_DIR"/* maven-local/
             rm -r "maven-local/$MAVEN_DIR"
           else
-            tar xzf maven-dist/apache-maven-bin.tar.gz -C maven-local --strip-components 1
+            tar xzf maven-dist/apache-maven-*-bin.tar.gz -C maven-local --strip-components 1
           fi
           echo "MAVEN_HOME=$PWD/maven-local" >> $GITHUB_ENV
           echo "$PWD/maven-local/bin" >> $GITHUB_PATH

diff --git a/apache-maven/pom.xml b/apache-maven/pom.xml
@@ -32,10 +32,6 @@ under the License.
   <name>Apache Maven Distribution</name>
   <description>The Apache Maven distribution, source and binary, in zip and tar.gz formats.</description>
 
-  <properties>
-    <distributionFileName>${distributionId}-${project.version}</distributionFileName>
-  </properties>
-
   <dependencies>
     <dependency>
       <groupId>org.apache.maven</groupId>
@@ -162,7 +158,6 @@ under the License.
   </pluginRepositories>
 
   <build>
-    <finalName>${distributionFileName}</finalName>
     <plugins>
       <plugin>
         <groupId>org.apache.maven.plugins</groupId>

diff --git a/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/LookupContext.java b/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/LookupContext.java
@@ -35,6 +35,7 @@
 import org.apache.maven.api.services.Lookup;
 import org.apache.maven.api.settings.Settings;
 import org.apache.maven.cling.logging.Slf4jConfiguration;
+import org.apache.maven.logging.BuildEventListener;
 import org.jline.terminal.Terminal;
 import org.slf4j.ILoggerFactory;
 
@@ -86,6 +87,8 @@ protected LookupContext(InvokerRequest invokerRequest) {
     public ContainerCapsule containerCapsule;
     public Lookup lookup;
 
+    public BuildEventListener buildEventListener;
+
     // paths user can override from CLI, and we need to set on MavenExReq
     public Path installationSettingsPath;
     public Path projectSettingsPath;

diff --git a/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/LookupInvoker.java b/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/LookupInvoker.java
@@ -69,7 +69,10 @@
 import org.apache.maven.internal.impl.SettingsUtilsV4;
 import org.apache.maven.jline.FastTerminal;
 import org.apache.maven.jline.MessageUtils;
+import org.apache.maven.logging.BuildEventListener;
 import org.apache.maven.logging.LoggingOutputStream;
+import org.apache.maven.logging.ProjectBuildLogAppender;
+import org.apache.maven.logging.SimpleBuildEventListener;
 import org.apache.maven.logging.api.LogLevelRecorder;
 import org.apache.maven.slf4j.MavenSimpleLogger;
 import org.jline.terminal.Terminal;
@@ -228,6 +231,23 @@ protected void configureLogging(C context) throws Exception {
         context.slf4jConfiguration.setRootLoggerLevel(context.loggerLevel);
         // else fall back to default log level specified in conf
         // see https://issues.apache.org/jira/browse/MNG-2570
+
+        // Create the build log appender; also sets MavenSimpleLogger sink
+        ProjectBuildLogAppender projectBuildLogAppender =
+                new ProjectBuildLogAppender(determineBuildEventListener(context));
+        context.closeables.add(projectBuildLogAppender);
+    }
+
+    protected BuildEventListener determineBuildEventListener(C context) {
+        if (context.buildEventListener == null) {
+            context.buildEventListener = doDetermineBuildEventListener(context);
+        }
+        return context.buildEventListener;
+    }
+
+    protected BuildEventListener doDetermineBuildEventListener(C context) {
+        Consumer<String> writer = determineWriter(context);
+        return new SimpleBuildEventListener(writer);
     }
 
     protected void createTerminal(C context) {

diff --git a/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvn/MavenContext.java b/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvn/MavenContext.java
@@ -22,15 +22,13 @@
 import org.apache.maven.api.cli.InvokerRequest;
 import org.apache.maven.cling.invoker.LookupContext;
 import org.apache.maven.eventspy.internal.EventSpyDispatcher;
-import org.apache.maven.logging.BuildEventListener;
 
 @SuppressWarnings("VisibilityModifier")
 public class MavenContext extends LookupContext {
     public MavenContext(InvokerRequest invokerRequest) {
         super(invokerRequest);
     }
 
-    public BuildEventListener buildEventListener;
     public EventSpyDispatcher eventSpyDispatcher;
     public Maven maven;
 

diff --git a/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvn/MavenInvoker.java b/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvn/MavenInvoker.java
@@ -27,7 +27,6 @@
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
-import java.util.function.Consumer;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
@@ -68,11 +67,8 @@
 import org.apache.maven.execution.ProjectActivation;
 import org.apache.maven.jline.MessageUtils;
 import org.apache.maven.lifecycle.LifecycleExecutionException;
-import org.apache.maven.logging.BuildEventListener;
 import org.apache.maven.logging.LoggingExecutionListener;
 import org.apache.maven.logging.MavenTransferListener;
-import org.apache.maven.logging.ProjectBuildLogAppender;
-import org.apache.maven.logging.SimpleBuildEventListener;
 import org.apache.maven.project.MavenProject;
 import org.codehaus.plexus.PlexusContainer;
 import org.eclipse.aether.DefaultRepositoryCache;
@@ -149,27 +145,6 @@ protected void postCommands(C context) throws Exception {
         }
     }
 
-    @Override
-    protected void configureLogging(C context) throws Exception {
-        super.configureLogging(context);
-        // Create the build log appender
-        ProjectBuildLogAppender projectBuildLogAppender =
-                new ProjectBuildLogAppender(determineBuildEventListener(context));
-        context.closeables.add(projectBuildLogAppender);
-    }
-
-    protected BuildEventListener determineBuildEventListener(C context) {
-        if (context.buildEventListener == null) {
-            context.buildEventListener = doDetermineBuildEventListener(context);
-        }
-        return context.buildEventListener;
-    }
-
-    protected BuildEventListener doDetermineBuildEventListener(C context) {
-        Consumer<String> writer = determineWriter(context);
-        return new SimpleBuildEventListener(writer);
-    }
-
     @Override
     protected void customizeSettingsRequest(C context, SettingsBuilderRequest settingsBuilderRequest) throws Exception {
         if (context.eventSpyDispatcher != null) {

diff --git a/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnenc/EncryptContext.java b/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnenc/EncryptContext.java
@@ -23,7 +23,6 @@
 
 import org.apache.maven.api.cli.InvokerRequest;
 import org.apache.maven.cling.invoker.LookupContext;
-import org.jline.consoleui.prompt.ConsolePrompt;
 import org.jline.reader.LineReader;
 import org.jline.utils.AttributedString;
 import org.jline.utils.AttributedStringBuilder;
@@ -40,7 +39,6 @@ protected EncryptContext(InvokerRequest invokerRequest) {
     public List<AttributedString> header;
     public AttributedStyle style;
     public LineReader reader;
-    public ConsolePrompt prompt;
 
     public void addInHeader(String text) {
         addInHeader(AttributedStyle.DEFAULT, text);

diff --git a/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnenc/EncryptInvoker.java b/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnenc/EncryptInvoker.java
@@ -26,13 +26,11 @@
 import org.apache.maven.cling.invoker.LookupInvoker;
 import org.apache.maven.cling.invoker.ProtoLookup;
 import org.apache.maven.cling.utils.CLIReportingUtils;
-import org.jline.consoleui.prompt.ConsolePrompt;
 import org.jline.reader.LineReaderBuilder;
 import org.jline.reader.UserInterruptException;
 import org.jline.terminal.Terminal;
 import org.jline.utils.AttributedStyle;
 import org.jline.utils.Colors;
-import org.jline.utils.OSUtils;
 
 /**
  * mvnenc invoker implementation.
@@ -76,17 +74,9 @@ protected int doExecute(EncryptContext context) throws Exception {
 
             Thread executeThread = Thread.currentThread();
             context.terminal.handle(Terminal.Signal.INT, signal -> executeThread.interrupt());
-            ConsolePrompt.UiConfig config;
-            if (OSUtils.IS_WINDOWS) {
-                config = new ConsolePrompt.UiConfig(">", "( )", "(x)", "( )");
-            } else {
-                config = new ConsolePrompt.UiConfig("❯", "◯ ", "◉ ", "◯ ");
-            }
-            config.setCancellableFirstPrompt(true);
 
             context.reader =
                     LineReaderBuilder.builder().terminal(context.terminal).build();
-            context.prompt = new ConsolePrompt(context.reader, context.terminal, config);
 
             EncryptOptions options = (EncryptOptions) context.invokerRequest.options();
             if (options.goals().isEmpty() || options.goals().get().size() != 1) {
@@ -102,14 +92,13 @@ protected int doExecute(EncryptContext context) throws Exception {
 
             return goal.execute(context);
         } catch (InterruptedException | InterruptedIOException | UserInterruptException e) {
-            context.terminal.writer().println("Goal canceled by user.");
+            context.logger.error("Goal canceled by user.");
             return CANCELED;
         } catch (Exception e) {
             if (context.invokerRequest.options().showErrors().orElse(false)) {
-                context.terminal.writer().println(e.getMessage());
-                e.printStackTrace(context.terminal.writer());
+                context.logger.error(e.getMessage(), e);
             } else {
-                context.terminal.writer().println(e.getMessage());
+                context.logger.error(e.getMessage());
             }
             return ERROR;
         } finally {
@@ -118,9 +107,9 @@ protected int doExecute(EncryptContext context) throws Exception {
     }
 
     protected int badGoalsErrorMessage(String message, EncryptContext context) {
-        context.terminal.writer().println(message);
-        context.terminal.writer().println("Supported goals are: " + String.join(", ", context.goals.keySet()));
-        context.terminal.writer().println("Use -h to display help.");
+        context.logger.error(message);
+        context.logger.error("Supported goals are: " + String.join(", ", context.goals.keySet()));
+        context.logger.error("Use -h to display help.");
         return BAD_OPERATION;
     }
 }
diff --git a/...-cli/src/main/java/org/apache/maven/cling/invoker/mvnenc/goals/ConfiguredGoalSupport.java b/...-cli/src/main/java/org/apache/maven/cling/invoker/mvnenc/goals/ConfiguredGoalSupport.java
@@ -39,12 +39,10 @@ protected ConfiguredGoalSupport(MessageBuilderFactory messageBuilderFactory, Sec
     @Override
     public int execute(EncryptContext context) throws Exception {
         if (!validateConfiguration(context)) {
-            context.terminal
-                    .writer()
-                    .println(messageBuilderFactory
-                            .builder()
-                            .error("Maven Encryption is not configured, run `mvnenc init` first.")
-                            .build());
+            context.logger.error(messageBuilderFactory
+                    .builder()
+                    .error("Maven Encryption is not configured, run `mvnenc init` first.")
+                    .build());
             return ERROR;
         }
         return doExecute(context);
@@ -59,36 +57,32 @@ protected boolean validateConfiguration(EncryptContext context) {
     }
 
     protected void dumpResponse(EncryptContext context, String indent, SecDispatcher.ValidationResponse response) {
-        context.terminal
-                .writer()
-                .println(messageBuilderFactory
-                        .builder()
-                        .format(
-                                response.isValid()
-                                        ? messageBuilderFactory
-                                                .builder()
-                                                .success("%sConfiguration validation of %s: %s")
-                                                .build()
-                                        : messageBuilderFactory
-                                                .builder()
-                                                .failure("%sConfiguration validation of %s: %s")
-                                                .build(),
-                                indent,
-                                response.getSource(),
-                                response.isValid() ? "VALID" : "INVALID"));
+        context.logger.info(messageBuilderFactory
+                .builder()
+                .format(
+                        response.isValid()
+                                ? messageBuilderFactory
+                                        .builder()
+                                        .success("%sConfiguration validation of %s: %s")
+                                        .build()
+                                : messageBuilderFactory
+                                        .builder()
+                                        .failure("%sConfiguration validation of %s: %s")
+                                        .build(),
+                        indent,
+                        response.getSource(),
+                        response.isValid() ? "VALID" : "INVALID")
+                .build());
         for (Map.Entry<SecDispatcher.ValidationResponse.Level, List<String>> entry :
                 response.getReport().entrySet()) {
-            Consumer<String> consumer = s -> context.terminal
-                    .writer()
-                    .println(messageBuilderFactory.builder().info(s).build());
+            Consumer<String> consumer = s ->
+                    context.logger.info(messageBuilderFactory.builder().info(s).build());
             if (entry.getKey() == SecDispatcher.ValidationResponse.Level.ERROR) {
-                consumer = s -> context.terminal
-                        .writer()
-                        .println(messageBuilderFactory.builder().error(s).build());
+                consumer = s -> context.logger.error(
+                        messageBuilderFactory.builder().error(s).build());
             } else if (entry.getKey() == SecDispatcher.ValidationResponse.Level.WARNING) {
-                consumer = s -> context.terminal
-                        .writer()
-                        .println(messageBuilderFactory.builder().warning(s).build());
+                consumer = s -> context.logger.warn(
+                        messageBuilderFactory.builder().warning(s).build());
             }
             for (String line : entry.getValue()) {
                 consumer.accept(indent + "  " + line);

diff --git a/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnenc/goals/Diag.java b/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnenc/goals/Diag.java
@@ -29,7 +29,7 @@
 import static org.apache.maven.cling.invoker.mvnenc.EncryptInvoker.OK;
 
 /**
- * The "diag" goal.
+ * The "diag" goal. It should always run, despite it overrides configured goal support.
  */
 @Singleton
 @Named("diag")
@@ -40,8 +40,13 @@ public Diag(MessageBuilderFactory messageBuilderFactory, SecDispatcher secDispat
     }
 
     @Override
-    protected int doExecute(EncryptContext context) {
+    public int execute(EncryptContext context) {
         dumpResponse(context, "", secDispatcher.validateConfiguration());
         return OK;
     }
+
+    @Override
+    protected int doExecute(EncryptContext context) throws Exception {
+        throw new IllegalStateException("Cannot reach here");
+    }
 }