From 8d4a01857ee1f29e5c86e1ec06ea6fe7c706a4ed Mon Sep 17 00:00:00 2001 From: cowwoc Date: Sun, 5 Oct 2025 17:48:08 -0400 Subject: [PATCH 1/2] Change default attachedOutputs to include classes and test-classes Previously, when attachedOutputs was not specified in maven-build-cache-config.xml, the build cache would return an empty list, meaning no directories were cached by default. This caused issues where compiled classes were not restored from cache, leading to build failures or inconsistent behavior. Changes: - Modified CacheConfigImpl.getAttachedOutputs() to return a default list containing 'classes' and 'test-classes' directories when attachedOutputs is not configured - Added getDefaultAttachedOutputs() helper method to create the default configuration - Updated usage.md to reflect that classes and test-classes are now restored by default - Updated build-cache-config.mdo documentation to document the new default behavior This change addresses user reports in issues #259 and #340 where builds failed after cache restoration due to missing compiled classes. It also aligns with user expectations from PR #177 discussion where this default was requested. Fixes: #259, #340 Related: #177 --- .../maven/buildcache/xml/CacheConfigImpl.java | 19 ++++++++++++++++++- src/main/mdo/build-cache-config.mdo | 6 ++++-- src/site/markdown/usage.md | 9 ++++----- 3 files changed, 26 insertions(+), 8 deletions(-) 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 cd6e87c0..e3b8a45b 100644 --- a/src/main/java/org/apache/maven/buildcache/xml/CacheConfigImpl.java +++ b/src/main/java/org/apache/maven/buildcache/xml/CacheConfigImpl.java @@ -575,7 +575,24 @@ public String getLocalRepositoryLocation() { public List getAttachedOutputs() { checkInitializedState(); final AttachedOutputs attachedOutputs = getConfiguration().getAttachedOutputs(); - return attachedOutputs == null ? Collections.emptyList() : attachedOutputs.getDirNames(); + if (attachedOutputs == null) { + return getDefaultAttachedOutputs(); + } + return attachedOutputs.getDirNames(); + } + + private List getDefaultAttachedOutputs() { + List defaults = new ArrayList<>(); + + DirName classes = new DirName(); + classes.setValue("classes"); + defaults.add(classes); + + DirName testClasses = new DirName(); + testClasses.setValue("test-classes"); + defaults.add(testClasses); + + return defaults; } @Override diff --git a/src/main/mdo/build-cache-config.mdo b/src/main/mdo/build-cache-config.mdo index 52ae0da0..0b7a2853 100644 --- a/src/main/mdo/build-cache-config.mdo +++ b/src/main/mdo/build-cache-config.mdo @@ -374,7 +374,8 @@ under the License. --> AttachedOutputs - Section relative to outputs which are not artifacts but need to be saved/restored. + Section relative to outputs which are not artifacts but need to be saved/restored. + If not specified, defaults to caching 'classes' and 'test-classes' directories. dirNames @@ -382,7 +383,8 @@ under the License. DirName * - Path to a directory containing files which needs to be saved/restored (relative to the build directory). + Path to a directory containing files which needs to be saved/restored (relative to the build directory). + When omitted, the cache defaults to saving and restoring the 'classes' and 'test-classes' directories. diff --git a/src/site/markdown/usage.md b/src/site/markdown/usage.md index 505dc460..893027e9 100644 --- a/src/site/markdown/usage.md +++ b/src/site/markdown/usage.md @@ -72,8 +72,7 @@ When a configuration is disabled by default in the config, it can be enabled via Build cache extension is generally compatible with IDEs with one limitation: -* The cache doesn't restore the entire project state. Compiled classes, unpacked artifacts, and similar ones typically - will not be restored in the build directory (aka `target`). Configure your IDE to not use Maven - build (`target`) directories for compilation and execution. In that case, IDE will provide fast compilation using - native caches, and - the build cache will supplement that with fast builds. +* The cache restores `classes` and `test-classes` directories by default, but may not restore other artifacts like + unpacked dependencies or generated resources. Configure your IDE to not use Maven build (`target`) directories for + compilation and execution if you experience issues. In that case, IDE will provide fast compilation using + native caches, and the build cache will supplement that with fast builds. From 636f243956c422211534d6b7ca3c2f7113430d6e Mon Sep 17 00:00:00 2001 From: cowwoc Date: Mon, 6 Oct 2025 12:40:04 -0400 Subject: [PATCH 2/2] Add project-model-based defaults with opt-in/opt-out for attachedOutputs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implemented reviewer suggestions to use project model for default attached outputs instead of hardcoded paths, with optional disable via property. Changes: - Modified CacheConfig.getAttachedOutputs() to accept MavenProject parameter - Updated CacheConfigImpl to compute defaults from project.getBuild(): * getOutputDirectory() for main classes * getTestOutputDirectory() for test classes * Compute relative paths from build directory - Added getRelativePath() helper method - Updated CacheControllerImpl.attachOutputs() to pass project - Added maven.build.cache.attachedOutputs.enabled property (default: true) to allow disabling automatic restoration for performance-sensitive scenarios - Updated tests to create mock MavenProject with Build configuration - Removed getAttachedOutputs from assertDefaults() helper (now requires parameter) Tests added: - testDefaultAttachedOutputsWhenNotConfigured() - verifies project-based defaults - testExplicitAttachedOutputsOverridesDefaults() - verifies XML config overrides - testDefaultAttachedOutputsDisabledViaProperty() - verifies opt-out functionality - testDefaultAttachedOutputsWithCustomDirectories() - verifies custom paths work This addresses reviewer concerns: ✓ Project-model-based approach respects custom output directory configurations ✓ Safe defaults matching cache-disabled behavior ✓ Opt-in/opt-out for performance-sensitive CI environments Usage: # Disable automatic restoration (CI optimization) mvn clean install -Dmaven.build.cache.attachedOutputs.enabled=false # Enable (default) mvn clean install -Dmaven.build.cache.attachedOutputs.enabled=true --- .../maven/buildcache/CacheControllerImpl.java | 2 +- .../maven/buildcache/xml/CacheConfig.java | 3 +- .../maven/buildcache/xml/CacheConfigImpl.java | 34 +++++- .../buildcache/xml/CacheConfigImplTest.java | 114 +++++++++++++++++- 4 files changed, 145 insertions(+), 8 deletions(-) diff --git a/src/main/java/org/apache/maven/buildcache/CacheControllerImpl.java b/src/main/java/org/apache/maven/buildcache/CacheControllerImpl.java index e71dcb7a..6baf52bd 100644 --- a/src/main/java/org/apache/maven/buildcache/CacheControllerImpl.java +++ b/src/main/java/org/apache/maven/buildcache/CacheControllerImpl.java @@ -930,7 +930,7 @@ public void attachGeneratedSources(MavenProject project) throws IOException { } private void attachOutputs(MavenProject project) throws IOException { - final List attachedDirs = cacheConfig.getAttachedOutputs(); + final List attachedDirs = cacheConfig.getAttachedOutputs(project); for (DirName dir : attachedDirs) { final Path targetDir = Paths.get(project.getBuild().getDirectory()); final Path outputDir = targetDir.resolve(dir.getValue()); 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..bd918a3f 100644 --- a/src/main/java/org/apache/maven/buildcache/xml/CacheConfig.java +++ b/src/main/java/org/apache/maven/buildcache/xml/CacheConfig.java @@ -35,6 +35,7 @@ import org.apache.maven.model.Plugin; import org.apache.maven.model.PluginExecution; import org.apache.maven.plugin.MojoExecution; +import org.apache.maven.project.MavenProject; /** * A java interface to the information configured in the maven-build-cache-config.xml file @@ -106,7 +107,7 @@ public interface CacheConfig { String getLocalRepositoryLocation(); - List getAttachedOutputs(); + List getAttachedOutputs(MavenProject project); boolean adjustMetaInfVersion(); 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 e3b8a45b..efc392af 100644 --- a/src/main/java/org/apache/maven/buildcache/xml/CacheConfigImpl.java +++ b/src/main/java/org/apache/maven/buildcache/xml/CacheConfigImpl.java @@ -67,6 +67,7 @@ import org.apache.maven.model.Plugin; import org.apache.maven.model.PluginExecution; import org.apache.maven.plugin.MojoExecution; +import org.apache.maven.project.MavenProject; import org.apache.maven.rtinfo.RuntimeInformation; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -95,6 +96,7 @@ public class CacheConfigImpl implements org.apache.maven.buildcache.xml.CacheCon public static final String LAZY_RESTORE_PROPERTY_NAME = "maven.build.cache.lazyRestore"; public static final String RESTORE_ON_DISK_ARTIFACTS_PROPERTY_NAME = "maven.build.cache.restoreOnDiskArtifacts"; public static final String RESTORE_GENERATED_SOURCES_PROPERTY_NAME = "maven.build.cache.restoreGeneratedSources"; + public static final String ATTACHED_OUTPUTS_ENABLED_PROPERTY_NAME = "maven.build.cache.attachedOutputs.enabled"; public static final String ALWAYS_RUN_PLUGINS = "maven.build.cache.alwaysRunPlugins"; public static final String MANDATORY_CLEAN = "maven.build.cache.mandatoryClean"; @@ -572,29 +574,51 @@ public String getLocalRepositoryLocation() { } @Override - public List getAttachedOutputs() { + public List getAttachedOutputs(MavenProject project) { checkInitializedState(); final AttachedOutputs attachedOutputs = getConfiguration().getAttachedOutputs(); if (attachedOutputs == null) { - return getDefaultAttachedOutputs(); + return getDefaultAttachedOutputs(project); } return attachedOutputs.getDirNames(); } - private List getDefaultAttachedOutputs() { + private List getDefaultAttachedOutputs(MavenProject project) { + boolean enabled = getProperty(ATTACHED_OUTPUTS_ENABLED_PROPERTY_NAME, true); + if (!enabled) { + return Collections.emptyList(); + } + List defaults = new ArrayList<>(); + // Get output directories from project build configuration + String buildDirectory = project.getBuild().getDirectory(); + String outputDirectory = project.getBuild().getOutputDirectory(); + String testOutputDirectory = project.getBuild().getTestOutputDirectory(); + + // Compute relative paths from build directory + String classesRelative = getRelativePath(buildDirectory, outputDirectory); + String testClassesRelative = getRelativePath(buildDirectory, testOutputDirectory); + DirName classes = new DirName(); - classes.setValue("classes"); + classes.setValue(classesRelative); defaults.add(classes); DirName testClasses = new DirName(); - testClasses.setValue("test-classes"); + testClasses.setValue(testClassesRelative); defaults.add(testClasses); return defaults; } + private String getRelativePath(String basePath, String fullPath) { + // Convert to Path objects and compute relative path + Path base = Paths.get(basePath); + Path full = Paths.get(fullPath); + Path relative = base.relativize(full); + return relative.toString(); + } + @Override public boolean adjustMetaInfVersion() { if (isEnabled()) { diff --git a/src/test/java/org/apache/maven/buildcache/xml/CacheConfigImplTest.java b/src/test/java/org/apache/maven/buildcache/xml/CacheConfigImplTest.java index b7106174..cc5f5b60 100644 --- a/src/test/java/org/apache/maven/buildcache/xml/CacheConfigImplTest.java +++ b/src/test/java/org/apache/maven/buildcache/xml/CacheConfigImplTest.java @@ -31,6 +31,7 @@ import java.util.Arrays; import java.util.Collections; import java.util.HashMap; +import java.util.List; import java.util.Map; import java.util.Optional; import java.util.Properties; @@ -39,13 +40,17 @@ import org.apache.commons.lang3.tuple.Pair; import org.apache.maven.buildcache.DefaultPluginScanConfig; import org.apache.maven.buildcache.hash.HashFactory; +import org.apache.maven.buildcache.xml.config.AttachedOutputs; import org.apache.maven.buildcache.xml.config.Configuration; +import org.apache.maven.buildcache.xml.config.DirName; import org.apache.maven.buildcache.xml.config.Remote; import org.apache.maven.execution.MavenExecutionRequest; import org.apache.maven.execution.MavenSession; +import org.apache.maven.model.Build; import org.apache.maven.model.Plugin; import org.apache.maven.model.PluginExecution; import org.apache.maven.plugin.MojoExecution; +import org.apache.maven.project.MavenProject; import org.apache.maven.rtinfo.RuntimeInformation; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -169,7 +174,7 @@ private void assertDefaults(Map overrides) { asserts.put("calculateProjectVersionChecksum", () -> assertFalse(testObject.calculateProjectVersionChecksum())); asserts.put("canIgnore", () -> assertFalse(testObject.canIgnore(mock(MojoExecution.class)))); asserts.put("getAlwaysRunPlugins", () -> assertNull(testObject.getAlwaysRunPlugins())); - asserts.put("getAttachedOutputs", () -> assertEquals(Collections.emptyList(), testObject.getAttachedOutputs())); + // getAttachedOutputs removed - requires MavenProject parameter, tested separately asserts.put("getBaselineCacheUrl", () -> assertNull(testObject.getBaselineCacheUrl())); asserts.put("getDefaultGlob", () -> assertEquals("*", testObject.getDefaultGlob())); asserts.put( @@ -482,4 +487,111 @@ void testRemoveSaveFinalIgnoredWhenRemoteSaveDisabled() { Pair.of("getUrl", () -> assertEquals("dummy.url.xyz", testObject.getUrl())), Pair.of("isRemoteCacheEnabled", () -> assertTrue(testObject.isRemoteCacheEnabled()))); } + + @Test + void testDefaultAttachedOutputsWhenNotConfigured() { + // When attachedOutputs is not configured in XML, should return default list + Configuration configuration = new Configuration(); + // Deliberately not setting attachedOutputs + testCacheConfig.setConfiguration(configuration); + + assertEquals(CacheState.INITIALIZED, testObject.initialize()); + + // Create mock project with default build configuration + MavenProject mockProject = mock(MavenProject.class); + Build mockBuild = mock(Build.class); + when(mockProject.getBuild()).thenReturn(mockBuild); + when(mockBuild.getDirectory()).thenReturn("/project/target"); + when(mockBuild.getOutputDirectory()).thenReturn("/project/target/classes"); + when(mockBuild.getTestOutputDirectory()).thenReturn("/project/target/test-classes"); + + List attachedOutputs = testObject.getAttachedOutputs(mockProject); + assertEquals(2, attachedOutputs.size(), "Should have 2 default attached outputs"); + + List dirNames = attachedOutputs.stream() + .map(DirName::getValue) + .collect(Collectors.toList()); + + assertTrue(dirNames.contains("classes"), "Should include 'classes' directory by default"); + assertTrue(dirNames.contains("test-classes"), "Should include 'test-classes' directory by default"); + } + + @Test + void testExplicitAttachedOutputsOverridesDefaults() { + // When attachedOutputs is explicitly configured, should use those values instead of defaults + Configuration configuration = new Configuration(); + AttachedOutputs attachedOutputs = new AttachedOutputs(); + + DirName customDir = new DirName(); + customDir.setValue("custom-output"); + attachedOutputs.addDirName(customDir); + + configuration.setAttachedOutputs(attachedOutputs); + testCacheConfig.setConfiguration(configuration); + + assertEquals(CacheState.INITIALIZED, testObject.initialize()); + + // Create mock project (not used when explicit config is set, but required by interface) + MavenProject mockProject = mock(MavenProject.class); + Build mockBuild = mock(Build.class); + when(mockProject.getBuild()).thenReturn(mockBuild); + when(mockBuild.getDirectory()).thenReturn("/project/target"); + when(mockBuild.getOutputDirectory()).thenReturn("/project/target/classes"); + when(mockBuild.getTestOutputDirectory()).thenReturn("/project/target/test-classes"); + + List result = testObject.getAttachedOutputs(mockProject); + assertEquals(1, result.size(), "Should have 1 explicitly configured output"); + assertEquals("custom-output", result.get(0).getValue(), + "Should use explicitly configured directory, not defaults"); + } + + @Test + void testDefaultAttachedOutputsDisabledViaProperty() { + // When attachedOutputs.enabled property is false, should return empty list + Configuration configuration = new Configuration(); + testCacheConfig.setConfiguration(configuration); + + when(mockProperties.getProperty(CacheConfigImpl.ATTACHED_OUTPUTS_ENABLED_PROPERTY_NAME)) + .thenReturn("false"); + + assertEquals(CacheState.INITIALIZED, testObject.initialize()); + + // Create mock project + MavenProject mockProject = mock(MavenProject.class); + Build mockBuild = mock(Build.class); + when(mockProject.getBuild()).thenReturn(mockBuild); + when(mockBuild.getDirectory()).thenReturn("/project/target"); + when(mockBuild.getOutputDirectory()).thenReturn("/project/target/classes"); + when(mockBuild.getTestOutputDirectory()).thenReturn("/project/target/test-classes"); + + List attachedOutputs = testObject.getAttachedOutputs(mockProject); + assertEquals(0, attachedOutputs.size(), "Should return empty list when disabled via property"); + } + + @Test + void testDefaultAttachedOutputsWithCustomDirectories() { + // When project has custom output directories, should use those + Configuration configuration = new Configuration(); + testCacheConfig.setConfiguration(configuration); + + assertEquals(CacheState.INITIALIZED, testObject.initialize()); + + // Create mock project with custom output directories + MavenProject mockProject = mock(MavenProject.class); + Build mockBuild = mock(Build.class); + when(mockProject.getBuild()).thenReturn(mockBuild); + when(mockBuild.getDirectory()).thenReturn("/project/build"); + when(mockBuild.getOutputDirectory()).thenReturn("/project/build/custom-classes"); + when(mockBuild.getTestOutputDirectory()).thenReturn("/project/build/custom-test-classes"); + + List attachedOutputs = testObject.getAttachedOutputs(mockProject); + assertEquals(2, attachedOutputs.size(), "Should have 2 default attached outputs"); + + List dirNames = attachedOutputs.stream() + .map(DirName::getValue) + .collect(Collectors.toList()); + + assertTrue(dirNames.contains("custom-classes"), "Should include custom output directory"); + assertTrue(dirNames.contains("custom-test-classes"), "Should include custom test output directory"); + } }