Support crater runs on Windows

.appveyor.yml

@@ -18,12 +18,15 @@ install:
   - rustc -V
   - cargo -V
 
-build: false
-
 branches:
   only:
     - auto
     - try
+    - master
 
-test_script:
+build_script:
   - cargo build
+
+test_script:
+  - cargo run -- create-lists
+  - cargo test

.gitattributes

@@ -5,3 +5,4 @@
 *.md text eol=lf
 *.gitignore text eol=lf
 *.gitattributes text eol=lf
+*.json text eol=lf

docs/agent-machine-setup-windows.md

@@ -119,20 +119,15 @@ cargo build --release
 cargo run --release -- prepare-local
 ```
 
-Running `minicrater` tests require that the `*.expected.json` files match the
-output of crater exactly. However, when installed via `chocolately`, `git` has
-config `core.autocrlf` set to `true` by default. This will override the
-preferences in `crater`'s `.gitattributes` file and cause the JSON files to
-have the wrong line endings (CRLF). Make sure you set `core.autocrlf = false`
-before cloning the repo.
+Remember to run `cargo run -- create-lists` before running the tests.
 
-If you've already cloned the repo, simply run the following from inside of it
-to replace the JSON files:
+# Troubleshooting
 
-```powershell
-git config core.autocrlf false
-rm -r tests/
-git checkout -- tests/
-```
+## Overlong paths
 
-Remember to run `cargo run -- create-lists` before running the tests.
+If you encounter warnings about paths being too long, you should disable the
+260 character limit. This option is called "Enable Win32 long paths" in the
+policy editor. See this [Stack Overflow question][so-long-path] for more
+details.
+
+[so-long-path]: https://superuser.com/questions/1119883/windows-10-enable-ntfs-long-paths-policy-option-missing

src/cli.rs

@@ -26,6 +26,10 @@ use std::path::PathBuf;
 use std::str::FromStr;
 use structopt::clap::AppSettings;
 
+#[cfg(windows)]
+static DEFAULT_DOCKER_ENV: &str = "rustops/crates-build-env-windows";
+
+#[cfg(not(windows))]
 static DEFAULT_DOCKER_ENV: &str = "rustops/crates-build-env";
 
 // An experiment name

src/crates/mod.rs

@@ -4,7 +4,7 @@ mod sources;
 use crate::dirs::LOCAL_CRATES_DIR;
 use crate::prelude::*;
 use std::fmt;
-use std::path::Path;
+use std::path::{Path, PathBuf};
 use std::str::FromStr;
 
 pub(crate) use crate::crates::sources::github::GitHubRepo;
@@ -26,6 +26,16 @@ impl Crate {
         }
     }
 
+    pub(crate) fn to_path(&self) -> PathBuf {
+        let components: Vec<&str> = match *self {
+            Crate::Registry(ref details) => vec!["reg", &details.name, &details.version],
+            Crate::GitHub(ref repo) => vec!["gh", &repo.org, &repo.name],
+            Crate::Local(ref name) => vec!["local", &name],
+        };
+
+        components.into_iter().collect()
+    }
+
     pub(crate) fn fetch(&self) -> Fallible<()> {
         match *self {
             Crate::Registry(ref krate) => krate.fetch(),

src/crates/sources/registry.rs

@@ -20,6 +20,7 @@ impl List for RegistryList {
         let mut list = Vec::new();
         let mut counts = HashMap::new();
 
+        fs::create_dir_all(&*LOCAL_DIR)?;
         let index = Index::new(LOCAL_DIR.join("crates.io-index"));
         index.retrieve_or_update().to_failure()?;
 

src/dirs.rs

@@ -44,5 +44,29 @@ pub(crate) fn crate_source_dir(ex: &Experiment, tc: &Toolchain, krate: &Crate) -
         .join(&ex.name)
         .join("sources")
         .join(tc.to_path_component())
-        .join(krate.id())
+        .join(krate.to_path())
+}
+
+pub mod container {
+    use std::path::{Path, PathBuf};
+
+    use lazy_static::lazy_static;
+
+    #[cfg(windows)]
+    lazy_static! {
+        pub static ref ROOT_DIR: PathBuf = Path::new(r"C:\crater").into();
+    }
+
+    #[cfg(not(windows))]
+    lazy_static! {
+        pub static ref ROOT_DIR: PathBuf = Path::new("/opt/crater").into();
+    }
+
+    lazy_static! {
+        pub static ref WORK_DIR: PathBuf = ROOT_DIR.join("workdir");
+        pub static ref TARGET_DIR: PathBuf = ROOT_DIR.join("target");
+        pub static ref CARGO_HOME: PathBuf = ROOT_DIR.join("cargo-home");
+        pub static ref RUSTUP_HOME: PathBuf = ROOT_DIR.join("rustup-home");
+        pub static ref CARGO_BIN_DIR: PathBuf = CARGO_HOME.join("bin");
+    }
 }

src/docker.rs

@@ -53,7 +53,7 @@ impl DockerEnv {
     }
 }
 
-#[derive(Copy, Clone)]
+#[derive(Copy, Clone, PartialEq, Eq)]
 pub(crate) enum MountPerms {
     ReadWrite,
     ReadOnly,
@@ -66,7 +66,7 @@ struct MountConfig {
 }
 
 impl MountConfig {
-    fn to_arg(&self) -> String {
+    fn to_volume_arg(&self) -> String {
         let perm = match self.perm {
             MountPerms::ReadWrite => "rw",
             MountPerms::ReadOnly => "ro",
@@ -78,6 +78,21 @@ impl MountConfig {
             perm
         )
     }
+
+    fn to_mount_arg(&self) -> String {
+        let mut opts_with_leading_comma = vec![];
+
+        if self.perm == MountPerms::ReadOnly {
+            opts_with_leading_comma.push(",readonly");
+        }
+
+        format!(
+            "type=bind,src={},dst={}{}",
+            absolute(&self.host_path).to_string_lossy(),
+            self.container_path.to_string_lossy(),
+            opts_with_leading_comma.join(""),
+        )
+    }
 }
 
 pub(crate) struct ContainerBuilder<'a> {
@@ -147,8 +162,16 @@ impl<'a> ContainerBuilder<'a> {
 
         for mount in &self.mounts {
             fs::create_dir_all(&mount.host_path)?;
-            args.push("-v".into());
-            args.push(mount.to_arg())
+
+            // On Windows, we mount paths containing a colon which don't work with `-v`, but on
+            // Linux we need the Z flag, which doesn't work with `--mount`, for SELinux relabeling.
+            if cfg!(windows) {
+                args.push("--mount".into());
+                args.push(mount.to_mount_arg())
+            } else {
+                args.push("-v".into());
+                args.push(mount.to_volume_arg())
+            }
         }
 
         for &(ref var, ref value) in &self.env {
@@ -171,6 +194,10 @@ impl<'a> ContainerBuilder<'a> {
             args.push("none".into());
         }
 
+        if cfg!(windows) {
+            args.push("--isolation=process".into());
+        }
+
         args.push(self.image.image.clone());
 
         for arg in self.cmd {

src/native/unix.rs

@@ -14,8 +14,8 @@ pub(crate) fn kill_process(id: u32) -> Fallible<()> {
     Ok(())
 }
 
-pub(crate) fn current_user() -> u32 {
-    Uid::effective().into()
+pub(crate) fn current_user() -> Option<u32> {
+    Some(Uid::effective().into())
 }
 
 fn current_group() -> u32 {
@@ -25,7 +25,7 @@ fn current_group() -> u32 {
 fn executable_mode_for(path: &Path) -> Fallible<u32> {
     let metadata = path.metadata()?;
 
-    if metadata.uid() == current_user() {
+    if metadata.uid() == current_user().unwrap() {
         Ok(EXECUTABLE_BITS << 6)
     } else if metadata.gid() == current_group() {
         Ok(EXECUTABLE_BITS << 3)
@@ -74,7 +74,7 @@ mod tests {
 
     #[test]
     fn test_current_user() {
-        assert_eq!(current_user(), u32::from(Uid::effective()));
+        assert_eq!(current_user(), Some(u32::from(Uid::effective())));
     }
 
     #[test]

src/native/windows.rs

@@ -1,4 +1,5 @@
 use crate::prelude::*;
+use std::fs::File;
 use std::path::Path;
 use winapi::um::handleapi::CloseHandle;
 use winapi::um::processthreadsapi::{OpenProcess, TerminateProcess};
@@ -7,6 +8,9 @@ use winapi::um::winnt::PROCESS_TERMINATE;
 pub(crate) fn kill_process(id: u32) -> Fallible<()> {
     unsafe {
         let handle = OpenProcess(PROCESS_TERMINATE, 0, id);
+        if handle.is_null() {
+            bail!("OpenProcess for process {} failed", id);
+        }
         if TerminateProcess(handle, 101) == 0 {
             bail!("TerminateProcess for process {} failed", id);
         }
@@ -18,14 +22,45 @@ pub(crate) fn kill_process(id: u32) -> Fallible<()> {
     Ok(())
 }
 
-pub(crate) fn current_user() -> u32 {
-    unimplemented!();
+pub(crate) fn current_user() -> Option<u32> {
+    None
+}
+
+fn path_ends_in_exe<P: AsRef<Path>>(path: P) -> Fallible<bool> {
+    path.as_ref()
+        .extension()
+        .ok_or_else(|| failure::format_err!("Unable to get `Path` extension"))
+        .map(|ext| ext == "exe")
 }
 
-pub(crate) fn is_executable<P: AsRef<Path>>(_path: P) -> Fallible<bool> {
-    unimplemented!();
+/// Check that the file exists and has `.exe` as its extension.
+pub(crate) fn is_executable<P: AsRef<Path>>(path: P) -> Fallible<bool> {
+    let path = path.as_ref();
+    File::open(path)
+        .map_err(Into::into)
+        .and_then(|_| path_ends_in_exe(path))
 }
 
-pub(crate) fn make_executable<P: AsRef<Path>>(_path: P) -> Fallible<()> {
-    unimplemented!();
+pub(crate) fn make_executable<P: AsRef<Path>>(path: P) -> Fallible<()> {
+    if is_executable(path)? {
+        Ok(())
+    } else {
+        failure::bail!("Downloaded binaries should be executable by default");
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use std::process::Command;
+
+    #[test]
+    fn test_kill_process() {
+        // Try to kill a sleep command
+        let mut cmd = Command::new("timeout").args(&["2"]).spawn().unwrap();
+        kill_process(cmd.id()).unwrap();
+
+        // Ensure it returns the code passed to `TerminateProcess`
+        assert_eq!(cmd.wait().unwrap().code(), Some(101));
+    }
 }

src/report/archives.rs

@@ -51,7 +51,12 @@ pub fn write_logs_archives<DB: ReadResults, W: ReportWriter>(
             let log_bytes = log_bytes.to_plain()?;
             let log_bytes = log_bytes.as_slice();
 
-            let path = format!("{}/{}/{}.txt", comparison, krate.id(), tc);
+            let path = format!(
+                "{}/{}/{}.txt",
+                comparison,
+                krate.id(),
+                tc.to_path_component(),
+            );
 
             let mut header = TarHeader::new_gnu();
             header.set_size(log_bytes.len() as u64);