This is an automated email from the ASF dual-hosted git repository. gnodet pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/maven.git
The following commit(s) were added to refs/heads/master by this push: new ddae283c2b [MNG-8245][MNG-8246] Warn when calling before: or after: phases (#1974) ddae283c2b is described below commit ddae283c2b318ba9814384c66a002b4821f53aa4 Author: Guillaume Nodet <gno...@gmail.com> AuthorDate: Sat Dec 14 00:07:01 2024 +0100 [MNG-8245][MNG-8246] Warn when calling before: or after: phases (#1974) --- .../DefaultLifecycleTaskSegmentCalculator.java | 14 +++ .../concurrent/ConcurrentLifecycleStarter.java | 11 +++ .../maven/it/MavenITmng8245BeforePhaseCliTest.java | 102 +++++++++++++++++++++ .../org/apache/maven/it/TestSuiteOrdering.java | 1 + .../mng-8245-before-after-phase-all/pom.xml | 45 +++++++++ 5 files changed, 173 insertions(+) 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 <a href="https://issues.apache.org/jira/browse/MNG-8245">MNG-8245</a> + * and <a href="https://issues.apache.org/jira/browse/MNG-8246">MNG-8246</a>. + */ +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 @@ +<?xml version="1.0" encoding="UTF-8"?> +<project xmlns="http://maven.apache.org/POM/4.1.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" root="true" xsi:schemaLocation="http://maven.apache.org/POM/4.1.0 https://maven.apache.org/xsd/maven-4.1.0.xsd"> + + <groupId>org.apahce.maven.its</groupId> + <artifactId>mng-8245</artifactId> + <version>1.0.0-SNAPSHOT</version> + + <build> + <plugins> + <plugin> + <groupId>com.soebes.maven.plugins</groupId> + <artifactId>echo-maven-plugin</artifactId> + <version>0.5.0</version> + <executions> + <execution> + <id>before-clean</id> + <goals> + <goal>echo</goal> + </goals> + <phase>before:clean</phase> + <configuration> + <echos> + <echo>Hallo 'before:clean' phase.</echo> + </echos> + </configuration> + </execution> + <execution> + <id>after-clean</id> + <goals> + <goal>echo</goal> + </goals> + <phase>after:clean</phase> + <configuration> + <echos> + <echo>Hallo 'after:clean' phase.</echo> + </echos> + </configuration> + </execution> + </executions> + </plugin> + </plugins> + + </build> + +</project>