feat: JENKINS-73837 - Adjusted DirScanner to allow empty directories in the TarArchiver

core/src/main/java/hudson/util/DirScanner.java

@@ -146,6 +146,12 @@ public void scan(File dir, FileVisitor visitor) throws IOException {
                     File file = new File(dir, f);
                     scanSingle(file, f, visitor);
                 }
+                for (String d : ds.getIncludedDirectories()) {
+                    if (!d.isEmpty()) {
+                        File file = new File(dir, d);
+                        scanSingle(file, d, visitor);
+                    }
+                }
             }
         }
 

core/src/main/java/jenkins/util/VirtualFile.java

@@ -327,11 +327,9 @@ public Collection<String> call() throws IOException {
 
         private static void collectFiles(VirtualFile d, Collection<String> names, String prefix) throws IOException {
             for (VirtualFile child : d.list()) {
-                if (child.isFile()) {
-                    names.add(prefix + child.getName());
-                } else if (child.isDirectory()) {
-                    collectFiles(child, names, prefix + child.getName() + "/");
-                }
+                // JENKINS-73837 - Empty Directories are now included in the list
+                names.add(prefix + child.getName());
+                collectFiles(child, names, prefix + child.getName() + "/");
             }
         }
     }

core/src/test/java/hudson/FilePathTest.java

@@ -806,7 +806,7 @@ public void testEOFbrokenFlush() throws IOException, InterruptedException {
         // Compress archive
         final FilePath tmpDirPath = new FilePath(srcFolder);
         int tarred = tmpDirPath.tar(Files.newOutputStream(archive.toPath()), "**");
-        assertEquals("One file should have been compressed", 3, tarred);
+        assertEquals("One file should have been compressed", 4, tarred);
 
         // Decompress
         final File dstFolder = temp.newFolder("dst");

core/src/test/java/hudson/util/io/TarArchiverTest.java

@@ -36,7 +36,6 @@
 import hudson.util.StreamTaskListener;
 import java.io.File;
 import java.io.IOException;
-import java.io.InputStream;
 import java.io.OutputStream;
 import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
@@ -46,7 +45,6 @@
 import java.util.Set;
 import org.apache.tools.tar.TarEntry;
 import org.apache.tools.tar.TarInputStream;
-import org.junit.Ignore;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.TemporaryFolder;
@@ -129,29 +127,6 @@ private static void run(FilePath dir, String... cmds) throws InterruptedExceptio
         }
     }
 
-    @Ignore("TODO fails to add empty directories to archive")
-    @Issue("JENKINS-73837")
-    @Test
-    public void emptyDirectory() throws Exception {
-        Path tar = tmp.newFile("test.tar").toPath();
-        Path root = tmp.newFolder().toPath();
-        Files.createDirectory(root.resolve("foo"));
-        Files.createDirectory(root.resolve("bar"));
-        Files.writeString(root.resolve("bar/file.txt"), "foobar", StandardCharsets.UTF_8);
-        try (OutputStream out = Files.newOutputStream(tar)) {
-            new FilePath(root.toFile()).tar(out, "**");
-        }
-        Set<String> names = new HashSet<>();
-        try (InputStream is = Files.newInputStream(tar);
-                TarInputStream tis = new TarInputStream(is, StandardCharsets.UTF_8.name())) {
-            TarEntry te;
-            while ((te = tis.getNextEntry()) != null) {
-                names.add(te.getName());
-            }
-        }
-        assertEquals(Set.of("foo/", "bar/", "bar/file.txt"), names);
-    }
-
     /**
      * Test backing up an open file
      */
@@ -171,6 +146,30 @@ public void emptyDirectory() throws Exception {
         t1.join();
     }
 
+    @Issue("JENKINS-73837")
+    @Test
+    public void emptyDirectory() throws Exception {
+        Path tar = tmp.newFile("test.tar").toPath();
+        Path root = tmp.newFolder().toPath();
+        Files.createDirectory(root.resolve("a"));
+        Files.createDirectory(root.resolve("b"));
+
+        Files.writeString(root.resolve("a/file.txt"), "empty_dir_test");
+
+        try (OutputStream outputStream = Files.newOutputStream(tar)) {
+            new FilePath(root.toFile()).tar(outputStream, "**");
+        }
+
+        Set<String> names = new HashSet<>();
+        try (TarInputStream tarInputStream = new TarInputStream(Files.newInputStream(tar), StandardCharsets.UTF_8.name())) {
+            TarEntry tarEntry;
+            while ((tarEntry = tarInputStream.getNextEntry()) != null) {
+                names.add(tarEntry.getName());
+            }
+        }
+        assertEquals(Set.of("a/", "b/", "a/file.txt"), names);
+    }
+
     private static class GrowingFileRunnable implements Runnable {
         private boolean finish = false;
         private Exception ex = null;

core/src/test/java/hudson/util/io/ZipArchiverTest.java

@@ -4,7 +4,6 @@
 
 import hudson.FilePath;
 import java.io.IOException;
-import java.io.InputStream;
 import java.io.OutputStream;
 import java.io.RandomAccessFile;
 import java.nio.charset.StandardCharsets;
@@ -16,7 +15,6 @@
 import java.util.zip.ZipFile;
 import java.util.zip.ZipInputStream;
 import org.junit.Assume;
-import org.junit.Ignore;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.TemporaryFolder;
@@ -81,26 +79,25 @@ public void huge64bitFile() throws IOException {
         }
     }
 
-    @Ignore("TODO fails to add empty directories to archive")
     @Issue("JENKINS-49296")
     @Test
     public void emptyDirectory() throws Exception {
         Path zip = tmp.newFile("test.zip").toPath();
         Path root = tmp.newFolder().toPath();
-        Files.createDirectory(root.resolve("foo"));
-        Files.createDirectory(root.resolve("bar"));
-        Files.writeString(root.resolve("bar/file.txt"), "foobar", StandardCharsets.UTF_8);
+        Files.createDirectory(root.resolve("a"));
+        Files.createDirectory(root.resolve("b"));
+        Files.writeString(root.resolve("a/file.txt"), "empty_dir_zip_test");
         try (OutputStream out = Files.newOutputStream(zip)) {
             new FilePath(root.toFile()).zip(out, "**");
         }
-        Set<String> names = new HashSet<>();
-        try (InputStream is = Files.newInputStream(zip);
-             ZipInputStream zis = new ZipInputStream(is, StandardCharsets.UTF_8)) {
-            ZipEntry ze;
-            while ((ze = zis.getNextEntry()) != null) {
-                names.add(ze.getName());
+        Set<String> actual = new HashSet<>();
+        Set<String> expected = Set.of("a/", "b/", "a/file.txt");
+        try (ZipInputStream zipInputStream = new ZipInputStream(Files.newInputStream(zip), StandardCharsets.UTF_8)) {
+            ZipEntry zipEntry;
+            while ((zipEntry = zipInputStream.getNextEntry()) != null) {
+                actual.add(zipEntry.getName());
             }
         }
-        assertEquals(Set.of("foo/", "bar/", "bar/file.txt"), names);
+        assertEquals(expected, actual);
     }
 }

core/src/test/java/jenkins/util/VirtualFileTest.java

@@ -27,9 +27,11 @@
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.contains;
 import static org.hamcrest.Matchers.containsInAnyOrder;
+import static org.hamcrest.Matchers.containsInRelativeOrder;
 import static org.hamcrest.Matchers.empty;
 import static org.hamcrest.Matchers.endsWith;
 import static org.hamcrest.Matchers.greaterThan;
+import static org.hamcrest.Matchers.hasItems;
 import static org.hamcrest.Matchers.hasSize;
 import static org.hamcrest.Matchers.is;
 import static org.hamcrest.Matchers.lessThan;
@@ -132,16 +134,16 @@ private static String modeString(int mode) throws IOException {
             System.err.println("testing " + vf.getClass().getName());
             assertEquals("[.hg/config.txt, sub/mid.txt, sub/subsub/lowest.txt, top.txt]", new TreeSet<>(vf.list("**/*.txt", null, false)).toString());
             assertEquals("[sub/mid.txt, sub/subsub/lowest.txt, top.txt]", new TreeSet<>(vf.list("**/*.txt", null, true)).toString());
-            assertEquals("[.hg/config.txt, sub/mid.txt, sub/subsub/lowest.txt, top.txt, very/deep/path/here]", new TreeSet<>(vf.list("**", null, false)).toString());
+            assertEquals("[.hg, .hg/config.txt, sub, sub/mid.txt, sub/subsub, sub/subsub/lowest.txt, top.txt, very, very/deep, very/deep/path, very/deep/path/here]", new TreeSet<>(vf.list("**", null, false)).toString());
             assertEquals("[]", new TreeSet<>(vf.list("", null, false)).toString());
-            assertEquals("[sub/mid.txt, sub/subsub/lowest.txt]", new TreeSet<>(vf.list("sub/", null, false)).toString());
-            assertEquals("[sub/mid.txt]", new TreeSet<>(vf.list("sub/", "sub/subsub/", false)).toString());
-            assertEquals("[sub/mid.txt]", new TreeSet<>(vf.list("sub/", "sub/subsub/**", false)).toString());
-            assertEquals("[sub/mid.txt]", new TreeSet<>(vf.list("sub/", "**/subsub/", false)).toString());
+            assertEquals("[sub, sub/mid.txt, sub/subsub, sub/subsub/lowest.txt]", new TreeSet<>(vf.list("sub/", null, false)).toString());
+            assertEquals("[sub, sub/mid.txt]", new TreeSet<>(vf.list("sub/", "sub/subsub/", false)).toString());
+            assertEquals("[sub, sub/mid.txt]", new TreeSet<>(vf.list("sub/", "sub/subsub/**", false)).toString());
+            assertEquals("[sub, sub/mid.txt]", new TreeSet<>(vf.list("sub/", "**/subsub/", false)).toString());
             assertEquals("[.hg/config.txt, sub/mid.txt]", new TreeSet<>(vf.list("**/mid*,**/conf*", null, false)).toString());
-            assertEquals("[sub/mid.txt, sub/subsub/lowest.txt]", new TreeSet<>(vf.list("sub/", "**/notthere/", false)).toString());
+            assertEquals("[sub, sub/mid.txt, sub/subsub, sub/subsub/lowest.txt]", new TreeSet<>(vf.list("sub/", "**/notthere/", false)).toString());
             assertEquals("[top.txt]", new TreeSet<>(vf.list("*.txt", null, false)).toString());
-            assertEquals("[sub/subsub/lowest.txt, top.txt, very/deep/path/here]", new TreeSet<>(vf.list("**", "**/mid*,**/conf*", false)).toString());
+            assertEquals("[.hg, sub, sub/subsub, sub/subsub/lowest.txt, top.txt, very, very/deep, very/deep/path, very/deep/path/here]", new TreeSet<>(vf.list("**", "**/mid*,**/conf*", false)).toString());
         }
     }
     /** Roughly analogous to {@code org.jenkinsci.plugins.compress_artifacts.ZipStorage}. */
@@ -237,7 +239,7 @@ public void list_Glob_NoFollowLinks_FileVF() throws Exception {
         File root = tmp.getRoot();
         VirtualFile virtualRoot = VirtualFile.forFile(root);
         Collection<String> children = virtualRoot.list("**", null, true, LinkOption.NOFOLLOW_LINKS);
-        assertThat(children, containsInAnyOrder(
+        assertThat(children, containsInRelativeOrder(
                 "a/aa/aa.txt",
                 "a/ab/ab.txt",
                 "b/ba/ba.txt"
@@ -253,7 +255,8 @@ public void list_Glob_NoFollowLinks_FilePathVF() throws Exception {
         File root = tmp.getRoot();
         VirtualFile virtualRoot = VirtualFile.forFilePath(new FilePath(root));
         Collection<String> children = virtualRoot.list("**", null, true, LinkOption.NOFOLLOW_LINKS);
-        assertThat(children, containsInAnyOrder(
+        // JENKINS-73837 - Since we now allow empty directories, this had to be changed to hasItems
+        assertThat(children, hasItems(
                 "a/aa/aa.txt",
                 "a/ab/ab.txt",
                 "b/ba/ba.txt"
@@ -281,7 +284,8 @@ public void zip_NoFollowLinks_FilePathVF() throws Exception {
         assertTrue(unzipPath.isDirectory());
         assertTrue(unzipPath.child("a").child("aa").child("aa.txt").exists());
         assertTrue(unzipPath.child("a").child("ab").child("ab.txt").exists());
-        assertFalse(unzipPath.child("a").child("aa").child("aaa").exists());
+        // JENKINS-73837 - Empty directories are included in the zip file
+        assertTrue(unzipPath.child("a").child("aa").child("aaa").exists());
         assertFalse(unzipPath.child("a").child("_b").exists());
         assertTrue(unzipPath.child("b").child("ba").child("ba.txt").exists());
         assertFalse(unzipPath.child("b").child("_a").exists());
@@ -311,7 +315,8 @@ public void zip_NoFollowLinks_FilePathVF_withPrefix() throws Exception {
         assertTrue(unzipPath.child(prefix).isDirectory());
         assertTrue(unzipPath.child(prefix).child("a").child("aa").child("aa.txt").exists());
         assertTrue(unzipPath.child(prefix).child("a").child("ab").child("ab.txt").exists());
-        assertFalse(unzipPath.child(prefix).child("a").child("aa").child("aaa").exists());
+        // JENKINS-73837 - Empty directories are included in the zip file
+        assertTrue(unzipPath.child(prefix).child("a").child("aa").child("aaa").exists());
         assertFalse(unzipPath.child(prefix).child("a").child("_b").exists());
         assertTrue(unzipPath.child(prefix).child("b").child("ba").child("ba.txt").exists());
         assertFalse(unzipPath.child(prefix).child("b").child("_a").exists());
@@ -339,7 +344,8 @@ public void zip_NoFollowLinks_FileVF() throws Exception {
         assertTrue(unzipPath.isDirectory());
         assertTrue(unzipPath.child("a").child("aa").child("aa.txt").exists());
         assertTrue(unzipPath.child("a").child("ab").child("ab.txt").exists());
-        assertFalse(unzipPath.child("a").child("aa").child("aaa").exists());
+        // JENKINS-73837 - Empty directories are included in the zip file
+        assertTrue(unzipPath.child("a").child("aa").child("aaa").exists());
         assertFalse(unzipPath.child("a").child("_b").exists());
         assertTrue(unzipPath.child("b").child("ba").child("ba.txt").exists());
         assertFalse(unzipPath.child("b").child("_a").exists());
@@ -369,7 +375,8 @@ public void zip_NoFollowLinks_FileVF_withPrefix() throws Exception {
         assertTrue(unzipPath.child(prefix).isDirectory());
         assertTrue(unzipPath.child(prefix).child("a").child("aa").child("aa.txt").exists());
         assertTrue(unzipPath.child(prefix).child("a").child("ab").child("ab.txt").exists());
-        assertFalse(unzipPath.child(prefix).child("a").child("aa").child("aaa").exists());
+        // JENKINS-73837 - Empty directories are included in the zip file
+        assertTrue(unzipPath.child(prefix).child("a").child("aa").child("aaa").exists());
         assertFalse(unzipPath.child(prefix).child("a").child("_b").exists());
         assertTrue(unzipPath.child(prefix).child("b").child("ba").child("ba.txt").exists());
         assertFalse(unzipPath.child(prefix).child("b").child("_a").exists());
@@ -504,7 +511,8 @@ public void list_Glob_NoFollowLinks_ExternalSymlink_FilePathVF() throws Exceptio
         VirtualFile symlinkVirtualPath = VirtualFile.forFilePath(symlinkPath);
         VirtualFile symlinkChildVirtualPath = symlinkVirtualPath.child("aa");
         Collection<String> children = symlinkChildVirtualPath.list("**", null, true, LinkOption.NOFOLLOW_LINKS);
-        assertThat(children, contains("aa.txt"));
+        // JENKINS-73837 - Since we now allow empty directories, this had to be changed to hasItems
+        assertThat(children, hasItems("aa.txt"));
     }
 
     @Test
@@ -519,7 +527,8 @@ public void list_Glob_NoFollowLinks_ExternalSymlink_FileVF() throws Exception {
         VirtualFile symlinkVirtualFile = VirtualFile.forFile(symlinkFile);
         VirtualFile symlinkChildVirtualFile = symlinkVirtualFile.child("aa");
         Collection<String> children = symlinkChildVirtualFile.list("**", null, true, LinkOption.NOFOLLOW_LINKS);
-        assertThat(children, contains("aa.txt"));
+        // JENKINS-73837 - Since we now allow empty directories, this had to be changed to hasItems
+        assertThat(children, hasItems("aa.txt"));
     }
 
     @Test