From 3261cd15cb7fdd75d518acdbc938b6c89a21944e Mon Sep 17 00:00:00 2001 From: ibaryshnikov Date: Sun, 3 Jun 2018 13:38:55 +0300 Subject: [PATCH 01/12] boilerplate for a new command --- src/bin/cargo/commands/add.rs | 22 ++++++++++++++++++++++ src/bin/cargo/commands/mod.rs | 3 +++ 2 files changed, 25 insertions(+) create mode 100644 src/bin/cargo/commands/add.rs diff --git a/src/bin/cargo/commands/add.rs b/src/bin/cargo/commands/add.rs new file mode 100644 index 00000000000..ca5def1841c --- /dev/null +++ b/src/bin/cargo/commands/add.rs @@ -0,0 +1,22 @@ +use command_prelude::*; + +use cargo::core::{SourceId}; + +pub fn cli() -> App { + subcommand("add") + .about("Add a new dependency") + .arg(Arg::with_name("crate").empty_values(false)) +} + +pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult { + println!("cargo add subcommand executed"); + + let krate = args.value_of("crate") + .unwrap_or_default(); + + println!("crate {:?}", krate); + + // let source = SourceId::crates_io(config)?; + + Ok(()) +} diff --git a/src/bin/cargo/commands/mod.rs b/src/bin/cargo/commands/mod.rs index 057dc2f07b4..c0413044c75 100644 --- a/src/bin/cargo/commands/mod.rs +++ b/src/bin/cargo/commands/mod.rs @@ -2,6 +2,7 @@ use command_prelude::*; pub fn builtin() -> Vec { vec![ + add::cli(), bench::cli(), build::cli(), check::cli(), @@ -37,6 +38,7 @@ pub fn builtin() -> Vec { pub fn builtin_exec(cmd: &str) -> Option CliResult> { let f = match cmd { + "add" => add::exec, "bench" => bench::exec, "build" => build::exec, "check" => check::exec, @@ -72,6 +74,7 @@ pub fn builtin_exec(cmd: &str) -> Option CliResu Some(f) } +pub mod add; pub mod bench; pub mod build; pub mod check; From 742da9db34665c45b67fea4e79c795bec2644b5b Mon Sep 17 00:00:00 2001 From: ibaryshnikov Date: Tue, 5 Jun 2018 21:45:53 +0300 Subject: [PATCH 02/12] cargo_add created inside ops --- src/cargo/ops/cargo_add.rs | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) create mode 100644 src/cargo/ops/cargo_add.rs diff --git a/src/cargo/ops/cargo_add.rs b/src/cargo/ops/cargo_add.rs new file mode 100644 index 00000000000..a01e71599c1 --- /dev/null +++ b/src/cargo/ops/cargo_add.rs @@ -0,0 +1,37 @@ +use core::{Package, SourceId} + +use util::errors::{CargoResult}; + +fn select() -> CargoResult { + let (pkg, source) = select_pkg( + map.load(source_id)?, + krate, + vers, + config, + is_first_install, + &mut |_| { + bail!( + "must specify a crate to add from \ + crates.io, or use --path or --git to \ + specify alternate source" + ) + }, + )?; + + pkg +} + +pub fn select_pkg<'a, T>( + mut source: T, + name: Option<&str>, + vers: Option<&str>, + config: &Config, + needs_update: bool, + list_all: &mut FnMut(&mut T) -> CargoResult>, +) -> CargoResult<(Package, Box)> +where + T: Source + 'a, +{ + + } +} From 097da43d4d644b868e43365eca1437391c782cbd Mon Sep 17 00:00:00 2001 From: ibaryshnikov Date: Wed, 6 Jun 2018 00:59:27 +0300 Subject: [PATCH 03/12] cargo add subcommand added using code from cargo install, will be replaced in process --- src/bin/cargo/commands/add.rs | 43 ++- src/cargo/ops/cargo_add.rs | 575 +++++++++++++++++++++++++++++++++- src/cargo/ops/mod.rs | 2 + 3 files changed, 601 insertions(+), 19 deletions(-) diff --git a/src/bin/cargo/commands/add.rs b/src/bin/cargo/commands/add.rs index ca5def1841c..313a8744194 100644 --- a/src/bin/cargo/commands/add.rs +++ b/src/bin/cargo/commands/add.rs @@ -1,6 +1,8 @@ use command_prelude::*; -use cargo::core::{SourceId}; +use cargo::core::{GitReference, SourceId}; +use cargo::ops; +use cargo::util::ToUrl; pub fn cli() -> App { subcommand("add") @@ -9,6 +11,9 @@ pub fn cli() -> App { } pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult { + let mut compile_opts = args.compile_options(config, CompileMode::Build)?; + compile_opts.build_config.release = !args.is_present("debug"); + println!("cargo add subcommand executed"); let krate = args.value_of("crate") @@ -16,7 +21,41 @@ pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult { println!("crate {:?}", krate); - // let source = SourceId::crates_io(config)?; + let mut from_cwd = false; + + let source = if let Some(url) = args.value_of("git") { + let url = url.to_url()?; + let gitref = if let Some(branch) = args.value_of("branch") { + GitReference::Branch(branch.to_string()) + } else if let Some(tag) = args.value_of("tag") { + GitReference::Tag(tag.to_string()) + } else if let Some(rev) = args.value_of("rev") { + GitReference::Rev(rev.to_string()) + } else { + GitReference::Branch("master".to_string()) + }; + SourceId::for_git(&url, gitref)? + } else if let Some(path) = args.value_of_path("path", config) { + SourceId::for_path(&path)? + } else if krate.is_empty() { + from_cwd = true; + SourceId::for_path(config.cwd())? + } else { + SourceId::crates_io(config)? + }; + + let version = args.value_of("version"); + let root = args.value_of("root"); + + ops::add( + root, + krate, + &source, + from_cwd, + version, + &compile_opts, + args.is_present("force"), + )?; Ok(()) } diff --git a/src/cargo/ops/cargo_add.rs b/src/cargo/ops/cargo_add.rs index a01e71599c1..5542b0d8117 100644 --- a/src/cargo/ops/cargo_add.rs +++ b/src/cargo/ops/cargo_add.rs @@ -1,37 +1,578 @@ -use core::{Package, SourceId} +use std::collections::{BTreeMap, BTreeSet}; +use std::{env, fs}; +use std::io::prelude::*; +use std::io::SeekFrom; +use std::path::{Path, PathBuf}; +use std::sync::Arc; -use util::errors::{CargoResult}; +use semver::{Version, VersionReq}; +use tempfile::Builder as TempFileBuilder; +use toml; -fn select() -> CargoResult { - let (pkg, source) = select_pkg( +use core::{Dependency, Edition, Package, Source, SourceId}; +use core::{PackageId, Workspace}; +use core::compiler::DefaultExecutor; +use ops::{self, CompileFilter}; +use sources::{GitSource, PathSource, SourceConfigMap}; +use util::{internal, Config}; +use util::{FileLock, Filesystem}; +use util::errors::{CargoResult, CargoResultExt}; +use util::paths; + +#[derive(Deserialize, Serialize)] +#[serde(untagged)] +enum CrateListing { + V1(CrateListingV1), + Empty(Empty), +} + +#[derive(Deserialize, Serialize)] +#[serde(deny_unknown_fields)] +struct Empty {} + +#[derive(Deserialize, Serialize)] +struct CrateListingV1 { + v1: BTreeMap>, +} + +struct Transaction { + bins: Vec, +} + +impl Transaction { + fn success(mut self) { + self.bins.clear(); + } +} + +impl Drop for Transaction { + fn drop(&mut self) { + for bin in self.bins.iter() { + let _ = paths::remove_file(bin); + } + } +} + +pub fn add( + root: Option<&str>, + krate: &str, + source_id: &SourceId, + from_cwd: bool, + vers: Option<&str>, + opts: &ops::CompileOptions, + force: bool, +) -> CargoResult<()> { + let root = resolve_root(root, opts.config)?; + let map = SourceConfigMap::new(opts.config)?; + + install_one( + &root, + &map, + krate, + source_id, + from_cwd, + vers, + opts, + force, + true, + )?; + + + // Print a warning that if this directory isn't in PATH that they won't be + // able to run these commands. + let dst = metadata(opts.config, &root)?.parent().join("bin"); + let path = env::var_os("PATH").unwrap_or_default(); + for path in env::split_paths(&path) { + if path == dst { + return Ok(()); + } + } + + opts.config.shell().warn(&format!( + "be sure to add `{}` to your PATH to be \ + able to run the installed binaries", + dst.display() + ))?; + + Ok(()) +} + +fn install_one( + root: &Filesystem, + map: &SourceConfigMap, + krate: &str, + source_id: &SourceId, + from_cwd: bool, + vers: Option<&str>, + opts: &ops::CompileOptions, + force: bool, + is_first_install: bool, +) -> CargoResult<()> { + let config = opts.config; + + let (pkg, source) = if source_id.is_git() { + select_pkg( + GitSource::new(source_id, config)?, + krate, + vers, + config, + is_first_install, + )? + } else if source_id.is_path() { + let path = source_id + .url() + .to_file_path() + .map_err(|()| format_err!("path sources must have a valid path"))?; + let mut src = PathSource::new(&path, source_id, config); + src.update().chain_err(|| { + format_err!( + "`{}` is not a crate root; specify a crate to \ + install from crates.io, or use --path or --git to \ + specify an alternate source", + path.display() + ) + })?; + select_pkg( + PathSource::new(&path, source_id, config), + krate, + vers, + config, + is_first_install, + )? + } else { + select_pkg( map.load(source_id)?, krate, vers, config, is_first_install, - &mut |_| { - bail!( - "must specify a crate to add from \ - crates.io, or use --path or --git to \ - specify alternate source" - ) - }, - )?; + )? + }; + + let mut td_opt = None; + let mut needs_cleanup = false; + let overidden_target_dir = if source_id.is_path() { + None + } else if let Some(dir) = config.target_dir()? { + Some(dir) + } else if let Ok(td) = TempFileBuilder::new().prefix("cargo-install").tempdir() { + let p = td.path().to_owned(); + td_opt = Some(td); + Some(Filesystem::new(p)) + } else { + needs_cleanup = true; + Some(Filesystem::new(config.cwd().join("target-install"))) + }; + + let ws = Workspace::ephemeral(pkg, config, overidden_target_dir, false)?; + let pkg = ws.current()?; + + if from_cwd { + match pkg.manifest().edition() { + Edition::Edition2015 => config.shell().warn( + "To build the current package use `cargo build`, \ + to install the current package run `cargo install --path .`", + )?, + Edition::Edition2018 => bail!( + "To build the current package use `cargo build`, \ + to install the current package run `cargo install --path .`, \ + otherwise specify a crate to install from \ + crates.io, or use --path or --git to \ + specify alternate source" + ), + } + }; + + config.shell().status("Installing", pkg)?; + + // Preflight checks to check up front whether we'll overwrite something. + // We have to check this again afterwards, but may as well avoid building + // anything if we're gonna throw it away anyway. + { + let metadata = metadata(config, root)?; + let list = read_crate_list(&metadata)?; + let dst = metadata.parent().join("bin"); + check_overwrites(&dst, pkg, &opts.filter, &list, force)?; + } + + let compile = + ops::compile_ws(&ws, Some(source), opts, Arc::new(DefaultExecutor)).chain_err(|| { + if let Some(td) = td_opt.take() { + // preserve the temporary directory, so the user can inspect it + td.into_path(); + } + + format_err!( + "failed to compile `{}`, intermediate artifacts can be \ + found at `{}`", + pkg, + ws.target_dir().display() + ) + })?; + let binaries: Vec<(&str, &Path)> = 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::>()?; + if binaries.is_empty() { + bail!( + "no binaries are available for install using the selected \ + features" + ); + } + + let metadata = metadata(config, root)?; + let mut list = read_crate_list(&metadata)?; + let dst = metadata.parent().join("bin"); + let duplicates = check_overwrites(&dst, pkg, &opts.filter, &list, force)?; + + fs::create_dir_all(&dst)?; + + // 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 = TempFileBuilder::new() + .prefix("cargo-install") + .tempdir_in(&dst)?; + 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() && fs::rename(src, &dst).is_ok() { + continue; + } + fs::copy(src, &dst).chain_err(|| { + format_err!("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)); - pkg + 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); + config.shell().status("Installing", dst.display())?; + fs::rename(&src, &dst).chain_err(|| { + format_err!("failed to move `{}` to `{}`", src.display(), dst.display()) + })?; + 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); + config.shell().status("Replacing", dst.display())?; + fs::rename(&src, &dst).chain_err(|| { + format_err!("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); + } + } + // Failsafe to force replacing metadata for git packages + // https://github.com/rust-lang/cargo/issues/4582 + if let Some(set) = list.v1.remove(&pkg.package_id().clone()) { + list.v1.insert(pkg.package_id().clone(), set); + } + 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::>(); + 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, 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) => result.chain_err(|| err)?, + Ok(_) => result?, + } + + // Reaching here means all actions have succeeded. Clean up. + installed.success(); + if needs_cleanup { + // Don't bother grabbing a lock as we're going to blow it all away + // anyway. + let target_dir = ws.target_dir().into_path_unlocked(); + paths::remove_dir_all(&target_dir)?; + } + + Ok(()) } -pub fn select_pkg<'a, T>( +fn select_pkg<'a, T>( mut source: T, - name: Option<&str>, + name: &str, vers: Option<&str>, config: &Config, needs_update: bool, - list_all: &mut FnMut(&mut T) -> CargoResult>, ) -> CargoResult<(Package, Box)> where T: Source + 'a, { - + if needs_update { + source.update()?; } + + let vers = match vers { + Some(v) => { + // If the version begins with character <, >, =, ^, ~ parse it as a + // version range, otherwise parse it as a specific version + let first = v.chars() + .nth(0) + .ok_or_else(|| format_err!("no version provided for the `--vers` flag"))?; + + match first { + '<' | '>' | '=' | '^' | '~' => match v.parse::() { + Ok(v) => Some(v.to_string()), + Err(_) => bail!( + "the `--vers` provided, `{}`, is \ + not a valid semver version requirement\n\n + Please have a look at \ + http://doc.crates.io/specifying-dependencies.html \ + for the correct format", + v + ), + }, + _ => match v.parse::() { + Ok(v) => Some(format!("={}", v)), + Err(_) => { + let mut msg = format!( + "\ + the `--vers` provided, `{}`, is \ + not a valid semver version\n\n\ + historically Cargo treated this \ + as a semver version requirement \ + accidentally\nand will continue \ + to do so, but this behavior \ + will be removed eventually", + v + ); + + // If it is not a valid version but it is a valid version + // requirement, add a note to the warning + if v.parse::().is_ok() { + msg.push_str(&format!( + "\nif you want to specify semver range, \ + add an explicit qualifier, like ^{}", + v + )); + } + config.shell().warn(&msg)?; + Some(v.to_string()) + } + }, + } + } + None => None, + }; + let vers = vers.as_ref().map(|s| &**s); + let dep = Dependency::parse_no_deprecated( + name, + Some(vers.unwrap_or("*")), + source.source_id(), + )?; + let deps = source.query_vec(&dep)?; + match deps.iter().map(|p| p.package_id()).max() { + Some(pkgid) => { + let pkg = source.download(pkgid)?; + Ok((pkg, Box::new(source))) + } + None => { + let vers_info = vers.map(|v| format!(" with version `{}`", v)) + .unwrap_or_default(); + Err(format_err!( + "could not find `{}` in {}{}", + name, + source.source_id(), + vers_info + )) + } + } +} + +fn check_overwrites( + dst: &Path, + pkg: &Package, + filter: &ops::CompileFilter, + prev: &CrateListingV1, + force: bool, +) -> CargoResult>> { + // 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 !filter.is_specific() && !pkg.targets().iter().any(|t| t.is_bin()) { + 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 (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(format_err!("{}", msg)) +} + +fn find_duplicates( + dst: &Path, + pkg: &Package, + filter: &ops::CompileFilter, + prev: &CrateListingV1, +) -> BTreeMap> { + let check = |name: String| { + // Need to provide type, works around Rust Issue #93349 + let name = format!("{}{}", name, env::consts::EXE_SUFFIX); + if fs::metadata(dst.join(&name)).is_err() { + None + } else if let Some((p, _)) = prev.v1.iter().find(|&(_, v)| v.contains(&name)) { + Some((name, Some(p.clone()))) + } else { + Some((name, None)) + } + }; + match *filter { + CompileFilter::Default { .. } => pkg.targets() + .iter() + .filter(|t| t.is_bin()) + .filter_map(|t| check(t.name().to_string())) + .collect(), + CompileFilter::Only { + ref bins, + ref examples, + .. + } => { + let all_bins: Vec = bins.try_collect().unwrap_or_else(|| { + pkg.targets() + .iter() + .filter(|t| t.is_bin()) + .map(|t| t.name().to_string()) + .collect() + }); + let all_examples: Vec = examples.try_collect().unwrap_or_else(|| { + pkg.targets() + .iter() + .filter(|t| t.is_bin_example()) + .map(|t| t.name().to_string()) + .collect() + }); + + all_bins + .iter() + .chain(all_examples.iter()) + .filter_map(|t| check(t.clone())) + .collect::>>() + } + } +} + +fn read_crate_list(file: &FileLock) -> CargoResult { + let listing = (|| -> CargoResult<_> { + let mut contents = String::new(); + file.file().read_to_string(&mut contents)?; + let listing = + toml::from_str(&contents).chain_err(|| internal("invalid TOML found for metadata"))?; + match listing { + CrateListing::V1(v1) => Ok(v1), + CrateListing::Empty(_) => Ok(CrateListingV1 { + v1: BTreeMap::new(), + }), + } + })() + .chain_err(|| { + format_err!( + "failed to parse crate metadata at `{}`", + file.path().to_string_lossy() + ) + })?; + Ok(listing) +} + +fn write_crate_list(file: &FileLock, listing: CrateListingV1) -> CargoResult<()> { + (|| -> CargoResult<_> { + let mut file = file.file(); + file.seek(SeekFrom::Start(0))?; + file.set_len(0)?; + let data = toml::to_string(&CrateListing::V1(listing))?; + file.write_all(data.as_bytes())?; + Ok(()) + })() + .chain_err(|| { + format_err!( + "failed to write crate metadata at `{}`", + file.path().to_string_lossy() + ) + })?; + Ok(()) +} + +fn metadata(config: &Config, root: &Filesystem) -> CargoResult { + root.open_rw(Path::new(".crates.toml"), config, "crate metadata") +} + +fn resolve_root(flag: Option<&str>, config: &Config) -> CargoResult { + let config_root = config.get_path("install.root")?; + Ok(flag.map(PathBuf::from) + .or_else(|| env::var_os("CARGO_INSTALL_ROOT").map(PathBuf::from)) + .or_else(move || config_root.map(|v| v.val)) + .map(Filesystem::new) + .unwrap_or_else(|| config.home().clone())) } diff --git a/src/cargo/ops/mod.rs b/src/cargo/ops/mod.rs index 9c09f14f5ed..df57157cd80 100644 --- a/src/cargo/ops/mod.rs +++ b/src/cargo/ops/mod.rs @@ -1,3 +1,4 @@ +pub use self::cargo_add::{add}; pub use self::cargo_clean::{clean, CleanOptions}; pub use self::cargo_compile::{compile, compile_with_exec, compile_ws, CompileOptions}; pub use self::cargo_compile::{CompileFilter, FilterRule, Packages}; @@ -23,6 +24,7 @@ pub use self::resolve::{add_overrides, get_resolved_packages, resolve_with_previ pub use self::cargo_output_metadata::{output_metadata, ExportInfo, OutputMetadataOptions}; pub use self::fix::{fix, FixOptions, fix_maybe_exec_rustc}; +mod cargo_add; mod cargo_clean; mod cargo_compile; mod cargo_doc; From 36978b3b4f06b80541457caac0d99fa694ad3b92 Mon Sep 17 00:00:00 2001 From: ibaryshnikov Date: Sat, 16 Jun 2018 21:38:39 +0300 Subject: [PATCH 04/12] #5586 args and afterhelp info added, some boilerplate removed --- src/bin/cargo/commands/add.rs | 35 ++- src/cargo/ops/cargo_add.rs | 570 +--------------------------------- 2 files changed, 26 insertions(+), 579 deletions(-) diff --git a/src/bin/cargo/commands/add.rs b/src/bin/cargo/commands/add.rs index 313a8744194..3f3c6249e72 100644 --- a/src/bin/cargo/commands/add.rs +++ b/src/bin/cargo/commands/add.rs @@ -8,11 +8,31 @@ pub fn cli() -> App { subcommand("add") .about("Add a new dependency") .arg(Arg::with_name("crate").empty_values(false)) + .arg( + opt("version", "Specify a version to install from crates.io") + .alias("vers") + .value_name("VERSION"), + ) + .arg(opt("git", "Git URL to add the specified crate from").value_name("URL")) + .arg(opt("branch", "Branch to use when add from git").value_name("BRANCH")) + .arg(opt("tag", "Tag to use when add from git").value_name("TAG")) + .arg(opt("rev", "Specific commit to use when adding from git").value_name("SHA")) + .arg(opt("path", "Filesystem path to local crate to add").value_name("PATH")) + .after_help( + "\ +This command allows you to add a dependency to a Cargo.toml manifest file. If is a github +or gitlab repository URL, or a local path, `cargo add` will try to automatically get the crate name +and set the appropriate `--git` or `--path` value. + +Please note that Cargo treats versions like \"1.2.3\" as \"^1.2.3\" (and that \"^1.2.3\" is specified +as \">=1.2.3 and <2.0.0\"). By default, `cargo add` will use this format, as it is the one that the +crates.io registry suggests. One goal of `cargo add` is to prevent you from using wildcard +dependencies (version set to \"*\").", + ) } pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult { - let mut compile_opts = args.compile_options(config, CompileMode::Build)?; - compile_opts.build_config.release = !args.is_present("debug"); + let ws = args.workspace(config)?; println!("cargo add subcommand executed"); @@ -21,8 +41,6 @@ pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult { println!("crate {:?}", krate); - let mut from_cwd = false; - let source = if let Some(url) = args.value_of("git") { let url = url.to_url()?; let gitref = if let Some(branch) = args.value_of("branch") { @@ -37,24 +55,17 @@ pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult { SourceId::for_git(&url, gitref)? } else if let Some(path) = args.value_of_path("path", config) { SourceId::for_path(&path)? - } else if krate.is_empty() { - from_cwd = true; - SourceId::for_path(config.cwd())? } else { SourceId::crates_io(config)? }; let version = args.value_of("version"); - let root = args.value_of("root"); ops::add( - root, + &ws, krate, &source, - from_cwd, version, - &compile_opts, - args.is_present("force"), )?; Ok(()) diff --git a/src/cargo/ops/cargo_add.rs b/src/cargo/ops/cargo_add.rs index 5542b0d8117..914218185d8 100644 --- a/src/cargo/ops/cargo_add.rs +++ b/src/cargo/ops/cargo_add.rs @@ -1,578 +1,14 @@ -use std::collections::{BTreeMap, BTreeSet}; -use std::{env, fs}; -use std::io::prelude::*; -use std::io::SeekFrom; -use std::path::{Path, PathBuf}; -use std::sync::Arc; -use semver::{Version, VersionReq}; -use tempfile::Builder as TempFileBuilder; -use toml; +use core::{PackageId, SourceId, Workspace}; -use core::{Dependency, Edition, Package, Source, SourceId}; -use core::{PackageId, Workspace}; -use core::compiler::DefaultExecutor; -use ops::{self, CompileFilter}; -use sources::{GitSource, PathSource, SourceConfigMap}; -use util::{internal, Config}; -use util::{FileLock, Filesystem}; -use util::errors::{CargoResult, CargoResultExt}; -use util::paths; - -#[derive(Deserialize, Serialize)] -#[serde(untagged)] -enum CrateListing { - V1(CrateListingV1), - Empty(Empty), -} - -#[derive(Deserialize, Serialize)] -#[serde(deny_unknown_fields)] -struct Empty {} - -#[derive(Deserialize, Serialize)] -struct CrateListingV1 { - v1: BTreeMap>, -} - -struct Transaction { - bins: Vec, -} - -impl Transaction { - fn success(mut self) { - self.bins.clear(); - } -} - -impl Drop for Transaction { - fn drop(&mut self) { - for bin in self.bins.iter() { - let _ = paths::remove_file(bin); - } - } -} +use util::errors::{CargoResult}; pub fn add( - root: Option<&str>, + ws: &Workspace, krate: &str, source_id: &SourceId, - from_cwd: bool, vers: Option<&str>, - opts: &ops::CompileOptions, - force: bool, ) -> CargoResult<()> { - let root = resolve_root(root, opts.config)?; - let map = SourceConfigMap::new(opts.config)?; - - install_one( - &root, - &map, - krate, - source_id, - from_cwd, - vers, - opts, - force, - true, - )?; - - - // Print a warning that if this directory isn't in PATH that they won't be - // able to run these commands. - let dst = metadata(opts.config, &root)?.parent().join("bin"); - let path = env::var_os("PATH").unwrap_or_default(); - for path in env::split_paths(&path) { - if path == dst { - return Ok(()); - } - } - - opts.config.shell().warn(&format!( - "be sure to add `{}` to your PATH to be \ - able to run the installed binaries", - dst.display() - ))?; Ok(()) } - -fn install_one( - root: &Filesystem, - map: &SourceConfigMap, - krate: &str, - source_id: &SourceId, - from_cwd: bool, - vers: Option<&str>, - opts: &ops::CompileOptions, - force: bool, - is_first_install: bool, -) -> CargoResult<()> { - let config = opts.config; - - let (pkg, source) = if source_id.is_git() { - select_pkg( - GitSource::new(source_id, config)?, - krate, - vers, - config, - is_first_install, - )? - } else if source_id.is_path() { - let path = source_id - .url() - .to_file_path() - .map_err(|()| format_err!("path sources must have a valid path"))?; - let mut src = PathSource::new(&path, source_id, config); - src.update().chain_err(|| { - format_err!( - "`{}` is not a crate root; specify a crate to \ - install from crates.io, or use --path or --git to \ - specify an alternate source", - path.display() - ) - })?; - select_pkg( - PathSource::new(&path, source_id, config), - krate, - vers, - config, - is_first_install, - )? - } else { - select_pkg( - map.load(source_id)?, - krate, - vers, - config, - is_first_install, - )? - }; - - let mut td_opt = None; - let mut needs_cleanup = false; - let overidden_target_dir = if source_id.is_path() { - None - } else if let Some(dir) = config.target_dir()? { - Some(dir) - } else if let Ok(td) = TempFileBuilder::new().prefix("cargo-install").tempdir() { - let p = td.path().to_owned(); - td_opt = Some(td); - Some(Filesystem::new(p)) - } else { - needs_cleanup = true; - Some(Filesystem::new(config.cwd().join("target-install"))) - }; - - let ws = Workspace::ephemeral(pkg, config, overidden_target_dir, false)?; - let pkg = ws.current()?; - - if from_cwd { - match pkg.manifest().edition() { - Edition::Edition2015 => config.shell().warn( - "To build the current package use `cargo build`, \ - to install the current package run `cargo install --path .`", - )?, - Edition::Edition2018 => bail!( - "To build the current package use `cargo build`, \ - to install the current package run `cargo install --path .`, \ - otherwise specify a crate to install from \ - crates.io, or use --path or --git to \ - specify alternate source" - ), - } - }; - - config.shell().status("Installing", pkg)?; - - // Preflight checks to check up front whether we'll overwrite something. - // We have to check this again afterwards, but may as well avoid building - // anything if we're gonna throw it away anyway. - { - let metadata = metadata(config, root)?; - let list = read_crate_list(&metadata)?; - let dst = metadata.parent().join("bin"); - check_overwrites(&dst, pkg, &opts.filter, &list, force)?; - } - - let compile = - ops::compile_ws(&ws, Some(source), opts, Arc::new(DefaultExecutor)).chain_err(|| { - if let Some(td) = td_opt.take() { - // preserve the temporary directory, so the user can inspect it - td.into_path(); - } - - format_err!( - "failed to compile `{}`, intermediate artifacts can be \ - found at `{}`", - pkg, - ws.target_dir().display() - ) - })?; - let binaries: Vec<(&str, &Path)> = 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::>()?; - if binaries.is_empty() { - bail!( - "no binaries are available for install using the selected \ - features" - ); - } - - let metadata = metadata(config, root)?; - let mut list = read_crate_list(&metadata)?; - let dst = metadata.parent().join("bin"); - let duplicates = check_overwrites(&dst, pkg, &opts.filter, &list, force)?; - - fs::create_dir_all(&dst)?; - - // 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 = TempFileBuilder::new() - .prefix("cargo-install") - .tempdir_in(&dst)?; - 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() && fs::rename(src, &dst).is_ok() { - continue; - } - fs::copy(src, &dst).chain_err(|| { - format_err!("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); - config.shell().status("Installing", dst.display())?; - fs::rename(&src, &dst).chain_err(|| { - format_err!("failed to move `{}` to `{}`", src.display(), dst.display()) - })?; - 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); - config.shell().status("Replacing", dst.display())?; - fs::rename(&src, &dst).chain_err(|| { - format_err!("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); - } - } - // Failsafe to force replacing metadata for git packages - // https://github.com/rust-lang/cargo/issues/4582 - if let Some(set) = list.v1.remove(&pkg.package_id().clone()) { - list.v1.insert(pkg.package_id().clone(), set); - } - 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::>(); - 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, 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) => result.chain_err(|| err)?, - Ok(_) => result?, - } - - // Reaching here means all actions have succeeded. Clean up. - installed.success(); - if needs_cleanup { - // Don't bother grabbing a lock as we're going to blow it all away - // anyway. - let target_dir = ws.target_dir().into_path_unlocked(); - paths::remove_dir_all(&target_dir)?; - } - - Ok(()) -} - -fn select_pkg<'a, T>( - mut source: T, - name: &str, - vers: Option<&str>, - config: &Config, - needs_update: bool, -) -> CargoResult<(Package, Box)> -where - T: Source + 'a, -{ - if needs_update { - source.update()?; - } - - let vers = match vers { - Some(v) => { - // If the version begins with character <, >, =, ^, ~ parse it as a - // version range, otherwise parse it as a specific version - let first = v.chars() - .nth(0) - .ok_or_else(|| format_err!("no version provided for the `--vers` flag"))?; - - match first { - '<' | '>' | '=' | '^' | '~' => match v.parse::() { - Ok(v) => Some(v.to_string()), - Err(_) => bail!( - "the `--vers` provided, `{}`, is \ - not a valid semver version requirement\n\n - Please have a look at \ - http://doc.crates.io/specifying-dependencies.html \ - for the correct format", - v - ), - }, - _ => match v.parse::() { - Ok(v) => Some(format!("={}", v)), - Err(_) => { - let mut msg = format!( - "\ - the `--vers` provided, `{}`, is \ - not a valid semver version\n\n\ - historically Cargo treated this \ - as a semver version requirement \ - accidentally\nand will continue \ - to do so, but this behavior \ - will be removed eventually", - v - ); - - // If it is not a valid version but it is a valid version - // requirement, add a note to the warning - if v.parse::().is_ok() { - msg.push_str(&format!( - "\nif you want to specify semver range, \ - add an explicit qualifier, like ^{}", - v - )); - } - config.shell().warn(&msg)?; - Some(v.to_string()) - } - }, - } - } - None => None, - }; - let vers = vers.as_ref().map(|s| &**s); - let dep = Dependency::parse_no_deprecated( - name, - Some(vers.unwrap_or("*")), - source.source_id(), - )?; - let deps = source.query_vec(&dep)?; - match deps.iter().map(|p| p.package_id()).max() { - Some(pkgid) => { - let pkg = source.download(pkgid)?; - Ok((pkg, Box::new(source))) - } - None => { - let vers_info = vers.map(|v| format!(" with version `{}`", v)) - .unwrap_or_default(); - Err(format_err!( - "could not find `{}` in {}{}", - name, - source.source_id(), - vers_info - )) - } - } -} - -fn check_overwrites( - dst: &Path, - pkg: &Package, - filter: &ops::CompileFilter, - prev: &CrateListingV1, - force: bool, -) -> CargoResult>> { - // 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 !filter.is_specific() && !pkg.targets().iter().any(|t| t.is_bin()) { - 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 (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(format_err!("{}", msg)) -} - -fn find_duplicates( - dst: &Path, - pkg: &Package, - filter: &ops::CompileFilter, - prev: &CrateListingV1, -) -> BTreeMap> { - let check = |name: String| { - // Need to provide type, works around Rust Issue #93349 - let name = format!("{}{}", name, env::consts::EXE_SUFFIX); - if fs::metadata(dst.join(&name)).is_err() { - None - } else if let Some((p, _)) = prev.v1.iter().find(|&(_, v)| v.contains(&name)) { - Some((name, Some(p.clone()))) - } else { - Some((name, None)) - } - }; - match *filter { - CompileFilter::Default { .. } => pkg.targets() - .iter() - .filter(|t| t.is_bin()) - .filter_map(|t| check(t.name().to_string())) - .collect(), - CompileFilter::Only { - ref bins, - ref examples, - .. - } => { - let all_bins: Vec = bins.try_collect().unwrap_or_else(|| { - pkg.targets() - .iter() - .filter(|t| t.is_bin()) - .map(|t| t.name().to_string()) - .collect() - }); - let all_examples: Vec = examples.try_collect().unwrap_or_else(|| { - pkg.targets() - .iter() - .filter(|t| t.is_bin_example()) - .map(|t| t.name().to_string()) - .collect() - }); - - all_bins - .iter() - .chain(all_examples.iter()) - .filter_map(|t| check(t.clone())) - .collect::>>() - } - } -} - -fn read_crate_list(file: &FileLock) -> CargoResult { - let listing = (|| -> CargoResult<_> { - let mut contents = String::new(); - file.file().read_to_string(&mut contents)?; - let listing = - toml::from_str(&contents).chain_err(|| internal("invalid TOML found for metadata"))?; - match listing { - CrateListing::V1(v1) => Ok(v1), - CrateListing::Empty(_) => Ok(CrateListingV1 { - v1: BTreeMap::new(), - }), - } - })() - .chain_err(|| { - format_err!( - "failed to parse crate metadata at `{}`", - file.path().to_string_lossy() - ) - })?; - Ok(listing) -} - -fn write_crate_list(file: &FileLock, listing: CrateListingV1) -> CargoResult<()> { - (|| -> CargoResult<_> { - let mut file = file.file(); - file.seek(SeekFrom::Start(0))?; - file.set_len(0)?; - let data = toml::to_string(&CrateListing::V1(listing))?; - file.write_all(data.as_bytes())?; - Ok(()) - })() - .chain_err(|| { - format_err!( - "failed to write crate metadata at `{}`", - file.path().to_string_lossy() - ) - })?; - Ok(()) -} - -fn metadata(config: &Config, root: &Filesystem) -> CargoResult { - root.open_rw(Path::new(".crates.toml"), config, "crate metadata") -} - -fn resolve_root(flag: Option<&str>, config: &Config) -> CargoResult { - let config_root = config.get_path("install.root")?; - Ok(flag.map(PathBuf::from) - .or_else(|| env::var_os("CARGO_INSTALL_ROOT").map(PathBuf::from)) - .or_else(move || config_root.map(|v| v.val)) - .map(Filesystem::new) - .unwrap_or_else(|| config.home().clone())) -} From 04ec76053f34bd855ecac86333464ddbd36afa5d Mon Sep 17 00:00:00 2001 From: ibaryshnikov Date: Sat, 16 Jun 2018 23:44:38 +0300 Subject: [PATCH 05/12] started to move manifest from cargo-edit to cargo --- Cargo.toml | 1 + src/bin/cargo/commands/add.rs | 2 +- src/cargo/lib.rs | 1 + src/cargo/ops/cargo_add.rs | 11 +- src/cargo/util/toml/dependency.rs | 106 +++++++ src/cargo/util/toml/manifest.rs | 489 ++++++++++++++++++++++++++++++ src/cargo/util/toml/mod.rs | 5 + 7 files changed, 608 insertions(+), 7 deletions(-) create mode 100644 src/cargo/util/toml/dependency.rs create mode 100644 src/cargo/util/toml/manifest.rs diff --git a/Cargo.toml b/Cargo.toml index 1c77b43ded1..a998934a8e6 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -53,6 +53,7 @@ tar = { version = "0.4.15", default-features = false } tempfile = "3.0" termcolor = "1.0" toml = "0.4.2" +toml_edit = "0.1.1" url = "1.1" clap = "2.31.2" unicode-width = "0.1.5" diff --git a/src/bin/cargo/commands/add.rs b/src/bin/cargo/commands/add.rs index 3f3c6249e72..946753d9a2c 100644 --- a/src/bin/cargo/commands/add.rs +++ b/src/bin/cargo/commands/add.rs @@ -9,7 +9,7 @@ pub fn cli() -> App { .about("Add a new dependency") .arg(Arg::with_name("crate").empty_values(false)) .arg( - opt("version", "Specify a version to install from crates.io") + opt("version", "Specify a version to add from crates.io") .alias("vers") .value_name("VERSION"), ) diff --git a/src/cargo/lib.rs b/src/cargo/lib.rs index 6792ba12c38..4b7644c0c3d 100644 --- a/src/cargo/lib.rs +++ b/src/cargo/lib.rs @@ -58,6 +58,7 @@ extern crate tar; extern crate tempfile; extern crate termcolor; extern crate toml; +extern crate toml_edit; extern crate unicode_width; extern crate url; diff --git a/src/cargo/ops/cargo_add.rs b/src/cargo/ops/cargo_add.rs index 914218185d8..b6235360d6e 100644 --- a/src/cargo/ops/cargo_add.rs +++ b/src/cargo/ops/cargo_add.rs @@ -1,14 +1,13 @@ -use core::{PackageId, SourceId, Workspace}; +use core::{SourceId, Workspace}; use util::errors::{CargoResult}; pub fn add( - ws: &Workspace, - krate: &str, - source_id: &SourceId, - vers: Option<&str>, + _ws: &Workspace, + _krate: &str, + _source_id: &SourceId, + _vers: Option<&str>, ) -> CargoResult<()> { - Ok(()) } diff --git a/src/cargo/util/toml/dependency.rs b/src/cargo/util/toml/dependency.rs new file mode 100644 index 00000000000..d6566fdd930 --- /dev/null +++ b/src/cargo/util/toml/dependency.rs @@ -0,0 +1,106 @@ +use toml_edit; + +#[derive(Debug, Hash, PartialEq, Eq, Clone)] +enum DependencySource { + Version(String), + Git(String), + Path(String), +} + +/// A dependency handled by Cargo +#[derive(Debug, Hash, PartialEq, Eq, Clone)] +pub struct Dependency { + /// The name of the dependency (as it is set in its `Cargo.toml` and known to crates.io) + pub name: String, + optional: bool, + source: DependencySource, +} + +impl Default for Dependency { + fn default() -> Dependency { + Dependency { + name: "".into(), + optional: false, + source: DependencySource::Version("0.1.0".into()), + } + } +} + +impl Dependency { + /// Create a new dependency with a name + pub fn new(name: &str) -> Dependency { + Dependency { + name: name.into(), + ..Dependency::default() + } + } + + /// Set dependency to a given version + pub fn set_version(mut self, version: &str) -> Dependency { + self.source = DependencySource::Version(version.into()); + self + } + + /// Set dependency to a given repository + pub fn set_git(mut self, repo: &str) -> Dependency { + self.source = DependencySource::Git(repo.into()); + self + } + + /// Set dependency to a given path + pub fn set_path(mut self, path: &str) -> Dependency { + self.source = DependencySource::Path(path.into()); + self + } + + /// Set whether the dependency is optional + pub fn set_optional(mut self, opt: bool) -> Dependency { + self.optional = opt; + self + } + + /// Get version of dependency + pub fn version(&self) -> Option<&str> { + if let DependencySource::Version(ref version) = self.source { + Some(version) + } else { + None + } + } + + /// Convert dependency to TOML + /// + /// Returns a tuple with the dependency's name and either the version as a `String` + /// or the path/git repository as an `InlineTable`. + /// (If the dependency is set as `optional`, an `InlineTable` is returned in any case.) + pub fn to_toml(&self) -> (String, toml_edit::Item) { + let data: toml_edit::Item = match (self.optional, self.source.clone()) { + // Extra short when version flag only + (false, DependencySource::Version(v)) => toml_edit::value(v), + // Other cases are represented as an inline table + (optional, source) => { + let mut data = toml_edit::InlineTable::default(); + + match source { + DependencySource::Version(v) => { + data.get_or_insert("version", v); + } + DependencySource::Git(v) => { + data.get_or_insert("git", v); + } + DependencySource::Path(v) => { + data.get_or_insert("path", v); + } + } + if self.optional { + data.get_or_insert("optional", optional); + } + + data.fmt(); + toml_edit::value(toml_edit::Value::InlineTable(data)) + } + }; + + (self.name.clone(), data) + } +} diff --git a/src/cargo/util/toml/manifest.rs b/src/cargo/util/toml/manifest.rs new file mode 100644 index 00000000000..9c3a90b8b75 --- /dev/null +++ b/src/cargo/util/toml/manifest.rs @@ -0,0 +1,489 @@ +use std::fs::{self, File, OpenOptions}; +use std::io::{Read, Write}; +use std::ops::Deref; +use std::path::{Path, PathBuf}; +use std::{env, str}; + +use termcolor::{BufferWriter, Color, ColorChoice, ColorSpec, WriteColor}; +use toml_edit; + +use util::errors::*; +use util::toml::dependency::Dependency; + +const MANIFEST_FILENAME: &str = "Cargo.toml"; + +/// A Cargo manifest +#[derive(Debug, Clone)] +pub struct Manifest { + /// Manifest contents as TOML data + pub data: toml_edit::Document, +} + +/// If a manifest is specified, return that one, otherise perform a manifest search starting from +/// the current directory. +/// If a manifest is specified, return that one. If a path is specified, perform a manifest search +/// starting from there. If nothing is specified, start searching from the current directory +/// (`cwd`). +pub fn find(specified: &Option) -> CargoResult { + match *specified { + Some(ref path) + if fs::metadata(&path) + .chain_err(|| "Failed to get cargo file metadata")? + .is_file() => + { + Ok(path.to_owned()) + } + Some(ref path) => search(path), + None => search(&env::current_dir().chain_err(|| "Failed to get current directory")?), + } +} + +/// Search for Cargo.toml in this directory and recursively up the tree until one is found. +fn search(dir: &Path) -> CargoResult { + let manifest = dir.join(MANIFEST_FILENAME); + + if fs::metadata(&manifest).is_ok() { + Ok(manifest) + } else { + dir.parent() + .ok_or_else(|| ErrorKind::MissingManifest.into()) + .and_then(|dir| search(dir)) + } +} + +fn merge_inline_table(old_dep: &mut toml_edit::Item, new: &toml_edit::Item) { + for (k, v) in new.as_inline_table() + .expect("expected an inline table") + .iter() + { + old_dep[k] = toml_edit::value(v.clone()); + } +} + +fn str_or_1_len_table(item: &toml_edit::Item) -> bool { + item.is_str() || item.as_table_like().map(|t| t.len() == 1).unwrap_or(false) +} +/// Merge a new dependency into an old entry. See `Dependency::to_toml` for what the format of the +/// new dependency will be. +fn merge_dependencies(old_dep: &mut toml_edit::Item, new: &Dependency) { + assert!(!old_dep.is_none()); + + let new_toml = new.to_toml().1; + + if str_or_1_len_table(old_dep) { + // The old dependency is just a version/git/path. We are safe to overwrite. + *old_dep = new_toml; + } else if old_dep.is_table_like() { + for key in &["version", "path", "git"] { + // remove this key/value pairs + old_dep[key] = toml_edit::Item::None; + } + if let Some(name) = new_toml.as_str() { + old_dep["version"] = toml_edit::value(name); + } else { + merge_inline_table(old_dep, &new_toml); + } + } else { + unreachable!("Invalid old dependency type"); + } + + old_dep.as_inline_table_mut().map(|t| t.fmt()); +} + +/// Print a message if the new dependency version is different from the old one. +fn print_upgrade_if_necessary( + crate_name: &str, + old_dep: &toml_edit::Item, + new_version: &toml_edit::Item, +) -> CargoResult<()> { + let old_version = if str_or_1_len_table(old_dep) { + old_dep.clone() + } else if old_dep.is_table_like() { + let version = old_dep["version"].clone(); + if version.is_none() { + return Err("Missing version field".into()); + } + version + } else { + unreachable!("Invalid old dependency type") + }; + + if let (Some(old_version), Some(new_version)) = (old_version.as_str(), new_version.as_str()) { + if old_version == new_version { + return Ok(()); + } + let bufwtr = BufferWriter::stdout(ColorChoice::Always); + let mut buffer = bufwtr.buffer(); + buffer + .set_color(ColorSpec::new().set_fg(Some(Color::Green)).set_bold(true)) + .chain_err(|| "Failed to set output colour")?; + write!(&mut buffer, " Upgrading ").chain_err(|| "Failed to write upgrade message")?; + buffer + .set_color(&ColorSpec::new()) + .chain_err(|| "Failed to clear output colour")?; + write!( + &mut buffer, + "{} v{} -> v{}\n", + crate_name, old_version, new_version, + ).chain_err(|| "Failed to write upgrade versions")?; + bufwtr + .print(&buffer) + .chain_err(|| "Failed to print upgrade message")?; + } + Ok(()) +} + +impl Manifest { + /// Look for a `Cargo.toml` file + /// + /// Starts at the given path an goes into its parent directories until the manifest file is + /// found. If no path is given, the process's working directory is used as a starting point. + pub fn find_file(path: &Option) -> CargoResult { + find(path).and_then(|path| { + OpenOptions::new() + .read(true) + .write(true) + .open(path) + .chain_err(|| "Failed to find Cargo.toml") + }) + } + + /// Open the `Cargo.toml` for a path (or the process' `cwd`) + pub fn open(path: &Option) -> CargoResult { + let mut file = Manifest::find_file(path)?; + let mut data = String::new(); + file.read_to_string(&mut data) + .chain_err(|| "Failed to read manifest contents")?; + + data.parse().chain_err(|| "Unable to parse Cargo.toml") + } + + /// Get the specified table from the manifest. + pub fn get_table<'a>(&'a mut self, table_path: &[String]) -> CargoResult<&'a mut toml_edit::Item> { + /// Descend into a manifest until the required table is found. + fn descend<'a>( + input: &'a mut toml_edit::Item, + path: &[String], + ) -> CargoResult<&'a mut toml_edit::Item> { + if let Some(segment) = path.get(0) { + let value = input[&segment].or_insert(toml_edit::table()); + + if value.is_table_like() { + descend(value, &path[1..]) + } else { + Err(ErrorKind::NonExistentTable(segment.clone()).into()) + } + } else { + Ok(input) + } + } + + descend(&mut self.data.root, table_path) + } + + /// Get all sections in the manifest that exist and might contain dependencies. + /// The returned items are always `Table` or `InlineTable`. + pub fn get_sections(&self) -> Vec<(Vec, toml_edit::Item)> { + let mut sections = Vec::new(); + + for dependency_type in &["dev-dependencies", "build-dependencies", "dependencies"] { + // Dependencies can be in the three standard sections... + if self.data[dependency_type].is_table_like() { + sections.push(( + vec![dependency_type.to_string()], + self.data[dependency_type].clone(), + )) + } + + // ... and in `target..(build-/dev-)dependencies`. + let target_sections = self.data + .as_table() + .get("target") + .and_then(toml_edit::Item::as_table_like) + .into_iter() + .flat_map(|t| t.iter()) + .filter_map(|(target_name, target_table)| { + let dependency_table = &target_table[dependency_type]; + dependency_table.as_table_like().map(|_| { + ( + vec![ + "target".to_string(), + target_name.to_string(), + dependency_type.to_string(), + ], + dependency_table.clone(), + ) + }) + }); + + sections.extend(target_sections); + } + + sections + } + + /// Overwrite a file with TOML data. + pub fn write_to_file(&self, file: &mut File) -> CargoResult<()> { + if self.data["package"].is_none() && self.data["project"].is_none() { + if !self.data["workspace"].is_none() { + Err(ErrorKind::UnexpectedRootManifest)?; + } else { + Err(ErrorKind::InvalidManifest)?; + } + } + + let s = self.data.to_string(); + let new_contents_bytes = s.as_bytes(); + + // We need to truncate the file, otherwise the new contents + // will be mixed up with the old ones. + file.set_len(new_contents_bytes.len() as u64) + .chain_err(|| "Failed to truncate Cargo.toml")?; + file.write_all(new_contents_bytes) + .chain_err(|| "Failed to write updated Cargo.toml") + } + + /// Add entry to a Cargo.toml. + pub fn insert_into_table(&mut self, table_path: &[String], dep: &Dependency) -> CargoResult<()> { + let table = self.get_table(table_path)?; + + if table[&dep.name].is_none() { + // insert a new entry + let (ref name, ref mut new_dependency) = dep.to_toml(); + table[name] = new_dependency.clone(); + } else { + // update an existing entry + merge_dependencies(&mut table[&dep.name], dep); + table.as_inline_table_mut().map(|t| t.fmt()); + } + Ok(()) + } + + /// Update an entry in Cargo.toml. + pub fn update_table_entry( + &mut self, + table_path: &[String], + dep: &Dependency, + dry_run: bool, + ) -> CargoResult<()> { + let table = self.get_table(table_path)?; + let new_dep = dep.to_toml().1; + + // If (and only if) there is an old entry, merge the new one in. + if !table[&dep.name].is_none() { + if let Err(e) = print_upgrade_if_necessary(&dep.name, &table[&dep.name], &new_dep) { + eprintln!("Error while displaying upgrade message, {}", e); + } + if !dry_run { + merge_dependencies(&mut table[&dep.name], dep); + table.as_inline_table_mut().map(|t| t.fmt()); + } + } + + Ok(()) + } + + /// Remove entry from a Cargo.toml. + /// + /// # Examples + /// + /// ``` + /// # extern crate cargo_edit; + /// # extern crate toml_edit; + /// # fn main() { + /// use cargo_edit::{Dependency, Manifest}; + /// use toml_edit; + /// + /// let mut manifest = Manifest { data: toml_edit::Document::new() }; + /// let dep = Dependency::new("cargo-edit").set_version("0.1.0"); + /// let _ = manifest.insert_into_table(&vec!["dependencies".to_owned()], &dep); + /// assert!(manifest.remove_from_table("dependencies", &dep.name).is_ok()); + /// assert!(manifest.remove_from_table("dependencies", &dep.name).is_err()); + /// assert!(manifest.data["dependencies"].is_none()); + /// # } + /// ``` + pub fn remove_from_table(&mut self, table: &str, name: &str) -> CargoResult<()> { + if !self.data[table].is_table_like() { + Err(ErrorKind::NonExistentTable(table.into()))?; + } else { + { + let dep = &mut self.data[table][name]; + if dep.is_none() { + Err(ErrorKind::NonExistentDependency(name.into(), table.into()))?; + } + // remove the dependency + *dep = toml_edit::Item::None; + } + + // remove table if empty + if self.data[table].as_table_like().unwrap().is_empty() { + self.data[table] = toml_edit::Item::None; + } + } + Ok(()) + } + + /// Add multiple dependencies to manifest + pub fn add_deps(&mut self, table: &[String], deps: &[Dependency]) -> CargoResult<()> { + deps.iter() + .map(|dep| self.insert_into_table(table, dep)) + .collect::>>()?; + + Ok(()) + } +} + +impl str::FromStr for Manifest { + type Err = CargoError; + + /// Read manifest data from string + fn from_str(input: &str) -> ::std::result::Result { + let d: toml_edit::Document = input.parse().chain_err(|| "Manifest not valid TOML")?; + + Ok(Manifest { data: d }) + } +} + +/// A Cargo manifest that is available locally. +#[derive(Debug)] +pub struct LocalManifest { + /// Path to the manifest + path: PathBuf, + /// Manifest contents + manifest: Manifest, +} + +impl Deref for LocalManifest { + type Target = Manifest; + + fn deref(&self) -> &Manifest { + &self.manifest + } +} + +impl LocalManifest { + /// Construct a `LocalManifest`. If no path is provided, make an educated guess as to which one + /// the user means. + pub fn find(path: &Option) -> CargoResult { + let path = find(path)?; + Self::try_new(&path) + } + + /// Construct the `LocalManifest` corresponding to the `Path` provided. + pub fn try_new(path: &Path) -> CargoResult { + let path = path.to_path_buf(); + Ok(LocalManifest { + manifest: Manifest::open(&Some(path.clone()))?, + path: path, + }) + } + + /// Get the `File` corresponding to this manifest. + fn get_file(&self) -> CargoResult { + Manifest::find_file(&Some(self.path.clone())) + } + + /// Instruct this manifest to upgrade a single dependency. If this manifest does not have that + /// dependency, it does nothing. + pub fn upgrade(&mut self, dependency: &Dependency, dry_run: bool) -> CargoResult<()> { + for (table_path, table) in self.get_sections() { + let table_like = table.as_table_like().expect("Unexpected non-table"); + for (name, _old_value) in table_like.iter() { + if name == dependency.name { + self.manifest + .update_table_entry(&table_path, dependency, dry_run)?; + } + } + } + + let mut file = self.get_file()?; + self.write_to_file(&mut file) + .chain_err(|| "Failed to write new manifest contents") + } +} + +#[cfg(test)] +mod tests { + use util::toml::dependency::Dependency; + use super::*; + use toml_edit; + + #[test] + fn add_remove_dependency() { + let mut manifest = Manifest { + data: toml_edit::Document::new(), + }; + let clone = manifest.clone(); + let dep = Dependency::new("cargo-edit").set_version("0.1.0"); + let _ = manifest.insert_into_table(&["dependencies".to_owned()], &dep); + assert!( + manifest + .remove_from_table("dependencies", &dep.name) + .is_ok() + ); + assert_eq!(manifest.data.to_string(), clone.data.to_string()); + } + + #[test] + fn update_dependency() { + let mut manifest = Manifest { + data: toml_edit::Document::new(), + }; + let dep = Dependency::new("cargo-edit").set_version("0.1.0"); + manifest + .insert_into_table(&["dependencies".to_owned()], &dep) + .unwrap(); + + let new_dep = Dependency::new("cargo-edit").set_version("0.2.0"); + manifest + .update_table_entry(&["dependencies".to_owned()], &new_dep, false) + .unwrap(); + } + + #[test] + fn update_wrong_dependency() { + let mut manifest = Manifest { + data: toml_edit::Document::new(), + }; + let dep = Dependency::new("cargo-edit").set_version("0.1.0"); + manifest + .insert_into_table(&["dependencies".to_owned()], &dep) + .unwrap(); + let original = manifest.clone(); + + let new_dep = Dependency::new("wrong-dep").set_version("0.2.0"); + manifest + .update_table_entry(&["dependencies".to_owned()], &new_dep, false) + .unwrap(); + + assert_eq!(manifest.data.to_string(), original.data.to_string()); + } + + #[test] + fn remove_dependency_no_section() { + let mut manifest = Manifest { + data: toml_edit::Document::new(), + }; + let dep = Dependency::new("cargo-edit").set_version("0.1.0"); + assert!( + manifest + .remove_from_table("dependencies", &dep.name) + .is_err() + ); + } + + #[test] + fn remove_dependency_non_existent() { + let mut manifest = Manifest { + data: toml_edit::Document::new(), + }; + let dep = Dependency::new("cargo-edit").set_version("0.1.0"); + let other_dep = Dependency::new("other-dep").set_version("0.1.0"); + let _ = manifest.insert_into_table(&["dependencies".to_owned()], &other_dep); + assert!( + manifest + .remove_from_table("dependencies", &dep.name) + .is_err() + ); + } +} diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index 9d83668569a..e1910a5b621 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -10,6 +10,7 @@ use serde::de::{self, Deserialize}; use serde::ser; use serde_ignored; use toml; +use toml_edit; use url::Url; use core::dependency::{Kind, Platform}; @@ -23,6 +24,10 @@ use util::errors::{CargoError, CargoResult, CargoResultExt}; use util::paths; use util::{self, Config, ToUrl}; +// mod err; +mod dependency; +mod manifest; + mod targets; use self::targets::targets; From 73294fb6e909618001b3b5d9b779acd83e698dd2 Mon Sep 17 00:00:00 2001 From: ibaryshnikov Date: Tue, 3 Jul 2018 00:07:21 +0300 Subject: [PATCH 06/12] replaced ErrorKind from cargo_edit with format_err --- src/cargo/util/toml/manifest.rs | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/cargo/util/toml/manifest.rs b/src/cargo/util/toml/manifest.rs index 9c3a90b8b75..7ede85b0905 100644 --- a/src/cargo/util/toml/manifest.rs +++ b/src/cargo/util/toml/manifest.rs @@ -46,7 +46,7 @@ fn search(dir: &Path) -> CargoResult { Ok(manifest) } else { dir.parent() - .ok_or_else(|| ErrorKind::MissingManifest.into()) + .ok_or_else(|| format_err!("Unable to find Cargo.toml")) .and_then(|dir| search(dir)) } } @@ -101,7 +101,7 @@ fn print_upgrade_if_necessary( } else if old_dep.is_table_like() { let version = old_dep["version"].clone(); if version.is_none() { - return Err("Missing version field".into()); + Err(format_err!("Missing version field"))?; } version } else { @@ -171,7 +171,7 @@ impl Manifest { if value.is_table_like() { descend(value, &path[1..]) } else { - Err(ErrorKind::NonExistentTable(segment.clone()).into()) + Err(format_err!("The table `{}` could not be found.", segment.clone()).into()) } } else { Ok(input) @@ -226,9 +226,10 @@ impl Manifest { pub fn write_to_file(&self, file: &mut File) -> CargoResult<()> { if self.data["package"].is_none() && self.data["project"].is_none() { if !self.data["workspace"].is_none() { - Err(ErrorKind::UnexpectedRootManifest)?; + Err(format_err!("Found virtual manifest, but this command requires running against an \ + actual package in this workspace."))?; } else { - Err(ErrorKind::InvalidManifest)?; + Err(format_err!("Cargo.toml missing expected `package` or `project` fields"))?; } } @@ -304,12 +305,12 @@ impl Manifest { /// ``` pub fn remove_from_table(&mut self, table: &str, name: &str) -> CargoResult<()> { if !self.data[table].is_table_like() { - Err(ErrorKind::NonExistentTable(table.into()))?; + Err(format_err!("The table `{}` could not be found.", table))?; } else { { let dep = &mut self.data[table][name]; if dep.is_none() { - Err(ErrorKind::NonExistentDependency(name.into(), table.into()))?; + Err(format_err!("The dependency `{}` could not be found in `{}`.", name.into(), table.into()))?; } // remove the dependency *dep = toml_edit::Item::None; From 2449eca09fcfd61fc8ee95760a327538ddcb1426 Mon Sep 17 00:00:00 2001 From: ibaryshnikov Date: Sat, 14 Jul 2018 00:05:32 +0300 Subject: [PATCH 07/12] chain_err usage fixed in manifest.rs --- src/cargo/util/toml/manifest.rs | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/cargo/util/toml/manifest.rs b/src/cargo/util/toml/manifest.rs index 7ede85b0905..2c758dca25b 100644 --- a/src/cargo/util/toml/manifest.rs +++ b/src/cargo/util/toml/manifest.rs @@ -145,6 +145,7 @@ impl Manifest { .write(true) .open(path) .chain_err(|| "Failed to find Cargo.toml") + .map_err(CargoError::from) }) } @@ -155,7 +156,9 @@ impl Manifest { file.read_to_string(&mut data) .chain_err(|| "Failed to read manifest contents")?; - data.parse().chain_err(|| "Unable to parse Cargo.toml") + data.parse() + .chain_err(|| "Unable to parse Cargo.toml") + .map_err(CargoError::from) } /// Get the specified table from the manifest. @@ -239,9 +242,12 @@ impl Manifest { // We need to truncate the file, otherwise the new contents // will be mixed up with the old ones. file.set_len(new_contents_bytes.len() as u64) - .chain_err(|| "Failed to truncate Cargo.toml")?; + .chain_err(|| "Failed to truncate Cargo.toml") + .map_err(CargoError::from)?; file.write_all(new_contents_bytes) .chain_err(|| "Failed to write updated Cargo.toml") + .map_err(CargoError::from)?; + Ok(()) } /// Add entry to a Cargo.toml. @@ -310,7 +316,7 @@ impl Manifest { { let dep = &mut self.data[table][name]; if dep.is_none() { - Err(format_err!("The dependency `{}` could not be found in `{}`.", name.into(), table.into()))?; + Err(format_err!("The dependency `{}` could not be found in `{}`.", name, table))?; } // remove the dependency *dep = toml_edit::Item::None; @@ -400,6 +406,7 @@ impl LocalManifest { let mut file = self.get_file()?; self.write_to_file(&mut file) .chain_err(|| "Failed to write new manifest contents") + .map_err(CargoError::from) } } From 833bbcb20bf6a1a4cf33bfaeca7230338782643c Mon Sep 17 00:00:00 2001 From: ibaryshnikov Date: Sat, 14 Jul 2018 01:13:17 +0300 Subject: [PATCH 08/12] errors simplified in manifest.rs --- src/cargo/util/toml/manifest.rs | 40 +++++++++++++++------------------ 1 file changed, 18 insertions(+), 22 deletions(-) diff --git a/src/cargo/util/toml/manifest.rs b/src/cargo/util/toml/manifest.rs index 2c758dca25b..c7bb4bfeaff 100644 --- a/src/cargo/util/toml/manifest.rs +++ b/src/cargo/util/toml/manifest.rs @@ -101,7 +101,7 @@ fn print_upgrade_if_necessary( } else if old_dep.is_table_like() { let version = old_dep["version"].clone(); if version.is_none() { - Err(format_err!("Missing version field"))?; + bail!("Missing version field"); } version } else { @@ -174,7 +174,7 @@ impl Manifest { if value.is_table_like() { descend(value, &path[1..]) } else { - Err(format_err!("The table `{}` could not be found.", segment.clone()).into()) + bail!("The table `{}` could not be found.", segment); } } else { Ok(input) @@ -228,12 +228,10 @@ impl Manifest { /// Overwrite a file with TOML data. pub fn write_to_file(&self, file: &mut File) -> CargoResult<()> { if self.data["package"].is_none() && self.data["project"].is_none() { - if !self.data["workspace"].is_none() { - Err(format_err!("Found virtual manifest, but this command requires running against an \ - actual package in this workspace."))?; - } else { - Err(format_err!("Cargo.toml missing expected `package` or `project` fields"))?; - } + ensure!(self.data["workspace"].is_none(), + "Found virtual manifest, but this command requires running against an \ + actual package in this workspace."); + bail!("Cargo.toml missing expected `package` or `project` fields"); } let s = self.data.to_string(); @@ -310,22 +308,20 @@ impl Manifest { /// # } /// ``` pub fn remove_from_table(&mut self, table: &str, name: &str) -> CargoResult<()> { - if !self.data[table].is_table_like() { - Err(format_err!("The table `{}` could not be found.", table))?; - } else { - { - let dep = &mut self.data[table][name]; - if dep.is_none() { - Err(format_err!("The dependency `{}` could not be found in `{}`.", name, table))?; - } - // remove the dependency - *dep = toml_edit::Item::None; + ensure!(self.data[table].is_table_like(), + "The table `{}` could not be found.", table); + { + let dep = &mut self.data[table][name]; + if dep.is_none() { + bail!("The dependency `{}` could not be found in `{}`.", name, table); } + // remove the dependency + *dep = toml_edit::Item::None; + } - // remove table if empty - if self.data[table].as_table_like().unwrap().is_empty() { - self.data[table] = toml_edit::Item::None; - } + // remove table if empty + if self.data[table].as_table_like().unwrap().is_empty() { + self.data[table] = toml_edit::Item::None; } Ok(()) } From 69e01769cb4b97dcb9d39ec8411df469a6e63ff1 Mon Sep 17 00:00:00 2001 From: ibaryshnikov Date: Sun, 16 Sep 2018 16:32:11 +0300 Subject: [PATCH 09/12] issue-5586 package selection implemented in 'cargo add' subcommand --- src/bin/cargo/commands/add.rs | 2 + src/cargo/ops/cargo_add.rs | 189 +++++++++++++++++++++++++++++++++- 2 files changed, 186 insertions(+), 5 deletions(-) diff --git a/src/bin/cargo/commands/add.rs b/src/bin/cargo/commands/add.rs index 946753d9a2c..7b9d2f3ddbe 100644 --- a/src/bin/cargo/commands/add.rs +++ b/src/bin/cargo/commands/add.rs @@ -33,6 +33,7 @@ dependencies (version set to \"*\").", pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult { let ws = args.workspace(config)?; + let compile_opts = args.compile_options(config, CompileMode::Build)?; println!("cargo add subcommand executed"); @@ -66,6 +67,7 @@ pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult { krate, &source, version, + &compile_opts, )?; Ok(()) diff --git a/src/cargo/ops/cargo_add.rs b/src/cargo/ops/cargo_add.rs index b6235360d6e..fe00f20feb4 100644 --- a/src/cargo/ops/cargo_add.rs +++ b/src/cargo/ops/cargo_add.rs @@ -1,13 +1,192 @@ +use semver::{Version, VersionReq}; -use core::{SourceId, Workspace}; +use core::{Dependency, Package, Source, SourceId, Workspace}; +use ops; +use sources::{SourceConfigMap}; +use util::{Config}; use util::errors::{CargoResult}; pub fn add( - _ws: &Workspace, - _krate: &str, - _source_id: &SourceId, - _vers: Option<&str>, + ws: &Workspace, + krate: &str, + source_id: &SourceId, + vers: Option<&str>, + opts: &ops::CompileOptions, ) -> CargoResult<()> { + // println!("ws is {:?}", ws); + println!("sourceid is {:?}", source_id); + println!("vers are {:?}", vers); + + let map = SourceConfigMap::new(opts.config)?; + + let needs_update = true; + + let (pkg, source) = select_pkg( + map.load(source_id)?, + Some(krate), + vers, + opts.config, + needs_update, + &mut |_| { + bail!( + "must specify a crate to install from \ + crates.io, or use --path or --git to \ + specify alternate source" + ) + }, + )?; + println!("pkg {:?}", pkg); Ok(()) } + +fn select_pkg<'a, T>( + mut source: T, + name: Option<&str>, + vers: Option<&str>, + config: &Config, + needs_update: bool, + list_all: &mut FnMut(&mut T) -> CargoResult>, +) -> CargoResult<(Package, Box)> +where + T: Source + 'a, +{ + if needs_update { + source.update()?; + } + + match name { + Some(name) => { + let vers = match vers { + Some(v) => { + // If the version begins with character <, >, =, ^, ~ parse it as a + // version range, otherwise parse it as a specific version + let first = v.chars() + .nth(0) + .ok_or_else(|| format_err!("no version provided for the `--vers` flag"))?; + + match first { + '<' | '>' | '=' | '^' | '~' => match v.parse::() { + Ok(v) => Some(v.to_string()), + Err(_) => bail!( + "the `--vers` provided, `{}`, is \ + not a valid semver version requirement\n\n + Please have a look at \ + http://doc.crates.io/specifying-dependencies.html \ + for the correct format", + v + ), + }, + _ => match v.parse::() { + Ok(v) => Some(format!("={}", v)), + Err(_) => { + let mut msg = format!( + "\ + the `--vers` provided, `{}`, is \ + not a valid semver version\n\n\ + historically Cargo treated this \ + as a semver version requirement \ + accidentally\nand will continue \ + to do so, but this behavior \ + will be removed eventually", + v + ); + + // If it is not a valid version but it is a valid version + // requirement, add a note to the warning + if v.parse::().is_ok() { + msg.push_str(&format!( + "\nif you want to specify semver range, \ + add an explicit qualifier, like ^{}", + v + )); + } + config.shell().warn(&msg)?; + Some(v.to_string()) + } + }, + } + } + None => None, + }; + let vers = vers.as_ref().map(|s| &**s); + let vers_spec = if vers.is_none() && source.source_id().is_registry() { + // Avoid pre-release versions from crate.io + // unless explicitly asked for + Some("*") + } else { + vers + }; + let dep = Dependency::parse_no_deprecated( + name, + vers_spec, + source.source_id(), + )?; + let deps = source.query_vec(&dep)?; + match deps.iter().map(|p| p.package_id()).max() { + Some(pkgid) => { + let pkg = source.download(pkgid)?; + Ok((pkg, Box::new(source))) + } + None => { + let vers_info = vers.map(|v| format!(" with version `{}`", v)) + .unwrap_or_default(); + Err(format_err!( + "could not find `{}` in {}{}", + name, + source.source_id(), + vers_info + )) + } + } + } + None => { + let candidates = list_all(&mut source)?; + let binaries = candidates + .iter() + .filter(|cand| cand.targets().iter().filter(|t| t.is_bin()).count() > 0); + let examples = candidates + .iter() + .filter(|cand| cand.targets().iter().filter(|t| t.is_example()).count() > 0); + let pkg = match one(binaries, |v| multi_err("binaries", v))? { + Some(p) => p, + None => match one(examples, |v| multi_err("examples", v))? { + Some(p) => p, + None => bail!( + "no packages found with binaries or \ + examples" + ), + }, + }; + return Ok((pkg.clone(), Box::new(source))); + + fn multi_err(kind: &str, mut pkgs: Vec<&Package>) -> String { + pkgs.sort_unstable_by_key(|a| a.name()); + format!( + "multiple packages with {} found: {}", + kind, + pkgs.iter() + .map(|p| p.name().as_str()) + .collect::>() + .join(", ") + ) + } + } + } +} + +fn one(mut i: I, f: F) -> CargoResult> +where + I: Iterator, + F: FnOnce(Vec) -> String, +{ + match (i.next(), i.next()) { + (Some(i1), Some(i2)) => { + let mut v = vec![i1, i2]; + v.extend(i); + Err(format_err!("{}", f(v))) + } + (Some(i), None) => Ok(Some(i)), + (None, _) => Ok(None), + } +} From 3786eef7f12fce1cec773227118f14e88cce0512 Mon Sep 17 00:00:00 2001 From: ibaryshnikov Date: Thu, 20 Sep 2018 00:29:55 +0300 Subject: [PATCH 10/12] issue-5586 first working example implemented --- src/cargo/ops/cargo_add.rs | 172 ++++---------------------------- src/cargo/ops/cargo_install.rs | 2 +- src/cargo/util/toml/manifest.rs | 2 +- src/cargo/util/toml/mod.rs | 5 +- 4 files changed, 21 insertions(+), 160 deletions(-) diff --git a/src/cargo/ops/cargo_add.rs b/src/cargo/ops/cargo_add.rs index fe00f20feb4..e4fbab1ac89 100644 --- a/src/cargo/ops/cargo_add.rs +++ b/src/cargo/ops/cargo_add.rs @@ -1,11 +1,9 @@ -use semver::{Version, VersionReq}; +use core::{SourceId, Workspace}; -use core::{Dependency, Package, Source, SourceId, Workspace}; - -use ops; +use ops::{self, cargo_install}; use sources::{SourceConfigMap}; -use util::{Config}; use util::errors::{CargoResult}; +use util::toml; pub fn add( ws: &Workspace, @@ -14,15 +12,14 @@ pub fn add( vers: Option<&str>, opts: &ops::CompileOptions, ) -> CargoResult<()> { - // println!("ws is {:?}", ws); - println!("sourceid is {:?}", source_id); - println!("vers are {:?}", vers); + let cwd = ws.config().cwd(); + println!("cwd is {:?}", cwd); let map = SourceConfigMap::new(opts.config)?; let needs_update = true; - let (pkg, source) = select_pkg( + let (pkg, _source) = cargo_install::select_pkg( map.load(source_id)?, Some(krate), vers, @@ -37,156 +34,21 @@ pub fn add( }, )?; println!("pkg {:?}", pkg); - Ok(()) -} + let manifest_path = Some(toml::manifest::find(&Some(cwd.to_path_buf()))?); + let mut manifest = toml::manifest::Manifest::open(&manifest_path)?; -fn select_pkg<'a, T>( - mut source: T, - name: Option<&str>, - vers: Option<&str>, - config: &Config, - needs_update: bool, - list_all: &mut FnMut(&mut T) -> CargoResult>, -) -> CargoResult<(Package, Box)> -where - T: Source + 'a, -{ - if needs_update { - source.update()?; - } + let dependency = toml::dependency::Dependency::new(&krate) + .set_version(&format!("{}", pkg.version())); - match name { - Some(name) => { - let vers = match vers { - Some(v) => { - // If the version begins with character <, >, =, ^, ~ parse it as a - // version range, otherwise parse it as a specific version - let first = v.chars() - .nth(0) - .ok_or_else(|| format_err!("no version provided for the `--vers` flag"))?; + println!("dependency is {:?}", dependency); - match first { - '<' | '>' | '=' | '^' | '~' => match v.parse::() { - Ok(v) => Some(v.to_string()), - Err(_) => bail!( - "the `--vers` provided, `{}`, is \ - not a valid semver version requirement\n\n - Please have a look at \ - http://doc.crates.io/specifying-dependencies.html \ - for the correct format", - v - ), - }, - _ => match v.parse::() { - Ok(v) => Some(format!("={}", v)), - Err(_) => { - let mut msg = format!( - "\ - the `--vers` provided, `{}`, is \ - not a valid semver version\n\n\ - historically Cargo treated this \ - as a semver version requirement \ - accidentally\nand will continue \ - to do so, but this behavior \ - will be removed eventually", - v - ); + manifest.insert_into_table(&get_section(), &dependency)?; - // If it is not a valid version but it is a valid version - // requirement, add a note to the warning - if v.parse::().is_ok() { - msg.push_str(&format!( - "\nif you want to specify semver range, \ - add an explicit qualifier, like ^{}", - v - )); - } - config.shell().warn(&msg)?; - Some(v.to_string()) - } - }, - } - } - None => None, - }; - let vers = vers.as_ref().map(|s| &**s); - let vers_spec = if vers.is_none() && source.source_id().is_registry() { - // Avoid pre-release versions from crate.io - // unless explicitly asked for - Some("*") - } else { - vers - }; - let dep = Dependency::parse_no_deprecated( - name, - vers_spec, - source.source_id(), - )?; - let deps = source.query_vec(&dep)?; - match deps.iter().map(|p| p.package_id()).max() { - Some(pkgid) => { - let pkg = source.download(pkgid)?; - Ok((pkg, Box::new(source))) - } - None => { - let vers_info = vers.map(|v| format!(" with version `{}`", v)) - .unwrap_or_default(); - Err(format_err!( - "could not find `{}` in {}{}", - name, - source.source_id(), - vers_info - )) - } - } - } - None => { - let candidates = list_all(&mut source)?; - let binaries = candidates - .iter() - .filter(|cand| cand.targets().iter().filter(|t| t.is_bin()).count() > 0); - let examples = candidates - .iter() - .filter(|cand| cand.targets().iter().filter(|t| t.is_example()).count() > 0); - let pkg = match one(binaries, |v| multi_err("binaries", v))? { - Some(p) => p, - None => match one(examples, |v| multi_err("examples", v))? { - Some(p) => p, - None => bail!( - "no packages found with binaries or \ - examples" - ), - }, - }; - return Ok((pkg.clone(), Box::new(source))); - - fn multi_err(kind: &str, mut pkgs: Vec<&Package>) -> String { - pkgs.sort_unstable_by_key(|a| a.name()); - format!( - "multiple packages with {} found: {}", - kind, - pkgs.iter() - .map(|p| p.name().as_str()) - .collect::>() - .join(", ") - ) - } - } - } + let mut file = toml::manifest::Manifest::find_file(&manifest_path)?; + manifest.write_to_file(&mut file)?; + Ok(()) } -fn one(mut i: I, f: F) -> CargoResult> -where - I: Iterator, - F: FnOnce(Vec) -> String, -{ - match (i.next(), i.next()) { - (Some(i1), Some(i2)) => { - let mut v = vec![i1, i2]; - v.extend(i); - Err(format_err!("{}", f(v))) - } - (Some(i), None) => Ok(Some(i)), - (None, _) => Ok(None), - } +fn get_section() -> Vec { + vec!["dependencies".to_owned()] } diff --git a/src/cargo/ops/cargo_install.rs b/src/cargo/ops/cargo_install.rs index 60e6c30ed8d..8ca81fe9d7a 100644 --- a/src/cargo/ops/cargo_install.rs +++ b/src/cargo/ops/cargo_install.rs @@ -416,7 +416,7 @@ fn path_source<'a>(source_id: &SourceId, config: &'a Config) -> CargoResult( +pub fn select_pkg<'a, T>( mut source: T, name: Option<&str>, vers: Option<&str>, diff --git a/src/cargo/util/toml/manifest.rs b/src/cargo/util/toml/manifest.rs index c7bb4bfeaff..fd265491d2c 100644 --- a/src/cargo/util/toml/manifest.rs +++ b/src/cargo/util/toml/manifest.rs @@ -19,7 +19,7 @@ pub struct Manifest { pub data: toml_edit::Document, } -/// If a manifest is specified, return that one, otherise perform a manifest search starting from +/// If a manifest is specified, return that one, otherwise perform a manifest search starting from /// the current directory. /// If a manifest is specified, return that one. If a path is specified, perform a manifest search /// starting from there. If nothing is specified, start searching from the current directory diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index e1910a5b621..a73b12b5eab 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -10,7 +10,6 @@ use serde::de::{self, Deserialize}; use serde::ser; use serde_ignored; use toml; -use toml_edit; use url::Url; use core::dependency::{Kind, Platform}; @@ -25,8 +24,8 @@ use util::paths; use util::{self, Config, ToUrl}; // mod err; -mod dependency; -mod manifest; +pub mod dependency; +pub mod manifest; mod targets; use self::targets::targets; From a730a2a36d0bde1c2b67da43e6c41a293a9a0d97 Mon Sep 17 00:00:00 2001 From: ibaryshnikov Date: Mon, 24 Sep 2018 01:03:16 +0300 Subject: [PATCH 11/12] issue-5586 allowed to add multiple dependencies, moved source_id extraction to a separate function --- src/bin/cargo/commands/add.rs | 33 +++++++----------------- src/bin/cargo/commands/install.rs | 42 +++++++++++++++++-------------- src/cargo/ops/cargo_add.rs | 40 +++++++++++++++++++++++------ 3 files changed, 65 insertions(+), 50 deletions(-) diff --git a/src/bin/cargo/commands/add.rs b/src/bin/cargo/commands/add.rs index 7b9d2f3ddbe..49f4572b4d4 100644 --- a/src/bin/cargo/commands/add.rs +++ b/src/bin/cargo/commands/add.rs @@ -1,13 +1,13 @@ use command_prelude::*; -use cargo::core::{GitReference, SourceId}; use cargo::ops; -use cargo::util::ToUrl; + +use super::install; pub fn cli() -> App { subcommand("add") .about("Add a new dependency") - .arg(Arg::with_name("crate").empty_values(false)) + .arg(Arg::with_name("crate").empty_values(false).multiple(true)) .arg( opt("version", "Specify a version to add from crates.io") .alias("vers") @@ -37,34 +37,19 @@ pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult { println!("cargo add subcommand executed"); - let krate = args.value_of("crate") - .unwrap_or_default(); + let krates = args.values_of("crate") + .unwrap_or_default() + .collect::>(); - println!("crate {:?}", krate); + println!("crate {:?}", krates); - let source = if let Some(url) = args.value_of("git") { - let url = url.to_url()?; - let gitref = if let Some(branch) = args.value_of("branch") { - GitReference::Branch(branch.to_string()) - } else if let Some(tag) = args.value_of("tag") { - GitReference::Tag(tag.to_string()) - } else if let Some(rev) = args.value_of("rev") { - GitReference::Rev(rev.to_string()) - } else { - GitReference::Branch("master".to_string()) - }; - SourceId::for_git(&url, gitref)? - } else if let Some(path) = args.value_of_path("path", config) { - SourceId::for_path(&path)? - } else { - SourceId::crates_io(config)? - }; + let (_from_cwd, source) = install::get_source_id(&config, &args, &krates)?; let version = args.value_of("version"); ops::add( &ws, - krate, + krates, &source, version, &compile_opts, diff --git a/src/bin/cargo/commands/install.rs b/src/bin/cargo/commands/install.rs index 6f6fd627648..e347a58a1b7 100644 --- a/src/bin/cargo/commands/install.rs +++ b/src/bin/cargo/commands/install.rs @@ -86,8 +86,29 @@ pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult { .unwrap_or_default() .collect::>(); - let mut from_cwd = false; + let (from_cwd, source) = get_source_id(&config, &args, &krates)?; + + let version = args.value_of("version"); + let root = args.value_of("root"); + + if args.is_present("list") { + ops::install_list(root, config)?; + } else { + ops::install( + root, + krates, + &source, + from_cwd, + version, + &compile_opts, + args.is_present("force"), + )?; + } + Ok(()) +} +pub fn get_source_id(config: &Config, args: &ArgMatches, krates: &Vec<&str>) -> Result<(bool, SourceId), CliError> { + let mut from_cwd = false; let source = if let Some(url) = args.value_of("git") { let url = url.to_url()?; let gitref = if let Some(branch) = args.value_of("branch") { @@ -108,22 +129,5 @@ pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult { } else { SourceId::crates_io(config)? }; - - let version = args.value_of("version"); - let root = args.value_of("root"); - - if args.is_present("list") { - ops::install_list(root, config)?; - } else { - ops::install( - root, - krates, - &source, - from_cwd, - version, - &compile_opts, - args.is_present("force"), - )?; - } - Ok(()) + Ok((from_cwd, source)) } diff --git a/src/cargo/ops/cargo_add.rs b/src/cargo/ops/cargo_add.rs index e4fbab1ac89..b54ca920269 100644 --- a/src/cargo/ops/cargo_add.rs +++ b/src/cargo/ops/cargo_add.rs @@ -7,7 +7,7 @@ use util::toml; pub fn add( ws: &Workspace, - krate: &str, + krates: Vec<&str>, source_id: &SourceId, vers: Option<&str>, opts: &ops::CompileOptions, @@ -17,8 +17,38 @@ pub fn add( let map = SourceConfigMap::new(opts.config)?; - let needs_update = true; + let manifest_path = Some(toml::manifest::find(&Some(cwd.to_path_buf()))?); + let mut manifest = toml::manifest::Manifest::open(&manifest_path)?; + + let mut needs_update = true; + for krate in krates { + add_one( + &map, + krate, + source_id, + vers, + opts, + needs_update, + &mut manifest, + )?; + needs_update = false; + } + let mut file = toml::manifest::Manifest::find_file(&manifest_path)?; + manifest.write_to_file(&mut file)?; + + Ok(()) +} + +fn add_one( + map: &SourceConfigMap, + krate: &str, + source_id: &SourceId, + vers: Option<&str>, + opts: &ops::CompileOptions, + needs_update: bool, + manifest: &mut toml::manifest::Manifest, + ) -> CargoResult<()> { let (pkg, _source) = cargo_install::select_pkg( map.load(source_id)?, Some(krate), @@ -33,9 +63,7 @@ pub fn add( ) }, )?; - println!("pkg {:?}", pkg); - let manifest_path = Some(toml::manifest::find(&Some(cwd.to_path_buf()))?); - let mut manifest = toml::manifest::Manifest::open(&manifest_path)?; + println!("pkg {:?}", pkg); let dependency = toml::dependency::Dependency::new(&krate) .set_version(&format!("{}", pkg.version())); @@ -44,8 +72,6 @@ pub fn add( manifest.insert_into_table(&get_section(), &dependency)?; - let mut file = toml::manifest::Manifest::find_file(&manifest_path)?; - manifest.write_to_file(&mut file)?; Ok(()) } From 3f8075cb5d8d0ef3f6eb219d0196bc9eb3326ee2 Mon Sep 17 00:00:00 2001 From: ibaryshnikov Date: Mon, 24 Sep 2018 23:27:03 +0300 Subject: [PATCH 12/12] issue-5586 both git and version can be specified --- src/cargo/ops/cargo_add.rs | 36 +++++++++++++++---- src/cargo/util/toml/dependency.rs | 57 ++++++++++++++++++++++++------- 2 files changed, 75 insertions(+), 18 deletions(-) diff --git a/src/cargo/ops/cargo_add.rs b/src/cargo/ops/cargo_add.rs index b54ca920269..de9dc442b1c 100644 --- a/src/cargo/ops/cargo_add.rs +++ b/src/cargo/ops/cargo_add.rs @@ -1,7 +1,7 @@ use core::{SourceId, Workspace}; use ops::{self, cargo_install}; -use sources::{SourceConfigMap}; +use sources::{GitSource, SourceConfigMap}; use util::errors::{CargoResult}; use util::toml; @@ -49,7 +49,23 @@ fn add_one( needs_update: bool, manifest: &mut toml::manifest::Manifest, ) -> CargoResult<()> { - let (pkg, _source) = cargo_install::select_pkg( + let (pkg, _source) = if source_id.is_git() { + cargo_install::select_pkg( + GitSource::new(source_id, opts.config)?, + Some(krate), + vers, + opts.config, + needs_update, + &mut |_| { + bail!( + "must specify a crate to install from \ + crates.io, or use --path or --git to \ + specify alternate source" + ) + }, + )? + } else { + cargo_install::select_pkg( map.load(source_id)?, Some(krate), vers, @@ -62,12 +78,20 @@ fn add_one( specify alternate source" ) }, - )?; - println!("pkg {:?}", pkg); + )? + }; + println!("pkg {:?}", pkg); + let package_id = pkg.package_id(); + println!("package_id is {:?}", package_id); + + let mut dependency = toml::dependency::Dependency::new(&krate) + .set_version(&format!("{}", package_id.version())); - let dependency = toml::dependency::Dependency::new(&krate) - .set_version(&format!("{}", pkg.version())); + if source_id.is_git() { + dependency = dependency.set_path(&format!("{}", package_id.source_id())); + } + println!("is git {}", source_id.is_git()); println!("dependency is {:?}", dependency); manifest.insert_into_table(&get_section(), &dependency)?; diff --git a/src/cargo/util/toml/dependency.rs b/src/cargo/util/toml/dependency.rs index d6566fdd930..c40878b38a8 100644 --- a/src/cargo/util/toml/dependency.rs +++ b/src/cargo/util/toml/dependency.rs @@ -2,9 +2,11 @@ use toml_edit; #[derive(Debug, Hash, PartialEq, Eq, Clone)] enum DependencySource { - Version(String), + Version { + version: Option, + path: Option, + }, Git(String), - Path(String), } /// A dependency handled by Cargo @@ -21,7 +23,10 @@ impl Default for Dependency { Dependency { name: "".into(), optional: false, - source: DependencySource::Version("0.1.0".into()), + source: DependencySource::Version { + version: None, + path: None, + }, } } } @@ -37,7 +42,15 @@ impl Dependency { /// Set dependency to a given version pub fn set_version(mut self, version: &str) -> Dependency { - self.source = DependencySource::Version(version.into()); + let old_source = self.source; + let old_path = match old_source { + DependencySource::Version { path, .. } => path, + _ => None, + }; + self.source = DependencySource::Version { + version: Some(version.into()), + path: old_path, + }; self } @@ -49,7 +62,15 @@ impl Dependency { /// Set dependency to a given path pub fn set_path(mut self, path: &str) -> Dependency { - self.source = DependencySource::Path(path.into()); + let old_source = self.source; + let old_version = match old_source { + DependencySource::Version { version, .. } => version, + _ => None, + }; + self.source = DependencySource::Version { + version: old_version, + path: Some(path.into()), + }; self } @@ -61,7 +82,11 @@ impl Dependency { /// Get version of dependency pub fn version(&self) -> Option<&str> { - if let DependencySource::Version(ref version) = self.source { + if let DependencySource::Version { + version: Some(ref version), + .. + } = self.source + { Some(version) } else { None @@ -76,21 +101,29 @@ impl Dependency { pub fn to_toml(&self) -> (String, toml_edit::Item) { let data: toml_edit::Item = match (self.optional, self.source.clone()) { // Extra short when version flag only - (false, DependencySource::Version(v)) => toml_edit::value(v), + ( + false, + DependencySource::Version { + version: Some(v), + path: None, + }, + ) => toml_edit::value(v), // Other cases are represented as an inline table (optional, source) => { let mut data = toml_edit::InlineTable::default(); match source { - DependencySource::Version(v) => { - data.get_or_insert("version", v); + DependencySource::Version { version, path } => { + if let Some(v) = version { + data.get_or_insert("version", v); + } + if let Some(p) = path { + data.get_or_insert("path", p); + } } DependencySource::Git(v) => { data.get_or_insert("git", v); } - DependencySource::Path(v) => { - data.get_or_insert("path", v); - } } if self.optional { data.get_or_insert("optional", optional);