From ad976d6e3cc4a24ccc77929018d9cc331868358d Mon Sep 17 00:00:00 2001 From: bors Date: Fri, 5 Feb 2021 18:11:29 +0000 Subject: [PATCH 1/4] Auto merge of #9142 - ehuss:fix-doc-orphan, r=Eh2406 Fix panic with doc collision orphan. As I feared, the collision removal added in #9077 caused problems due to orphans left in the unit graph. Ironically, it was the collision warning detection code which failed, as it iterates over all the keys of the graph. The solution is to remove all orphans from the graph after collisions have been removed. Fixes #9136 --- src/cargo/ops/cargo_compile.rs | 29 +++++++++++++------ tests/testsuite/collisions.rs | 51 ++++++++++++++++++++++++++++++++-- 2 files changed, 69 insertions(+), 11 deletions(-) diff --git a/src/cargo/ops/cargo_compile.rs b/src/cargo/ops/cargo_compile.rs index b14bb5d690f..981afe23580 100644 --- a/src/cargo/ops/cargo_compile.rs +++ b/src/cargo/ops/cargo_compile.rs @@ -507,7 +507,7 @@ pub fn create_bcx<'a, 'cfg>( // TODO: In theory, Cargo should also dedupe the roots, but I'm uncertain // what heuristics to use in that case. if build_config.mode == (CompileMode::Doc { deps: true }) { - remove_duplicate_doc(build_config, &mut unit_graph); + remove_duplicate_doc(build_config, &units, &mut unit_graph); } if build_config @@ -1508,14 +1508,11 @@ fn opt_patterns_and_names( /// - Different sources. See `collision_doc_sources` test. /// /// Ideally this would not be necessary. -fn remove_duplicate_doc(build_config: &BuildConfig, unit_graph: &mut UnitGraph) { - // NOTE: There is some risk that this can introduce problems because it - // may create orphans in the unit graph (parts of the tree get detached - // from the roots). I currently can't think of any ways this will cause a - // problem because all other parts of Cargo traverse the graph starting - // from the roots. Perhaps this should scan for detached units and remove - // them too? - // +fn remove_duplicate_doc( + build_config: &BuildConfig, + root_units: &[Unit], + unit_graph: &mut UnitGraph, +) { // First, create a mapping of crate_name -> Unit so we can see where the // duplicates are. let mut all_docs: HashMap> = HashMap::new(); @@ -1601,4 +1598,18 @@ fn remove_duplicate_doc(build_config: &BuildConfig, unit_graph: &mut UnitGraph) for unit_deps in unit_graph.values_mut() { unit_deps.retain(|unit_dep| !removed_units.contains(&unit_dep.unit)); } + // Remove any orphan units that were detached from the graph. + let mut visited = HashSet::new(); + fn visit(unit: &Unit, graph: &UnitGraph, visited: &mut HashSet) { + if !visited.insert(unit.clone()) { + return; + } + for dep in &graph[unit] { + visit(&dep.unit, graph, visited); + } + } + for unit in root_units { + visit(unit, unit_graph, &mut visited); + } + unit_graph.retain(|unit, _| visited.contains(unit)); } diff --git a/tests/testsuite/collisions.rs b/tests/testsuite/collisions.rs index 4315498169a..d7deebc175c 100644 --- a/tests/testsuite/collisions.rs +++ b/tests/testsuite/collisions.rs @@ -3,9 +3,8 @@ //! Ideally these should never happen, but I don't think we'll ever be able to //! prevent all collisions. -use cargo_test_support::basic_manifest; -use cargo_test_support::project; use cargo_test_support::registry::Package; +use cargo_test_support::{basic_manifest, cross_compile, project}; use std::env; #[cargo_test] @@ -431,3 +430,51 @@ the same path; see . ) .run(); } + +#[cargo_test] +fn collision_doc_target() { + // collision in doc with --target, doesn't fail due to orphans + if cross_compile::disabled() { + return; + } + + Package::new("orphaned", "1.0.0").publish(); + Package::new("bar", "1.0.0") + .dep("orphaned", "1.0") + .publish(); + Package::new("bar", "2.0.0").publish(); + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.1.0" + + [dependencies] + bar2 = { version = "2.0", package="bar" } + bar = "1.0" + "#, + ) + .file("src/lib.rs", "") + .build(); + + p.cargo("doc --target") + .arg(cross_compile::alternate()) + .with_stderr_unordered( + "\ +[UPDATING] [..] +[DOWNLOADING] crates ... +[DOWNLOADED] orphaned v1.0.0 [..] +[DOWNLOADED] bar v2.0.0 [..] +[DOWNLOADED] bar v1.0.0 [..] +[CHECKING] orphaned v1.0.0 +[DOCUMENTING] bar v2.0.0 +[CHECKING] bar v2.0.0 +[CHECKING] bar v1.0.0 +[DOCUMENTING] foo v0.1.0 [..] +[FINISHED] [..] +", + ) + .run(); +} From a9652c077944335bd6ed5d79fd4865d414e6ce94 Mon Sep 17 00:00:00 2001 From: bors Date: Fri, 19 Feb 2021 15:32:24 +0000 Subject: [PATCH 2/4] Auto merge of #9185 - horacimacias:master, r=ehuss Do not exit prematurely if anything failed installing. https://github.com/rust-lang/cargo/issues/9180 --- src/cargo/ops/cargo_install.rs | 16 ++++----- tests/testsuite/install.rs | 61 ++++++++++++++++++++++++++++++++++ 2 files changed, 68 insertions(+), 9 deletions(-) diff --git a/src/cargo/ops/cargo_install.rs b/src/cargo/ops/cargo_install.rs index c15c73a215e..7d72c712cc1 100644 --- a/src/cargo/ops/cargo_install.rs +++ b/src/cargo/ops/cargo_install.rs @@ -119,17 +119,15 @@ pub fn install( // able to run these commands. let dst = root.join("bin").into_path_unlocked(); let path = env::var_os("PATH").unwrap_or_default(); - for path in env::split_paths(&path) { - if path == dst { - return Ok(()); - } - } + let dst_in_path = env::split_paths(&path).any(|path| path == dst); - config.shell().warn(&format!( - "be sure to add `{}` to your PATH to be \ + if !dst_in_path { + config.shell().warn(&format!( + "be sure to add `{}` to your PATH to be \ able to run the installed binaries", - dst.display() - ))?; + dst.display() + ))?; + } } if scheduled_error { diff --git a/tests/testsuite/install.rs b/tests/testsuite/install.rs index 7a38607cc35..6e45c3d19fa 100644 --- a/tests/testsuite/install.rs +++ b/tests/testsuite/install.rs @@ -14,6 +14,8 @@ use cargo_test_support::install::{ assert_has_installed_exe, assert_has_not_installed_exe, cargo_home, }; use cargo_test_support::paths; +use std::env; +use std::path::PathBuf; fn pkg(name: &str, vers: &str) { Package::new(name, vers) @@ -129,6 +131,65 @@ fn multiple_pkgs() { assert_has_not_installed_exe(cargo_home(), "bar"); } +fn path() -> Vec { + env::split_paths(&env::var_os("PATH").unwrap_or_default()).collect() +} + +#[cargo_test] +fn multiple_pkgs_path_set() { + // confirm partial failure results in 101 status code and does not have the + // '[WARNING] be sure to add `[..]` to your PATH to be able to run the installed binaries' + // even if CARGO_HOME/bin is in the PATH + pkg("foo", "0.0.1"); + pkg("bar", "0.0.2"); + + // add CARGO_HOME/bin to path + let mut path = path(); + path.push(cargo_home().join("bin")); + let new_path = env::join_paths(path).unwrap(); + cargo_process("install foo bar baz") + .env("PATH", new_path) + .with_status(101) + .with_stderr( + "\ +[UPDATING] `[..]` index +[DOWNLOADING] crates ... +[DOWNLOADED] foo v0.0.1 (registry `[CWD]/registry`) +[INSTALLING] foo v0.0.1 +[COMPILING] foo v0.0.1 +[FINISHED] release [optimized] target(s) in [..] +[INSTALLING] [CWD]/home/.cargo/bin/foo[EXE] +[INSTALLED] package `foo v0.0.1` (executable `foo[EXE]`) +[DOWNLOADING] crates ... +[DOWNLOADED] bar v0.0.2 (registry `[CWD]/registry`) +[INSTALLING] bar v0.0.2 +[COMPILING] bar v0.0.2 +[FINISHED] release [optimized] target(s) in [..] +[INSTALLING] [CWD]/home/.cargo/bin/bar[EXE] +[INSTALLED] package `bar v0.0.2` (executable `bar[EXE]`) +[ERROR] could not find `baz` in registry `[..]` with version `*` +[SUMMARY] Successfully installed foo, bar! Failed to install baz (see error(s) above). +[ERROR] some crates failed to install +", + ) + .run(); + assert_has_installed_exe(cargo_home(), "foo"); + assert_has_installed_exe(cargo_home(), "bar"); + + cargo_process("uninstall foo bar") + .with_stderr( + "\ +[REMOVING] [CWD]/home/.cargo/bin/foo[EXE] +[REMOVING] [CWD]/home/.cargo/bin/bar[EXE] +[SUMMARY] Successfully uninstalled foo, bar! +", + ) + .run(); + + assert_has_not_installed_exe(cargo_home(), "foo"); + assert_has_not_installed_exe(cargo_home(), "bar"); +} + #[cargo_test] fn pick_max_version() { pkg("foo", "0.1.0"); From 19fd5cb6bd08f5d6670c61a128c87d6a09dec506 Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Wed, 10 Feb 2021 10:58:07 -0800 Subject: [PATCH 3/4] Add a schema version to the index. --- crates/cargo-test-support/src/registry.rs | 19 ++++++++++-- crates/crates-io/lib.rs | 2 ++ src/cargo/ops/registry.rs | 1 + src/cargo/sources/registry/index.rs | 37 ++++++++++++++++++++--- src/cargo/sources/registry/mod.rs | 18 +++++++++++ tests/testsuite/registry.rs | 30 ++++++++++++++++++ 6 files changed, 99 insertions(+), 8 deletions(-) diff --git a/crates/cargo-test-support/src/registry.rs b/crates/cargo-test-support/src/registry.rs index 4a5ff4955df..1be91502868 100644 --- a/crates/cargo-test-support/src/registry.rs +++ b/crates/cargo-test-support/src/registry.rs @@ -327,6 +327,7 @@ pub struct Package { links: Option, rust_version: Option, cargo_features: Vec, + v: Option, } #[derive(Clone)] @@ -401,6 +402,7 @@ impl Package { links: None, rust_version: None, cargo_features: Vec::new(), + v: None, } } @@ -554,6 +556,14 @@ impl Package { self } + /// Sets the index schema version for this package. + /// + /// See [`cargo::sources::registry::RegistryPackage`] for more information. + pub fn schema_version(&mut self, version: u32) -> &mut Package { + self.v = Some(version); + self + } + /// Creates the package and place it in the registry. /// /// This does not actually use Cargo's publishing system, but instead @@ -599,7 +609,7 @@ impl Package { } else { serde_json::json!(self.name) }; - let line = serde_json::json!({ + let mut json = serde_json::json!({ "name": name, "vers": self.vers, "deps": deps, @@ -607,8 +617,11 @@ impl Package { "features": self.features, "yanked": self.yanked, "links": self.links, - }) - .to_string(); + }); + if let Some(v) = self.v { + json["v"] = serde_json::json!(v); + } + let line = json.to_string(); let file = match self.name.len() { 1 => format!("1/{}", self.name), diff --git a/crates/crates-io/lib.rs b/crates/crates-io/lib.rs index 3f76b4b3451..4ae5069de02 100644 --- a/crates/crates-io/lib.rs +++ b/crates/crates-io/lib.rs @@ -56,6 +56,8 @@ pub struct NewCrate { pub repository: Option, pub badges: BTreeMap>, pub links: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub v: Option, } #[derive(Serialize)] diff --git a/src/cargo/ops/registry.rs b/src/cargo/ops/registry.rs index afb1adbf9d2..7032ae13090 100644 --- a/src/cargo/ops/registry.rs +++ b/src/cargo/ops/registry.rs @@ -305,6 +305,7 @@ fn transmit( license_file: license_file.clone(), badges: badges.clone(), links: links.clone(), + v: None, }, tarball, ); diff --git a/src/cargo/sources/registry/index.rs b/src/cargo/sources/registry/index.rs index c88726402f5..2489491c974 100644 --- a/src/cargo/sources/registry/index.rs +++ b/src/cargo/sources/registry/index.rs @@ -72,7 +72,8 @@ use crate::sources::registry::{RegistryData, RegistryPackage}; use crate::util::interning::InternedString; use crate::util::paths; use crate::util::{internal, CargoResult, Config, Filesystem, ToSemver}; -use log::info; +use anyhow::bail; +use log::{debug, info}; use semver::{Version, VersionReq}; use std::collections::{HashMap, HashSet}; use std::fs; @@ -233,6 +234,8 @@ enum MaybeIndexSummary { pub struct IndexSummary { pub summary: Summary, pub yanked: bool, + /// Schema version, see [`RegistryPackage`]. + v: u32, } /// A representation of the cache on disk that Cargo maintains of summaries. @@ -305,6 +308,7 @@ impl<'cfg> RegistryIndex<'cfg> { // minimize the amount of work being done here and parse as little as // necessary. let raw_data = &summaries.raw_data; + let max_version = 1; Ok(summaries .versions .iter_mut() @@ -318,6 +322,19 @@ impl<'cfg> RegistryIndex<'cfg> { } }, ) + .filter(move |is| { + if is.v > max_version { + debug!( + "unsupported schema version {} ({} {})", + is.v, + is.summary.name(), + is.summary.version() + ); + false + } else { + true + } + }) .filter(move |is| { is.summary .unstable_gate(namespaced_features, weak_dep_features) @@ -578,7 +595,14 @@ impl Summaries { // actually happens to verify that our cache is indeed fresh and // computes exactly the same value as before. if cfg!(debug_assertions) && cache_contents.is_some() { - assert_eq!(cache_bytes, cache_contents); + if cache_bytes != cache_contents { + panic!( + "original cache contents:\n{:?}\n\ + does not equal new cache contents:\n{:?}\n", + cache_contents.as_ref().map(|s| String::from_utf8_lossy(s)), + cache_bytes.as_ref().map(|s| String::from_utf8_lossy(s)), + ); + } } // Once we have our `cache_bytes` which represents the `Summaries` we're @@ -659,19 +683,19 @@ impl<'a> SummariesCache<'a> { .split_first() .ok_or_else(|| anyhow::format_err!("malformed cache"))?; if *first_byte != CURRENT_CACHE_VERSION { - anyhow::bail!("looks like a different Cargo's cache, bailing out"); + bail!("looks like a different Cargo's cache, bailing out"); } let mut iter = split(rest, 0); if let Some(update) = iter.next() { if update != last_index_update.as_bytes() { - anyhow::bail!( + bail!( "cache out of date: current index ({}) != cache ({})", last_index_update, str::from_utf8(update)?, ) } } else { - anyhow::bail!("malformed file"); + bail!("malformed file"); } let mut ret = SummariesCache::default(); while let Some(version) = iter.next() { @@ -749,7 +773,9 @@ impl IndexSummary { features, yanked, links, + v, } = serde_json::from_slice(line)?; + let v = v.unwrap_or(1); log::trace!("json parsed registry {}/{}", name, vers); let pkgid = PackageId::new(name, &vers, source_id)?; let deps = deps @@ -761,6 +787,7 @@ impl IndexSummary { Ok(IndexSummary { summary, yanked: yanked.unwrap_or(false), + v, }) } } diff --git a/src/cargo/sources/registry/mod.rs b/src/cargo/sources/registry/mod.rs index 495df40bfce..3142e719b45 100644 --- a/src/cargo/sources/registry/mod.rs +++ b/src/cargo/sources/registry/mod.rs @@ -269,6 +269,24 @@ pub struct RegistryPackage<'a> { /// Added early 2018 (see ), /// can be `None` if published before then. links: Option, + /// The schema version for this entry. + /// + /// If this is None, it defaults to version 1. Entries with unknown + /// versions are ignored. + /// + /// This provides a method to safely introduce changes to index entries + /// and allow older versions of cargo to ignore newer entries it doesn't + /// understand. This is honored as of 1.51, so unfortunately older + /// versions will ignore it, and potentially misinterpret version 1 and + /// newer entries. + /// + /// The intent is that versions older than 1.51 will work with a + /// pre-existing `Cargo.lock`, but they may not correctly process `cargo + /// update` or build a lock from scratch. In that case, cargo may + /// incorrectly select a new package that uses a new index format. A + /// workaround is to downgrade any packages that are incompatible with the + /// `--precise` flag of `cargo update`. + v: Option, } #[test] diff --git a/tests/testsuite/registry.rs b/tests/testsuite/registry.rs index acba4a2c413..441e965744a 100644 --- a/tests/testsuite/registry.rs +++ b/tests/testsuite/registry.rs @@ -2170,3 +2170,33 @@ fn package_lock_inside_package_is_overwritten() { assert_eq!(ok.metadata().unwrap().len(), 2); } + +#[cargo_test] +fn ignores_unknown_index_version() { + // If the version field is not understood, it is ignored. + Package::new("bar", "1.0.0").publish(); + Package::new("bar", "1.0.1").schema_version(9999).publish(); + + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.1.0" + + [dependencies] + bar = "1.0" + "#, + ) + .file("src/lib.rs", "") + .build(); + + p.cargo("tree") + .with_stdout( + "foo v0.1.0 [..]\n\ + └── bar v1.0.0\n\ + ", + ) + .run(); +} From d8649716e56d14c85cda16f756342aa36f5e6211 Mon Sep 17 00:00:00 2001 From: bors Date: Sat, 6 Feb 2021 19:12:01 +0000 Subject: [PATCH 4/4] Auto merge of #9148 - bjorn3:fix_ci, r=ehuss Fix warnings of the new non_fmt_panic lint This fixes the CI failure since the latest nightly. See https://github.com/rust-lang/rust/pull/81645 --- crates/cargo-test-support/src/cross_compile.rs | 2 +- tests/testsuite/cargo_command.rs | 2 +- tests/testsuite/new.rs | 14 +++++++++++--- 3 files changed, 13 insertions(+), 5 deletions(-) diff --git a/crates/cargo-test-support/src/cross_compile.rs b/crates/cargo-test-support/src/cross_compile.rs index 7d3ec335301..ff272d482f3 100644 --- a/crates/cargo-test-support/src/cross_compile.rs +++ b/crates/cargo-test-support/src/cross_compile.rs @@ -177,7 +177,7 @@ rustup does not appear to be installed. Make sure that the appropriate }, } - panic!(message); + panic!("{}", message); } /// The alternate target-triple to build with. diff --git a/tests/testsuite/cargo_command.rs b/tests/testsuite/cargo_command.rs index 75eec676dfb..04e3051c785 100644 --- a/tests/testsuite/cargo_command.rs +++ b/tests/testsuite/cargo_command.rs @@ -365,5 +365,5 @@ fn closed_output_ok() { .unwrap(); let status = child.wait().unwrap(); assert!(status.success()); - assert!(s.is_empty(), s); + assert!(s.is_empty(), "{}", s); } diff --git a/tests/testsuite/new.rs b/tests/testsuite/new.rs index 548810bce26..9a7f71cf125 100644 --- a/tests/testsuite/new.rs +++ b/tests/testsuite/new.rs @@ -370,7 +370,11 @@ fn finds_git_author() { let toml = paths::root().join("foo/Cargo.toml"); let contents = fs::read_to_string(&toml).unwrap(); - assert!(contents.contains(r#"authors = ["foo "]"#), contents); + assert!( + contents.contains(r#"authors = ["foo "]"#), + "{}", + contents + ); } #[cargo_test] @@ -411,7 +415,11 @@ fn finds_git_author_in_included_config() { cargo_process("new foo/bar").run(); let toml = paths::root().join("foo/bar/Cargo.toml"); let contents = fs::read_to_string(&toml).unwrap(); - assert!(contents.contains(r#"authors = ["foo "]"#), contents,); + assert!( + contents.contains(r#"authors = ["foo "]"#), + "{}", + contents + ); } #[cargo_test] @@ -632,7 +640,7 @@ fn new_with_blank_email() { .run(); let contents = fs::read_to_string(paths::root().join("foo/Cargo.toml")).unwrap(); - assert!(contents.contains(r#"authors = ["Sen"]"#), contents); + assert!(contents.contains(r#"authors = ["Sen"]"#), "{}", contents); } #[cargo_test]