From 1c85628457b2165886319cd4efaa917377ab4d85 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9da=20Housni=20Alaoui?= Date: Wed, 21 Aug 2024 17:09:52 +0200 Subject: [PATCH] Allow incremental restore in case of plugin parameter mismatch --- .../BuildCacheMojosExecutionStrategy.java | 74 +++++++++++++++---- .../maven/buildcache/CacheController.java | 2 +- .../maven/buildcache/CacheControllerImpl.java | 6 +- .../maven/buildcache/xml/CacheConfig.java | 2 + .../maven/buildcache/xml/CacheConfigImpl.java | 9 +++ src/main/mdo/build-cache-config.mdo | 8 ++ .../maven/buildcache/its/Issue103Test.java | 72 ++++++++++++++++++ .../.mvn/maven-build-cache-config.xml | 35 +++++++++ src/test/projects/mbuildcache-103/pom.xml | 42 +++++++++++ .../org/apache/maven/buildcache/Test.java | 24 ++++++ 10 files changed, 257 insertions(+), 17 deletions(-) create mode 100644 src/test/java/org/apache/maven/buildcache/its/Issue103Test.java create mode 100644 src/test/projects/mbuildcache-103/.mvn/maven-build-cache-config.xml create mode 100644 src/test/projects/mbuildcache-103/pom.xml create mode 100644 src/test/projects/mbuildcache-103/src/main/java/org/apache/maven/buildcache/Test.java diff --git a/src/main/java/org/apache/maven/buildcache/BuildCacheMojosExecutionStrategy.java b/src/main/java/org/apache/maven/buildcache/BuildCacheMojosExecutionStrategy.java index 5790d19f..373515e6 100644 --- 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 forcedExecutionMojos = new HashSet<>(); + Set 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 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 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, @@ -320,9 +367,7 @@ private boolean verifyCacheConsistency( final String fullGoalName = cacheCandidate.getMojoDescriptor().getFullGoalName(); if (completedExecution != null && !isParamsMatched(project, cacheCandidate, mojo, completedExecution)) { - LOGGER.info( - "Mojo cached parameters mismatch with actual, forcing full project build. Mojo: {}", - fullGoalName); + LOGGER.info("Mojo cached parameters mismatch with actual. Mojo: {}", fullGoalName); consistent = false; } @@ -433,6 +478,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 index 7acf7850..37114661 100644 --- 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 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 index 6cc644f9..3b12b500 100644 --- 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 index d86b15d4..e25fa00b 100644 --- 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 index 566a067a..645fd64e 100644 --- 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 index ecb15e63..b5c1e06e 100644 --- a/src/main/mdo/build-cache-config.mdo +++ b/src/main/mdo/build-cache-config.mdo @@ -241,6 +241,14 @@ under the License. FileHash (causes file hash is saved in build metadata) or EffectivePom (causes effective pom info is saved in build metadata) + + incrementalReconciliationOnParameterMismatch + boolean + false + 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. + diff --git a/src/test/java/org/apache/maven/buildcache/its/Issue103Test.java b/src/test/java/org/apache/maven/buildcache/its/Issue103Test.java new file mode 100644 index 00000000..1b7599c5 --- /dev/null +++ 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 new file mode 100644 index 00000000..502f9448 --- /dev/null +++ b/src/test/projects/mbuildcache-103/.mvn/maven-build-cache-config.xml @@ -0,0 +1,35 @@ + + + + + + + + + + + + + + + + + diff --git a/src/test/projects/mbuildcache-103/pom.xml b/src/test/projects/mbuildcache-103/pom.xml new file mode 100644 index 00000000..a04d7924 --- /dev/null +++ b/src/test/projects/mbuildcache-103/pom.xml @@ -0,0 +1,42 @@ + + + + 4.0.0 + org.apache.maven.caching.test.mbuildcache-103 + simple + 0.0.1-SNAPSHOT + jar + + + 1.8 + 1.8 + + + + + + org.apache.maven.extensions + maven-build-cache-extension + ${projectVersion} + + + + + 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 new file mode 100644 index 00000000..03f66a82 --- /dev/null +++ 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 +{ + +} \ No newline at end of file