From c57fef459a7ab1a49044710b728bd57cd7c1c4d9 Mon Sep 17 00:00:00 2001 From: Yehuda Katz + Carl Lerche Date: Wed, 11 Jun 2014 14:50:54 -0700 Subject: [PATCH] Add namespace to PackageId --- src/bin/cargo-read-manifest.rs | 4 +-- src/cargo/core/package_id.rs | 6 ++++ src/cargo/core/resolver.rs | 9 +++-- src/cargo/ops/cargo_compile.rs | 5 ++- src/cargo/ops/cargo_read_manifest.rs | 10 +++--- src/cargo/ops/cargo_rustc.rs | 4 +-- src/cargo/sources/git/source.rs | 13 +++++--- src/cargo/sources/mod.rs | 3 ++ src/cargo/sources/path.rs | 49 +++++++++++++++++++--------- src/cargo/util/mod.rs | 2 ++ src/cargo/util/paths.rs | 40 +++++++++++++++++++++++ src/cargo/util/result.rs | 4 ++- src/cargo/util/toml.rs | 11 ++++--- tests/support.rs | 43 ++---------------------- tests/test_cargo_compile.rs | 12 ++++--- 15 files changed, 135 insertions(+), 80 deletions(-) create mode 100644 src/cargo/util/paths.rs diff --git a/src/bin/cargo-read-manifest.rs b/src/bin/cargo-read-manifest.rs index 32a3500a5..b1a08de0e 100644 --- a/src/bin/cargo-read-manifest.rs +++ b/src/bin/cargo-read-manifest.rs @@ -7,7 +7,7 @@ extern crate hammer; use hammer::FlagConfig; use cargo::{execute_main_without_stdin,CLIResult,CLIError}; use cargo::core::Package; -use cargo::ops; +use cargo::sources::{PathSource}; #[deriving(PartialEq,Clone,Decodable)] struct Options { @@ -21,6 +21,6 @@ fn main() { } fn execute(options: Options) -> CLIResult> { - ops::read_package(&Path::new(options.manifest_path.as_slice())).map(|m| Some(m)) + PathSource::read_package(&Path::new(options.manifest_path.as_slice())).map(|m| Some(m)) .map_err(|err| CLIError::new(err.get_desc(), Some(err.get_detail()), 1)) } diff --git a/src/cargo/core/package_id.rs b/src/cargo/core/package_id.rs index ba5594f7a..4b3ea8eb9 100644 --- a/src/cargo/core/package_id.rs +++ b/src/cargo/core/package_id.rs @@ -42,6 +42,12 @@ impl ToUrl for Url { } } +impl<'a> ToUrl for &'a Url { + fn to_url(self) -> Option { + Some(self.clone()) + } +} + #[deriving(Clone,PartialEq)] pub struct PackageId { name: String, diff --git a/src/cargo/core/resolver.rs b/src/cargo/core/resolver.rs index 724968b71..a67cbf1d1 100644 --- a/src/cargo/core/resolver.rs +++ b/src/cargo/core/resolver.rs @@ -1,3 +1,4 @@ +use std::fmt::Show; use std::collections::HashMap; use core::{ Dependency, @@ -11,7 +12,9 @@ use util::result::CargoResult; * - The correct input here is not a registry. Resolves should be performable * on package summaries vs. the packages themselves. */ -pub fn resolve(deps: &[Dependency], registry: &Registry) -> CargoResult> { +pub fn resolve(deps: &[Dependency], registry: &R) -> CargoResult> { + log!(5, "resolve; deps={}; registry={}", deps, registry); + let mut remaining = Vec::from_slice(deps); let mut resolve = HashMap::<&str, &Summary>::new(); @@ -19,7 +22,9 @@ pub fn resolve(deps: &[Dependency], registry: &Registry) -> CargoResult curr, None => { - return Ok(resolve.values().map(|summary| summary.get_package_id().clone()).collect()); + let ret = resolve.values().map(|summary| summary.get_package_id().clone()).collect(); + log!(5, "resolve complete; ret={}", ret); + return Ok(ret); } }; diff --git a/src/cargo/ops/cargo_compile.rs b/src/cargo/ops/cargo_compile.rs index bb69aa700..d68543a12 100644 --- a/src/cargo/ops/cargo_compile.rs +++ b/src/cargo/ops/cargo_compile.rs @@ -26,7 +26,8 @@ use util::{other_error, CargoResult, Wrap}; pub fn compile(manifest_path: &Path) -> CargoResult<()> { log!(4, "compile; manifest-path={}", manifest_path.display()); - let package = try!(ops::read_package(manifest_path)); + // TODO: Move this into PathSource + let package = try!(PathSource::read_package(manifest_path)); debug!("loaded package; package={}", package); @@ -51,6 +52,8 @@ pub fn compile(manifest_path: &Path) -> CargoResult<()> { let packages = try!(source.get(resolved.as_slice()).wrap("unable to get packages from source")); + log!(5, "fetch packages from source; packages={}; ids={}", packages, resolved); + let package_set = PackageSet::new(packages.as_slice()); try!(ops::compile_packages(&package, &package_set)); diff --git a/src/cargo/ops/cargo_read_manifest.rs b/src/cargo/ops/cargo_read_manifest.rs index 95f1062d0..97ea1755d 100644 --- a/src/cargo/ops/cargo_read_manifest.rs +++ b/src/cargo/ops/cargo_read_manifest.rs @@ -1,16 +1,18 @@ use std::io::File; use util; +use url::Url; use core::{Package,Manifest}; use util::{CargoResult,io_error}; -pub fn read_manifest(contents: &[u8]) -> CargoResult { - util::toml::to_manifest(contents) +pub fn read_manifest(contents: &[u8], namespace: &Url) -> CargoResult { + util::toml::to_manifest(contents, namespace) } -pub fn read_package(path: &Path) -> CargoResult { +pub fn read_package(path: &Path, namespace: &Url) -> CargoResult { + log!(5, "read_package; path={}; namespace={}", path.display(), namespace); let mut file = try!(File::open(path).map_err(io_error)); let data = try!(file.read_to_end().map_err(io_error)); - let manifest = try!(read_manifest(data.as_slice())); + let manifest = try!(read_manifest(data.as_slice(), namespace)); Ok(Package::new(&manifest, &path.dir_path())) } diff --git a/src/cargo/ops/cargo_rustc.rs b/src/cargo/ops/cargo_rustc.rs index 733e5d91e..6e2925cbd 100644 --- a/src/cargo/ops/cargo_rustc.rs +++ b/src/cargo/ops/cargo_rustc.rs @@ -10,7 +10,7 @@ use util::result::ProcessError; type Args = Vec; pub fn compile_packages(pkg: &Package, deps: &PackageSet) -> CargoResult<()> { - debug!("compiling; pkg={}; deps={}", pkg, deps); + debug!("compile_packages; pkg={}; deps={}", pkg, deps); let target_dir = pkg.get_absolute_target_dir(); let deps_target_dir = target_dir.join("deps"); @@ -33,7 +33,7 @@ pub fn compile_packages(pkg: &Package, deps: &PackageSet) -> CargoResult<()> { } fn compile_pkg(pkg: &Package, dest: &Path, deps_dir: &Path, primary: bool) -> CargoResult<()> { - debug!("compiling; pkg={}; targets={}", pkg, pkg.get_targets()); + debug!("compile_pkg; pkg={}; targets={}", pkg, pkg.get_targets()); // compile for target in pkg.get_targets().iter() { diff --git a/src/cargo/sources/git/source.rs b/src/cargo/sources/git/source.rs index 1c88f450f..ec33b9b02 100644 --- a/src/cargo/sources/git/source.rs +++ b/src/cargo/sources/git/source.rs @@ -1,4 +1,5 @@ use ops; +use url; use core::source::Source; use core::{Package,PackageId,Summary}; use util::CargoResult; @@ -18,6 +19,10 @@ impl GitSource { pub fn new(remote: GitRemote, reference: String, db: Path, checkout: Path, verbose: bool) -> GitSource { GitSource { remote: remote, reference: GitReference::for_str(reference), db_path: db, checkout_path: checkout, verbose: verbose } } + + pub fn get_namespace<'a>(&'a self) -> &'a url::Url { + self.remote.get_url() + } } impl Show for GitSource { @@ -40,7 +45,7 @@ impl Source for GitSource { } fn list(&self) -> CargoResult> { - let pkg = try!(read_manifest(&self.checkout_path)); + let pkg = try!(read_manifest(&self.checkout_path, self.get_namespace())); Ok(vec!(pkg.get_summary().clone())) } @@ -50,7 +55,7 @@ impl Source for GitSource { fn get(&self, package_ids: &[PackageId]) -> CargoResult> { // TODO: Support multiple manifests per repo - let pkg = try!(read_manifest(&self.checkout_path)); + let pkg = try!(read_manifest(&self.checkout_path, self.remote.get_url())); if package_ids.iter().any(|pkg_id| pkg_id == pkg.get_package_id()) { Ok(vec!(pkg)) @@ -60,7 +65,7 @@ impl Source for GitSource { } } -fn read_manifest(path: &Path) -> CargoResult { +fn read_manifest(path: &Path, url: &url::Url) -> CargoResult { let path = path.join("Cargo.toml"); - ops::read_package(&path) + ops::read_package(&path, url) } diff --git a/src/cargo/sources/mod.rs b/src/cargo/sources/mod.rs index dc14a54ba..876c4836e 100644 --- a/src/cargo/sources/mod.rs +++ b/src/cargo/sources/mod.rs @@ -1,2 +1,5 @@ +pub use self::path::PathSource; +pub use self::git::GitSource; + pub mod path; pub mod git; diff --git a/src/cargo/sources/path.rs b/src/cargo/sources/path.rs index dcef3ed29..07d9678d3 100644 --- a/src/cargo/sources/path.rs +++ b/src/cargo/sources/path.rs @@ -3,17 +3,31 @@ use std::fmt::{Show,Formatter}; use core::{Package,PackageId,Summary}; use core::source::Source; use ops; -use util::{CargoResult}; +use url; +use util::{CargoResult,simple_human,io_error,realpath}; -pub struct PathSource { - paths: Vec -} +/* + * TODO: Consider whether it may be more appropriate for a PathSource to only + * take in a single path vs. a vec of paths. The pros / cons are unknown at + * this point. + */ +pub struct PathSource { paths: Vec } impl PathSource { pub fn new(paths: Vec) -> PathSource { - log!(4, "new; paths={}", display(paths.as_slice())); + log!(5, "new; paths={}", display(paths.as_slice())); PathSource { paths: paths } } + + pub fn read_package(path: &Path) -> CargoResult { + log!(5, "read_package; path={}", path.display()); + + // TODO: Use a realpath fn + let dir = path.dir_path(); + let namespace = try!(namespace(&dir)); + + ops::read_package(path, &namespace) + } } impl Show for PathSource { @@ -27,10 +41,10 @@ impl Source for PathSource { fn list(&self) -> CargoResult> { Ok(self.paths.iter().filter_map(|path| { - match read_manifest(path) { + match PathSource::read_package(&path.join("Cargo.toml")) { Ok(ref pkg) => Some(pkg.get_summary().clone()), Err(e) => { - log!(4, "failed to read manifest; path={}; err={}", path.display(), e); + debug!("failed to read manifest; path={}; err={}", path.display(), e); None } } @@ -41,11 +55,15 @@ impl Source for PathSource { Ok(()) } - fn get(&self, name_vers: &[PackageId]) -> CargoResult> { + fn get(&self, ids: &[PackageId]) -> CargoResult> { + log!(5, "getting packages; ids={}", ids); + Ok(self.paths.iter().filter_map(|path| { - match read_manifest(path) { + match PathSource::read_package(&path.join("Cargo.toml")) { Ok(pkg) => { - if name_vers.iter().any(|pkg_id| pkg.get_package_id() == pkg_id) { + log!(5, "comparing; pkg={}", pkg); + + if ids.iter().any(|pkg_id| pkg.get_package_id() == pkg_id) { Some(pkg) } else { None @@ -57,11 +75,12 @@ impl Source for PathSource { } } -fn read_manifest(path: &Path) -> CargoResult { - let path = path.join("Cargo.toml"); - ops::read_package(&path) -} - fn display(paths: &[Path]) -> Vec { paths.iter().map(|p| p.display().to_str()).collect() } + +fn namespace(path: &Path) -> CargoResult { + let real = try!(realpath(path).map_err(io_error)); + url::from_str(format!("file://{}", real.display()).as_slice()).map_err(|err| + simple_human(err.as_slice())) +} diff --git a/src/cargo/util/mod.rs b/src/cargo/util/mod.rs index 2d2229c3c..58eaf1829 100644 --- a/src/cargo/util/mod.rs +++ b/src/cargo/util/mod.rs @@ -1,5 +1,6 @@ pub use self::process_builder::{process,ProcessBuilder}; pub use self::result::{CargoError,CargoResult,Wrap,Require,ToCLI,other_error,human_error,simple_human,toml_error,io_error,process_error}; +pub use self::paths::realpath; pub mod graph; pub mod process_builder; @@ -7,3 +8,4 @@ pub mod config; pub mod important_paths; pub mod result; pub mod toml; +pub mod paths; diff --git a/src/cargo/util/paths.rs b/src/cargo/util/paths.rs new file mode 100644 index 000000000..bfdf0ca90 --- /dev/null +++ b/src/cargo/util/paths.rs @@ -0,0 +1,40 @@ +use std::{io,os}; +use std::io::fs; + +pub fn realpath(original: &Path) -> io::IoResult { + static MAX_LINKS_FOLLOWED: uint = 256; + let original = os::make_absolute(original); + + // Right now lstat on windows doesn't work quite well + if cfg!(windows) { + return Ok(original) + } + + let result = original.root_path(); + let mut result = result.expect("make_absolute has no root_path"); + let mut followed = 0; + + for part in original.components() { + result.push(part); + + loop { + if followed == MAX_LINKS_FOLLOWED { + return Err(io::standard_error(io::InvalidInput)) + } + + match fs::lstat(&result) { + Err(..) => break, + Ok(ref stat) if stat.kind != io::TypeSymlink => break, + Ok(..) => { + followed += 1; + let path = try!(fs::readlink(&result)); + result.pop(); + result.push(path); + } + } + } + } + + return Ok(result); +} + diff --git a/src/cargo/util/result.rs b/src/cargo/util/result.rs index 66489790e..3f943d1eb 100644 --- a/src/cargo/util/result.rs +++ b/src/cargo/util/result.rs @@ -82,7 +82,9 @@ impl Show for CargoError { match self.desc { StaticDescription(string) => write!(f, "{}", string), BoxedDescription(ref string) => write!(f, "{}", string) - } + }; + + write!(f, "; kind={}", self.kind) } } diff --git a/src/cargo/util/toml.rs b/src/cargo/util/toml.rs index 24aab1269..cfbe60147 100644 --- a/src/cargo/util/toml.rs +++ b/src/cargo/util/toml.rs @@ -1,18 +1,19 @@ use toml; +use url::Url; use std::collections::HashMap; use serialize::Decodable; use core::{Summary,Manifest,Target,Dependency,PackageId}; use util::{CargoResult,Require,simple_human,toml_error}; -pub fn to_manifest(contents: &[u8]) -> CargoResult { +pub fn to_manifest(contents: &[u8], namespace: &Url) -> CargoResult { let root = try!(toml::parse_from_bytes(contents).map_err(|_| simple_human("Cargo.toml is not valid Toml"))); let toml = try!(toml_to_manifest(root).map_err(|_| simple_human("Cargo.toml is not a valid Cargo manifest"))); - toml.to_manifest() + toml.to_manifest(namespace) } fn toml_to_manifest(root: toml::Value) -> CargoResult { @@ -104,13 +105,13 @@ pub struct TomlProject { } impl TomlProject { - pub fn to_package_id(&self, namespace: &str) -> PackageId { + pub fn to_package_id(&self, namespace: &Url) -> PackageId { PackageId::new(self.name.as_slice(), self.version.as_slice(), namespace) } } impl TomlManifest { - pub fn to_manifest(&self) -> CargoResult { + pub fn to_manifest(&self, namespace: &Url) -> CargoResult { // Get targets let targets = normalize(self.lib.as_ref().map(|l| l.as_slice()), self.bin.as_ref().map(|b| b.as_slice())); @@ -137,7 +138,7 @@ impl TomlManifest { } Ok(Manifest::new( - &Summary::new(&self.project.to_package_id("http://rust-lang.org/central-repo"), deps.as_slice()), + &Summary::new(&self.project.to_package_id(namespace), deps.as_slice()), targets.as_slice(), &Path::new("target"))) } diff --git a/tests/support.rs b/tests/support.rs index efc34259a..ec4f6172d 100644 --- a/tests/support.rs +++ b/tests/support.rs @@ -152,43 +152,6 @@ pub fn cargo_dir() -> Path { /// Returns an absolute path in the filesystem that `path` points to. The /// returned path does not contain any symlinks in its hierarchy. -pub fn realpath(original: &Path) -> io::IoResult { - static MAX_LINKS_FOLLOWED: uint = 256; - let original = os::make_absolute(original); - - // Right now lstat on windows doesn't work quite well - if cfg!(windows) { - return Ok(original) - } - - let result = original.root_path(); - let mut result = result.expect("make_absolute has no root_path"); - let mut followed = 0; - - for part in original.components() { - result.push(part); - - loop { - if followed == MAX_LINKS_FOLLOWED { - return Err(io::standard_error(io::InvalidInput)) - } - - match fs::lstat(&result) { - Err(..) => break, - Ok(ref stat) if stat.kind != io::TypeSymlink => break, - Ok(..) => { - followed += 1; - let path = try!(fs::readlink(&result)); - result.pop(); - result.push(path); - } - } - } - } - - return Ok(result); -} - /* * * ===== Matchers ===== @@ -205,12 +168,12 @@ struct Execs { impl Execs { - pub fn with_stdout(mut ~self, expected: &str) -> Box { + pub fn with_stdout(mut ~self, expected: S) -> Box { self.expect_stdout = Some(expected.to_str()); self } - pub fn with_stderr(mut ~self, expected: &str) -> Box { + pub fn with_stderr(mut ~self, expected: S) -> Box { self.expect_stderr = Some(expected.to_str()); self } @@ -252,7 +215,7 @@ impl Execs { match str::from_utf8(actual) { None => Err(format!("{} was not utf8 encoded", description)), Some(actual) => { - ham::expect(actual == out, format!("{} was:\n`{}`\n\nexpected:\n`{}`\n\nother output:\n`{}`", description, actual, out, str::from_utf8(extra))) + ham::expect(actual == out, format!("{} was:\n`{}`\n\nexpected:\n`{}`\n\nother output:\n`{}`", description, actual, out, str::from_utf8_lossy(extra))) } } } diff --git a/tests/test_cargo_compile.rs b/tests/test_cargo_compile.rs index f212d8ef8..deeb20fbc 100644 --- a/tests/test_cargo_compile.rs +++ b/tests/test_cargo_compile.rs @@ -1,7 +1,7 @@ -use support::{ResultTest,project,execs,realpath}; +use support::{ResultTest,project,execs}; use hamcrest::{assert_that,existing_file}; use cargo; -use cargo::util::process; +use cargo::util::{process,realpath}; fn setup() { } @@ -120,9 +120,13 @@ test!(cargo_compile_with_warnings_in_a_dep_package { fn dead() {} "#); + let bar = realpath(&p.root().join("bar")).assert(); + let main = realpath(&p.root()).assert(); + assert_that(p.cargo_process("cargo-compile"), execs() - .with_stdout("Compiling bar v0.5.0\nCompiling foo v0.5.0\n") + .with_stdout(format!("Compiling bar v0.5.0 (file:{})\nCompiling foo v0.5.0 (file:{})\n", + bar.display(), main.display())) .with_stderr("")); assert_that(&p.root().join("target/foo"), existing_file()); @@ -132,7 +136,7 @@ test!(cargo_compile_with_warnings_in_a_dep_package { execs().with_stdout("test passed\n")); }) -test!(cargo_compile_with_nested_deps { +test!(cargo_compile_with_nested_deps_shorthand { let mut p = project("foo"); let bar = p.root().join("bar"); let baz = p.root().join("baz"); -- 2.30.2