Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ignore broken symlinks when compressing #224

Merged
merged 2 commits into from
Dec 9, 2021
Merged

Conversation

sigmaSd
Copy link
Contributor

@sigmaSd sigmaSd commented Dec 8, 2021

No description provided.

@figsoda
Copy link
Member

figsoda commented Dec 8, 2021

as an alternative we can add a flag to preserve symlinks

@marcospb19 marcospb19 merged commit 9c34469 into ouch-org:master Dec 9, 2021
@marcospb19
Copy link
Member

as an alternative we can add a flag to preserve symlinks

Tar seems to support symlinks, but zip doesn't, so this flag would only work for tar?

@figsoda
Copy link
Member

figsoda commented Dec 9, 2021

man zip gave me this

--symlinks
       For  UNIX  and VMS (V8.3 and later), store symbolic links as such in the zip archive, instead of compressing and storing the file referred to by the link.
       This can avoid multiple copies of files being included in the archive as zip recurses the directory trees and accesses files directly and by links.

@sigmaSd
Copy link
Contributor Author

sigmaSd commented Dec 9, 2021

That doesn't mention broken symlinks explicitly and in practice if you try ziping a directory containing a broken symlink using zip it wont be in the zip archive

@marcospb19
Copy link
Member

I think I should then create an issue to add a --symlink flag that does the same, @sigmaSd do you also agree?

@sigmaSd
Copy link
Contributor Author

sigmaSd commented Dec 14, 2021

I guess its a simple flag and can be added easily

But I think it should be named -something like --dont--ignore--broken--symlinks

My hunch is this is an advanced feature and the flag should be explicit like the above to show it

Or another idea is to wait until somone open an issue about missing broken symlinks and explain his usecase, then it becomes obvious why we want this

@marcospb19
Copy link
Member

--dont--ignore--broken--symlinks is different from the --symlink I was thinking about.

The former is about throwing errors when symlinks are broken, and the latter is about not following symlinks when creating archives, adding the symlinks instead of the files they point to.

I think that the latter would be a more interesting and useful flag, could be named as --store-symlinks.

@sigmaSd
Copy link
Contributor Author

sigmaSd commented Dec 14, 2021

So I did some testing and there is actually some weird edge cases in how zip does this, but overall:

  • zip follows symlinks by default (-y flag to not follow)
  • tar doesn't follow symlinks by default(-h ? to follow)

Since ouch already is similar to zip in this area, I guess adding --store-symlinks is a good idea

@marcospb19
Copy link
Member

marcospb19 commented Dec 14, 2021

Thanks for your research!
(Edit: both of you, actually ;) )

Comment on lines +123 to +128
/// Returns true if a path is a symlink.
/// This is the same as the nightly https://doc.rust-lang.org/std/path/struct.Path.html#method.is_symlink
// Useful to detect broken symlinks when compressing. (So we can safely ignore them)
pub fn is_symlink(path: &Path) -> bool {
fs::symlink_metadata(path).map(|m| m.file_type().is_symlink()).unwrap_or(false)
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lol a couple months later this was stabilized.

https://blog.rust-lang.org/2022/01/13/Rust-1.58.0.html

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants