fix: allow strip_archive_path_components to strip a dir containing the same filename#6405
fix: allow strip_archive_path_components to strip a dir containing the same filename#6405
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR fixes an issue in the archive path stripping functionality where it previously failed when a directory contained a file with the same name as the directory. The fix implements a temporary rename strategy to avoid filename conflicts during the file moving process.
- Replaces the previous two-phase approach (collect all files first, then move) with a safer single-phase approach
- Introduces temporary directory renaming to prevent conflicts when moving files out of directories
- Simplifies the logic by processing directories one at a time instead of collecting all entries first
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
Bugbot run |
| "{}_tmp_strip", | ||
| path.file_name().unwrap().to_string_lossy() | ||
| )); | ||
| fs::rename(&path, &temp_path)?; |
There was a problem hiding this comment.
I think you can get around this with https://docs.rs/tempfile/latest/tempfile/struct.NamedTempFile.html#method.new_in
There was a problem hiding this comment.
We need to rename the parent directory with a unique name, so I think we cannot directly use the tempfile crate.
We can use it to get a unique directory name, but it creates a tempdir and removes it immediately, so I think it's not good for performance.
Also, the directory name with _tmp_strip could collide, but I believe it's not a common case.
mv .../age-plugin-yubikey/age-plugin-yubikey .../age-plugin-yubikeyfails, so we need to rename the dir before moving the file.