Option to stay on same file system

[mckaymatt]

https://github.com/BurntSushi/walkdir/issues/8

There's a good bit going on here.

• added --same-file-system/-x opt
• added is_same_file_system function to DirEntry iterator.
• added a windows module that uses winapi to call for BY_HANDLE_FILE_INFORMATION. All that is technically need from there is dwVolumeSerialNumber, but I thought it would be a waste to go through the trouble of getting that metadata without giving users the option to use it themselves. Maybe that's a leaky abstraction, but it's essentially what we do with the unix metadata. Also, there are a few places where unwrap is called because I'm not sure we would want to recover if someone calls -x and we can't find the root device info.
• when --same-file-system is true the root_device is compared to the DirEntry metadata.dev (unix) or BY_HANDLE_FILE_INFORMATION.dwVolumeSerialNumber (NT).
• the Windows version calls for BY_HANDLE_FILE_INFORMATION every single time. It might make sense to have something like a get_metadata flag that same_file_system would trigger.

I was able to manually test this in a Windows VM, but I didn't add tests. That will probably involve a mock. I want to get some feedback before doing that though.

Going forward it might might sense to do a full abstraction the Unix and Windows metadata.

@BurntSushi

C:\vagrant\walkdir>cargo build --examples && target\debug\examples\walkdir.exe -L --same-file-system C:\Users\vagrant\Downloads
   Compiling walkdir v2.0.1 (file:///C:/vagrant/walkdir)
    Finished dev [unoptimized + debuginfo] target(s) in 6.50 secs
C:\Users\vagrant\Downloads
C:\Users\vagrant\Downloads\desktop.ini

C:\vagrant\walkdir>cargo build --examples && target\debug\examples\walkdir.exe -L  C:\Users\vagrant\Downloads
    Finished dev [unoptimized + debuginfo] target(s) in 0.23 secs
C:\Users\vagrant\Downloads
C:\Users\vagrant\Downloads\ddrive
C:\Users\vagrant\Downloads\ddrive\ddrivetext.txt
C:\Users\vagrant\Downloads\desktop.ini

[KodrAus]

Thanks @mckaymatt! My two cents are that I think we should avoid creating a public dependency out of winapi if we can get away with it. I would keep the BY_HANDLE_FILE_INFORMATION private for now and just use it to drive the is_same_file_system behaviour. I also wouldn't be too worried about unwraping in the examples, but we should think more about the unwraps in the is_same_file_system methods. I wouldn't expect a method that returns Result to panic.

For the Builder methods instead of panicking if we can't determine the initial filesystem maybe we could capture that Err and throw it back when attempting to traverse. So the self.opts.root_device becomes a Result<u64, io::Error>. Alternatively we could make that builder method return a Result so you need to ? on it. I think you're right and folks will probably just want to panic, but we should avoid making that decision for them.

What do you think?

[mckaymatt]

Sounds good 👍

[mckaymatt]

@KodrAus I removed the BY_HANDLE_FILE_INFORMATION struct from the public part of the lib.

root_device is now an Option<Result<u64>> since it really should be able to be None when we aren't collecting drive id's.

is_same_file_system will now return errors if something goes wrong. That being said, I had to make a new error every time since I can't keep taking and returning the borrowed error from the root_device. That's no ideal, but if you don't have that root_drive id you are not getting very far either way.

[mckaymatt]

@KodrAus I made the change you suggested and merged in master. Take a look.

[KodrAus]

@mckaymatt This looks good to me! @BurntSushi (who is really busy and might not get to this for a while) might have some more feedback. The gist of it is:

• We try get the device for the root passed in to WalkDir when calling same_filesystem(true)
• We stash the result of this call until you start iterating
• If the device is different, we return None
• If the device failed, we return Some(Err). Since this is checked for all files, if getting to root failed, the traversal will theoretically continue, but Err on each item

[mckaymatt]

@KodrAus Is this still being considered?

[agriffis]

@BurntSushi @mckaymatt This looks like it's nearly done but stalled. I would love to see this in ripgrep, since otherwise I often fall back on find | grep. Any chance one of you will find a chance to finish this up?

[mckaymatt]

I still need to fix the error handling so don't review yet.

[agriffis]

@BurntSushi Thanks to @mckaymatt I think this has your requested changes now, and it builds successfully on Windows. Do you mind taking another look at merging this?

[agriffis]

Actually it looks like some of the changes from code review didn't survive between my commits and Matt's, I'm not sure how. I'll take another swipe at it.

[agriffis]

@BurntSushi @mckaymatt I opened #107 with the additional changes requested by Andrew. I don't care if you merge that one or pull the additional commit into this PR... I'd just like to see this finally land in ripgrep!

[mckaymatt]

I will close this since https://github.com/BurntSushi/walkdir/pull/107 is further ahead.