diff --git a/src/main/java/org/apache/maven/buildcache/CacheControllerImpl.java b/src/main/java/org/apache/maven/buildcache/CacheControllerImpl.java index e71dcb7a..bb4c37bb 100644 --- a/src/main/java/org/apache/maven/buildcache/CacheControllerImpl.java +++ b/src/main/java/org/apache/maven/buildcache/CacheControllerImpl.java @@ -881,7 +881,7 @@ private boolean zipAndAttachArtifact(MavenProject project, Path dir, String clas throws IOException { Path temp = Files.createTempFile("maven-incremental-", project.getArtifactId()); temp.toFile().deleteOnExit(); - boolean hasFile = CacheUtils.zip(dir, temp, glob); + boolean hasFile = CacheUtils.zip(dir, temp, glob, cacheConfig.isPreserveTimestamps()); if (hasFile) { projectHelper.attachArtifact(project, "zip", classifier, temp.toFile()); } @@ -896,7 +896,7 @@ private void restoreGeneratedSources(Artifact artifact, Path artifactFilePath, M if (!Files.exists(outputDir)) { Files.createDirectories(outputDir); } - CacheUtils.unzip(artifactFilePath, outputDir); + CacheUtils.unzip(artifactFilePath, outputDir, cacheConfig.isPreserveTimestamps()); } // TODO: move to config diff --git a/src/main/java/org/apache/maven/buildcache/CacheUtils.java b/src/main/java/org/apache/maven/buildcache/CacheUtils.java index 2eded63a..3c17335f 100644 --- a/src/main/java/org/apache/maven/buildcache/CacheUtils.java +++ b/src/main/java/org/apache/maven/buildcache/CacheUtils.java @@ -27,12 +27,18 @@ import java.nio.file.PathMatcher; import java.nio.file.SimpleFileVisitor; import java.nio.file.StandardCopyOption; +import java.nio.file.attribute.BasicFileAttributeView; import java.nio.file.attribute.BasicFileAttributes; import java.nio.file.attribute.FileTime; import java.util.Arrays; import java.util.Collection; +import java.util.Collections; +import java.util.HashMap; +import java.util.HashSet; import java.util.List; +import java.util.Map; import java.util.NoSuchElementException; +import java.util.Set; import java.util.stream.Stream; import java.util.zip.ZipEntry; import java.util.zip.ZipInputStream; @@ -154,24 +160,59 @@ public static boolean isArchive(File file) { * @param dir directory to zip * @param zip zip to populate * @param glob glob to apply to filenames + * @param preserveTimestamps whether to preserve file and directory timestamps in the zip. + *

Important: When {@code true}, timestamps are stored in ZIP entry headers, + * which means they become part of the ZIP file's binary content. As a result, hashing + * the ZIP file (e.g., for cache keys) will include timestamp information, ensuring + * cache invalidation when file timestamps change. This behavior is similar to how Git + * includes file mode in tree hashes.

* @return true if at least one file has been included in the zip. * @throws IOException */ - public static boolean zip(final Path dir, final Path zip, final String glob) throws IOException { + public static boolean zip(final Path dir, final Path zip, final String glob, boolean preserveTimestamps) + throws IOException { final MutableBoolean hasFiles = new MutableBoolean(); try (ZipOutputStream zipOutputStream = new ZipOutputStream(Files.newOutputStream(zip))) { PathMatcher matcher = "*".equals(glob) ? null : FileSystems.getDefault().getPathMatcher("glob:" + glob); + + // Track directories that contain matching files for glob filtering + final Set directoriesWithMatchingFiles = new HashSet<>(); + // Track directory attributes for timestamp preservation + final Map directoryAttributes = + preserveTimestamps ? new HashMap<>() : Collections.emptyMap(); + Files.walkFileTree(dir, new SimpleFileVisitor() { + @Override + public FileVisitResult preVisitDirectory(Path path, BasicFileAttributes attrs) throws IOException { + if (preserveTimestamps) { + // Store attributes for use in postVisitDirectory + directoryAttributes.put(path, attrs); + } + return FileVisitResult.CONTINUE; + } + @Override public FileVisitResult visitFile(Path path, BasicFileAttributes basicFileAttributes) throws IOException { if (matcher == null || matcher.matches(path.getFileName())) { + if (preserveTimestamps) { + // Mark all parent directories as containing matching files + Path parent = path.getParent(); + while (parent != null && !parent.equals(dir)) { + directoriesWithMatchingFiles.add(parent); + parent = parent.getParent(); + } + } + final ZipEntry zipEntry = new ZipEntry(dir.relativize(path).toString()); + if (preserveTimestamps) { + zipEntry.setTime(basicFileAttributes.lastModifiedTime().toMillis()); + } zipOutputStream.putNextEntry(zipEntry); Files.copy(path, zipOutputStream); hasFiles.setTrue(); @@ -179,12 +220,38 @@ public FileVisitResult visitFile(Path path, BasicFileAttributes basicFileAttribu } return FileVisitResult.CONTINUE; } + + @Override + public FileVisitResult postVisitDirectory(Path path, IOException exc) throws IOException { + // Propagate any exception that occurred during directory traversal + if (exc != null) { + throw exc; + } + + // Add directory entry only if preserving timestamps and: + // 1. It's not the root directory, AND + // 2. Either no glob filter (matcher is null) OR directory contains matching files + if (preserveTimestamps + && !path.equals(dir) + && (matcher == null || directoriesWithMatchingFiles.contains(path))) { + BasicFileAttributes attrs = directoryAttributes.get(path); + if (attrs != null) { + String relativePath = dir.relativize(path).toString() + "/"; + ZipEntry zipEntry = new ZipEntry(relativePath); + zipEntry.setTime(attrs.lastModifiedTime().toMillis()); + zipOutputStream.putNextEntry(zipEntry); + zipOutputStream.closeEntry(); + } + } + return FileVisitResult.CONTINUE; + } }); } return hasFiles.booleanValue(); } - public static void unzip(Path zip, Path out) throws IOException { + public static void unzip(Path zip, Path out, boolean preserveTimestamps) throws IOException { + Map directoryTimestamps = preserveTimestamps ? new HashMap<>() : Collections.emptyMap(); try (ZipInputStream zis = new ZipInputStream(Files.newInputStream(zip))) { ZipEntry entry = zis.getNextEntry(); while (entry != null) { @@ -193,16 +260,53 @@ public static void unzip(Path zip, Path out) throws IOException { throw new RuntimeException("Bad zip entry"); } if (entry.isDirectory()) { - Files.createDirectory(file); + Files.createDirectories(file); + if (preserveTimestamps) { + directoryTimestamps.put(file, entry.getTime()); + } } else { Path parent = file.getParent(); - Files.createDirectories(parent); + if (parent != null) { + Files.createDirectories(parent); + } Files.copy(zis, file, StandardCopyOption.REPLACE_EXISTING); + + if (preserveTimestamps) { + setAllTimestamps(file, entry.getTime()); + } } - Files.setLastModifiedTime(file, FileTime.fromMillis(entry.getTime())); entry = zis.getNextEntry(); } } + + if (preserveTimestamps) { + // Set directory timestamps after all files have been extracted to avoid them being + // updated by file creation operations + for (Map.Entry dirEntry : directoryTimestamps.entrySet()) { + setAllTimestamps(dirEntry.getKey(), dirEntry.getValue()); + } + } + } + + /** + * Sets all timestamps (lastModifiedTime, lastAccessTime, and creationTime) for a path + * to the same value to ensure consistency. This is a best-effort operation that silently + * ignores errors on filesystems that don't support timestamp modification. + * + * @param path the path to update + * @param timestampMillis the timestamp in milliseconds since epoch + */ + private static void setAllTimestamps(Path path, long timestampMillis) { + try { + BasicFileAttributeView attributes = Files.getFileAttributeView(path, BasicFileAttributeView.class); + if (attributes != null) { + FileTime time = FileTime.fromMillis(timestampMillis); + attributes.setTimes(time, time, time); + } + } catch (IOException e) { + // Timestamp setting is best-effort; log but don't fail extraction + // This can happen on filesystems that don't support modification times + } } public static void debugPrintCollection( 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..6cc9a4b2 100644 --- a/src/main/java/org/apache/maven/buildcache/xml/CacheConfig.java +++ b/src/main/java/org/apache/maven/buildcache/xml/CacheConfig.java @@ -108,6 +108,8 @@ public interface CacheConfig { List getAttachedOutputs(); + boolean isPreserveTimestamps(); + boolean adjustMetaInfVersion(); boolean calculateProjectVersionChecksum(); 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..e9f2b15f 100644 --- a/src/main/java/org/apache/maven/buildcache/xml/CacheConfigImpl.java +++ b/src/main/java/org/apache/maven/buildcache/xml/CacheConfigImpl.java @@ -578,6 +578,13 @@ public List getAttachedOutputs() { return attachedOutputs == null ? Collections.emptyList() : attachedOutputs.getDirNames(); } + @Override + public boolean isPreserveTimestamps() { + checkInitializedState(); + final AttachedOutputs attachedOutputs = getConfiguration().getAttachedOutputs(); + return attachedOutputs == null || attachedOutputs.isPreserveTimestamps(); + } + @Override public boolean adjustMetaInfVersion() { if (isEnabled()) { diff --git a/src/main/mdo/build-cache-config.mdo b/src/main/mdo/build-cache-config.mdo index 52ae0da0..b6146ab7 100644 --- a/src/main/mdo/build-cache-config.mdo +++ b/src/main/mdo/build-cache-config.mdo @@ -376,6 +376,12 @@ under the License. AttachedOutputs Section relative to outputs which are not artifacts but need to be saved/restored. + + preserveTimestamps + boolean + true + Preserve file and directory timestamps when saving/restoring attached outputs. Disabling this may improve performance on some filesystems but can cause Maven warnings about files being more recent than packaged artifacts. + dirNames diff --git a/src/test/java/org/apache/maven/buildcache/CacheUtilsTimestampTest.java b/src/test/java/org/apache/maven/buildcache/CacheUtilsTimestampTest.java new file mode 100644 index 00000000..4f1ce286 --- /dev/null +++ b/src/test/java/org/apache/maven/buildcache/CacheUtilsTimestampTest.java @@ -0,0 +1,497 @@ +/* + * 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; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +import java.io.IOException; +import java.io.OutputStream; +import java.nio.charset.StandardCharsets; +import java.nio.file.FileVisitResult; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.SimpleFileVisitor; +import java.nio.file.attribute.BasicFileAttributes; +import java.nio.file.attribute.FileTime; +import java.time.Instant; +import java.time.temporal.ChronoUnit; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; +import java.util.zip.ZipEntry; +import java.util.zip.ZipInputStream; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.fail; + +/** + * Tests for timestamp preservation in CacheUtils.zip() and CacheUtils.unzip() methods. + * These tests verify the fix for the issue where file and directory timestamps were not + * preserved during cache operations, causing Maven warnings about files being "more recent + * than the packaged artifact". + */ +class CacheUtilsTimestampTest { + + /** + * Tolerance for timestamp comparison in milliseconds. + * Zip format stores timestamps with 2-second precision, so we use 2000ms tolerance. + */ + private static final long TIMESTAMP_TOLERANCE_MS = 2000; + + @TempDir + Path tempDir; + + @Test + void testFileTimestampPreservation() throws IOException { + // Given: Files with specific timestamp (1 hour ago) + Instant oldTime = Instant.now().minus(1, ChronoUnit.HOURS); + Path sourceDir = tempDir.resolve("source"); + Path packageDir = sourceDir.resolve("com").resolve("example"); + Files.createDirectories(packageDir); + + Path file1 = packageDir.resolve("Service.class"); + Path file2 = packageDir.resolve("Repository.class"); + writeString(file1, "// Service.class content"); + writeString(file2, "// Repository.class content"); + Files.setLastModifiedTime(file1, FileTime.from(oldTime)); + Files.setLastModifiedTime(file2, FileTime.from(oldTime)); + + long originalTimestamp = Files.getLastModifiedTime(file1).toMillis(); + + // When: Zip and unzip using CacheUtils + Path zipFile = tempDir.resolve("cache.zip"); + CacheUtils.zip(sourceDir, zipFile, "*", true); + + Path extractDir = tempDir.resolve("extracted"); + Files.createDirectories(extractDir); + CacheUtils.unzip(zipFile, extractDir, true); + + // Then: File timestamps should be preserved + Path extractedFile1 = extractDir.resolve("com").resolve("example").resolve("Service.class"); + Path extractedFile2 = extractDir.resolve("com").resolve("example").resolve("Repository.class"); + + long extractedTimestamp1 = Files.getLastModifiedTime(extractedFile1).toMillis(); + long extractedTimestamp2 = Files.getLastModifiedTime(extractedFile2).toMillis(); + + assertTimestampPreserved( + "Service.class", + originalTimestamp, + extractedTimestamp1, + "File timestamp should be preserved through zip/unzip cycle"); + + assertTimestampPreserved( + "Repository.class", + originalTimestamp, + extractedTimestamp2, + "File timestamp should be preserved through zip/unzip cycle"); + } + + @Test + void testDirectoryTimestampPreservation() throws IOException { + // Given: Directories with specific timestamp (1 hour ago) + Instant oldTime = Instant.now().minus(1, ChronoUnit.HOURS); + Path sourceDir = tempDir.resolve("source"); + Path subDir = sourceDir.resolve("com").resolve("example"); + Files.createDirectories(subDir); + + Files.setLastModifiedTime(subDir, FileTime.from(oldTime)); + Files.setLastModifiedTime(sourceDir.resolve("com"), FileTime.from(oldTime)); + + // Add a file so zip has content + Path file = subDir.resolve("Test.class"); + writeString(file, "// Test content"); + Files.setLastModifiedTime(file, FileTime.from(oldTime)); + + long originalDirTimestamp = Files.getLastModifiedTime(subDir).toMillis(); + + // When: Zip and unzip using CacheUtils + Path zipFile = tempDir.resolve("cache.zip"); + CacheUtils.zip(sourceDir, zipFile, "*", true); + + Path extractDir = tempDir.resolve("extracted"); + Files.createDirectories(extractDir); + CacheUtils.unzip(zipFile, extractDir, true); + + // Then: Directory timestamps should be preserved + Path extractedDir = extractDir.resolve("com").resolve("example"); + long extractedDirTimestamp = Files.getLastModifiedTime(extractedDir).toMillis(); + + assertTimestampPreserved( + "com/example directory", + originalDirTimestamp, + extractedDirTimestamp, + "Directory timestamp should be preserved through zip/unzip cycle"); + } + + @Test + void testDirectoryEntriesStoredInZip() throws IOException { + // Given: Directory structure + Path sourceDir = tempDir.resolve("source"); + Path subDir = sourceDir.resolve("com").resolve("example"); + Files.createDirectories(subDir); + + // Add a file + Path file = subDir.resolve("Test.class"); + writeString(file, "// Test content"); + + // When: Create zip + Path zipFile = tempDir.resolve("cache.zip"); + CacheUtils.zip(sourceDir, zipFile, "*", true); + + // Then: Zip should contain directory entries + List entries = new ArrayList<>(); + try (ZipInputStream zis = new ZipInputStream(Files.newInputStream(zipFile))) { + ZipEntry entry; + while ((entry = zis.getNextEntry()) != null) { + entries.add(entry.getName()); + } + } + + boolean hasComDirectory = entries.stream().anyMatch(e -> e.equals("com/")); + boolean hasExampleDirectory = entries.stream().anyMatch(e -> e.equals("com/example/")); + boolean hasFile = entries.stream().anyMatch(e -> e.equals("com/example/Test.class")); + + assertTrue(hasComDirectory, "Zip should contain 'com/' directory entry"); + assertTrue(hasExampleDirectory, "Zip should contain 'com/example/' directory entry"); + assertTrue(hasFile, "Zip should contain 'com/example/Test.class' file entry"); + } + + @Test + void testTimestampsInZipEntries() throws IOException { + // Given: Files with specific timestamp + Instant oldTime = Instant.now().minus(1, ChronoUnit.HOURS); + Path sourceDir = tempDir.resolve("source"); + Path subDir = sourceDir.resolve("com").resolve("example"); + Files.createDirectories(subDir); + + Path file = subDir.resolve("Test.class"); + writeString(file, "// Test content"); + Files.setLastModifiedTime(file, FileTime.from(oldTime)); + Files.setLastModifiedTime(subDir, FileTime.from(oldTime)); + Files.setLastModifiedTime(sourceDir.resolve("com"), FileTime.from(oldTime)); + + long expectedTimestamp = oldTime.toEpochMilli(); + + // When: Create zip + Path zipFile = tempDir.resolve("cache.zip"); + CacheUtils.zip(sourceDir, zipFile, "*", true); + + // Then: Zip entries should have correct timestamps + try (ZipInputStream zis = new ZipInputStream(Files.newInputStream(zipFile))) { + ZipEntry entry; + while ((entry = zis.getNextEntry()) != null) { + long entryTimestamp = entry.getTime(); + + assertTimestampPreserved( + entry.getName() + " in zip", + expectedTimestamp, + entryTimestamp, + "Zip entry timestamp should match original file/directory timestamp"); + } + } + } + + @Test + void testMavenWarningScenario() throws IOException { + // This test simulates the exact scenario that causes Maven warning: + // "File 'target/classes/Foo.class' is more recent than the packaged artifact" + // + // Scenario: + // 1. Maven compiles .class files to target/classes at T1 + // 2. Maven packages them into JAR at T2 (slightly later) + // 3. Build cache stores the JAR + // 4. On next build, cache restores JAR contents back to target/classes + // 5. If timestamps aren't preserved, restored files have timestamp=NOW (T3) + // 6. Maven sees files (T3) are newer than JAR (T2) and warns + // + // MANUAL REPRODUCTION STEPS (to see actual Maven warning): + // 1. Configure maven-build-cache-extension in a multi-module Maven project + // 2. Build the project: mvn clean package + // - This populates the build cache with compiled outputs + // 3. Touch a source file in module A: touch module-a/src/main/java/Foo.java + // 4. Rebuild: mvn package + // - Module A rebuilds (source changed) + // - Module B restores from cache (no changes) + // - WITHOUT FIX: Module B's restored .class files have current timestamp + // - Maven detects: "File 'module-b/target/classes/Bar.class' is more recent + // than the packaged artifact 'module-b/target/module-b-1.0.jar'" + // - WITH FIX: Timestamps preserved, no warning + + // Given: Simulated first build - compile at T1, package at T2 + Instant compileTime = Instant.now().minus(1, ChronoUnit.HOURS); + + Path classesDir = tempDir.resolve("target").resolve("classes"); + Path packageDir = classesDir.resolve("com").resolve("example"); + Files.createDirectories(packageDir); + + Path classFile = packageDir.resolve("Service.class"); + writeString(classFile, "// Compiled Service.class"); + Files.setLastModifiedTime(classFile, FileTime.from(compileTime)); + + // Create JAR at T2 (slightly after compilation) + Instant packageTime = compileTime.plus(5, ChronoUnit.SECONDS); + Path jarFile = tempDir.resolve("target").resolve("my-module-1.0.jar"); + Files.createDirectories(jarFile.getParent()); + CacheUtils.zip(classesDir, jarFile, "*", true); + Files.setLastModifiedTime(jarFile, FileTime.from(packageTime)); + + long jarTimestamp = Files.getLastModifiedTime(jarFile).toMillis(); + + // Simulate mvn clean - delete target/classes + deleteRecursively(classesDir); + + // When: Simulate cache restoration - restore JAR contents back to target/classes + CacheUtils.unzip(jarFile, classesDir, true); + + // Then: Restored file should NOT be newer than JAR + Path restoredClass = classesDir.resolve("com").resolve("example").resolve("Service.class"); + long restoredTimestamp = Files.getLastModifiedTime(restoredClass).toMillis(); + + // The restored file should have the original compile time (T1), not current time (T3) + // This means it should be OLDER than the JAR (JAR was created at T2, 5 seconds after T1) + if (restoredTimestamp > jarTimestamp) { + long diffSeconds = (restoredTimestamp - jarTimestamp) / 1000; + fail(String.format( + "[WARNING] File 'target/classes/com/example/Service.class' is more recent%n" + + " than the packaged artifact 'my-module-1.0.jar'%n" + + " (difference: %d seconds)%n" + + " Please run a full 'mvn clean package' build%n%n" + + "This indicates timestamps are not being preserved correctly during cache restoration.", + diffSeconds)); + } + + // Additionally verify the timestamp is close to the original compile time + long originalTimestamp = compileTime.toEpochMilli(); + assertTimestampPreserved( + "Service.class", + originalTimestamp, + restoredTimestamp, + "Restored file should have original compile time, not current time"); + } + + @Test + void testMultipleFilesTimestampConsistency() throws IOException { + // Given: Multiple files all created at the same time + Instant buildTime = Instant.now().minus(1, ChronoUnit.HOURS); + Path sourceDir = tempDir.resolve("source"); + Path packageDir = sourceDir.resolve("com").resolve("example"); + Files.createDirectories(packageDir); + + List files = Arrays.asList( + packageDir.resolve("Service.class"), + packageDir.resolve("Repository.class"), + packageDir.resolve("Controller.class"), + packageDir.resolve("Model.class") + ); + + for (Path file : files) { + writeString(file, "// " + file.getFileName() + " content"); + Files.setLastModifiedTime(file, FileTime.from(buildTime)); + } + + long originalTimestamp = buildTime.toEpochMilli(); + + // When: Zip and unzip + Path zipFile = tempDir.resolve("cache.zip"); + CacheUtils.zip(sourceDir, zipFile, "*", true); + + Path extractDir = tempDir.resolve("extracted"); + Files.createDirectories(extractDir); + CacheUtils.unzip(zipFile, extractDir, true); + + // Then: All files should have consistent timestamps + for (Path originalFile : files) { + Path extractedFile = extractDir.resolve("com").resolve("example").resolve(originalFile.getFileName()); + long extractedTimestamp = Files.getLastModifiedTime(extractedFile).toMillis(); + + assertTimestampPreserved( + originalFile.getFileName().toString(), + originalTimestamp, + extractedTimestamp, + "All files from same build should have consistent timestamps"); + } + } + + @Test + void testPreserveTimestampsFalse() throws IOException { + // This test verifies that when preserveTimestamps=false, timestamps are NOT preserved + + // Given: Files with specific old timestamp + Instant oldTime = Instant.now().minus(1, ChronoUnit.HOURS); + Path sourceDir = tempDir.resolve("source"); + Path packageDir = sourceDir.resolve("com").resolve("example"); + Files.createDirectories(packageDir); + + Path file = packageDir.resolve("Service.class"); + writeString(file, "// Service.class content"); + Files.setLastModifiedTime(file, FileTime.from(oldTime)); + + long originalTimestamp = Files.getLastModifiedTime(file).toMillis(); + + // When: Zip and unzip with preserveTimestamps=false + Path zipFile = tempDir.resolve("cache.zip"); + CacheUtils.zip(sourceDir, zipFile, "*", false); + + Path extractDir = tempDir.resolve("extracted"); + Files.createDirectories(extractDir); + CacheUtils.unzip(zipFile, extractDir, false); + + // Then: Extracted file should NOT have the original timestamp + // (it should have a timestamp close to now, not 1 hour ago) + Path extractedFile = extractDir.resolve("com").resolve("example").resolve("Service.class"); + long extractedTimestamp = Files.getLastModifiedTime(extractedFile).toMillis(); + long currentTime = Instant.now().toEpochMilli(); + + // Verify the extracted file timestamp is NOT close to the original old timestamp + long diffFromOriginal = Math.abs(extractedTimestamp - originalTimestamp); + long diffFromCurrent = Math.abs(extractedTimestamp - currentTime); + + // The extracted file should be much closer to current time than to the old timestamp + assertTrue(diffFromCurrent < diffFromOriginal, + String.format("When preserveTimestamps=false, extracted file timestamp should be close to current time.%n" + + "Original timestamp (1 hour ago): %s (%d)%n" + + "Extracted timestamp: %s (%d)%n" + + "Current time: %s (%d)%n" + + "Diff from original: %d seconds%n" + + "Diff from current: %d seconds%n" + + "Expected: diff from current < diff from original", + Instant.ofEpochMilli(originalTimestamp), originalTimestamp, + Instant.ofEpochMilli(extractedTimestamp), extractedTimestamp, + Instant.ofEpochMilli(currentTime), currentTime, + diffFromOriginal / 1000, + diffFromCurrent / 1000)); + } + + /** + * Asserts that an extracted timestamp is preserved within tolerance. + * Fails with a detailed error message if timestamps differ significantly. + */ + private void assertTimestampPreserved(String fileName, long expectedMs, long actualMs, String message) { + long diffMs = Math.abs(actualMs - expectedMs); + long diffSeconds = diffMs / 1000; + + if (diffMs > TIMESTAMP_TOLERANCE_MS) { + String errorMessage = String.format( + "%s%n" + + "File: %s%n" + + "Expected timestamp: %s (%d)%n" + + "Actual timestamp: %s (%d)%n" + + "Difference: %d seconds (%.2f hours)%n" + + "%n" + + "Timestamps must be preserved within %d ms tolerance.%n" + + "This failure indicates CacheUtils.zip() or CacheUtils.unzip() is not%n" + + "correctly preserving file/directory timestamps.", + message, + fileName, + Instant.ofEpochMilli(expectedMs), + expectedMs, + Instant.ofEpochMilli(actualMs), + actualMs, + diffSeconds, + diffSeconds / 3600.0, + TIMESTAMP_TOLERANCE_MS); + + fail(errorMessage); + } + + // For debugging: log when timestamps are correctly preserved + assertEquals(expectedMs, actualMs, TIMESTAMP_TOLERANCE_MS, + String.format("%s (diff: %.2f seconds)", message, diffMs / 1000.0)); + } + + /** + * Java 8 compatible version of Files.writeString(). + */ + private void writeString(Path path, String content) throws IOException { + try (OutputStream out = Files.newOutputStream(path)) { + out.write(content.getBytes(StandardCharsets.UTF_8)); + } + } + + /** + * Tests that ZIP file hash changes when timestamps change (when preserveTimestamps=true). + * This ensures that the cache invalidates when file timestamps change, maintaining + * cache correctness similar to how Git includes file mode in tree hashes. + */ + @Test + void testTimestampsAffectFileHash() throws IOException { + // Given: Same directory content with different timestamps + Path sourceDir1 = tempDir.resolve("source1"); + Files.createDirectories(sourceDir1); + Path file1 = sourceDir1.resolve("test.txt"); + writeString(file1, "Same content"); + + Instant time1 = Instant.now().minus(2, ChronoUnit.HOURS); + Files.setLastModifiedTime(file1, FileTime.from(time1)); + + // Create second directory with identical content but different timestamp + Path sourceDir2 = tempDir.resolve("source2"); + Files.createDirectories(sourceDir2); + Path file2 = sourceDir2.resolve("test.txt"); + writeString(file2, "Same content"); // Identical content + + Instant time2 = Instant.now().minus(1, ChronoUnit.HOURS); // Different timestamp + Files.setLastModifiedTime(file2, FileTime.from(time2)); + + // When: Create ZIP files with preserveTimestamps=true + Path zip1 = tempDir.resolve("cache1.zip"); + Path zip2 = tempDir.resolve("cache2.zip"); + CacheUtils.zip(sourceDir1, zip1, "*", true); + CacheUtils.zip(sourceDir2, zip2, "*", true); + + // Then: ZIP files should have different hashes despite identical content + byte[] hash1 = Files.readAllBytes(zip1); + byte[] hash2 = Files.readAllBytes(zip2); + + boolean hashesAreDifferent = !Arrays.equals(hash1, hash2); + assertTrue(hashesAreDifferent, + "ZIP files with same content but different timestamps should have different hashes " + + "when preserveTimestamps=true. This ensures cache invalidation when timestamps change."); + + // Note: When preserveTimestamps=false, ZIP entries use current system time during creation, + // which means ZIP files created at different times will still differ. However, the critical + // distinction is that with preserveTimestamps=true, the original file timestamps are + // deterministically stored in ZIP entries, ensuring consistent cache behavior and proper + // cache invalidation when file timestamps change in the source. + } + + /** + * Recursively deletes a directory and all its contents. + */ + private void deleteRecursively(Path directory) throws IOException { + if (!Files.exists(directory)) { + return; + } + Files.walkFileTree(directory, new SimpleFileVisitor() { + @Override + public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IOException { + Files.delete(file); + return FileVisitResult.CONTINUE; + } + + @Override + public FileVisitResult postVisitDirectory(Path dir, IOException exc) throws IOException { + Files.delete(dir); + return FileVisitResult.CONTINUE; + } + }); + } +} diff --git a/src/test/java/org/apache/maven/buildcache/ModuleInfoCachingTest.java b/src/test/java/org/apache/maven/buildcache/ModuleInfoCachingTest.java new file mode 100644 index 00000000..063b650c --- /dev/null +++ b/src/test/java/org/apache/maven/buildcache/ModuleInfoCachingTest.java @@ -0,0 +1,71 @@ +/* + * 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; + +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * Test to verify that module-info.class files are properly cached and restored. + * + * Bug: Build cache does not properly cache JPMS module-info.class descriptors, + * causing module resolution failures when builds are restored from cache. + */ +public class ModuleInfoCachingTest { + + @Test + public void testModuleInfoClassIsIncludedInZip(@TempDir Path testDir) throws IOException { + // Create a mock classes directory with module-info.class + Path classesDir = testDir.resolve("classes"); + Files.createDirectories(classesDir); + + Path moduleInfoClass = classesDir.resolve("module-info.class"); + Files.write(moduleInfoClass, "fake module-info content".getBytes(StandardCharsets.UTF_8)); + + Path regularClass = classesDir.resolve("com/example/MyClass.class"); + Files.createDirectories(regularClass.getParent()); + Files.write(regularClass, "fake class content".getBytes(StandardCharsets.UTF_8)); + + // Zip using the default glob pattern "*" (same as attachedOutputs uses) + Path zipFile = testDir.resolve("test.zip"); + boolean hasFiles = CacheUtils.zip(classesDir, zipFile, "*", true); + + assertTrue(hasFiles, "Zip should contain files"); + assertTrue(Files.exists(zipFile), "Zip file should be created"); + + // Extract and verify module-info.class is present + Path extractDir = testDir.resolve("extracted"); + CacheUtils.unzip(zipFile, extractDir, true); + + Path extractedModuleInfo = extractDir.resolve("module-info.class"); + assertTrue(Files.exists(extractedModuleInfo), + "module-info.class should be present after extraction"); + + Path extractedRegularClass = extractDir.resolve("com/example/MyClass.class"); + assertTrue(Files.exists(extractedRegularClass), + "Regular .class file should be present after extraction"); + } +}