builder: add support for archiving special files on Unix

[liushuyu]

Added support for archiving character, block and FIFO files on Unix, this is useful when you want to take a whole system backup or re-archiving an extracted system image.

The file handling in this pull request isn't particularly graceful since I am not very experienced with the codebase, any suggestions are welcome!

---

This change is 

[alexcrichton]

Seems reasonable enough to me, thanks!

Could this perhaps be refactored a bit though to avoid a few extra calls to fs::metadata being called? I think this could have an internal append function which takes a FileType from libstd and dispatches internally about how to append it? (e.g. more cases would be handled on Unix than on Windows, but regular files would all roughly be handled in the same place)

[liushuyu]

Could this perhaps be refactored a bit though to avoid a few extra calls to fs::metadata being called? I think this could have an internal append function which takes a FileType from libstd and dispatches internally about how to append it? (e.g. more cases would be handled on Unix than on Windows, but regular files would all roughly be handled in the same place)

Sorry for the late reply. I am a bit confused by this suggestion: I saw the

tar-rs/src/builder.rs

Line 542 in f07b3c1

let stat = fs::symlink_metadata(&src)?;

part so I assumed one more stat() should be fine here? https://github.com/alexcrichton/tar-rs/pull/246/files#diff-e4f794ca308aa712cbad31feb714df7cada1bbe453a30e232e45891571430263R607

Did you mean to refactor the append_dir_all function so that the stat could be avoided? Or should I do something about append_special function?

[alexcrichton]

I mostly just got the feeling reading this that it'd be better to refactor where at some point there's the equivalent of a match on the FileType (or similar) of what to archive, and right now it seems spread across a few locations with perhaps an extra metadata call here or there.

[liushuyu]

I have made an attempt to reduce stat calls:

[alexcrichton]

This unfortunately has a side effect where on platforms like Unix this ends up doing a syscall whereas previously accessing file_type generally doesn't require a syscall on most major platforms. The intention was that this loop should primarily only do fs::read_dir as a sycall and otherwise other syscalls can be avoided.

My thinking about refactoring this was largely that this loop has one iteration of "look at the file type and delegate appropriately" but above in append_path_with_name it's basically the same loop. With the append_path_with_name addition it's getting more complicated so I was wondering if it would be possible to refactor this loop and append_path_with_name above to call a common function which is the one location where special files are handled.

[liushuyu]

This unfortunately has a side effect where on platforms like Unix this ends up doing a syscall whereas previously accessing file_type generally doesn't require a syscall on most major platforms. The intention was that this loop should primarily only do fs::read_dir as a sycall and otherwise other syscalls can be avoided.

I have reverted the changes made to this loop.

My thinking about refactoring this was largely that this loop has one iteration of "look at the file type and delegate appropriately" but above in append_path_with_name it's basically the same loop. With the append_path_with_name addition it's getting more complicated so I was wondering if it would be possible to refactor this loop and append_path_with_name above to call a common function which is the one location where special files are handled.

I am not quite sure which one is better:

1. Make a new delegate function append_special and rename the current append_special to append_special_unix. Then in the append_special function, use fs::metadata to get the metadata and pass it to append_special_unix. In which case, the loop needs to be modified to differentiate a regular file from a special one.
2. Similar to (1) but instead of using fs::metadata in that function, fs::metadata is used in the "special file" branch of the loop. The advantage of this is one less stat when called from append_path_with_name (since there is one stat in that function as well).
3. Similar to (1) but making append_special to accept Option<fs::metadata> and stat on-demand.

[liushuyu]

Ping? Is there any update for this Pull Request?

[alexcrichton]

Sorry I think this got lost in my inbox, but this looks good to me now!