Merge pull request #57 from GabrielSimonetto/issue-41

Allow compression of empty folders

src/archive/tar.rs

@@ -51,7 +51,9 @@ where
             let path = entry.path();
 
             println!("Compressing '{}'.", utils::to_utf(path));
-            if !path.is_dir() {
+            if path.is_dir() {
+                builder.append_dir(path, path)?;
+            } else {
                 let mut file = fs::File::open(path)?;
                 builder.append_file(path, &mut file)?;
             }

src/archive/zip.rs

@@ -88,14 +88,16 @@ where
             let path = entry.path();
 
             if path.is_dir() {
-                continue;
+                if dir_is_empty(path)? {
+                    writer.add_directory(path.to_str().unwrap().to_owned(), options)?;
+                }
+                // If a dir has files, the files are responsible for creating them.
+            } else {
+                writer.start_file(path.to_str().unwrap().to_owned(), options)?;
+                // TODO: better error messages
+                let file_bytes = fs::read(entry.path())?;
+                writer.write_all(&*file_bytes)?;
             }
-
-            writer.start_file(path.to_str().unwrap().to_owned(), options)?;
-
-            // TODO: better error messages
-            let file_bytes = fs::read(entry.path())?;
-            writer.write_all(&*file_bytes)?;
         }
 
         env::set_current_dir(previous_location)?;
@@ -112,6 +114,10 @@ fn check_for_comments(file: &ZipFile) {
     }
 }
 
+fn dir_is_empty(dir_path: &Path) -> crate::Result<bool> {
+    Ok(dir_path.read_dir()?.next().is_none())
+}
+
 #[cfg(unix)]
 fn __unix_set_permissions(file_path: &Path, file: &ZipFile) -> crate::Result<()> {
     use std::os::unix::fs::PermissionsExt;

tests/compress_and_decompress.rs

@@ -1,3 +1,5 @@
+mod utils;
+
 use std::{
     env, fs,
     io::prelude::*,
@@ -6,6 +8,7 @@ use std::{
 
 use ouch::{cli::Command, commands::run, oof};
 use rand::{rngs::SmallRng, RngCore, SeedableRng};
+use utils::*;
 
 #[test]
 /// Tests each format that supports multiple files with random input.
@@ -94,16 +97,6 @@ fn create_files(at: &Path, contents: &[FileContent]) -> Vec<PathBuf> {
         .collect()
 }
 
-fn compress_files(at: &Path, paths_to_compress: &[PathBuf], format: &str) -> PathBuf {
-    let archive_path = String::from("archive.") + format;
-    let archive_path = at.join(archive_path);
-
-    let command = Command::Compress { files: paths_to_compress.to_vec(), output_path: archive_path.to_path_buf() };
-    run(command, &oof::Flags::default()).expect("Failed to compress test dummy files");
-
-    archive_path
-}
-
 fn extract_files(archive_path: &Path) -> Vec<PathBuf> {
     // We will extract in the same folder as the archive
     // If the archive is at:

tests/compress_empty_dir.rs

@@ -0,0 +1,42 @@
+mod utils;
+
+use std::{env, path::PathBuf};
+
+use utils::*;
+
+#[test]
+fn test_each_format() {
+    test_compress_decompress_with_empty_dir("tar");
+    test_compress_decompress_with_empty_dir("zip");
+}
+
+fn test_compress_decompress_with_empty_dir(format: &str) {
+    // System temporary directory depends on the platform, for linux it's /tmp
+    let system_tmp = env::temp_dir();
+
+    // Create a temporary testing folder that will be deleted on scope drop
+    let testing_dir =
+        tempfile::Builder::new().prefix("ouch-testing").tempdir_in(system_tmp).expect("Could not create testing_dir");
+
+    let testing_dir_path = testing_dir.path();
+
+    let empty_dir_path: PathBuf = create_empty_dir(&testing_dir_path, "dummy_empty_dir_name");
+
+    let mut file_paths: Vec<PathBuf> = vec![empty_dir_path];
+
+    let compressed_archive_path: PathBuf = compress_files(&testing_dir_path, &file_paths, &format);
+
+    let mut extracted_paths = extract_files(&compressed_archive_path);
+
+    // // DEBUG UTIL:
+    // // Uncomment line below to freeze the code and see compressed and extracted files in
+    // // the temporary directory before their auto-destruction.
+    // std::thread::sleep(std::time::Duration::from_secs(10));
+
+    // no need to sort a unitary value vector but i will keep this
+    //  for retrocompatibility, for now.
+    file_paths.sort();
+    extracted_paths.sort();
+
+    assert_correct_paths(&file_paths, &extracted_paths, format);
+}

tests/utils.rs

@@ -0,0 +1,59 @@
+#[allow(dead_code)]
+use std::{
+    fs,
+    path::{Path, PathBuf},
+};
+
+use ouch::{cli::Command, commands::run, oof};
+
+pub fn create_empty_dir(at: &Path, filename: &str) -> PathBuf {
+    let dirname = Path::new(filename);
+    let full_path = at.join(dirname);
+
+    fs::create_dir(&full_path).expect("Failed to create an empty directory");
+
+    full_path
+}
+
+pub fn compress_files(at: &Path, paths_to_compress: &[PathBuf], format: &str) -> PathBuf {
+    let archive_path = String::from("archive.") + format;
+    let archive_path = at.join(archive_path);
+
+    let command = Command::Compress { files: paths_to_compress.to_vec(), output_path: archive_path.to_path_buf() };
+    run(command, &oof::Flags::default()).expect("Failed to compress test dummy files");
+
+    archive_path
+}
+
+pub fn extract_files(archive_path: &Path) -> Vec<PathBuf> {
+    // We will extract in the same folder as the archive
+    // If the archive is at:
+    //   /tmp/ouch-testing-tar.Rbq4DusBrtF8/archive.tar
+    // Then the extraction_output_folder will be:
+    //   /tmp/ouch-testing-tar.Rbq4DusBrtF8/extraction_results/
+    let mut extraction_output_folder = archive_path.to_path_buf();
+    // Remove the name of the extracted archive
+    assert!(extraction_output_folder.pop());
+    // Add the suffix "results"
+    extraction_output_folder.push("extraction_results");
+
+    let command = Command::Decompress {
+        files: vec![archive_path.to_owned()],
+        output_folder: Some(extraction_output_folder.clone()),
+    };
+    run(command, &oof::Flags::default()).expect("Failed to extract");
+
+    fs::read_dir(extraction_output_folder).unwrap().map(Result::unwrap).map(|entry| entry.path()).collect()
+}
+
+pub fn assert_correct_paths(original: &[PathBuf], extracted: &[PathBuf], format: &str) {
+    assert_eq!(
+        original.len(),
+        extracted.len(),
+        "Number of compressed files does not match number of decompressed when testing archive format '{:?}'.",
+        format
+    );
+    for (original, extracted) in original.iter().zip(extracted) {
+        assert_eq!(original.file_name(), extracted.file_name());
+    }
+}