Add --force flag to cargo install

src/bin/install.rs

@@ -15,6 +15,7 @@ pub struct Options {
     flag_color: Option<String>,
     flag_root: Option<String>,
     flag_list: bool,
+    flag_force: bool,
 
     arg_crate: Option<String>,
     flag_vers: Option<String>,
@@ -46,6 +47,7 @@ Build and install options:
     -h, --help                Print this message
     -j N, --jobs N            The number of jobs to run in parallel
     --features FEATURES       Space-separated list of features to activate
+    -f, --force               Force overwriting existing crates or binaries
     --no-default-features     Do not build the `default` feature
     --debug                   Build in debug mode instead of release mode
     --bin NAME                Only install the binary NAME
@@ -55,7 +57,7 @@ Build and install options:
     -q, --quiet               Less output printed to stdout
     --color WHEN              Coloring: auto, always, never
 
-This command manages Cargo's local set of install binary crates. Only packages
+This command manages Cargo's local set of installed binary crates. Only packages
 which have [[bin]] targets can be installed, and all binaries are installed into
 the installation root's `bin` folder. The installation root is determined, in
 order of precedence, by `--root`, `$CARGO_INSTALL_ROOT`, the `install.root`
@@ -75,6 +77,10 @@ crate has multiple binaries, the `--bin` argument can selectively install only
 one of them, and if you'd rather install examples the `--example` argument can
 be used as well.
 
+By default cargo will refuse to overwrite existing binaries. The `--force` flag
+enables overwriting existing binaries. Thus you can reinstall a crate with
+`cargo install --force <crate>`.
+
 As a special convenience, omitting the <crate> specification entirely will
 install the crate in the current directory. That is, `install` is equivalent to
 the more explicit `install --path .`.
@@ -130,7 +136,7 @@ pub fn execute(options: Options, config: &Config) -> CliResult<Option<()>> {
     if options.flag_list {
         try!(ops::install_list(root, config));
     } else {
-        try!(ops::install(root, krate, &source, vers, &compile_opts));
+        try!(ops::install(root, krate, &source, vers, &compile_opts, options.flag_force));
     }
     Ok(None)
 }

src/cargo/ops/cargo_install.rs

@@ -32,6 +32,12 @@ struct Transaction {
     bins: Vec<PathBuf>,
 }
 
+impl Transaction {
+    fn success(mut self) {
+        self.bins.clear();
+    }
+}
+
 impl Drop for Transaction {
     fn drop(&mut self) {
         for bin in self.bins.iter() {
@@ -44,7 +50,8 @@ pub fn install(root: Option<&str>,
                krate: Option<&str>,
                source_id: &SourceId,
                vers: Option<&str>,
-               opts: &ops::CompileOptions) -> CargoResult<()> {
+               opts: &ops::CompileOptions,
+               force: bool) -> CargoResult<()> {
     let config = opts.config;
     let root = try!(resolve_root(root, config));
     let (pkg, source) = if source_id.is_git() {
@@ -77,7 +84,7 @@ pub fn install(root: Option<&str>,
         let metadata = try!(metadata(config, &root));
         let list = try!(read_crate_list(metadata.file()));
         let dst = metadata.parent().join("bin");
-        try!(check_overwrites(&dst, &pkg, &opts.filter, &list));
+        try!(check_overwrites(&dst, &pkg, &opts.filter, &list, force));
     }
 
     let mut td_opt = None;
@@ -102,40 +109,122 @@ pub fn install(root: Option<&str>,
         human(format!("failed to compile `{}`, intermediate artifacts can be \
                        found at `{}`", pkg, target_dir.display()))
     }));
+    let binaries: Vec<(&str, &Path)> = try!(compile.binaries.iter().map(|bin| {
+        let name = bin.file_name().unwrap();
+        if let Some(s) = name.to_str() {
+            Ok((s, bin.as_ref()))
+        } else {
+            bail!("Binary `{:?}` name can't be serialized into string", name)
+        }
+    }).collect::<CargoResult<_>>());
 
     let metadata = try!(metadata(config, &root));
     let mut list = try!(read_crate_list(metadata.file()));
     let dst = metadata.parent().join("bin");
-    try!(check_overwrites(&dst, &pkg, &opts.filter, &list));
+    let duplicates = try!(check_overwrites(&dst, &pkg, &opts.filter, &list, force));
 
-    let mut t = Transaction { bins: Vec::new() };
     try!(fs::create_dir_all(&dst));
-    for bin in compile.binaries.iter() {
-        let dst = dst.join(bin.file_name().unwrap());
+
+    // Copy all binaries to a temporary directory under `dst` first, catching
+    // some failure modes (e.g. out of space) before touching the existing
+    // binaries. This directory will get cleaned up via RAII.
+    let staging_dir = try!(TempDir::new_in(&dst, "cargo-install"));
+    for &(bin, src) in binaries.iter() {
+        let dst = staging_dir.path().join(bin);
+        // Try to move if `target_dir` is transient.
+        if !source_id.is_path() {
+            if fs::rename(src, &dst).is_ok() {
+                continue
+            }
+        }
+        try!(fs::copy(src, &dst).chain_error(|| {
+            human(format!("failed to copy `{}` to `{}`", src.display(),
+                          dst.display()))
+        }));
+    }
+
+    let (to_replace, to_install): (Vec<&str>, Vec<&str>) =
+        binaries.iter().map(|&(bin, _)| bin)
+                       .partition(|&bin| duplicates.contains_key(bin));
+
+    let mut installed = Transaction { bins: Vec::new() };
+
+    // Move the temporary copies into `dst` starting with new binaries.
+    for bin in to_install.iter() {
+        let src = staging_dir.path().join(bin);
+        let dst = dst.join(bin);
         try!(config.shell().status("Installing", dst.display()));
-        try!(fs::copy(&bin, &dst).chain_error(|| {
-            human(format!("failed to copy `{}` to `{}`", bin.display(),
+        try!(fs::rename(&src, &dst).chain_error(|| {
+            human(format!("failed to move `{}` to `{}`", src.display(),
                           dst.display()))
         }));
-        t.bins.push(dst);
+        installed.bins.push(dst);
+    }
+
+    // Repeat for binaries which replace existing ones but don't pop the error
+    // up until after updating metadata.
+    let mut replaced_names = Vec::new();
+    let result = {
+        let mut try_install = || -> CargoResult<()> {
+            for &bin in to_replace.iter() {
+                let src = staging_dir.path().join(bin);
+                let dst = dst.join(bin);
+                try!(config.shell().status("Replacing", dst.display()));
+                try!(fs::rename(&src, &dst).chain_error(|| {
+                    human(format!("failed to move `{}` to `{}`", src.display(),
+                                  dst.display()))
+                }));
+                replaced_names.push(bin);
+            }
+            Ok(())
+        };
+        try_install()
+    };
+
+    // Update records of replaced binaries.
+    for &bin in replaced_names.iter() {
+        if let Some(&Some(ref p)) = duplicates.get(bin) {
+            if let Some(set) = list.v1.get_mut(p) {
+                set.remove(bin);
+            }
+        }
+        list.v1.entry(pkg.package_id().clone())
+               .or_insert_with(|| BTreeSet::new())
+               .insert(bin.to_string());
+    }
+
+    // Remove empty metadata lines.
+    let pkgs = list.v1.iter()
+                      .filter_map(|(p, set)| if set.is_empty() { Some(p.clone()) } else { None })
+                      .collect::<Vec<_>>();
+    for p in pkgs.iter() {
+        list.v1.remove(p);
     }
 
+    // If installation was successful record newly installed binaries.
+    if result.is_ok() {
+        list.v1.entry(pkg.package_id().clone())
+               .or_insert_with(|| BTreeSet::new())
+               .extend(to_install.iter().map(|s| s.to_string()));
+    }
+
+    let write_result = write_crate_list(metadata.file(), list);
+    match write_result {
+        // Replacement error (if any) isn't actually caused by write error
+        // but this seems to be the only way to show both.
+        Err(err) => try!(result.chain_error(|| err)),
+        Ok(_) => try!(result),
+    }
+
+    // Reaching here means all actions have succeeded. Clean up.
+    installed.success();
     if !source_id.is_path() {
         // Don't bother grabbing a lock as we're going to blow it all away
         // anyway.
         let target_dir = target_dir.into_path_unlocked();
         try!(fs::remove_dir_all(&target_dir));
     }
 
-    list.v1.entry(pkg.package_id().clone()).or_insert_with(|| {
-        BTreeSet::new()
-    }).extend(t.bins.iter().map(|t| {
-        t.file_name().unwrap().to_string_lossy().into_owned()
-    }));
-    try!(write_crate_list(metadata.file(), list));
-
-    t.bins.truncate(0);
-
     // Print a warning that if this directory isn't in PATH that they won't be
     // able to run these commands.
     let path = env::var_os("PATH").unwrap_or(OsString::new());
@@ -225,38 +314,61 @@ fn one<I, F>(mut i: I, f: F) -> CargoResult<Option<I::Item>>
 fn check_overwrites(dst: &Path,
                     pkg: &Package,
                     filter: &ops::CompileFilter,
-                    prev: &CrateListingV1) -> CargoResult<()> {
+                    prev: &CrateListingV1,
+                    force: bool) -> CargoResult<BTreeMap<String, Option<PackageId>>> {
+    if let CompileFilter::Everything = *filter {
+        // If explicit --bin or --example flags were passed then those'll
+        // get checked during cargo_compile, we only care about the "build
+        // everything" case here
+        if pkg.targets().iter().filter(|t| t.is_bin()).next().is_none() {
+            bail!("specified package has no binaries")
+        }
+    }
+    let duplicates = find_duplicates(dst, pkg, filter, prev);
+    if force || duplicates.is_empty() {
+        return Ok(duplicates)
+    }
+    // Format the error message.
+    let mut msg = String::new();
+    for (ref bin, p) in duplicates.iter() {
+        msg.push_str(&format!("binary `{}` already exists in destination", bin));
+        if let Some(p) = p.as_ref() {
+            msg.push_str(&format!(" as part of `{}`\n", p));
+        } else {
+            msg.push_str("\n");
+        }
+    }
+    msg.push_str("Add --force to overwrite");
+    Err(human(msg))
+}
+
+fn find_duplicates(dst: &Path,
+                   pkg: &Package,
+                   filter: &ops::CompileFilter,
+                   prev: &CrateListingV1) -> BTreeMap<String, Option<PackageId>> {
     let check = |name| {
         let name = format!("{}{}", name, env::consts::EXE_SUFFIX);
         if fs::metadata(dst.join(&name)).is_err() {
-            return Ok(())
-        }
-        let mut msg = format!("binary `{}` already exists in destination", name);
-        if let Some((p, _)) = prev.v1.iter().find(|&(_, v)| v.contains(&name)) {
-            msg.push_str(&format!(" as part of `{}`", p));
+            None
+        } else if let Some((p, _)) = prev.v1.iter().find(|&(_, v)| v.contains(&name)) {
+            Some((name, Some(p.clone())))
+        } else {
+            Some((name, None))
         }
-        Err(human(msg))
     };
     match *filter {
         CompileFilter::Everything => {
-            // If explicit --bin or --example flags were passed then those'll
-            // get checked during cargo_compile, we only care about the "build
-            // everything" case here
-            if pkg.targets().iter().filter(|t| t.is_bin()).next().is_none() {
-                bail!("specified package has no binaries")
-            }
-
-            for target in pkg.targets().iter().filter(|t| t.is_bin()) {
-                try!(check(target.name()));
-            }
+            pkg.targets().iter()
+                         .filter(|t| t.is_bin())
+                         .filter_map(|t| check(t.name()))
+                         .collect()
         }
         CompileFilter::Only { bins, examples, .. } => {
-            for bin in bins.iter().chain(examples) {
-                try!(check(bin));
-            }
+            bins.iter().chain(examples)
+                       .filter_map(|t| check(t))
+                       .collect()
         }
     }
-    Ok(())
 }
 
 fn read_crate_list(mut file: &File) -> CargoResult<CrateListingV1> {

tests/support/mod.rs

@@ -672,3 +672,4 @@ pub static UPLOADING:   &'static str = "   Uploading";
 pub static VERIFYING:   &'static str = "   Verifying";
 pub static ARCHIVING:   &'static str = "   Archiving";
 pub static INSTALLING:  &'static str = "  Installing";
+pub static REPLACING:   &'static str = "   Replacing";