Fix empty folder missing problem when zip files. (#330)

Fix: #328 --------- Co-authored-by: Li Haoyi <[email protected]>
com-lihaoyi · Dec 8, 2024 · 90626df · 90626df
1 parent a3a901c
commit 90626df
Show file tree

Hide file tree

Showing 8 changed files with 70 additions and 35 deletions.
diff --git a/os/src/ZipOps.scala b/os/src/ZipOps.scala
@@ -62,7 +62,7 @@ object zip {
           excludePatterns,
           includePatterns,
           (path, sub) => {
-            os.copy(path, opened / sub, createFolders = true)
+            os.copy.over(path, opened / sub, createFolders = true)
             if (!preserveMtimes) {
               os.mtime.set(opened / sub, 0)
               // This is the only way we can properly zero out filesystem metadata within the
@@ -98,10 +98,12 @@ object zip {
     sources.foreach { source =>
       if (os.isDir(source.src)) {
         for (path <- os.walk(source.src)) {
-          if (os.isFile(path) && shouldInclude(path.toString, excludePatterns, includePatterns)) {
-            makeZipEntry0(path, source.dest.getOrElse(os.sub) / path.subRelativeTo(source.src))
+          if (shouldInclude(path.toString, excludePatterns, includePatterns)) {
+            val target = source.dest.getOrElse(os.sub) / path.subRelativeTo(source.src / os.up)
+            makeZipEntry0(path, target)
           }
         }
+        makeZipEntry0(source.src, source.dest.getOrElse(os.sub / source.src.last))
       } else if (shouldInclude(source.src.last, excludePatterns, includePatterns)) {
         makeZipEntry0(source.src, source.dest.getOrElse(os.sub / source.src.last))
       }
@@ -153,6 +155,7 @@ object zip {
     val mtimeOpt = if (preserveMtimes) Some(os.mtime(file)) else None
 
     val fis = if (os.isFile(file)) Some(os.read.inputStream(file)) else None
+
     try makeZipEntry0(sub, fis, mtimeOpt, zipOut)
     finally fis.foreach(_.close())
   }
@@ -163,7 +166,14 @@ object zip {
       preserveMtimes: Option[Long],
       zipOut: ZipOutputStream
   ) = {
-    val zipEntry = new ZipEntry(sub.toString)
+    val path = is match {
+      // for folder
+      case None => sub.toString + "/"
+      // for file
+      case Some(_) => sub.toString
+    }
+
+    val zipEntry = new ZipEntry(path)
 
     preserveMtimes match {
       case Some(mtime) => zipEntry.setTime(mtime)

diff --git a/os/test/resources/test/emptyFolder/.gitkeep b/os/test/resources/test/emptyFolder/.gitkeep
diff --git a/os/test/src-jvm/SpawningSubprocessesNewTests.scala b/os/test/src-jvm/SpawningSubprocessesNewTests.scala
@@ -86,7 +86,7 @@ object SpawningSubprocessesNewTests extends TestSuite {
             stdout =
               os.ProcessOutput((buf, len) => lineCount += buf.slice(0, len).count(_ == '\n'))
           )
-          lineCount ==> 22
+          lineCount ==> 24
         }
       }
       test - prep { wd =>
@@ -97,7 +97,7 @@ object SpawningSubprocessesNewTests extends TestSuite {
             cwd = wd,
             stdout = os.ProcessOutput.Readlines(line => lineCount += 1)
           )
-          lineCount ==> 22
+          lineCount ==> 24
         }
       }
     }

diff --git a/os/test/src-jvm/SpawningSubprocessesTests.scala b/os/test/src-jvm/SpawningSubprocessesTests.scala
@@ -82,7 +82,7 @@ object SpawningSubprocessesTests extends TestSuite {
             stdout =
               os.ProcessOutput((buf, len) => lineCount += buf.slice(0, len).count(_ == '\n'))
           )
-          lineCount ==> 22
+          lineCount ==> 24
         }
       }
       test - prep { wd =>
@@ -92,7 +92,7 @@ object SpawningSubprocessesTests extends TestSuite {
             cwd = wd,
             stdout = os.ProcessOutput.Readlines(line => lineCount += 1)
           )
-          lineCount ==> 22
+          lineCount ==> 24
         }
       }
     }

diff --git a/os/test/src-jvm/ZipOpJvmTests.scala b/os/test/src-jvm/ZipOpJvmTests.scala
@@ -36,19 +36,21 @@ object ZipOpJvmTests extends TestSuite {
         dest = wd / "unzipped folder"
       )
 
-      val paths = os.walk(unzippedFolder)
-      val expected = Seq(
+      val paths = os.walk(unzippedFolder).toList.sorted
+      val expected = List(
         // Files get included in the zip root using their name
         wd / "unzipped folder/File.txt",
         wd / "unzipped folder/Multi Line.txt",
         // Folder contents get included relative to the source root
-        wd / "unzipped folder/nestedA",
-        wd / "unzipped folder/nestedB",
-        wd / "unzipped folder/one.txt",
-        wd / "unzipped folder/nestedA/a.txt",
-        wd / "unzipped folder/nestedB/b.txt"
-      )
-      assert(paths.sorted == expected)
+        wd / "unzipped folder/folder1",
+        wd / "unzipped folder/folder1/one.txt",
+        wd / "unzipped folder/folder2",
+        wd / "unzipped folder/folder2/nestedA",
+        wd / "unzipped folder/folder2/nestedA/a.txt",
+        wd / "unzipped folder/folder2/nestedB",
+        wd / "unzipped folder/folder2/nestedB/b.txt"
+      ).sorted
+      assert(paths == expected)
     }
 
     test("zipAndUnzipPreserveMtimes") - prep { wd =>

diff --git a/os/test/src/CheckerTests.scala b/os/test/src/CheckerTests.scala
@@ -453,8 +453,9 @@ object CheckerTests extends TestSuite {
       val unzipDir = os.unzip(zipFile, wd / "unzipped")
       os.walk(unzipDir).sorted ==> Seq(
         unzipDir / "File.txt",
-        unzipDir / "one.txt"
-      )
+        unzipDir / "folder1/one.txt",
+        unzipDir / "folder1"
+      ).sorted
     }
     test("unzip") - prepChecker { wd =>
       val zipFileName = "zipped.zip"
@@ -478,7 +479,7 @@ object CheckerTests extends TestSuite {
         source = zipFile,
         dest = wd / "unzipped"
       )
-      os.walk(unzipDir).length ==> 2
+      os.walk(unzipDir).length ==> 3
     }
   }
 }
diff --git a/os/test/src/OpTests.scala b/os/test/src/OpTests.scala
@@ -18,7 +18,8 @@ object OpTests extends TestSuite {
         res / "misc",
         res / "os",
         res / "File.txt",
-        res / "Multi Line.txt"
+        res / "Multi Line.txt",
+        res / "emptyFolder"
       ),
       os.list(res / "folder2").toSet == Set(
         res / "folder2/nestedA",

diff --git a/os/test/src/ZipOpTests.scala b/os/test/src/ZipOpTests.scala
@@ -54,9 +54,10 @@ object ZipOpTests extends TestSuite {
       val expected = Seq(
         wd / "unzipped folder/renamed-file.txt",
         wd / "unzipped folder/renamed-folder",
-        wd / "unzipped folder/renamed-folder/one.txt"
+        wd / "unzipped folder/renamed-folder/folder1",
+        wd / "unzipped folder/renamed-folder/folder1/one.txt"
       )
-      assert(paths.sorted == expected)
+      assert(paths.sorted == expected.sorted)
     }
 
     test("excludePatterns") - prep { wd =>
@@ -80,8 +81,9 @@ object ZipOpTests extends TestSuite {
         zipFile1,
         dest = wd / "zipByExcludingCertainFiles"
       )
-      val paths = os.walk(outputZipFilePath).sorted
-      val expected = Seq(wd / "zipByExcludingCertainFiles/File.amx")
+      val paths = os.walk(outputZipFilePath).toList.sorted
+      val expected =
+        Seq(wd / "zipByExcludingCertainFiles/File.amx").toList.sorted
       assert(paths == expected)
     }
 
@@ -104,8 +106,8 @@ object ZipOpTests extends TestSuite {
       // Unzip file to check for contents
       val outputZipFilePath =
         os.unzip(zipFile1, dest = wd / "zipByIncludingCertainFiles")
-      val paths = os.walk(outputZipFilePath)
-      val expected = Seq(wd / "zipByIncludingCertainFiles" / amxFile)
+      val paths = os.walk(outputZipFilePath).toSeq.sorted
+      val expected = List(wd / "zipByIncludingCertainFiles" / amxFile).sorted
       assert(paths == expected)
     }
 
@@ -124,8 +126,8 @@ object ZipOpTests extends TestSuite {
         dest = wd / "zipStreamFunction"
       )
 
-      val paths = os.walk(unzippedFolder)
-      assert(paths == Seq(unzippedFolder / "File.txt"))
+      val paths = os.walk(unzippedFolder).toSeq
+      assert(paths.sorted == Seq(unzippedFolder / "File.txt").sorted)
     }
 
     test("list") - prep { wd =>
@@ -140,9 +142,10 @@ object ZipOpTests extends TestSuite {
       )
 
       // Unzip file to a destination folder
-      val listedContents = os.unzip.list(source = wd / zipFileName).toSeq
+      val listedContents = os.unzip.list(source = wd / zipFileName).toList.sorted
 
-      val expected = Seq(os.sub / "File.txt", os.sub / "one.txt")
+      val expected =
+        List(os.sub / "File.txt", os.sub / "folder1/one.txt", os.sub / "folder1").sorted
       assert(listedContents == expected)
     }
 
@@ -167,11 +170,12 @@ object ZipOpTests extends TestSuite {
         excludePatterns = Seq(amxFile.r)
       )
 
-      val paths = os.walk(unzippedFolder)
-      val expected = Seq(
+      val paths = os.walk(unzippedFolder).toList.sorted
+      val expected = List(
         wd / "unzipAllExceptExcludingCertainFiles/File.txt",
-        wd / "unzipAllExceptExcludingCertainFiles/one.txt"
-      )
+        wd / "unzipAllExceptExcludingCertainFiles/folder1/one.txt",
+        wd / "unzipAllExceptExcludingCertainFiles/folder1"
+      ).sorted
 
       assert(paths == expected)
     }
@@ -222,5 +226,22 @@ object ZipOpTests extends TestSuite {
       assert(file2Content == "Content of file2")
     }
 
+    test("emptyFolder") - prep { wd =>
+      val zipFileName = "zipCheckEmptyDirectory.zip"
+      val zipFile = os.zip(
+        dest = wd / zipFileName,
+        sources = Seq(
+          wd / "emptyFolder",
+          wd / "File.txt"
+        )
+      )
+
+      val unzippedFolder = os.unzip(
+        source = wd / zipFileName,
+        dest = wd / "unzipped-empty-directory"
+      )
+
+      assert(os.exists(unzippedFolder / "emptyFolder"))
+    }
   }
 }