From ddae283c2b318ba9814384c66a002b4821f53aa4 Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Sat, 14 Dec 2024 00:07:01 +0100 Subject: [PATCH] [MNG-8245][MNG-8246] Warn when calling before: or after: phases (#1974) --- ...DefaultLifecycleTaskSegmentCalculator.java | 14 +++ .../ConcurrentLifecycleStarter.java | 11 ++ .../it/MavenITmng8245BeforePhaseCliTest.java | 102 ++++++++++++++++++ .../apache/maven/it/TestSuiteOrdering.java | 1 + .../mng-8245-before-after-phase-all/pom.xml | 45 ++++++++ 5 files changed, 173 insertions(+) create mode 100644 its/core-it-suite/src/test/java/org/apache/maven/it/MavenITmng8245BeforePhaseCliTest.java create mode 100644 its/core-it-suite/src/test/resources/mng-8245-before-after-phase-all/pom.xml diff --git a/impl/maven-core/src/main/java/org/apache/maven/lifecycle/internal/DefaultLifecycleTaskSegmentCalculator.java b/impl/maven-core/src/main/java/org/apache/maven/lifecycle/internal/DefaultLifecycleTaskSegmentCalculator.java index 39faca5300..6d17a835db 100644 --- a/impl/maven-core/src/main/java/org/apache/maven/lifecycle/internal/DefaultLifecycleTaskSegmentCalculator.java +++ b/impl/maven-core/src/main/java/org/apache/maven/lifecycle/internal/DefaultLifecycleTaskSegmentCalculator.java @@ -27,6 +27,7 @@ import java.util.List; import java.util.stream.Collectors; import java.util.stream.Stream; +import org.apache.maven.api.Lifecycle; import org.apache.maven.execution.MavenSession; import org.apache.maven.lifecycle.LifecycleNotFoundException; import org.apache.maven.lifecycle.LifecyclePhaseNotFoundException; @@ -39,6 +40,8 @@ import org.apache.maven.plugin.descriptor.MojoDescriptor; import org.apache.maven.plugin.prefix.NoPluginFoundForPrefixException; import org.apache.maven.plugin.version.PluginVersionResolutionException; import org.apache.maven.project.MavenProject; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import static java.util.Objects.requireNonNull; @@ -53,6 +56,8 @@ import static java.util.Objects.requireNonNull; @Named @Singleton public class DefaultLifecycleTaskSegmentCalculator implements LifecycleTaskSegmentCalculator { + private static final Logger LOGGER = LoggerFactory.getLogger(DefaultLifecycleTaskSegmentCalculator.class); + private final MojoDescriptorCreator mojoDescriptorCreator; private final LifecyclePluginResolver lifecyclePluginResolver; @@ -95,6 +100,11 @@ public class DefaultLifecycleTaskSegmentCalculator implements LifecycleTaskSegme TaskSegment currentSegment = null; for (String task : tasks) { + if (isBeforeOrAfterPhase(task)) { + String prevTask = task; + task = PhaseId.of(task).phase(); + LOGGER.warn("Illegal call to phase '{}'. The main phase '{}' will be used instead.", prevTask, task); + } if (isGoalSpecification(task)) { // "pluginPrefix[:version]:goal" or "groupId:artifactId[:version]:goal" @@ -139,6 +149,10 @@ public class DefaultLifecycleTaskSegmentCalculator implements LifecycleTaskSegme return false; } + private boolean isBeforeOrAfterPhase(String task) { + return task.startsWith(Lifecycle.BEFORE) || task.startsWith(Lifecycle.AFTER); + } + private boolean isGoalSpecification(String task) { return task.indexOf(':') >= 0; } diff --git a/impl/maven-core/src/main/java/org/apache/maven/lifecycle/internal/concurrent/ConcurrentLifecycleStarter.java b/impl/maven-core/src/main/java/org/apache/maven/lifecycle/internal/concurrent/ConcurrentLifecycleStarter.java index e4422ffc1c..3ba6c99499 100644 --- a/impl/maven-core/src/main/java/org/apache/maven/lifecycle/internal/concurrent/ConcurrentLifecycleStarter.java +++ b/impl/maven-core/src/main/java/org/apache/maven/lifecycle/internal/concurrent/ConcurrentLifecycleStarter.java @@ -27,6 +27,7 @@ import java.util.List; import java.util.stream.Collectors; import java.util.stream.Stream; +import org.apache.maven.api.Lifecycle; import org.apache.maven.execution.ExecutionEvent; import org.apache.maven.execution.MavenSession; import org.apache.maven.lifecycle.DefaultLifecycles; @@ -38,6 +39,7 @@ import org.apache.maven.lifecycle.internal.LifecyclePluginResolver; import org.apache.maven.lifecycle.internal.LifecycleStarter; import org.apache.maven.lifecycle.internal.LifecycleTask; import org.apache.maven.lifecycle.internal.MojoDescriptorCreator; +import org.apache.maven.lifecycle.internal.PhaseId; import org.apache.maven.lifecycle.internal.ReactorBuildStatus; import org.apache.maven.lifecycle.internal.ReactorContext; import org.apache.maven.lifecycle.internal.TaskSegment; @@ -138,6 +140,11 @@ public class ConcurrentLifecycleStarter implements LifecycleStarter { TaskSegment currentSegment = null; for (String task : tasks) { + if (isBeforeOrAfterPhase(task)) { + String prevTask = task; + task = PhaseId.of(task).phase(); + logger.warn("Illegal call to phase '{}'. The main phase '{}' will be used instead.", prevTask, task); + } if (isGoalSpecification(task)) { // "pluginPrefix[:version]:goal" or "groupId:artifactId[:version]:goal" @@ -185,6 +192,10 @@ public class ConcurrentLifecycleStarter implements LifecycleStarter { return false; } + private boolean isBeforeOrAfterPhase(String task) { + return task.startsWith(Lifecycle.BEFORE) || task.startsWith(Lifecycle.AFTER); + } + private boolean isGoalSpecification(String task) { return task.indexOf(':') >= 0; } diff --git a/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITmng8245BeforePhaseCliTest.java b/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITmng8245BeforePhaseCliTest.java new file mode 100644 index 0000000000..8cda53a3be --- /dev/null +++ b/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITmng8245BeforePhaseCliTest.java @@ -0,0 +1,102 @@ +/* + * 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. + */ +package org.apache.maven.it; + +import java.io.File; + +import org.junit.jupiter.api.Test; + +/** + * This is a test set for MNG-8245 + * and MNG-8246. + */ +class MavenITmng8245BeforePhaseCliTest extends AbstractMavenIntegrationTestCase { + + MavenITmng8245BeforePhaseCliTest() { + super("[4.0.0-rc-2,)"); + } + + /** + * Verify phase before:clean spits a warning and calls clean + */ + @Test + void testPhaseBeforeCleanAllWihConcurrentBuilder() throws Exception { + File testDir = extractResources("/mng-8245-before-after-phase-all"); + + Verifier verifier = newVerifier(testDir.getAbsolutePath()); + verifier.setLogFileName("before-clean-concurrent.txt"); + verifier.addCliArguments("-b", "concurrent", "before:clean"); + verifier.execute(); + + verifier.verifyTextInLog("Illegal call to phase 'before:clean'. The main phase 'clean' will be used instead."); + verifier.verifyTextInLog("Hallo 'before:clean' phase."); + verifier.verifyTextInLog("Hallo 'after:clean' phase."); + } + + /** + * Verify phase before:clean spits a warning and calls clean + */ + @Test + void testPhaseBeforeCleanAllWithLegacyBuilder() throws Exception { + File testDir = extractResources("/mng-8245-before-after-phase-all"); + + Verifier verifier = newVerifier(testDir.getAbsolutePath()); + verifier.setLogFileName("before-clean-legacy.txt"); + verifier.addCliArguments("before:clean"); + verifier.execute(); + + verifier.verifyTextInLog("Illegal call to phase 'before:clean'. The main phase 'clean' will be used instead."); + verifier.verifyTextInLog("Hallo 'before:clean' phase."); + verifier.verifyTextInLog("Hallo 'after:clean' phase."); + } + + /** + * Verify phase after:clean spits a warning and calls clean + */ + @Test + void testPhaseAfterCleanAllWihConcurrentBuilder() throws Exception { + File testDir = extractResources("/mng-8245-before-after-phase-all"); + + Verifier verifier = newVerifier(testDir.getAbsolutePath()); + verifier.setLogFileName("after-clean-concurrent.txt"); + verifier.addCliArguments("-b", "concurrent", "after:clean"); + verifier.execute(); + + verifier.verifyTextInLog("Illegal call to phase 'after:clean'. The main phase 'clean' will be used instead."); + verifier.verifyTextInLog("Hallo 'before:clean' phase."); + verifier.verifyTextInLog("Hallo 'after:clean' phase."); + } + + /** + * Verify phase after:clean spits a warning and calls clean + */ + @Test + void testPhaseAfterCleanAllWithLegacyBuilder() throws Exception { + File testDir = extractResources("/mng-8245-before-after-phase-all"); + + Verifier verifier = newVerifier(testDir.getAbsolutePath()); + verifier.setLogFileName("after-clean-legacy.txt"); + verifier.addCliArguments("after:clean"); + verifier.execute(); + + verifier.verifyTextInLog("Illegal call to phase 'after:clean'. The main phase 'clean' will be used instead."); + verifier.verifyTextInLog("Hallo 'before:clean' phase."); + verifier.verifyTextInLog("Hallo 'after:clean' phase."); + } +} diff --git a/its/core-it-suite/src/test/java/org/apache/maven/it/TestSuiteOrdering.java b/its/core-it-suite/src/test/java/org/apache/maven/it/TestSuiteOrdering.java index 5a5a4c149a..87b599b69f 100644 --- a/its/core-it-suite/src/test/java/org/apache/maven/it/TestSuiteOrdering.java +++ b/its/core-it-suite/src/test/java/org/apache/maven/it/TestSuiteOrdering.java @@ -100,6 +100,7 @@ public class TestSuiteOrdering implements ClassOrderer { * the tests are to finishing. Newer tests are also more likely to fail, so this is * a fail fast technique as well. */ + suite.addTestSuite(MavenITmng8245BeforePhaseCliTest.class); suite.addTestSuite(MavenITmng8244PhaseAllTest.class); suite.addTestSuite(MavenITmng8421MavenEncryptionTest.class); suite.addTestSuite(MavenITmng8400CanonicalMavenHomeTest.class); diff --git a/its/core-it-suite/src/test/resources/mng-8245-before-after-phase-all/pom.xml b/its/core-it-suite/src/test/resources/mng-8245-before-after-phase-all/pom.xml new file mode 100644 index 0000000000..910de385d0 --- /dev/null +++ b/its/core-it-suite/src/test/resources/mng-8245-before-after-phase-all/pom.xml @@ -0,0 +1,45 @@ + + + + org.apahce.maven.its + mng-8245 + 1.0.0-SNAPSHOT + + + + + com.soebes.maven.plugins + echo-maven-plugin + 0.5.0 + + + before-clean + + echo + + before:clean + + + Hallo 'before:clean' phase. + + + + + after-clean + + echo + + after:clean + + + Hallo 'after:clean' phase. + + + + + + + + + +