From 3858076274d8035ee1a9ba5b7b73d83c87313d63 Mon Sep 17 00:00:00 2001 From: tommady Date: Thu, 17 Jul 2025 08:38:11 +0000 Subject: [PATCH 1/5] Fix folder softlink is not preserved after packing Signed-off-by: tommady --- src/archive/tar.rs | 6 +++--- src/archive/zip.rs | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/archive/tar.rs b/src/archive/tar.rs index cc80b95..7bcf9cd 100644 --- a/src/archive/tar.rs +++ b/src/archive/tar.rs @@ -141,9 +141,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 ((path.is_dir() && path.symlink_metadata()?.is_symlink()) || path.is_symlink()) && !follow_symlinks { let target_path = path.read_link()?; let mut header = tar::Header::new_gnu(); @@ -155,6 +153,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, diff --git a/src/archive/zip.rs b/src/archive/zip.rs index 28546a1..11954cc 100644 --- a/src/archive/zip.rs +++ b/src/archive/zip.rs @@ -242,9 +242,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 ((path.is_dir() && path.symlink_metadata()?.is_symlink()) || path.is_symlink()) && !follow_symlinks { 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 +257,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) { From 13898bbef354ea0ddd33c7d547f85fb82d3b0c05 Mon Sep 17 00:00:00 2001 From: tommady Date: Thu, 17 Jul 2025 08:52:14 +0000 Subject: [PATCH 2/5] enhance test case Signed-off-by: tommady --- tests/integration.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/tests/integration.rs b/tests/integration.rs index 7df3995..c7f3503 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -685,7 +685,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() @@ -700,10 +701,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,7 +734,7 @@ fn symlink_pack_and_unpack( // 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()) } } From 37e2caee36f03e580a0a1bc32c3618fd23677330 Mon Sep 17 00:00:00 2001 From: tommady Date: Thu, 17 Jul 2025 09:11:19 +0000 Subject: [PATCH 3/5] fix test case Signed-off-by: tommady --- src/archive/tar.rs | 16 ++++++++++++---- src/archive/zip.rs | 16 +++++++++++++++- tests/integration.rs | 1 - 3 files changed, 27 insertions(+), 6 deletions(-) diff --git a/src/archive/tar.rs b/src/archive/tar.rs index 7bcf9cd..b0285e8 100644 --- a/src/archive/tar.rs +++ b/src/archive/tar.rs @@ -38,10 +38,18 @@ pub fn unpack_archive(reader: Box, output_folder: &Path, quiet: bool) .link_name()? .ok_or_else(|| std::io::Error::new(std::io::ErrorKind::InvalidData, "Missing symlink target"))?; - #[cfg(unix)] - std::os::unix::fs::symlink(&target, &full_path)?; - #[cfg(windows)] - std::os::windows::fs::symlink_file(&target, &full_path)?; + if target.is_file() { + #[cfg(unix)] + std::os::unix::fs::symlink(&target, &full_path)?; + #[cfg(windows)] + std::os::windows::fs::symlink_file(&target, &full_path)?; + } + if target.is_dir() { + #[cfg(unix)] + std::os::unix::fs::symlink(&target, &full_path)?; + #[cfg(windows)] + std::os::windows::fs::symlink_dir(&target, &full_path)?; + } } tar::EntryType::Regular | tar::EntryType::Directory => { file.unpack_in(output_folder)?; diff --git a/src/archive/zip.rs b/src/archive/zip.rs index 11954cc..59fd226 100644 --- a/src/archive/zip.rs +++ b/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() { diff --git a/tests/integration.rs b/tests/integration.rs index c7f3503..88de748 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -730,7 +730,6 @@ 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?; From 8280b9edb2997bc3e8d2ea05700dd0c22f95dc40 Mon Sep 17 00:00:00 2001 From: tommady Date: Thu, 17 Jul 2025 09:12:58 +0000 Subject: [PATCH 4/5] add changelog Signed-off-by: tommady --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f4b20ad..465edb4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,6 +37,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 folder softlink is not preserved after packing [\#850](https://github.com/ouch-org/ouch/pull/850) ([tommady](https://github.com/tommady)) ### Tweaks From e365c93f118e57ef6909992a778f57126626a2de Mon Sep 17 00:00:00 2001 From: tommady Date: Thu, 17 Jul 2025 09:51:57 +0000 Subject: [PATCH 5/5] fixup windows zip while unpacking should use symlink_dir Signed-off-by: tommady --- src/archive/tar.rs | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/src/archive/tar.rs b/src/archive/tar.rs index b0285e8..7b9b53a 100644 --- a/src/archive/tar.rs +++ b/src/archive/tar.rs @@ -38,18 +38,14 @@ pub fn unpack_archive(reader: Box, output_folder: &Path, quiet: bool) .link_name()? .ok_or_else(|| std::io::Error::new(std::io::ErrorKind::InvalidData, "Missing symlink target"))?; - if target.is_file() { - #[cfg(unix)] - std::os::unix::fs::symlink(&target, &full_path)?; - #[cfg(windows)] - std::os::windows::fs::symlink_file(&target, &full_path)?; - } - if target.is_dir() { - #[cfg(unix)] - std::os::unix::fs::symlink(&target, &full_path)?; - #[cfg(windows)] - std::os::windows::fs::symlink_dir(&target, &full_path)?; - } + #[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_file + #[cfg(windows)] + std::os::windows::fs::symlink_file(&target, &full_path)?; } tar::EntryType::Regular | tar::EntryType::Directory => { file.unpack_in(output_folder)?;