Fix npm not working because of wrong file copying (#284)

Cargo.lock

@@ -347,16 +347,6 @@ dependencies = [
  "generic-array",
 ]
 
-[[package]]
-name = "dircpy"
-version = "0.3.4"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "f4479788c6c76674c1551ef44c953101554e7edadb5ea0920797a2280d87eb3d"
-dependencies = [
- "log",
- "walkdir",
-]
-
 [[package]]
 name = "dirs"
 version = "3.0.1"
@@ -487,7 +477,6 @@ dependencies = [
  "clap",
  "colored",
  "csv",
- "dircpy",
  "dirs",
  "duct",
  "embed-resource",
@@ -1414,15 +1403,6 @@ version = "1.0.5"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "71d301d4193d031abdd79ff7e3dd721168a9572ef3fe51a1517aba235bd8f86e"
 
-[[package]]
-name = "same-file"
-version = "1.0.6"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "93fc1dc3aaa9bfed95e02e6eadabb4baf7e3078b0bd1b4d7b6b0b68378900502"
-dependencies = [
- "winapi-util",
-]
-
 [[package]]
 name = "schannel"
 version = "0.1.19"
@@ -1980,17 +1960,6 @@ dependencies = [
  "libc",
 ]
 
-[[package]]
-name = "walkdir"
-version = "2.3.1"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "777182bc735b6424e1a57516d35ed72cb8019d85c8c9bf536dccb3445c1a2f7d"
-dependencies = [
- "same-file",
- "winapi 0.3.9",
- "winapi-util",
-]
-
 [[package]]
 name = "want"
 version = "0.3.0"

Cargo.toml

@@ -25,7 +25,6 @@ log = "0.4.11"
 env_logger = "0.8.1"
 atty = "0.2.14"
 encoding_rs_io = "0.1.7"
-dircpy = "0.3.4"
 
 [dev-dependencies]
 pretty_assertions = "0.6.1"

src/directory_portal.rs

@@ -1,17 +1,23 @@
-use dircpy::copy_dir;
 use log::*;
 use std::path::Path;
-use tempfile::{tempdir, TempDir};
+use tempfile::TempDir;
 
+/// A "work-in-progress" directory, which will "teleport" into the path
+/// given in `target` only on successful, guarding from invalid state in the file system.
+///
+/// Underneath, it uses `fs::rename`, so make sure to make the `temp_dir` inside the same
+/// mount as `target`. This is why we have the `new_in` constructor.
 pub struct DirectoryPortal<P: AsRef<Path>> {
     temp_dir: TempDir,
     target: P,
 }
 
 impl<P: AsRef<Path>> DirectoryPortal<P> {
+    /// Create a new portal which will keep the temp files in
+    /// a subdirectory of `parent_dir` until teleporting to `target`.
     #[must_use]
-    pub fn new(target: P) -> Self {
-        let temp_dir = tempdir().expect("Can't generate a temp directory");
+    pub fn new_in(parent_dir: impl AsRef<Path>, target: P) -> Self {
+        let temp_dir = TempDir::new_in(parent_dir).expect("Can't generate a temp directory");
         debug!("Created a temp directory in {:?}", temp_dir.path());
         Self { target, temp_dir }
     }
@@ -22,8 +28,7 @@ impl<P: AsRef<Path>> DirectoryPortal<P> {
             self.temp_dir.path(),
             self.target.as_ref()
         );
-        copy_dir(&self.temp_dir, &self.target)?;
-        std::fs::remove_dir_all(&self.temp_dir)?;
+        std::fs::rename(&self.temp_dir, &self.target)?;
         Ok(self.target)
     }
 }
@@ -45,11 +50,12 @@ impl<P: AsRef<Path>> AsRef<Path> for DirectoryPortal<P> {
 mod tests {
     use super::*;
     use pretty_assertions::assert_eq;
+    use tempfile::tempdir;
 
-    #[test]
+    #[test_env_log::test]
     fn test_portal() {
         let tempdir = tempdir().expect("Can't generate a temp directory");
-        let portal = DirectoryPortal::new(tempdir.path().join("subdir"));
+        let portal = DirectoryPortal::new_in(std::env::temp_dir(), tempdir.path().join("subdir"));
         let new_file_path = portal.to_path_buf().join("README.md");
         std::fs::write(&new_file_path, "Hello world!").expect("Can't write file");
         let target = portal.teleport().expect("Can't close directory portal");

src/downloader.rs

@@ -88,7 +88,11 @@ pub fn install_node_dist<P: AsRef<Path>>(
     );
 
     std::fs::create_dir_all(installations_dir.as_ref()).context(IoError)?;
-    let portal = DirectoryPortal::new(installation_dir);
+
+    let temp_installations_dir = installations_dir.as_ref().join(".downloads");
+    std::fs::create_dir_all(&temp_installations_dir).context(IoError)?;
+
+    let portal = DirectoryPortal::new_in(&temp_installations_dir, installation_dir);
 
     let url = download_url(node_dist_mirror, version);
     debug!("Going to call for {}", &url);
@@ -123,37 +127,55 @@ mod tests {
     use crate::downloader::install_node_dist;
     use crate::version::Version;
     use pretty_assertions::assert_eq;
-    use std::io::Read;
     use tempfile::tempdir;
 
     #[test_env_log::test]
     fn test_installing_node_12() {
+        let installations_dir = tempdir().unwrap();
+        let node_path = install_in(installations_dir.path()).join("node");
+
+        let stdout = duct::cmd(node_path.to_str().unwrap(), vec!["--version"])
+            .stdout_capture()
+            .run()
+            .expect("Can't run Node binary")
+            .stdout;
+
+        let result = String::from_utf8(stdout).expect("Can't read `node --version` output");
+
+        assert_eq!(result.trim(), "v12.0.0");
+    }
+
+    #[test_env_log::test]
+    fn test_installing_npm() {
+        let installations_dir = tempdir().unwrap();
+        let npm_path = install_in(installations_dir.path()).join(if cfg!(windows) {
+            "npm.cmd"
+        } else {
+            "npm"
+        });
+
+        let stdout = duct::cmd(npm_path.to_str().unwrap(), vec!["--version"])
+            .stdout_capture()
+            .run()
+            .expect("Can't run npm")
+            .stdout;
+
+        let result = String::from_utf8(stdout).expect("Can't read npm output");
+
+        assert_eq!(result.trim(), "6.9.0");
+    }
+
+    fn install_in(path: &Path) -> PathBuf {
         let version = Version::parse("12.0.0").unwrap();
         let node_dist_mirror = Url::parse("https://nodejs.org/dist/").unwrap();
-        let installations_dir = tempdir().unwrap();
-        install_node_dist(&version, &node_dist_mirror, &installations_dir)
-            .expect("Can't install Node 12");
+        install_node_dist(&version, &node_dist_mirror, &path).expect("Can't install Node 12");
 
-        let mut location_path = PathBuf::from(&installations_dir.path());
-        location_path.push(version.v_str());
-        location_path.push("installation");
+        let mut location_path = path.join(version.v_str()).join("installation");
 
         if cfg!(unix) {
             location_path.push("bin");
         }
 
-        location_path.push("node");
-
-        let mut result = String::new();
-        std::process::Command::new(location_path.to_str().unwrap())
-            .arg("--version")
-            .stdout(std::process::Stdio::piped())
-            .spawn()
-            .expect("Can't find node executable")
-            .stdout
-            .expect("Can't capture stdout")
-            .read_to_string(&mut result)
-            .expect("Failed reading stdout");
-        assert_eq!(result.trim(), "v12.0.0");
+        location_path
     }
 }