From 5e326291036f6d2712123fbbae7ea97207e5a7f8 Mon Sep 17 00:00:00 2001 From: "J.W" Date: Tue, 21 Nov 2017 16:10:09 +0800 Subject: [PATCH 1/8] Include unpublishable path dependencies in package Before this patch, all path deps must be published before the root crate is published to the registry. This patch allow `cargo package` to include files of path dependencies in the generated package if the dependencies have the manifest key "package.publish" set to false, which means the crate cannot be registered as a sharable crate by itself at a registry, but still allowed to be published along with the root crate. --- src/cargo/core/package.rs | 4 +-- src/cargo/ops/cargo_package.rs | 19 ++++++---- src/cargo/sources/path.rs | 51 ++++++++++++++++++++------- src/cargo/util/toml/mod.rs | 64 +++++++++++++++++++++++++++++----- 4 files changed, 110 insertions(+), 28 deletions(-) diff --git a/src/cargo/core/package.rs b/src/cargo/core/package.rs index 1edc51b03ea..a820ab6b6ed 100644 --- a/src/cargo/core/package.rs +++ b/src/cargo/core/package.rs @@ -142,8 +142,8 @@ impl Package { #\n\ # When uploading crates to the registry Cargo will automatically\n\ # \"normalize\" Cargo.toml files for maximal compatibility\n\ - # with all versions of Cargo and also rewrite `path` dependencies\n\ - # to registry (e.g. crates.io) dependencies\n\ + # with all versions of Cargo and also change publishable `path`\n\ + # dependencies to registry (e.g. crates.io) dependencies\n\ #\n\ # If you believe there's an error in this file please file an\n\ # issue against the rust-lang/cargo repository. If you're\n\ diff --git a/src/cargo/ops/cargo_package.rs b/src/cargo/ops/cargo_package.rs index 04c3f6d971c..7bfa326b724 100644 --- a/src/cargo/ops/cargo_package.rs +++ b/src/cargo/ops/cargo_package.rs @@ -7,6 +7,7 @@ use std::sync::Arc; use flate2::read::GzDecoder; use flate2::{GzBuilder, Compression}; use git2; +use same_file::is_same_file; use tar::{Archive, Builder, Header, EntryType}; use core::{Package, Workspace, Source, SourceId}; @@ -203,8 +204,9 @@ fn tar(ws: &Workspace, let pkg = ws.current()?; let config = ws.config(); let root = pkg.root(); - for file in src.list_files(pkg)?.iter() { - let relative = util::without_prefix(file, root).unwrap(); + let manifest_path = pkg.manifest_path(); + for filepath in src.list_files(pkg)?.iter() { + let relative = util::without_prefix(filepath, root).unwrap(); check_filename(relative)?; let relative = relative.to_str().ok_or_else(|| { format!("non-utf8 path in source directory: {}", @@ -238,15 +240,15 @@ fn tar(ws: &Workspace, header.set_path(&path).chain_err(|| { format!("failed to add to archive: `{}`", relative) })?; - let mut file = File::open(file).chain_err(|| { - format!("failed to open for archiving: `{}`", file.display()) + let mut file = File::open(filepath).chain_err(|| { + format!("failed to open for archiving: `{}`", filepath.display()) })?; let metadata = file.metadata().chain_err(|| { format!("could not learn metadata for: `{}`", relative) })?; header.set_metadata(&metadata); - if relative == "Cargo.toml" { + if filepath.ends_with("Cargo.toml") { let orig = Path::new(&path).with_file_name("Cargo.toml.orig"); header.set_path(&orig)?; header.set_cksum(); @@ -255,7 +257,12 @@ fn tar(ws: &Workspace, })?; let mut header = Header::new_ustar(); - let toml = pkg.to_registry_toml()?; + let toml = if is_same_file(manifest_path, filepath)? { + pkg.to_registry_toml()? + } else { + let pkg = Package::for_path(&filepath, config).unwrap(); + pkg.to_registry_toml()? + }; header.set_path(&path)?; header.set_entry_type(EntryType::file()); header.set_mode(0o644); diff --git a/src/cargo/sources/path.rs b/src/cargo/sources/path.rs index 11760c29a63..26c5a070fa3 100644 --- a/src/cargo/sources/path.rs +++ b/src/cargo/sources/path.rs @@ -1,3 +1,4 @@ +use std::collections::HashSet; use std::fmt::{self, Debug, Formatter}; use std::fs; use std::path::{Path, PathBuf}; @@ -7,11 +8,13 @@ use git2; use glob::Pattern; use ignore::Match; use ignore::gitignore::GitignoreBuilder; +use same_file::is_same_file; use core::{Package, PackageId, Summary, SourceId, Source, Dependency, Registry}; use ops; use util::{self, CargoError, CargoResult, internal}; use util::Config; +use util::toml::is_remote_source; pub struct PathSource<'cfg> { source_id: SourceId, @@ -188,6 +191,26 @@ impl<'cfg> PathSource<'cfg> { } }; + // filter for publish-able packages + + let mut remote_prefixes = HashSet::::new(); + let mut local_should_package = |path: &Path| -> bool { + for ref prefix in remote_prefixes.iter() { + if path.starts_with(prefix) { + return false; + } + } + if let Some(dirpath) = path.parent() { + if !is_same_file(dirpath, root).unwrap_or(false) { + if is_remote_source(&dirpath.join("Cargo.toml")) { + remote_prefixes.insert(dirpath.to_path_buf()); + return false; + } + } + } + true + }; + // matching to paths let mut filter = |path: &Path| -> CargoResult { @@ -238,7 +261,7 @@ impl<'cfg> PathSource<'cfg> { } // Update to ignore_should_package for Stage 2 - Ok(glob_should_package) + Ok(glob_should_package && local_should_package(path)) }; // attempt git-prepopulate only if no `include` (rust-lang/cargo#4135) @@ -332,7 +355,7 @@ impl<'cfg> PathSource<'cfg> { } }); - let mut subpackages_found = Vec::new(); + let mut ignored_subpackages_found = Vec::new(); for (file_path, is_dir) in index_files.chain(untracked) { let file_path = file_path?; @@ -351,16 +374,20 @@ impl<'cfg> PathSource<'cfg> { Some("Cargo.lock") | Some("target") => continue, - // Keep track of all sub-packages found and also strip out all - // matches we've found so far. Note, though, that if we find - // our own `Cargo.toml` we keep going. + // Keep track of all published sub-packages found and also + // strip out all matches we've found so far. Note, though, + // that if we find our own `Cargo.toml` we keep going. Some("Cargo.toml") => { let path = file_path.parent().unwrap(); - if path != pkg_path { + if !is_same_file(&path, pkg_path)? { warn!("subpackage found: {}", path.display()); - ret.retain(|p| !p.starts_with(path)); - subpackages_found.push(path.to_path_buf()); - continue + if is_remote_source(&file_path) { + // if package is allowed to be published, + // then path dependency will become crates.io deps + ret.retain(|p| !p.starts_with(&path)); + ignored_subpackages_found.push(path.to_path_buf()); + continue + } } } @@ -369,7 +396,7 @@ impl<'cfg> PathSource<'cfg> { // If this file is part of any other sub-package we've found so far, // skip it. - if subpackages_found.iter().any(|p| file_path.starts_with(p)) { + if ignored_subpackages_found.iter().any(|p| file_path.starts_with(p)) { continue } @@ -433,8 +460,8 @@ impl<'cfg> PathSource<'cfg> { } return Ok(()) } - // Don't recurse into any sub-packages that we have - if !is_root && fs::metadata(&path.join("Cargo.toml")).is_ok() { + + if !is_root && is_remote_source(&path.join("Cargo.toml")) { return Ok(()) } diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index a2b33bfb196..b91a5e82356 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -136,6 +136,41 @@ in the future.", file.display()); }) } +#[derive(Deserialize)] +struct SourceManifest { + package: Option, + project: Option, +} + +#[derive(Deserialize)] +struct SourcePackage { + publish: Option, +} + +/// Check if the manifest file (if exists) has package.publish=true +/// +/// We do not validate the manifest here to avoid dealing with recursion. +pub fn is_remote_source(manifest_path: &Path) -> bool { + use ::std::io::Read; + + if let Ok(mut file) = fs::File::open(manifest_path) { + let mut content = String::new(); + if file.read_to_string(&mut content).is_ok() { + if let Ok(config) = toml::from_str::(&content) { + let pkg = config.package.or(config.project); + if let Some(pkg) = pkg { + return match pkg.publish { + Some(VecStringOrBool::VecString(ref registries)) => !registries.is_empty(), + Some(VecStringOrBool::Bool(false)) => false, + Some(VecStringOrBool::Bool(true)) | None => true, + }; + } + } + } + } + false +} + type TomlLibTarget = TomlTarget; type TomlBinTarget = TomlTarget; type TomlExampleTarget = TomlTarget; @@ -447,7 +482,7 @@ pub struct TomlProject { metadata: Option, } -#[derive(Debug, Deserialize, Serialize)] +#[derive(Clone, Debug, Deserialize, Serialize)] pub struct TomlWorkspace { members: Option>, exclude: Option>, @@ -473,11 +508,10 @@ struct Context<'a, 'b> { impl TomlManifest { pub fn prepare_for_publish(&self) -> TomlManifest { - let mut package = self.package.as_ref() + let package = self.package.as_ref() .or_else(|| self.project.as_ref()) .unwrap() .clone(); - package.workspace = None; return TomlManifest { package: Some(package), project: None, @@ -510,7 +544,7 @@ impl TomlManifest { }), replace: None, patch: None, - workspace: None, + workspace: self.workspace.clone(), badges: self.badges.clone(), cargo_features: self.cargo_features.clone(), }; @@ -528,9 +562,7 @@ impl TomlManifest { fn map_dependency(dep: &TomlDependency) -> TomlDependency { match *dep { TomlDependency::Detailed(ref d) => { - let mut d = d.clone(); - d.path.take(); // path dependencies become crates.io deps - TomlDependency::Detailed(d) + TomlDependency::Detailed(d.clone()) } TomlDependency::Simple(ref s) => { TomlDependency::Detailed(DetailedTomlDependency { @@ -888,7 +920,7 @@ impl TomlDependency { cx: &mut Context, kind: Option) -> CargoResult { - let details = match *self { + let mut details = match *self { TomlDependency::Simple(ref version) => DetailedTomlDependency { version: Some(version.clone()), .. Default::default() @@ -905,6 +937,22 @@ impl TomlDependency { cx.warnings.push(msg); } + if details.git.is_none() && details.registry.is_none() && + details.path.is_some() { + match kind { + // `cargo [build|test|bench]` are not affected. + Some(Kind::Normal) => { + let path = cx.root.join(PathBuf::from(details.path.clone().unwrap())); + if is_remote_source(&path.join("Cargo.toml")) { + // if package is allowed to be published, + // then path dependency will become crates.io deps + details.path.take(); + } + } + _ => (), + } + } + if details.git.is_none() { let git_only_keys = [ (&details.branch, "branch"), From 58238436877ff78045fe66a36a9a08d43c6f39bb Mon Sep 17 00:00:00 2001 From: "J.W" Date: Tue, 21 Nov 2017 16:10:09 +0800 Subject: [PATCH 2/8] Update tests files --- tests/package.rs | 76 ++++++++++++++++++++++++++++++++++--- tests/registry.rs | 97 +++++++++++++++++++++++++++++++++++++++++------ 2 files changed, 156 insertions(+), 17 deletions(-) diff --git a/tests/package.rs b/tests/package.rs index 42a491ebf7c..68691f8ec25 100644 --- a/tests/package.rs +++ b/tests/package.rs @@ -490,7 +490,67 @@ src/main.rs } #[test] -fn ignore_nested() { +fn exclude_publishable_path_deps() { + let p = project("foo") + .file("Cargo.toml", r#" + [project] + name = "foo" + version = "0.0.1" + authors = [] + [dependencies] + bar = { path = "bar", version = "0.1" } + "#) + .file("src/main.rs", "") + .file("bar/Cargo.toml", r#" + [package] + name = "bar" + version = "0.1.0" + authors = [] + "#) + .file("bar/src/lib.rs", "") + .build(); + + assert_that(p.cargo("package").arg("--list"), + execs().with_status(0).with_stdout("\ +Cargo.toml +src/main.rs +")); +} + +#[test] +fn include_unpublishable_path_deps() { + let p = project("foo") + .file("Cargo.toml", r#" + [project] + name = "foo" + version = "0.0.1" + authors = [] + + [dependencies] + bar = { path = "bar", version = "0.1" } + "#) + .file("src/main.rs", "") + .file("bar/Cargo.toml", r#" + [package] + publish = false + name = "bar" + version = "0.1.0" + authors = [] + "#) + .file("bar/src/lib.rs", "") + .build(); + + assert_that(p.cargo("package").arg("--list"), + execs().with_status(0).with_stdout("\ +Cargo.toml +bar/Cargo.toml +bar/src/lib.rs +src/main.rs +")); +} + +#[test] +fn keep_nested() { let cargo_toml = r#" [project] name = "nested" @@ -765,8 +825,8 @@ r#"# THIS FILE IS AUTOMATICALLY GENERATED BY CARGO # # When uploading crates to the registry Cargo will automatically # "normalize" Cargo.toml files for maximal compatibility -# with all versions of Cargo and also rewrite `path` dependencies -# to registry (e.g. crates.io) dependencies +# with all versions of Cargo and also change publishable `path` +# dependencies to registry (e.g. crates.io) dependencies # # If you believe there's an error in this file please file an # issue against the rust-lang/cargo repository. If you're @@ -788,17 +848,20 @@ version = "1.0" [dependencies.bar] version = "0.1" +path = "bar" [dependencies.def] version = "1.0" [dependencies.ghi] version = "1.0" + +[workspace] "#)); } #[test] -fn ignore_workspace_specifier() { +fn keep_workspace_specifier() { let p = project("foo") .file("Cargo.toml", r#" [project] @@ -842,8 +905,8 @@ r#"# THIS FILE IS AUTOMATICALLY GENERATED BY CARGO # # When uploading crates to the registry Cargo will automatically # "normalize" Cargo.toml files for maximal compatibility -# with all versions of Cargo and also rewrite `path` dependencies -# to registry (e.g. crates.io) dependencies +# with all versions of Cargo and also change publishable `path` +# dependencies to registry (e.g. crates.io) dependencies # # If you believe there's an error in this file please file an # issue against the rust-lang/cargo repository. If you're @@ -854,6 +917,7 @@ r#"# THIS FILE IS AUTOMATICALLY GENERATED BY CARGO name = "bar" version = "0.1.0" authors = [] +workspace = ".." "#)); } diff --git a/tests/registry.rs b/tests/registry.rs index 862e1bcb28f..e25029eb303 100644 --- a/tests/registry.rs +++ b/tests/registry.rs @@ -224,7 +224,7 @@ version required: >= 0.0.0 } #[test] -fn package_with_path_deps() { +fn package_with_publishable_path_deps() { Package::new("init", "0.0.1").publish(); let p = project("foo") @@ -252,14 +252,91 @@ fn package_with_path_deps() { .build(); assert_that(p.cargo("package").arg("-v"), - execs().with_status(101).with_stderr_contains("\ -[ERROR] failed to verify package tarball + execs().with_status(101).with_stderr(format!("\ +[PACKAGING] foo v0.0.1 ({dir}) +[ARCHIVING] [..] +[ARCHIVING] [..] +[VERIFYING] foo v0.0.1 ({dir}) +error: failed to verify package tarball Caused by: - no matching package named `notyet` found (required by `foo`) -location searched: registry [..] -version required: ^0.0.1 -")); + failed to load source for a dependency on `notyet` + +Caused by: + Unable to update {dir}[..] + +Caused by: + failed to read `[..]notyet[/]Cargo.toml` + +Caused by: + [..] (os error [..]) +", dir = p.url()))); + + Package::new("notyet", "0.0.1").publish(); + + assert_that(p.cargo("package"), + execs().with_status(101).with_stderr(format!("\ +[PACKAGING] foo v0.0.1 ({dir}) +[VERIFYING] foo v0.0.1 ({dir}) +error: failed to verify package tarball + +Caused by: + failed to load source for a dependency on `notyet` + +Caused by: + Unable to update file://[..]notyet + +Caused by: + failed to read `[..]notyet[/]Cargo.toml` + +Caused by: + [..] (os error [..]) +", dir = p.url()))); +} + +#[test] +fn package_with_unpublishable_path_deps() { + Package::new("init", "0.0.1").publish(); + + let p = project("foo") + .file("Cargo.toml", r#" + [project] + name = "foo" + version = "0.0.1" + authors = [] + license = "MIT" + description = "foo" + repository = "bar" + + [dependencies.notyet] + version = "0.0.1" + path = "notyet" + "#) + .file("src/main.rs", "fn main() {}") + .file("notyet/Cargo.toml", r#" + [package] + publish = false + name = "notyet" + version = "0.0.1" + authors = [] + "#) + .file("notyet/src/lib.rs", "") + .build(); + + assert_that(p.cargo("package").arg("-v"), + execs().with_status(0).with_stderr(format!("\ +[PACKAGING] foo v0.0.1 ({dir}) +[ARCHIVING] [..] +[ARCHIVING] [..] +[ARCHIVING] [..] +[ARCHIVING] [..] +[VERIFYING] foo v0.0.1 ({dir}[..]) +[COMPILING] notyet v0.0.1 ({dir}[..]) +[RUNNING] `rustc [..]notyet[/]src[/]lib.rs [..]` +[COMPILING] foo v0.0.1 ({dir}[..]) +[RUNNING] `rustc [..]src[/]main.rs [..]` +[FINISHED] dev [unoptimized + debuginfo] target(s) in [..] +", dir = p.url()))); Package::new("notyet", "0.0.1").publish(); @@ -267,11 +344,9 @@ version required: ^0.0.1 execs().with_status(0).with_stderr(format!("\ [PACKAGING] foo v0.0.1 ({dir}) [VERIFYING] foo v0.0.1 ({dir}) -[UPDATING] registry `[..]` -[DOWNLOADING] notyet v0.0.1 (registry `file://[..]`) -[COMPILING] notyet v0.0.1 +[COMPILING] notyet v0.0.1 ({dir}[..]) [COMPILING] foo v0.0.1 ({dir}[..]) -[FINISHED] dev [unoptimized + debuginfo] target(s) in [..] secs +[FINISHED] dev [unoptimized + debuginfo] target(s) in [..] ", dir = p.url()))); } From 1c9c2adca92a828fe7708ca11038b9f19b4d94e1 Mon Sep 17 00:00:00 2001 From: "J.W" Date: Tue, 21 Nov 2017 16:59:12 +0800 Subject: [PATCH 3/8] fix path seperators --- tests/package.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/package.rs b/tests/package.rs index 68691f8ec25..266a9c3a33d 100644 --- a/tests/package.rs +++ b/tests/package.rs @@ -485,7 +485,7 @@ fn no_duplicates_from_modified_tracked_files() { assert_that(cargo.arg("package").arg("--list"), execs().with_status(0).with_stdout("\ Cargo.toml -src/main.rs +src[/]main.rs ")); } @@ -513,7 +513,7 @@ fn exclude_publishable_path_deps() { assert_that(p.cargo("package").arg("--list"), execs().with_status(0).with_stdout("\ Cargo.toml -src/main.rs +src[/]main.rs ")); } @@ -543,9 +543,9 @@ fn include_unpublishable_path_deps() { assert_that(p.cargo("package").arg("--list"), execs().with_status(0).with_stdout("\ Cargo.toml -bar/Cargo.toml -bar/src/lib.rs -src/main.rs +bar[/]Cargo.toml +bar[/]src[/]lib.rs +src[/]main.rs ")); } From 66e1e7d73d8e2a6e8b02bbe0cfdbd307f0be0c11 Mon Sep 17 00:00:00 2001 From: "J.W" Date: Wed, 22 Nov 2017 19:18:41 +0800 Subject: [PATCH 4/8] Speed up the filter for remote packages --- src/cargo/sources/path.rs | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/src/cargo/sources/path.rs b/src/cargo/sources/path.rs index 26c5a070fa3..ba22e671a8e 100644 --- a/src/cargo/sources/path.rs +++ b/src/cargo/sources/path.rs @@ -1,4 +1,4 @@ -use std::collections::HashSet; +use std::collections::HashMap; use std::fmt::{self, Debug, Formatter}; use std::fs; use std::path::{Path, PathBuf}; @@ -193,19 +193,18 @@ impl<'cfg> PathSource<'cfg> { // filter for publish-able packages - let mut remote_prefixes = HashSet::::new(); + let mut remote_prefixes = HashMap::::new(); let mut local_should_package = |path: &Path| -> bool { - for ref prefix in remote_prefixes.iter() { + for (ref prefix, is_remote) in remote_prefixes.iter() { if path.starts_with(prefix) { - return false; + return !is_remote; } } if let Some(dirpath) = path.parent() { if !is_same_file(dirpath, root).unwrap_or(false) { - if is_remote_source(&dirpath.join("Cargo.toml")) { - remote_prefixes.insert(dirpath.to_path_buf()); - return false; - } + let is_remote = is_remote_source(&dirpath.join("Cargo.toml")); + remote_prefixes.insert(dirpath.to_path_buf(), is_remote); + return !is_remote; } } true From 29ed87e1356afbdb928ff0cf579b092204a5a565 Mon Sep 17 00:00:00 2001 From: "J.W" Date: Wed, 22 Nov 2017 19:50:40 +0800 Subject: [PATCH 5/8] fix logic --- src/cargo/core/package.rs | 3 ++- src/cargo/util/toml/mod.rs | 49 ++++++++++++++++---------------------- 2 files changed, 23 insertions(+), 29 deletions(-) diff --git a/src/cargo/core/package.rs b/src/cargo/core/package.rs index a820ab6b6ed..454d46092bc 100644 --- a/src/cargo/core/package.rs +++ b/src/cargo/core/package.rs @@ -135,7 +135,8 @@ impl Package { } pub fn to_registry_toml(&self) -> CargoResult { - let manifest = self.manifest().original().prepare_for_publish(); + let root = self.root(); + let manifest = self.manifest().original().prepare_for_publish(root); let toml = toml::to_string(&manifest)?; Ok(format!("\ # THIS FILE IS AUTOMATICALLY GENERATED BY CARGO\n\ diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index b91a5e82356..55127077dcf 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -507,7 +507,7 @@ struct Context<'a, 'b> { } impl TomlManifest { - pub fn prepare_for_publish(&self) -> TomlManifest { + pub fn prepare_for_publish(&self, root: &Path) -> TomlManifest { let package = self.package.as_ref() .or_else(|| self.project.as_ref()) .unwrap() @@ -521,23 +521,23 @@ impl TomlManifest { example: self.example.clone(), test: self.test.clone(), bench: self.bench.clone(), - dependencies: map_deps(self.dependencies.as_ref()), + dependencies: map_deps(self.dependencies.as_ref(), root), dev_dependencies: map_deps(self.dev_dependencies.as_ref() - .or_else(|| self.dev_dependencies2.as_ref())), + .or_else(|| self.dev_dependencies2.as_ref()), root), dev_dependencies2: None, build_dependencies: map_deps(self.build_dependencies.as_ref() - .or_else(|| self.build_dependencies2.as_ref())), + .or_else(|| self.build_dependencies2.as_ref()), root), build_dependencies2: None, features: self.features.clone(), target: self.target.as_ref().map(|target_map| { target_map.iter().map(|(k, v)| { (k.clone(), TomlPlatform { - dependencies: map_deps(v.dependencies.as_ref()), + dependencies: map_deps(v.dependencies.as_ref(), root), dev_dependencies: map_deps(v.dev_dependencies.as_ref() - .or_else(|| v.dev_dependencies2.as_ref())), + .or_else(|| v.dev_dependencies2.as_ref()), root), dev_dependencies2: None, build_dependencies: map_deps(v.build_dependencies.as_ref() - .or_else(|| v.build_dependencies2.as_ref())), + .or_else(|| v.build_dependencies2.as_ref()), root), build_dependencies2: None, }) }).collect() @@ -549,20 +549,29 @@ impl TomlManifest { cargo_features: self.cargo_features.clone(), }; - fn map_deps(deps: Option<&BTreeMap>) + fn map_deps(deps: Option<&BTreeMap>, root: &Path) -> Option> { let deps = match deps { Some(deps) => deps, None => return None }; - Some(deps.iter().map(|(k, v)| (k.clone(), map_dependency(v))).collect()) + Some(deps.iter().map(|(k, v)| (k.clone(), map_dependency(v, root))).collect()) } - fn map_dependency(dep: &TomlDependency) -> TomlDependency { + fn map_dependency(dep: &TomlDependency, root: &Path) -> TomlDependency { match *dep { TomlDependency::Detailed(ref d) => { - TomlDependency::Detailed(d.clone()) + let mut dep = d.clone(); + if let Some(ref path) = d.path { + let path = root.join(PathBuf::from(path)); + if is_remote_source(&path.join("Cargo.toml")) { + // if package is allowed to be published, + // then path dependency will become crates.io deps + dep.path.take(); + } + } + TomlDependency::Detailed(dep) } TomlDependency::Simple(ref s) => { TomlDependency::Detailed(DetailedTomlDependency { @@ -920,7 +929,7 @@ impl TomlDependency { cx: &mut Context, kind: Option) -> CargoResult { - let mut details = match *self { + let details = match *self { TomlDependency::Simple(ref version) => DetailedTomlDependency { version: Some(version.clone()), .. Default::default() @@ -937,22 +946,6 @@ impl TomlDependency { cx.warnings.push(msg); } - if details.git.is_none() && details.registry.is_none() && - details.path.is_some() { - match kind { - // `cargo [build|test|bench]` are not affected. - Some(Kind::Normal) => { - let path = cx.root.join(PathBuf::from(details.path.clone().unwrap())); - if is_remote_source(&path.join("Cargo.toml")) { - // if package is allowed to be published, - // then path dependency will become crates.io deps - details.path.take(); - } - } - _ => (), - } - } - if details.git.is_none() { let git_only_keys = [ (&details.branch, "branch"), From fcb559fff144517900ce8228c9933916c889c13c Mon Sep 17 00:00:00 2001 From: "J.W" Date: Wed, 22 Nov 2017 20:12:59 +0800 Subject: [PATCH 6/8] Update tests --- tests/registry.rs | 34 ++++++++++------------------------ 1 file changed, 10 insertions(+), 24 deletions(-) diff --git a/tests/registry.rs b/tests/registry.rs index e25029eb303..572b4d52a03 100644 --- a/tests/registry.rs +++ b/tests/registry.rs @@ -257,40 +257,26 @@ fn package_with_publishable_path_deps() { [ARCHIVING] [..] [ARCHIVING] [..] [VERIFYING] foo v0.0.1 ({dir}) +[UPDATING] registry [..] error: failed to verify package tarball Caused by: - failed to load source for a dependency on `notyet` - -Caused by: - Unable to update {dir}[..] - -Caused by: - failed to read `[..]notyet[/]Cargo.toml` - -Caused by: - [..] (os error [..]) + no matching package named `notyet` found (required by `foo`) +location searched: registry [..] +version required: ^0.0.1 ", dir = p.url()))); Package::new("notyet", "0.0.1").publish(); assert_that(p.cargo("package"), - execs().with_status(101).with_stderr(format!("\ + execs().with_status(0).with_stderr(format!("\ [PACKAGING] foo v0.0.1 ({dir}) [VERIFYING] foo v0.0.1 ({dir}) -error: failed to verify package tarball - -Caused by: - failed to load source for a dependency on `notyet` - -Caused by: - Unable to update file://[..]notyet - -Caused by: - failed to read `[..]notyet[/]Cargo.toml` - -Caused by: - [..] (os error [..]) +[UPDATING] registry [..] +[DOWNLOADING] notyet v0.0.1 (registry [..]) +[COMPILING] notyet v0.0.1 +[COMPILING] foo v0.0.1 [..] +[FINISHED] dev [unoptimized + debuginfo] target(s) in [..] ", dir = p.url()))); } From f454f2703d7b799156838c6519dd765b88aad608 Mon Sep 17 00:00:00 2001 From: "J.W" Date: Wed, 22 Nov 2017 20:21:36 +0800 Subject: [PATCH 7/8] Update tests --- tests/package.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/package.rs b/tests/package.rs index 266a9c3a33d..a16bc8264f9 100644 --- a/tests/package.rs +++ b/tests/package.rs @@ -848,7 +848,6 @@ version = "1.0" [dependencies.bar] version = "0.1" -path = "bar" [dependencies.def] version = "1.0" From 251f2d820fc2019f8316cc66b33ff5a034f914a7 Mon Sep 17 00:00:00 2001 From: "J.W" Date: Wed, 22 Nov 2017 23:37:01 +0800 Subject: [PATCH 8/8] Update tests --- tests/publish.rs | 84 +++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 73 insertions(+), 11 deletions(-) diff --git a/tests/publish.rs b/tests/publish.rs index d4af2e99a85..1cf200a1fcd 100644 --- a/tests/publish.rs +++ b/tests/publish.rs @@ -15,6 +15,19 @@ use flate2::read::GzDecoder; use hamcrest::assert_that; use tar::Archive; +fn decode_uploaded_package() -> std::io::Result> { + let mut f = File::open(&publish::upload_path().join("api/v1/crates/new")).unwrap(); + // Skip the metadata payload and the size of the tarball + let mut sz = [0; 4]; + assert_eq!(f.read(&mut sz).unwrap(), 4); + let sz = ((sz[0] as u32) << 0) | + ((sz[1] as u32) << 8) | + ((sz[2] as u32) << 16) | + ((sz[3] as u32) << 24); + f.seek(SeekFrom::Current(sz as i64 + 4))?; + GzDecoder::new(f) +} + #[test] fn simple() { publish::setup(); @@ -178,22 +191,13 @@ See [..] dir = p.url(), reg = publish::registry()))); - let mut f = File::open(&publish::upload_path().join("api/v1/crates/new")).unwrap(); - // Skip the metadata payload and the size of the tarball - let mut sz = [0; 4]; - assert_eq!(f.read(&mut sz).unwrap(), 4); - let sz = ((sz[0] as u32) << 0) | - ((sz[1] as u32) << 8) | - ((sz[2] as u32) << 16) | - ((sz[3] as u32) << 24); - f.seek(SeekFrom::Current(sz as i64 + 4)).unwrap(); - // Verify the tarball - let mut rdr = GzDecoder::new(f).unwrap(); + let mut rdr = decode_uploaded_package().unwrap(); assert_eq!(rdr.header().filename().unwrap(), "foo-0.0.1.crate".as_bytes()); let mut contents = Vec::new(); rdr.read_to_end(&mut contents).unwrap(); let mut ar = Archive::new(&contents[..]); + let mut count = 0; for file in ar.entries().unwrap() { let file = file.unwrap(); let fname = file.header().path_bytes(); @@ -202,7 +206,9 @@ See [..] fname == b"foo-0.0.1/Cargo.toml.orig" || fname == b"foo-0.0.1/src/main.rs", "unexpected filename: {:?}", file.header().path()); + count = count + 1; } + assert!(count == 3, "some files are missing"); } #[test] @@ -646,3 +652,59 @@ fn block_publish_no_registry() { `foo` is marked as unpublishable ")); } + +#[test] +fn with_path_dependencies() { + publish::setup(); + + let p = project("foo") + .file("Cargo.toml", r#" + [package] + name = "foo" + version = "0.0.1" + authors = [] + + [dependencies] + bar = { path = "bar", version = "*" } + + [workspace] + members = ["bar"] + "#) + .file("src/main.rs", "fn main() {}") + .file("bar/Cargo.toml", r#" + [package] + publish = false + name = "bar" + version = "0.1.0" + authors = [] + "#) + .file("bar/src/lib.rs", "") + .build(); + + assert_that(p.cargo("publish").arg("--index").arg(publish::registry().to_string()), + execs().with_status(0).with_stderr_contains(&format!("\ +[UPLOADING] foo v0.0.1 ({dir}) +", dir = p.url()))); + + // Verify the tarball + let mut rdr = decode_uploaded_package().unwrap(); + assert_eq!(rdr.header().filename().unwrap(), "foo-0.0.1.crate".as_bytes()); + let mut contents = Vec::new(); + rdr.read_to_end(&mut contents).unwrap(); + let mut ar = Archive::new(&contents[..]); + let mut count = 0; + for file in ar.entries().unwrap() { + let file = file.unwrap(); + let fname = file.header().path_bytes(); + let fname = &*fname; + assert!(fname == b"foo-0.0.1/Cargo.toml" || + fname == b"foo-0.0.1/Cargo.toml.orig" || + fname == b"foo-0.0.1/src/main.rs" || + fname == b"foo-0.0.1/bar/Cargo.toml" || + fname == b"foo-0.0.1/bar/Cargo.toml.orig" || + fname == b"foo-0.0.1/bar/src/lib.rs", + "unexpected filename: {:?}", file.header().path()); + count = count + 1; + } + assert!(count == 6, "some files are missing"); +}