Skip to content

Commit 6fc92c8

Browse files
committed
Improve logging messages and javadoc per review feedback
- Differentiate cache skip message when cacheCompile=false vs true - Change empty directory artifact log from warn to info level - Add artifact examples to stagePreExistingArtifacts javadoc - Add proper javadoc to collectCachedArtifactPaths method - Fix resource leak: wrap Files.walk() in try-with-resources
1 parent 2db9ffc commit 6fc92c8

2 files changed

Lines changed: 33 additions & 7 deletions

File tree

src/main/java/org/apache/maven/buildcache/CacheControllerImpl.java

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -611,7 +611,9 @@ public void save(
611611
boolean isPomProjectWithWork = "pom".equals(project.getPackaging()) && !completedExecution.isEmpty();
612612

613613
if (!hasArtifactFile && !hasAttachedArtifacts && !isPomProjectWithWork) {
614-
LOGGER.info("Skipping cache save: no artifacts to save (only metadata present)");
614+
LOGGER.info(
615+
"Skipping cache save: no artifacts to save ({}only metadata present)",
616+
cacheCompile ? "" : "cacheCompile=false, ");
615617
return;
616618
}
617619

@@ -711,7 +713,7 @@ private void saveDirectoryArtifact(
711713
// Clean up temp file after it's been saved to cache
712714
Files.deleteIfExists(tempZip);
713715
} else {
714-
LOGGER.warn("Directory artifact has no files to cache: {}", originalFile);
716+
LOGGER.info("Skipping empty directory artifact: {}", originalFile);
715717
}
716718
}
717719

@@ -1167,6 +1169,14 @@ public FileVisitResult visitFile(Path path, BasicFileAttributes basicFileAttribu
11671169
/**
11681170
* Move pre-existing build artifacts to staging directory to prevent caching stale files.
11691171
*
1172+
* <p><b>Artifacts Staged:</b>
1173+
* <ul>
1174+
* <li>{@code target/classes} - Compiled main classes directory</li>
1175+
* <li>{@code target/test-classes} - Compiled test classes directory</li>
1176+
* <li>{@code target/*.jar} - Main project artifact (JAR/WAR files)</li>
1177+
* <li>Other directories configured via {@code attachedOutputs} in cache configuration</li>
1178+
* </ul>
1179+
*
11701180
* <p><b>DESIGN RATIONALE - Staleness Detection via Staging Directory:</b>
11711181
*
11721182
* <p>This approach solves three critical problems that timestamp-based checking cannot handle:
@@ -1277,7 +1287,19 @@ public void stagePreExistingArtifacts(MavenSession session, MavenProject project
12771287
}
12781288

12791289
/**
1280-
* Collect paths to all artifacts that will be cached (main artifact + attachedOutputs).
1290+
* Collects paths to all artifacts that will be considered for caching for the given project.
1291+
*
1292+
* <p>This includes:
1293+
* <ul>
1294+
* <li>the main project artifact file (for example, the built JAR), if it has been produced, and</li>
1295+
* <li>any attached output directories configured via {@code cacheConfig.getAttachedOutputs()} under the
1296+
* project's target directory, when {@code cacheConfig.isCacheCompile()} is enabled.</li>
1297+
* </ul>
1298+
* Only paths that currently exist on disk are included in the returned set; non-existent files or directories
1299+
* are ignored.
1300+
*
1301+
* @param project the Maven project whose artifact and attached output paths should be collected
1302+
* @return a set of existing filesystem paths for the project's main artifact and configured attached outputs
12811303
*/
12821304
private Set<Path> collectCachedArtifactPaths(MavenProject project) {
12831305
Set<Path> paths = new HashSet<>();

src/test/java/org/apache/maven/buildcache/its/CacheCompileDisabledTest.java

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -62,8 +62,10 @@ void compileDoesNotCacheWhenDisabled(Verifier verifier) throws VerificationExcep
6262
verifier.verifyErrorFreeLog();
6363

6464
// Verify NO cache entry was created (no buildinfo.xml in local cache)
65-
boolean hasCacheEntry =
66-
Files.walk(localCache).anyMatch(p -> p.getFileName().toString().equals("buildinfo.xml"));
65+
boolean hasCacheEntry;
66+
try (Stream<Path> walk = Files.walk(localCache)) {
67+
hasCacheEntry = walk.anyMatch(p -> p.getFileName().toString().equals("buildinfo.xml"));
68+
}
6769
assertFalse(hasCacheEntry, "Cache entry should NOT be created when maven.build.cache.cacheCompile=false");
6870

6971
// Clean project and run compile again
@@ -103,8 +105,10 @@ void compileCreatesCacheEntryWhenEnabled(Verifier verifier) throws VerificationE
103105
verifier.verifyErrorFreeLog();
104106

105107
// Verify cache entry WAS created
106-
boolean hasCacheEntry =
107-
Files.walk(localCache).anyMatch(p -> p.getFileName().toString().equals("buildinfo.xml"));
108+
boolean hasCacheEntry;
109+
try (Stream<Path> walk = Files.walk(localCache)) {
110+
hasCacheEntry = walk.anyMatch(p -> p.getFileName().toString().equals("buildinfo.xml"));
111+
}
108112
assertTrue(hasCacheEntry, "Cache entry should be created when maven.build.cache.cacheCompile=true (default)");
109113

110114
// Clean project and run compile again

0 commit comments

Comments
 (0)