Support Windows as target OS

[fawick]

I was trying to build diskus with on Windows 7 (Rust 1.37.0, stable-x86_64-pc-windows-msvc toolchain). This failed, as the trait st::os::unix::fs::MetadataExt is used in src/walk.rs.

Here is the full error message of the failing build from my machine.

$ cargo build
   Compiling diskus v0.5.0 (C:\Users\fabian\rust\diskus)
error[E0433]: failed to resolve: could not find `unix` in `os`
 --> src\walk.rs:3:14
  |
3 | use std::os::unix::fs::MetadataExt;
  |              ^^^^ could not find `unix` in `os`

warning: unused import: `std::os::unix::fs::MetadataExt`
 --> src\walk.rs:3:5
  |
3 | use std::os::unix::fs::MetadataExt;
  |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  |
  = note: #[warn(unused_imports)] on by default

error[E0599]: no method named `nlink` found for type `std::fs::Metadata` in the current scope
  --> src\walk.rs:27:63
   |
27 |             let unique_id = if metadata.is_file() && metadata.nlink() > 1 {
   |                                                               ^^^^^

error[E0599]: no method named `dev` found for type `std::fs::Metadata` in the current scope
  --> src\walk.rs:28:40
   |
28 |                 Some(UniqueID(metadata.dev(), metadata.ino()))
   |                                        ^^^

error[E0599]: no method named `ino` found for type `std::fs::Metadata` in the current scope
  --> src\walk.rs:28:56
   |
28 |                 Some(UniqueID(metadata.dev(), metadata.ino()))
   |                                                        ^^^

error: aborting due to 4 previous errors

Some errors have detailed explanations: E0433, E0599.
For more information about an error, try `rustc --explain E0433`.
error: Could not compile `diskus`.

To learn more, run the command again with --verbose.  

I dug into the source and came up with a patch that allowed me a compile and run the tool. Basically, I refactored the generation of UniqueID into a function and made a explicit distinction between unix and windows when implementing that function.

Here is the diff of the patch.

$ git diff  src
diff --git a/src/walk.rs b/src/walk.rs
index 0125713..580a6e0 100644
--- a/src/walk.rs
+++ b/src/walk.rs
@@ -1,9 +1,11 @@
 use std::collections::HashSet;
 use std::fs;
-use std::os::unix::fs::MetadataExt;
 use std::path::PathBuf;
 use std::thread;

+#[cfg(target_os = "unix")]
+use std::os::unix::fs::MetadataExt;
+
 use crossbeam_channel as channel;

 use rayon;
@@ -18,17 +20,27 @@ enum Message {
     CouldNotReadDir(PathBuf),
 }

+#[cfg(target_os = "unix")]
+fn generate_unique_id(metadata: &std::fs::Metadata) -> Option<UniqueID> {
+    // If the entry has more than one hard link, generate
+    // a unique ID consisting of device and inode in order
+    // not to count this entry twice.
+    if metadata.is_file() && metadata.nlink() > 1 {
+        Some(UniqueID(metadata.dev(), metadata.ino()))
+    } else {
+        None
+    }
+}
+
+#[cfg(target_os = "windows")]
+fn generate_unique_id(_metadata: &std::fs::Metadata) -> Option<UniqueID> {
+    None
+}
+
 fn walk(tx: channel::Sender<Message>, entries: &[PathBuf]) {
     entries.into_par_iter().for_each_with(tx, |tx_ref, entry| {
         if let Ok(metadata) = entry.symlink_metadata() {
-            // If the entry has more than one hard link, generate
-            // a unique ID consisting of device and inode in order
-            // not to count this entry twice.
-            let unique_id = if metadata.is_file() && metadata.nlink() > 1 {
-                Some(UniqueID(metadata.dev(), metadata.ino()))
-            } else {
-                None
-            };
+            let unique_id = generate_unique_id(&metadata);

             let size = metadata.len();

While it serves my immediate usecase, the Windows implementation is incorrect/incomplete, as there is no handling of NTFS Hard Links and Junctions. A quick search with cargo revealed https://crates.io/crates/junction which would probably be a suitable candidate for coming up with a proper implementation.

The issue I'd like to raise is whether diskus should have Windows support. If so, is the approach via [cfg(target_os = "...")] acceptable for diskus. I found a discussion in the Rust forum that contains links to similar branching in the libc crate source.

[sharkdp]

Thank you for your feedback.

The issue I'd like to raise is whether diskus should have Windows support

Yes, ideally I'd like to support Windows, if there isn't too much OS-specific code that needs to be added.

While it serves my immediate usecase, the Windows implementation is incorrect/incomplete, as there is no handling of NTFS Hard Links and Junctions.

Yes, we should make sure that links can be correctly before adding Windows support.

If so, is the approach via [cfg(target_os = "...")] acceptable for diskus.

Yes. Adding things like #[cfg(windows)] or #[cfg(unix)] is pretty common. Ideally though, OS-specific code should be moved to separate functions and/or modules.

[fawick]

Thanks for the feedback! Based on that, I am going to look closer into the junction crate mentioned above and see whether I can come with a working proof of concept. It's going to take a while, I can only spare about two hours a month for FOSS.

[fawick]

Windows sure is strange.

While there is the mklink command to create hardlinks, symlinks and junction points, even Windows'
own tools dir and Powershell do not handle it correctly. Here is an example of file of 10^6 bytes (orig.txt), a hardlink to it (hl.txt), as well as a symlink and a junction point to the containing directory. All Windows tools return four times the size of orig.txt.

C:\Users\fabian\rust\diskus>mkdir foo

C:\Users\fabian\rust\diskus>mkdir foo\orig

C:\Users\fabian\rust\diskus>fsutil file createnew foo/orig/orig.txt 1000000
File C:\Users\fabian\rust\diskus\foo\orig\orig.txt is created

C:\Users\fabian\rust\diskus>mklink /h foo\orig\hl.txt foo\orig\orig.txt
Hardlink created for foo\orig\hl.txt <<===>> foo\orig\orig.txt

C:\Users\fabian\rust\diskus>mklink /J foo\JP foo\orig
Junction created for foo\JP <<===>> foo\orig

C:\Users\fabian\rust\diskus>mklink foo\softlink foo\orig
symbolic link created for foo\softlink <<===>> foo\orig

C:\Users\fabian\rust\diskus>dir /s /q foo
 Volume in drive C is Windows
 Volume Serial Number is 10F9-E64D

 Directory of C:\Users\fabian\rust\diskus\foo

09/18/2019  08:43 PM    <DIR>          GORRAMBOX\fabian       .
09/18/2019  08:43 PM    <DIR>          GORRAMBOX\fabian       ..
09/18/2019  08:43 PM    <JUNCTION>     GORRAMBOX\fabian       JP [C:\Users\fabian\rust\diskus\foo\orig]
09/18/2019  08:36 PM    <DIR>          GORRAMBOX\fabian       orig
09/18/2019  08:43 PM    <SYMLINK>      GORRAMBOX\fabian       softlink [foo\orig]
               1 File(s)              0 bytes

 Directory of C:\Users\fabian\rust\diskus\foo\JP

09/18/2019  08:36 PM    <DIR>          GORRAMBOX\fabian       .
09/18/2019  08:36 PM    <DIR>          GORRAMBOX\fabian       ..
09/18/2019  08:36 PM         1,000,000 GORRAMBOX\fabian       hl.txt
09/18/2019  08:36 PM         1,000,000 GORRAMBOX\fabian       orig.txt
               2 File(s)      2,000,000 bytes

 Directory of C:\Users\fabian\rust\diskus\foo\orig

09/18/2019  08:36 PM    <DIR>          GORRAMBOX\fabian       .
09/18/2019  08:36 PM    <DIR>          GORRAMBOX\fabian       ..
09/18/2019  08:36 PM         1,000,000 GORRAMBOX\fabian       hl.txt
09/18/2019  08:36 PM         1,000,000 GORRAMBOX\fabian       orig.txt
               2 File(s)      2,000,000 bytes

     Total Files Listed:
               5 File(s)      4,000,000 bytes
               8 Dir(s)  55,115,661,312 bytes free

And in Powershell

PS C:\Users\fabian\rust\diskus\foo> $totalsize = [long]0
PS C:\Users\fabian\rust\diskus\foo> Get-ChildItem -File -Recurse -Force -ErrorAction SilentlyContinue | % {$totalsize += $_.Length}
PS C:\Users\fabian\rust\diskus\foo> $totalsize
4000000

Even Windows Explorer cannot get it right

[sharkdp]

It seems to me that links are not very common in Windows. Could we just choose the same route and ignore their existence (+ document it)?

[fawick]

Yes, I agree, that's most likely the best route. At least then diskus will have parity with the official tools.

In that case my patch from the first message already has everything on board and I can create a PR from that. Would you happen to have a chance to test a build on MacOS? I only have access to Win (7/10) and Linux.

[sharkdp]

In that case my patch from the first message already has everything on board and I can create a PR from that.

Sounds great!

Would you happen to have a chance to test a build on MacOS?

I don't. But every supported OS should be part of the CI toolchain (see .travis.yml file). We already perform OSX builds. You can take a look at the .travis.yml file in some of my other projects to see how Windows testing can be enabled: https://github.com/sharkdp/pastel/blob/df58d834b89e62cf1071eb2fed3a9564b906fc25/.travis.yml#L15-L17

Unfortunately, I never had the time to write proper tests for diskus, so Travis CI really just tests if everything builds correctly.

[sharkdp]

(A proper way to write integration tests for command-line programs in Rust can be found here: https://github.com/sharkdp/pastel/blob/master/tests/integration_tests.rs)

[sharkdp]

Released in v0.6.0.