Allow incremental restore in case of plugin parameter mismatch

reda-alaoui · reda-alaoui · commit 846045af14b9 · 2024-08-21T23:00:52.000+02:00
diff --git a/src/main/java/org/apache/maven/buildcache/BuildCacheMojosExecutionStrategy.java b/src/main/java/org/apache/maven/buildcache/BuildCacheMojosExecutionStrategy.java
@@ -24,10 +24,13 @@
 
 import java.io.File;
 import java.nio.file.Path;
+import java.util.Collection;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
 
 import org.apache.commons.lang3.ArrayUtils;
 import org.apache.commons.lang3.StringUtils;
@@ -130,14 +133,14 @@ public void execute(
             }
 
             boolean restorable = result.isSuccess() || result.isPartialSuccess();
-            boolean restored = false; // if partially restored need to save increment
+            CacheRestorationStatus restorationStatus =
+                    CacheRestorationStatus.FAILURE; // if partially restored need to save increment
             if (restorable) {
-                CacheRestorationStatus cacheRestorationStatus =
-                        restoreProject(result, mojoExecutions, mojoExecutionRunner, cacheConfig);
-                restored = CacheRestorationStatus.SUCCESS == cacheRestorationStatus;
-                executeExtraCleanPhaseIfNeeded(cacheRestorationStatus, cleanPhase, mojoExecutionRunner);
+                restorationStatus = restoreProject(result, mojoExecutions, mojoExecutionRunner, cacheConfig);
+                executeExtraCleanPhaseIfNeeded(restorationStatus, cleanPhase, mojoExecutionRunner);
             }
-            if (!restored) {
+            if (restorationStatus != CacheRestorationStatus.SUCCESS
+                    && restorationStatus != CacheRestorationStatus.INCREMENTAL_SUCCESS) {
                 for (MojoExecution mojoExecution : mojoExecutions) {
                     if (source == Source.CLI
                             || mojoExecution.getLifecyclePhase() == null
@@ -147,7 +150,8 @@ public void execute(
                 }
             }
 
-            if (cacheState == INITIALIZED && (!result.isSuccess() || !restored)) {
+            if (cacheState == INITIALIZED
+                    && (!result.isSuccess() || restorationStatus != CacheRestorationStatus.SUCCESS)) {
                 if (cacheConfig.isSkipSave()) {
                     LOGGER.info("Cache saving is disabled.");
                 } else if (cacheConfig.isMandatoryClean()
@@ -228,20 +232,35 @@ private CacheRestorationStatus restoreProject(
             // Verify cache consistency for cached mojos
             LOGGER.debug("Verify consistency on cached mojos");
             Set<MojoExecution> forcedExecutionMojos = new HashSet<>();
+            Set<MojoExecution> reconciliationExecutionMojos = new HashSet<>();
             for (MojoExecution cacheCandidate : cachedSegment) {
                 if (cacheController.isForcedExecution(project, cacheCandidate)) {
                     forcedExecutionMojos.add(cacheCandidate);
                 } else {
+                    if (!reconciliationExecutionMojos.isEmpty()) {
+                        reconciliationExecutionMojos.add(cacheCandidate);
+                        continue;
+                    }
                     if (!verifyCacheConsistency(
                             cacheCandidate, build, project, session, mojoExecutionRunner, cacheConfig)) {
-                        LOGGER.info("A cached mojo is not consistent, continuing with non cached build");
-                        return CacheRestorationStatus.FAILURE;
+                        if (!cacheConfig.isIncrementalReconciliationOnParameterMismatch()) {
+                            LOGGER.info("A cached mojo is not consistent, continuing with non cached build");
+                            return CacheRestorationStatus.FAILURE;
+                        } else {
+                            LOGGER.info("A cached mojo is not consistent, will reconciliate from here");
+                            reconciliationExecutionMojos.add(cacheCandidate);
+                        }
                     }
                 }
             }
 
+            Set<MojoExecution> plannedExecutions = Stream.concat(
+                            forcedExecutionMojos.stream(), reconciliationExecutionMojos.stream())
+                    .collect(Collectors.toSet());
             // Restore project artifacts
-            ArtifactRestorationReport restorationReport = cacheController.restoreProjectArtifacts(cacheResult);
+            ArtifactRestorationReport restorationReport = cacheController.restoreProjectArtifacts(
+                    cacheResult,
+                    !containsExecution(plannedExecutions, "org.apache.maven.plugins", "maven-jar-plugin", "jar"));
             if (!restorationReport.isSuccess()) {
                 LOGGER.info("Cannot restore project artifacts, continuing with non cached build");
                 return restorationReport.isRestoredFilesInProjectDirectory()
@@ -267,6 +286,12 @@ private CacheRestorationStatus restoreProject(
                     // mojoExecutionScope.seed(
                     //        org.apache.maven.api.MojoExecution.class, new DefaultMojoExecution(cacheCandidate));
                     mojoExecutionRunner.run(cacheCandidate);
+                } else if (reconciliationExecutionMojos.contains(cacheCandidate)) {
+                    LOGGER.info(
+                            "Mojo execution is needed for reconciliation: {}",
+                            cacheCandidate.getMojoDescriptor().getFullGoalName());
+                    mojoExecutionScope.seed(MojoExecution.class, cacheCandidate);
+                    mojoExecutionRunner.run(cacheCandidate);
                 } else {
                     LOGGER.info(
                             "Skipping plugin execution (cached): {}",
@@ -295,12 +320,34 @@ private CacheRestorationStatus restoreProject(
             for (MojoExecution mojoExecution : postCachedSegment) {
                 mojoExecutionRunner.run(mojoExecution);
             }
-            return CacheRestorationStatus.SUCCESS;
+
+            if (reconciliationExecutionMojos.isEmpty()) {
+                return CacheRestorationStatus.SUCCESS;
+            } else {
+                return CacheRestorationStatus.INCREMENTAL_SUCCESS;
+            }
         } finally {
             mojoExecutionScope.exit();
         }
     }
 
+    private boolean containsExecution(
+            Collection<MojoExecution> executions, String groupId, String artifactId, String goal) {
+        for (MojoExecution execution : executions) {
+            if (!groupId.equals(execution.getGroupId())) {
+                continue;
+            }
+            if (!artifactId.equals(execution.getArtifactId())) {
+                continue;
+            }
+            if (!goal.equals(execution.getGoal())) {
+                continue;
+            }
+            return true;
+        }
+        return false;
+    }
+
     private boolean verifyCacheConsistency(
             MojoExecution cacheCandidate,
             Build cachedBuild,
@@ -433,6 +480,7 @@ private static String normalizedPath(Path path, Path baseDirPath) {
 
     private enum CacheRestorationStatus {
         SUCCESS,
+        INCREMENTAL_SUCCESS,
         FAILURE,
         FAILURE_NEEDS_CLEAN
     }
diff --git a/src/main/java/org/apache/maven/buildcache/CacheController.java b/src/main/java/org/apache/maven/buildcache/CacheController.java
@@ -35,7 +35,7 @@ public interface CacheController {
     CacheResult findCachedBuild(
             MavenSession session, MavenProject project, List<MojoExecution> mojoExecutions, boolean skipCache);
 
-    ArtifactRestorationReport restoreProjectArtifacts(CacheResult cacheResult);
+    ArtifactRestorationReport restoreProjectArtifacts(CacheResult cacheResult, boolean setProjectArtifact);
 
     void save(
             CacheResult cacheResult,
diff --git a/src/main/java/org/apache/maven/buildcache/CacheControllerImpl.java b/src/main/java/org/apache/maven/buildcache/CacheControllerImpl.java
@@ -345,7 +345,7 @@ private void verifyRestorationInsideProject(final MavenProject project, Path pat
     }
 
     @Override
-    public ArtifactRestorationReport restoreProjectArtifacts(CacheResult cacheResult) {
+    public ArtifactRestorationReport restoreProjectArtifacts(CacheResult cacheResult, boolean setProjectArtifact) {
 
         LOGGER.debug("Restore project artifacts");
         final Build build = cacheResult.getBuildInfo();
@@ -412,7 +412,9 @@ public ArtifactRestorationReport restoreProjectArtifacts(CacheResult cacheResult
             // Actually modify project at the end in case something went wrong during restoration,
             // in which case, the project is unmodified and we continue with normal build.
             if (restoredProjectArtifact != null) {
-                project.setArtifact(restoredProjectArtifact);
+                if (setProjectArtifact) {
+                    project.setArtifact(restoredProjectArtifact);
+                }
                 // need to include package lifecycle to save build info for incremental builds
                 if (!project.hasLifecyclePhase("package")) {
                     project.addLifecyclePhase("package");
diff --git a/src/main/java/org/apache/maven/buildcache/xml/CacheConfig.java b/src/main/java/org/apache/maven/buildcache/xml/CacheConfig.java
@@ -152,4 +152,6 @@ public interface CacheConfig {
      * Flag to save in cache only if a build went through the clean lifecycle
      */
     boolean isMandatoryClean();
+
+    boolean isIncrementalReconciliationOnParameterMismatch();
 }
diff --git a/src/main/java/org/apache/maven/buildcache/xml/CacheConfigImpl.java b/src/main/java/org/apache/maven/buildcache/xml/CacheConfigImpl.java
@@ -95,6 +95,8 @@ public class CacheConfigImpl implements org.apache.maven.buildcache.xml.CacheCon
     public static final String RESTORE_GENERATED_SOURCES_PROPERTY_NAME = "maven.build.cache.restoreGeneratedSources";
     public static final String ALWAYS_RUN_PLUGINS = "maven.build.cache.alwaysRunPlugins";
     public static final String MANDATORY_CLEAN = "maven.build.cache.mandatoryClean";
+    public static final String INCREMENTAL_RECONCILIATION_ON_PARAMETER_MISMATCH =
+            "maven.build.cache.incrementalReconciliationOnParameterMismatch";
 
     /**
      * Flag to control if we should skip lookup for cached artifacts globally or for a particular project even if
@@ -538,6 +540,13 @@ public boolean isMandatoryClean() {
         return getProperty(MANDATORY_CLEAN, getConfiguration().isMandatoryClean());
     }
 
+    @Override
+    public boolean isIncrementalReconciliationOnParameterMismatch() {
+        return getProperty(
+                INCREMENTAL_RECONCILIATION_ON_PARAMETER_MISMATCH,
+                getConfiguration().isIncrementalReconciliationOnParameterMismatch());
+    }
+
     @Override
     public String getId() {
         checkInitializedState();
diff --git a/src/main/mdo/build-cache-config.mdo b/src/main/mdo/build-cache-config.mdo
@@ -241,6 +241,14 @@ under the License.
                     <description>FileHash (causes file hash is saved in build metadata) or
                         EffectivePom (causes effective pom info is saved in build metadata)</description>
                 </field>
+                <field>
+                    <name>incrementalReconciliationOnParameterMismatch</name>
+                    <type>boolean</type>
+                    <defaultValue>false</defaultValue>
+                    <description>If true, on plugin execution parameter mismatch, the build will try to complete by
+                    running the mismatched execution and executions that follow. If false, on plugin execution parameter
+                    mismatch, a full build will be performed.</description>
+                </field>
             </fields>
         </class>
 
diff --git a/src/test/java/org/apache/maven/buildcache/its/Issue103Test.java b/src/test/java/org/apache/maven/buildcache/its/Issue103Test.java
@@ -0,0 +1,72 @@
+/*
+ * 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.buildcache.its;
+
+import java.util.Arrays;
+
+import org.apache.maven.buildcache.its.junit.IntegrationTest;
+import org.apache.maven.it.VerificationException;
+import org.apache.maven.it.Verifier;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+
+import static org.apache.maven.buildcache.util.LogFileUtils.findFirstLineContainingTextsInLogs;
+
+/**
+ * @author Réda Housni Alaoui
+ */
+@IntegrationTest("src/test/projects/mbuildcache-103")
+class Issue103Test {
+
+    private static final String PROJECT_NAME = "org.apache.maven.caching.test.mbuildcache-103:simple";
+
+    @Test
+    void simple(Verifier verifier) throws VerificationException {
+        verifier.setAutoclean(false);
+
+        verifier.getCliOptions().clear();
+        verifier.addCliOption("-Dmaven.build.cache.incrementalReconciliationOnParameterMismatch");
+        verifier.addCliOption("-DskipTests");
+        verifier.setLogFileName("../log-1.txt");
+        verifier.executeGoals(Arrays.asList("clean", "verify"));
+        verifier.verifyErrorFreeLog();
+
+        verifier.getCliOptions().clear();
+        verifier.addCliOption("-Dmaven.build.cache.incrementalReconciliationOnParameterMismatch");
+        verifier.setLogFileName("../log-2.txt");
+        verifier.executeGoals(Arrays.asList("clean", "verify"));
+        verifier.verifyErrorFreeLog();
+        verifier.verifyTextInLog("No tests to run.");
+        verifier.verifyTextInLog("Building jar");
+        verifier.verifyTextInLog("Saved Build to local file");
+        verifyNoTextInLog(verifier, "A cached mojo is not consistent, continuing with non cached build");
+
+        verifier.getCliOptions().clear();
+        verifier.addCliOption("-Dmaven.build.cache.incrementalReconciliationOnParameterMismatch");
+        verifier.setLogFileName("../log-3.txt");
+        verifier.executeGoals(Arrays.asList("clean", "verify"));
+        verifier.verifyErrorFreeLog();
+        verifier.verifyTextInLog("Found cached build, restoring " + PROJECT_NAME + " from cache by");
+        verifier.verifyTextInLog("Skipping plugin execution (cached): surefire:test");
+    }
+
+    private static void verifyNoTextInLog(Verifier verifier, String text) throws VerificationException {
+        Assertions.assertNull(findFirstLineContainingTextsInLogs(verifier, text));
+    }
+}
diff --git a/src/test/projects/mbuildcache-103/.mvn/maven-build-cache-config.xml b/src/test/projects/mbuildcache-103/.mvn/maven-build-cache-config.xml
@@ -0,0 +1,35 @@
+<?xml version="1.0" encoding="UTF-8" ?>
+
+<!--
+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.
+-->
+
+<cache xmlns="http://maven.apache.org/BUILD-CACHE-CONFIG/1.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+       xsi:schemaLocation="http://maven.apache.org/BUILD-CACHE-CONFIG/1.0.0 https://maven.apache.org/xsd/build-cache-config-1.0.0.xsd">
+  <executionControl>
+    <reconcile>
+      <plugins>
+        <plugin artifactId="maven-surefire-plugin" goal="test">
+          <reconciles>
+            <reconcile propertyName="skipTests" skipValue="true"/>
+          </reconciles>
+        </plugin>
+      </plugins>
+    </reconcile>
+  </executionControl>
+</cache>
diff --git a/src/test/projects/mbuildcache-103/pom.xml b/src/test/projects/mbuildcache-103/pom.xml
@@ -0,0 +1,42 @@
+<!--
+
+    Copyright 2021 the original author or authors.
+
+    Licensed 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.
+
+-->
+<project xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns="http://maven.apache.org/POM/4.0.0"
+         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
+
+    <modelVersion>4.0.0</modelVersion>
+    <groupId>org.apache.maven.caching.test.mbuildcache-103</groupId>
+    <artifactId>simple</artifactId>
+    <version>0.0.1-SNAPSHOT</version>
+    <packaging>jar</packaging>
+
+    <properties>
+        <maven.compiler.source>1.8</maven.compiler.source>
+        <maven.compiler.target>1.8</maven.compiler.target>
+    </properties>
+
+    <build>
+        <extensions>
+            <extension>
+                <groupId>org.apache.maven.extensions</groupId>
+                <artifactId>maven-build-cache-extension</artifactId>
+                <version>${projectVersion}</version>
+            </extension>
+        </extensions>
+    </build>
+
+</project>
diff --git a/src/test/projects/mbuildcache-103/src/main/java/org/apache/maven/buildcache/Test.java b/src/test/projects/mbuildcache-103/src/main/java/org/apache/maven/buildcache/Test.java
@@ -0,0 +1,24 @@
+/*
+ * 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.buildcache;
+
+class Test
+{
+
+}

Original file line number	Diff line number	Diff line change
`@@ -152,4 +152,6 @@ public interface CacheConfig {`
`152`	`152`	`* Flag to save in cache only if a build went through the clean lifecycle`
`153`	`153`	`*/`
`154`	`154`	`boolean isMandatoryClean();`
	`155`	`+`
	`156`	`+ boolean isIncrementalReconciliationOnParameterMismatch();`
`155`	`157`	`}`