Skip to content

Commit

Permalink
Auto merge of #2405 - gkoz:reinstall, r=alexcrichton
Browse files Browse the repository at this point in the history
Add `--force` flag to cargo install

Close #2082.

Adding `--force` (`-f`) instructs cargo to overwrite existing binaries (updating the metadata accordingly).
This allows updating crates via `cargo install -f <crate>`.

Installation happens in two stages now: binaries are copied into a temporary subdirectory of the destination first, then moved into destination. This should catch some errors earlier.

In case of installation error cargo will remove new binaries but won't attempt to undo successful overwrites.
  • Loading branch information
bors committed May 2, 2016
2 parents 2d84f3e + 9f0fa24 commit 401209f
Show file tree
Hide file tree
Showing 4 changed files with 302 additions and 44 deletions.
10 changes: 8 additions & 2 deletions src/bin/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>,
Expand Down Expand Up @@ -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
Expand All @@ -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`
Expand All @@ -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 .`.
Expand Down Expand Up @@ -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)
}
190 changes: 151 additions & 39 deletions src/cargo/ops/cargo_install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand All @@ -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() {
Expand Down Expand Up @@ -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;
Expand All @@ -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());
Expand Down Expand Up @@ -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> {
Expand Down
1 change: 1 addition & 0 deletions tests/support/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Loading

0 comments on commit 401209f

Please sign in to comment.