From: Alex Crichton Date: Tue, 6 Mar 2018 02:24:36 +0000 (-0800) Subject: Remove `Source::for_path` X-Git-Tag: archive/raspbian/0.35.0-2+rpi1~3^2^2^2^2^2^2^2~22^2~2^2~38^2~1 X-Git-Url: https://dgit.raspbian.org/?a=commitdiff_plain;h=9659f5603276ae883e92b202b6d2954b3354e8da;p=cargo.git Remove `Source::for_path` This commit removes the `Source::for_path` constructor in favor of `Workspace::load`. This prevents re-parsing manifests multiple times as Cargo loads up as this can sometimes be an expensive operation. Instead the `Workspace` now retains a cache of packages that can be updated as it goes along. Finally, this should mean that we're only parsing path dependencies at most once rather than multiple times. --- diff --git a/src/bin/commands/read_manifest.rs b/src/bin/commands/read_manifest.rs index 5365b5bac..1e54c79e8 100644 --- a/src/bin/commands/read_manifest.rs +++ b/src/bin/commands/read_manifest.rs @@ -1,6 +1,5 @@ use command_prelude::*; -use cargo::core::Package; use cargo::print_json; pub fn cli() -> App { @@ -13,8 +12,7 @@ Print a JSON representation of a Cargo.toml manifest.", } pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult { - let root = args.root_manifest(config)?; - let pkg = Package::for_path(&root, config)?; - print_json(&pkg); + let ws = args.workspace(config)?; + print_json(&ws.current()?); Ok(()) } diff --git a/src/cargo/core/package.rs b/src/cargo/core/package.rs index d66a89554..f61ee5277 100644 --- a/src/cargo/core/package.rs +++ b/src/cargo/core/package.rs @@ -81,14 +81,6 @@ impl Package { } } - /// Calculate the Package from the manifest path (and cargo configuration). - pub fn for_path(manifest_path: &Path, config: &Config) -> CargoResult { - let path = manifest_path.parent().unwrap(); - let source_id = SourceId::for_path(path)?; - let (pkg, _) = ops::read_package(manifest_path, &source_id, config)?; - Ok(pkg) - } - /// Get the manifest dependencies pub fn dependencies(&self) -> &[Dependency] { self.manifest.dependencies() diff --git a/src/cargo/core/resolver/encode.rs b/src/cargo/core/resolver/encode.rs index 63cd778db..ee172416f 100644 --- a/src/cargo/core/resolver/encode.rs +++ b/src/cargo/core/resolver/encode.rs @@ -188,7 +188,7 @@ impl EncodableResolve { } } -fn build_path_deps(ws: &Workspace) -> HashMap { +fn build_path_deps(ws: &Workspace) -> (HashMap) { // If a crate is *not* a path source, then we're probably in a situation // such as `cargo install` with a lock file from a remote dependency. In // that case we don't need to fixup any path dependencies (as they're not @@ -207,33 +207,33 @@ fn build_path_deps(ws: &Workspace) -> HashMap { visited.insert(member.package_id().source_id().clone()); } for member in members.iter() { - build_pkg(member, ws.config(), &mut ret, &mut visited); + build_pkg(member, ws, &mut ret, &mut visited); } for deps in ws.root_patch().values() { for dep in deps { - build_dep(dep, ws.config(), &mut ret, &mut visited); + build_dep(dep, ws, &mut ret, &mut visited); } } for &(_, ref dep) in ws.root_replace() { - build_dep(dep, ws.config(), &mut ret, &mut visited); + build_dep(dep, ws, &mut ret, &mut visited); } return ret; fn build_pkg( pkg: &Package, - config: &Config, + ws: &Workspace, ret: &mut HashMap, visited: &mut HashSet, ) { for dep in pkg.dependencies() { - build_dep(dep, config, ret, visited); + build_dep(dep, ws, ret, visited); } } fn build_dep( dep: &Dependency, - config: &Config, + ws: &Workspace, ret: &mut HashMap, visited: &mut HashSet, ) { @@ -245,13 +245,13 @@ fn build_path_deps(ws: &Workspace) -> HashMap { Ok(p) => p.join("Cargo.toml"), Err(_) => return, }; - let pkg = match Package::for_path(&path, config) { + let pkg = match ws.load(&path) { Ok(p) => p, Err(_) => return, }; ret.insert(pkg.name().to_string(), pkg.package_id().source_id().clone()); visited.insert(pkg.package_id().source_id().clone()); - build_pkg(&pkg, config, ret, visited); + build_pkg(&pkg, ws, ret, visited); } } diff --git a/src/cargo/core/workspace.rs b/src/cargo/core/workspace.rs index d6ba3594d..38be0ea29 100644 --- a/src/cargo/core/workspace.rs +++ b/src/cargo/core/workspace.rs @@ -1,17 +1,19 @@ -use std::collections::hash_map::{Entry, HashMap}; +use std::cell::RefCell; use std::collections::BTreeMap; +use std::collections::hash_map::{Entry, HashMap}; use std::path::{Path, PathBuf}; use std::slice; use glob::glob; use url::Url; -use core::{EitherManifest, Package, SourceId, VirtualManifest}; use core::{Dependency, PackageIdSpec, Profile, Profiles}; -use util::{Config, Filesystem}; +use core::{EitherManifest, Package, SourceId, VirtualManifest}; +use ops; use util::errors::{CargoResult, CargoResultExt}; use util::paths; use util::toml::read_manifest; +use util::{Config, Filesystem}; /// The core abstraction in Cargo for working with a workspace of crates. /// @@ -67,6 +69,8 @@ pub struct Workspace<'cfg> { // needed by the current configuration (such as in cargo install). In some // cases `false` also results in the non-enforcement of dev-dependencies. require_optional_deps: bool, + + loaded_packages: RefCell>, } // Separate structure for tracking loaded packages (to avoid loading anything @@ -137,6 +141,7 @@ impl<'cfg> Workspace<'cfg> { default_members: Vec::new(), is_ephemeral: false, require_optional_deps: true, + loaded_packages: RefCell::new(HashMap::new()), }; ws.root_manifest = ws.find_root(manifest_path)?; ws.find_members()?; @@ -172,6 +177,7 @@ impl<'cfg> Workspace<'cfg> { default_members: Vec::new(), is_ephemeral: true, require_optional_deps, + loaded_packages: RefCell::new(HashMap::new()), }; { let key = ws.current_manifest.parent().unwrap(); @@ -669,11 +675,32 @@ impl<'cfg> Workspace<'cfg> { Ok(()) } + + pub fn load(&self, manifest_path: &Path) -> CargoResult { + match self.packages.maybe_get(manifest_path) { + Some(&MaybePackage::Package(ref p)) => return Ok(p.clone()), + Some(&MaybePackage::Virtual(_)) => bail!("cannot load workspace root"), + None => {} + } + + let mut loaded = self.loaded_packages.borrow_mut(); + if let Some(p) = loaded.get(manifest_path).cloned() { + return Ok(p); + } + let source_id = SourceId::for_path(manifest_path.parent().unwrap())?; + let (package, _nested_paths) = ops::read_package(manifest_path, &source_id, self.config)?; + loaded.insert(manifest_path.to_path_buf(), package.clone()); + Ok(package) + } } impl<'cfg> Packages<'cfg> { fn get(&self, manifest_path: &Path) -> &MaybePackage { - &self.packages[manifest_path.parent().unwrap()] + self.maybe_get(manifest_path).unwrap() + } + + fn maybe_get(&self, manifest_path: &Path) -> Option<&MaybePackage> { + self.packages.get(manifest_path.parent().unwrap()) } fn load(&mut self, manifest_path: &Path) -> CargoResult<&MaybePackage> { diff --git a/src/cargo/ops/registry.rs b/src/cargo/ops/registry.rs index cc92dda73..60e4641c1 100644 --- a/src/cargo/ops/registry.rs +++ b/src/cargo/ops/registry.rs @@ -447,8 +447,8 @@ pub fn modify_owners(config: &Config, opts: &OwnersOptions) -> CargoResult<()> { Some(ref name) => name.clone(), None => { let manifest_path = find_root_manifest_for_wd(None, config.cwd())?; - let pkg = Package::for_path(&manifest_path, config)?; - pkg.name().to_string() + let ws = Workspace::new(&manifest_path, config)?; + ws.current()?.package_id().name().to_string() } }; @@ -508,8 +508,8 @@ pub fn yank( Some(name) => name, None => { let manifest_path = find_root_manifest_for_wd(None, config.cwd())?; - let pkg = Package::for_path(&manifest_path, config)?; - pkg.name().to_string() + let ws = Workspace::new(&manifest_path, config)?; + ws.current()?.package_id().name().to_string() } }; let version = match version {