Fix folder softlink is not preserved after packing (#850)

* Fix folder softlink is not preserved after packing

Signed-off-by: tommady <[email protected]>

* enhance test case

Signed-off-by: tommady <[email protected]>

* fix test case

Signed-off-by: tommady <[email protected]>

* add changelog

Signed-off-by: tommady <[email protected]>

* fixup windows zip while unpacking should use symlink_dir

Signed-off-by: tommady <[email protected]>

* address comment

Signed-off-by: tommady <[email protected]>

---------

Signed-off-by: tommady <[email protected]>
Co-authored-by: Vinícius Miguel <[email protected]>

Author: tommady

CHANGELOG.md

@@ -43,6 +43,7 @@ Categories Used:
 - Fix tar extraction count when --quiet [\#824](https://github.com/ouch-org/ouch/pull/824) ([marcospb19](https://github.com/marcospb19))
 - Fix 7z BadSignature error when compressing and then listing [\#819](https://github.com/ouch-org/ouch/pull/819) ([tommady](https://github.com/tommady))
 - Fix unpacking with merge flag failing without --dir flag [\#826](https://github.com/ouch-org/ouch/pull/826) ([tommady](https://github.com/tommady))
+- Fix folder softlink is not preserved after packing [\#850](https://github.com/ouch-org/ouch/pull/850) ([tommady](https://github.com/tommady))
 
 ### Tweaks
 

src/archive/tar.rs

@@ -40,6 +40,10 @@ pub fn unpack_archive(reader: Box<dyn Read>, output_folder: &Path, quiet: bool)
 
                 #[cfg(unix)]
                 std::os::unix::fs::symlink(&target, &full_path)?;
+
+                // FIXME: how to detect whether the destination is a folder or a regular file?
+                // regular file should use fs::symlink_file
+                // folder should use fs::symlink_dir
                 #[cfg(windows)]
                 std::os::windows::fs::symlink_file(&target, &full_path)?;
             }
@@ -141,9 +145,7 @@ where
                 info(format!("Compressing '{}'", EscapedPathDisplay::new(path)));
             }
 
-            if path.is_dir() {
-                builder.append_dir(path, path)?;
-            } else if path.is_symlink() && !follow_symlinks {
+            if !follow_symlinks && path.symlink_metadata()?.is_symlink() {
                 let target_path = path.read_link()?;
 
                 let mut header = tar::Header::new_gnu();
@@ -155,6 +157,8 @@ where
                         .detail("Unexpected error while trying to read link")
                         .detail(format!("Error: {err}."))
                 })?;
+            } else if path.is_dir() {
+                builder.append_dir(path, path)?;
             } else {
                 let mut file = match fs::File::open(path) {
                     Ok(f) => f,

src/archive/zip.rs

@@ -64,7 +64,21 @@ where
                 if !quiet {
                     info(format!("File {} extracted to \"{}\"", idx, file_path.display()));
                 }
-                fs::create_dir_all(&file_path)?;
+
+                let mode = file.unix_mode();
+                let is_symlink = mode.is_some_and(|mode| mode & 0o170000 == 0o120000);
+
+                if is_symlink {
+                    let mut target = String::new();
+                    file.read_to_string(&mut target)?;
+
+                    #[cfg(unix)]
+                    std::os::unix::fs::symlink(&target, &file_path)?;
+                    #[cfg(windows)]
+                    std::os::windows::fs::symlink_dir(&target, file_path)?;
+                } else {
+                    fs::create_dir_all(&file_path)?;
+                }
             }
             _is_file @ false => {
                 if let Some(path) = file_path.parent() {
@@ -242,9 +256,7 @@ where
                     .detail(format!("File at '{path:?}' has a non-UTF-8 name"))
             })?;
 
-            if metadata.is_dir() {
-                writer.add_directory(entry_name, options)?;
-            } else if path.is_symlink() && !follow_symlinks {
+            if !follow_symlinks && path.symlink_metadata()?.is_symlink() {
                 let target_path = path.read_link()?;
                 let target_name = target_path.to_str().ok_or_else(|| {
                     FinalError::with_title("Zip requires that all directories names are valid UTF-8")
@@ -259,6 +271,8 @@ where
                 let symlink_options = options.unix_permissions(0o120777);
 
                 writer.add_symlink(entry_name, target_name, symlink_options)?;
+            } else if path.is_dir() {
+                writer.add_directory(entry_name, options)?;
             } else {
                 #[cfg(not(unix))]
                 let options = if is_executable::is_executable(path) {

tests/integration.rs

@@ -689,7 +689,8 @@ fn symlink_pack_and_unpack(
     let root_path = temp_dir.path();
 
     let src_files_path = root_path.join("src_files");
-    fs::create_dir_all(&src_files_path)?;
+    let folder_path = src_files_path.join("folder");
+    fs::create_dir_all(&folder_path)?;
 
     let mut files_path = ["file1.txt", "file2.txt", "file3.txt", "file4.txt", "file5.txt"]
         .into_iter()
@@ -704,10 +705,15 @@ fn symlink_pack_and_unpack(
     fs::create_dir_all(&dest_files_path)?;
 
     let symlink_path = src_files_path.join(Path::new("symlink"));
+    let symlink_folder_path = src_files_path.join(Path::new("symlink_folder"));
     #[cfg(unix)]
     std::os::unix::fs::symlink(&files_path[0], &symlink_path)?;
+    #[cfg(unix)]
+    std::os::unix::fs::symlink(&folder_path, &symlink_folder_path)?;
     #[cfg(windows)]
     std::os::windows::fs::symlink_file(&files_path[0], &symlink_path)?;
+    #[cfg(windows)]
+    std::os::windows::fs::symlink_dir(&folder_path, &symlink_folder_path)?;
 
     files_path.push(symlink_path);
 
@@ -728,11 +734,10 @@ fn symlink_pack_and_unpack(
         .assert()
         .success();
 
-    assert_same_directory(&src_files_path, &dest_files_path, false);
     // check the symlink stand still
     for f in dest_files_path.as_path().read_dir()? {
         let f = f?;
-        if f.file_name() == "symlink" {
+        if f.file_name() == "symlink" || f.file_name() == "symlink_folder" {
             assert!(f.file_type()?.is_symlink())
         }
     }