fix: add tolerant tar extraction on windows to solve failing on symlinks #1138
Conversation
|
Thank you! On first glance, looks good! |
|
@wolfv i think I've fixed the linting issue. Let me know if there is anything else I should consider. |
| match file.header().entry_type() { | ||
| EntryType::Symlink if cfg!(target_os = "windows") => { | ||
| if let Err(e) = file.unpack(&path) { | ||
| println!( |
There was a problem hiding this comment.
We should always use tracing::warn! or tracing::error!.
| } | ||
| } | ||
| Err(e) => { | ||
| println!("Warning: failed to read an entry due to {:?}", e); |
There was a problem hiding this comment.
I think in this case, we should also return the error and fail the operation.
| } | ||
| _ => { | ||
| if let Err(e) = file.unpack(&path) { | ||
| println!("Warning: failed to unpack {:?} due to {:?}", path, e); |
There was a problem hiding this comment.
Again we should return the error here :)
|
Hi @Krande - let me know if you want to make the changes or if you would prefer me taking over :) |
|
If you have time to implement the changes, feel free to finish them:) if not, I'll likely start to implement them after lunch time tomorrow! |
|
I looked a little bit deeper and I think unfortunately this doesn't cover a bunch of (other) edge cases that the original tar extraction code covers. That's really unfortunate. Ideally we could implement this kind of behavior / filtering in tar-rs directly. |
|
I see. I checked tar-rs and found this alexcrichton/tar-rs#280 which I guess is related. |
|
Would it be possible to make this tolerant extraction feature optional by hiding it behind an opt-in flag until a better long-term solution can be found? That way, it is at least possible for us forced to use windows in our day jobs to use rattler-build for projects containing symlinks :) |
Hey guys,
this is supposed to close #940.
This is my first time writing rust beyond just "hello world", so any suggestions for improvements are most welcome!