From ab3d218d6f24dfcd3bc2411bb129f4641fd98314 Mon Sep 17 00:00:00 2001 From: Eh2406 Date: Tue, 29 Jan 2019 16:24:41 -0500 Subject: [PATCH 01/10] keep track of crates that are whitelisted to be used even if yanked --- src/cargo/core/registry.rs | 12 +++++-- src/cargo/core/source/source_id.rs | 15 ++++++-- src/cargo/ops/cargo_install.rs | 4 +-- src/cargo/ops/registry.rs | 6 ++-- src/cargo/ops/resolve.rs | 1 + src/cargo/sources/config.rs | 18 ++++++---- src/cargo/sources/registry/index.rs | 7 ++-- src/cargo/sources/registry/mod.rs | 55 +++++++++++++++++++++-------- tests/testsuite/search.rs | 3 +- 9 files changed, 86 insertions(+), 35 deletions(-) diff --git a/src/cargo/core/registry.rs b/src/cargo/core/registry.rs index 533b4cb0b72..5767493a650 100644 --- a/src/cargo/core/registry.rs +++ b/src/cargo/core/registry.rs @@ -1,4 +1,4 @@ -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use log::{debug, trace}; use semver::VersionReq; @@ -71,6 +71,7 @@ pub struct PackageRegistry<'cfg> { source_ids: HashMap, locked: LockedMap, + yanked_whitelist: HashSet, source_config: SourceConfigMap<'cfg>, patches: HashMap>, @@ -97,6 +98,7 @@ impl<'cfg> PackageRegistry<'cfg> { overrides: Vec::new(), source_config, locked: HashMap::new(), + yanked_whitelist: HashSet::new(), patches: HashMap::new(), patches_locked: false, patches_available: HashMap::new(), @@ -166,6 +168,10 @@ impl<'cfg> PackageRegistry<'cfg> { self.add_source(source, Kind::Override); } + pub fn add_to_yanked_whitelist(&mut self, iter: impl Iterator) { + self.yanked_whitelist.extend(iter) + } + pub fn register_lock(&mut self, id: PackageId, deps: Vec) { trace!("register_lock: {}", id); for dep in deps.iter() { @@ -301,7 +307,9 @@ impl<'cfg> PackageRegistry<'cfg> { fn load(&mut self, source_id: SourceId, kind: Kind) -> CargoResult<()> { (|| { debug!("loading source {}", source_id); - let source = self.source_config.load(source_id)?; + let source = self + .source_config + .load(source_id, self.yanked_whitelist.clone())?; assert_eq!(source.source_id(), source_id); if kind == Kind::Override { diff --git a/src/cargo/core/source/source_id.rs b/src/cargo/core/source/source_id.rs index 3fe12ebe1ad..385e606e242 100644 --- a/src/cargo/core/source/source_id.rs +++ b/src/cargo/core/source/source_id.rs @@ -4,8 +4,8 @@ use std::fmt::{self, Formatter}; use std::hash::{self, Hash}; use std::path::Path; use std::ptr; -use std::sync::atomic::Ordering::SeqCst; use std::sync::atomic::AtomicBool; +use std::sync::atomic::Ordering::SeqCst; use std::sync::Mutex; use log::trace; @@ -13,6 +13,7 @@ use serde::de; use serde::ser; use url::Url; +use crate::core::PackageId; use crate::ops; use crate::sources::git; use crate::sources::DirectorySource; @@ -257,7 +258,11 @@ impl SourceId { } /// Creates an implementation of `Source` corresponding to this ID. - pub fn load<'a>(self, config: &'a Config) -> CargoResult> { + pub fn load<'a>( + self, + config: &'a Config, + yanked_whitelist: HashSet, + ) -> CargoResult> { trace!("loading SourceId; {}", self); match self.inner.kind { Kind::Git(..) => Ok(Box::new(GitSource::new(self, config)?)), @@ -268,7 +273,11 @@ impl SourceId { }; Ok(Box::new(PathSource::new(&path, self, config))) } - Kind::Registry => Ok(Box::new(RegistrySource::remote(self, config))), + Kind::Registry => Ok(Box::new(RegistrySource::remote( + self, + yanked_whitelist, + config, + ))), Kind::LocalRegistry => { let path = match self.inner.url.to_file_path() { Ok(p) => p, diff --git a/src/cargo/ops/cargo_install.rs b/src/cargo/ops/cargo_install.rs index b193131a927..e32e5274444 100644 --- a/src/cargo/ops/cargo_install.rs +++ b/src/cargo/ops/cargo_install.rs @@ -1,5 +1,5 @@ use std::collections::btree_map::Entry; -use std::collections::{BTreeMap, BTreeSet}; +use std::collections::{BTreeMap, BTreeSet, HashSet}; use std::io::prelude::*; use std::io::SeekFrom; use std::path::{Path, PathBuf}; @@ -187,7 +187,7 @@ fn install_one( })? } else { select_pkg( - map.load(source_id)?, + map.load(source_id, HashSet::new())?, krate, vers, config, diff --git a/src/cargo/ops/registry.rs b/src/cargo/ops/registry.rs index 04cf2886918..9889336821e 100644 --- a/src/cargo/ops/registry.rs +++ b/src/cargo/ops/registry.rs @@ -1,4 +1,4 @@ -use std::collections::BTreeMap; +use std::collections::{BTreeMap, HashSet}; use std::fs::{self, File}; use std::io::{self, BufRead}; use std::iter::repeat; @@ -350,7 +350,7 @@ pub fn registry( let token = token.or(token_config); let sid = get_source_id(config, index_config.or(index), registry)?; let api_host = { - let mut src = RegistrySource::remote(sid, config); + let mut src = RegistrySource::remote(sid, HashSet::new(), config); // Only update the index if the config is not available or `force` is set. let cfg = src.config(); let cfg = if force_update || cfg.is_err() { @@ -696,7 +696,7 @@ fn get_source_id( (_, Some(i)) => SourceId::for_registry(&i.to_url()?), _ => { let map = SourceConfigMap::new(config)?; - let src = map.load(SourceId::crates_io(config)?)?; + let src = map.load(SourceId::crates_io(config)?, HashSet::new())?; Ok(src.replaced_source_id()) } } diff --git a/src/cargo/ops/resolve.rs b/src/cargo/ops/resolve.rs index 33f2a2c038c..069670326a3 100644 --- a/src/cargo/ops/resolve.rs +++ b/src/cargo/ops/resolve.rs @@ -471,6 +471,7 @@ fn register_previous_locks( // package's dependencies here as that'll be covered below to poison those // if they changed. let mut avoid_locking = HashSet::new(); + registry.add_to_yanked_whitelist(resolve.iter()); for node in resolve.iter() { if !keep(&node) { add_deps(resolve, node, &mut avoid_locking); diff --git a/src/cargo/sources/config.rs b/src/cargo/sources/config.rs index ccc66b693f2..cee22da457b 100644 --- a/src/cargo/sources/config.rs +++ b/src/cargo/sources/config.rs @@ -4,13 +4,13 @@ //! structure usable by Cargo itself. Currently this is primarily used to map //! sources to one another via the `replace-with` key in `.cargo/config`. -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use std::path::{Path, PathBuf}; use log::debug; use url::Url; -use crate::core::{GitReference, Source, SourceId}; +use crate::core::{GitReference, PackageId, Source, SourceId}; use crate::sources::{ReplacedSource, CRATES_IO_REGISTRY}; use crate::util::config::ConfigValue; use crate::util::errors::{CargoResult, CargoResultExt}; @@ -73,11 +73,15 @@ impl<'cfg> SourceConfigMap<'cfg> { self.config } - pub fn load(&self, id: SourceId) -> CargoResult> { + pub fn load( + &self, + id: SourceId, + yanked_whitelist: HashSet, + ) -> CargoResult> { debug!("loading: {}", id); let mut name = match self.id2name.get(&id) { Some(name) => name, - None => return Ok(id.load(self.config)?), + None => return Ok(id.load(self.config, yanked_whitelist)?), }; let mut path = Path::new("/"); let orig_name = name; @@ -99,7 +103,7 @@ impl<'cfg> SourceConfigMap<'cfg> { name = s; path = p; } - None if id == cfg.id => return Ok(id.load(self.config)?), + None if id == cfg.id => return Ok(id.load(self.config, yanked_whitelist)?), None => { new_id = cfg.id.with_precise(id.precise().map(|s| s.to_string())); break; @@ -116,8 +120,8 @@ impl<'cfg> SourceConfigMap<'cfg> { ) } } - let new_src = new_id.load(self.config)?; - let old_src = id.load(self.config)?; + let new_src = new_id.load(self.config, yanked_whitelist.clone())?; + let old_src = id.load(self.config, yanked_whitelist.clone())?; if !new_src.supports_checksums() && old_src.supports_checksums() { failure::bail!( "\ diff --git a/src/cargo/sources/registry/index.rs b/src/cargo/sources/registry/index.rs index b165c4c65d1..51b39c5ae99 100644 --- a/src/cargo/sources/registry/index.rs +++ b/src/cargo/sources/registry/index.rs @@ -1,4 +1,4 @@ -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use std::path::Path; use std::str; @@ -273,6 +273,7 @@ impl<'cfg> RegistryIndex<'cfg> { &mut self, dep: &Dependency, load: &mut dyn RegistryData, + yanked_whitelist: &HashSet, f: &mut dyn FnMut(Summary), ) -> CargoResult<()> { let source_id = self.source_id; @@ -280,7 +281,9 @@ impl<'cfg> RegistryIndex<'cfg> { let summaries = self.summaries(name, load)?; let summaries = summaries .iter() - .filter(|&&(_, yanked)| dep.source_id().precise().is_some() || !yanked) + .filter(|&(summary, yanked)| { + !yanked || yanked_whitelist.contains(&summary.package_id()) + }) .map(|s| s.0.clone()); // Handle `cargo update --precise` here. If specified, our own source diff --git a/src/cargo/sources/registry/mod.rs b/src/cargo/sources/registry/mod.rs index e2f756aeec0..3c96063e6fa 100644 --- a/src/cargo/sources/registry/mod.rs +++ b/src/cargo/sources/registry/mod.rs @@ -160,6 +160,7 @@ use std::borrow::Cow; use std::collections::BTreeMap; +use std::collections::HashSet; use std::fs::File; use std::path::{Path, PathBuf}; @@ -191,6 +192,7 @@ pub struct RegistrySource<'cfg> { updated: bool, ops: Box, index: index::RegistryIndex<'cfg>, + yanked_whitelist: HashSet, index_locked: bool, } @@ -384,16 +386,34 @@ fn short_name(id: SourceId) -> String { } impl<'cfg> RegistrySource<'cfg> { - pub fn remote(source_id: SourceId, config: &'cfg Config) -> RegistrySource<'cfg> { + pub fn remote( + source_id: SourceId, + yanked_whitelist: HashSet, + config: &'cfg Config, + ) -> RegistrySource<'cfg> { let name = short_name(source_id); let ops = remote::RemoteRegistry::new(source_id, config, &name); - RegistrySource::new(source_id, config, &name, Box::new(ops), true) + RegistrySource::new( + source_id, + config, + &name, + Box::new(ops), + yanked_whitelist, + true, + ) } pub fn local(source_id: SourceId, path: &Path, config: &'cfg Config) -> RegistrySource<'cfg> { let name = short_name(source_id); let ops = local::LocalRegistry::new(path, config, &name); - RegistrySource::new(source_id, config, &name, Box::new(ops), false) + RegistrySource::new( + source_id, + config, + &name, + Box::new(ops), + HashSet::new(), + false, + ) } fn new( @@ -401,6 +421,7 @@ impl<'cfg> RegistrySource<'cfg> { config: &'cfg Config, name: &str, ops: Box, + yanked_whitelist: HashSet, index_locked: bool, ) -> RegistrySource<'cfg> { RegistrySource { @@ -409,6 +430,7 @@ impl<'cfg> RegistrySource<'cfg> { source_id, updated: false, index: index::RegistryIndex::new(source_id, ops.index_path(), config, index_locked), + yanked_whitelist, index_locked, ops, } @@ -505,12 +527,13 @@ impl<'cfg> Source for RegistrySource<'cfg> { if dep.source_id().precise().is_some() && !self.updated { debug!("attempting query without update"); let mut called = false; - self.index.query_inner(dep, &mut *self.ops, &mut |s| { - if dep.matches(&s) { - called = true; - f(s); - } - })?; + self.index + .query_inner(dep, &mut *self.ops, &self.yanked_whitelist, &mut |s| { + if dep.matches(&s) { + called = true; + f(s); + } + })?; if called { return Ok(()); } else { @@ -519,15 +542,17 @@ impl<'cfg> Source for RegistrySource<'cfg> { } } - self.index.query_inner(dep, &mut *self.ops, &mut |s| { - if dep.matches(&s) { - f(s); - } - }) + self.index + .query_inner(dep, &mut *self.ops, &self.yanked_whitelist, &mut |s| { + if dep.matches(&s) { + f(s); + } + }) } fn fuzzy_query(&mut self, dep: &Dependency, f: &mut dyn FnMut(Summary)) -> CargoResult<()> { - self.index.query_inner(dep, &mut *self.ops, f) + self.index + .query_inner(dep, &mut *self.ops, &self.yanked_whitelist, f) } fn supports_checksums(&self) -> bool { diff --git a/tests/testsuite/search.rs b/tests/testsuite/search.rs index f80925a2afd..f150a887302 100644 --- a/tests/testsuite/search.rs +++ b/tests/testsuite/search.rs @@ -1,3 +1,4 @@ +use std::collections::HashSet; use std::fs::{self, File}; use std::io::prelude::*; use std::path::Path; @@ -107,7 +108,7 @@ fn not_update() { let sid = SourceId::for_registry(®istry_url()).unwrap(); let cfg = Config::new(Shell::new(), paths::root(), paths::home().join(".cargo")); - let mut regsrc = RegistrySource::remote(sid, &cfg); + let mut regsrc = RegistrySource::remote(sid, HashSet::new(), &cfg); regsrc.update().unwrap(); cargo_process("search postgres") From 448127631dd940d4f2f7efd669d81a7d0841a106 Mon Sep 17 00:00:00 2001 From: Eh2406 Date: Tue, 29 Jan 2019 16:58:47 -0500 Subject: [PATCH 02/10] only whitelist packages we are keeping from the lockfile --- src/cargo/core/registry.rs | 5 +---- src/cargo/ops/resolve.rs | 1 - 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/src/cargo/core/registry.rs b/src/cargo/core/registry.rs index 5767493a650..d08a0ff37d0 100644 --- a/src/cargo/core/registry.rs +++ b/src/cargo/core/registry.rs @@ -168,15 +168,12 @@ impl<'cfg> PackageRegistry<'cfg> { self.add_source(source, Kind::Override); } - pub fn add_to_yanked_whitelist(&mut self, iter: impl Iterator) { - self.yanked_whitelist.extend(iter) - } - pub fn register_lock(&mut self, id: PackageId, deps: Vec) { trace!("register_lock: {}", id); for dep in deps.iter() { trace!("\t-> {}", dep); } + self.yanked_whitelist.insert(id); let sub_map = self .locked .entry(id.source_id()) diff --git a/src/cargo/ops/resolve.rs b/src/cargo/ops/resolve.rs index 069670326a3..33f2a2c038c 100644 --- a/src/cargo/ops/resolve.rs +++ b/src/cargo/ops/resolve.rs @@ -471,7 +471,6 @@ fn register_previous_locks( // package's dependencies here as that'll be covered below to poison those // if they changed. let mut avoid_locking = HashSet::new(); - registry.add_to_yanked_whitelist(resolve.iter()); for node in resolve.iter() { if !keep(&node) { add_deps(resolve, node, &mut avoid_locking); From 865a847ffd91a1a83e0d65df88e752a14d7ea3d1 Mon Sep 17 00:00:00 2001 From: Eh2406 Date: Tue, 29 Jan 2019 17:12:59 -0500 Subject: [PATCH 03/10] push the rest of the clones to the bottom --- src/cargo/core/registry.rs | 4 +--- src/cargo/core/source/source_id.rs | 2 +- src/cargo/ops/cargo_install.rs | 2 +- src/cargo/ops/registry.rs | 4 ++-- src/cargo/sources/config.rs | 6 +++--- src/cargo/sources/registry/mod.rs | 8 ++++---- tests/testsuite/search.rs | 2 +- 7 files changed, 13 insertions(+), 15 deletions(-) diff --git a/src/cargo/core/registry.rs b/src/cargo/core/registry.rs index d08a0ff37d0..348f2b33a7f 100644 --- a/src/cargo/core/registry.rs +++ b/src/cargo/core/registry.rs @@ -304,9 +304,7 @@ impl<'cfg> PackageRegistry<'cfg> { fn load(&mut self, source_id: SourceId, kind: Kind) -> CargoResult<()> { (|| { debug!("loading source {}", source_id); - let source = self - .source_config - .load(source_id, self.yanked_whitelist.clone())?; + let source = self.source_config.load(source_id, &self.yanked_whitelist)?; assert_eq!(source.source_id(), source_id); if kind == Kind::Override { diff --git a/src/cargo/core/source/source_id.rs b/src/cargo/core/source/source_id.rs index 385e606e242..a96ec53e629 100644 --- a/src/cargo/core/source/source_id.rs +++ b/src/cargo/core/source/source_id.rs @@ -261,7 +261,7 @@ impl SourceId { pub fn load<'a>( self, config: &'a Config, - yanked_whitelist: HashSet, + yanked_whitelist: &HashSet, ) -> CargoResult> { trace!("loading SourceId; {}", self); match self.inner.kind { diff --git a/src/cargo/ops/cargo_install.rs b/src/cargo/ops/cargo_install.rs index e32e5274444..5c898c195fe 100644 --- a/src/cargo/ops/cargo_install.rs +++ b/src/cargo/ops/cargo_install.rs @@ -187,7 +187,7 @@ fn install_one( })? } else { select_pkg( - map.load(source_id, HashSet::new())?, + map.load(source_id, &HashSet::new())?, krate, vers, config, diff --git a/src/cargo/ops/registry.rs b/src/cargo/ops/registry.rs index 9889336821e..4447dc9699e 100644 --- a/src/cargo/ops/registry.rs +++ b/src/cargo/ops/registry.rs @@ -350,7 +350,7 @@ pub fn registry( let token = token.or(token_config); let sid = get_source_id(config, index_config.or(index), registry)?; let api_host = { - let mut src = RegistrySource::remote(sid, HashSet::new(), config); + let mut src = RegistrySource::remote(sid, &HashSet::new(), config); // Only update the index if the config is not available or `force` is set. let cfg = src.config(); let cfg = if force_update || cfg.is_err() { @@ -696,7 +696,7 @@ fn get_source_id( (_, Some(i)) => SourceId::for_registry(&i.to_url()?), _ => { let map = SourceConfigMap::new(config)?; - let src = map.load(SourceId::crates_io(config)?, HashSet::new())?; + let src = map.load(SourceId::crates_io(config)?, &HashSet::new())?; Ok(src.replaced_source_id()) } } diff --git a/src/cargo/sources/config.rs b/src/cargo/sources/config.rs index cee22da457b..ea873af647b 100644 --- a/src/cargo/sources/config.rs +++ b/src/cargo/sources/config.rs @@ -76,7 +76,7 @@ impl<'cfg> SourceConfigMap<'cfg> { pub fn load( &self, id: SourceId, - yanked_whitelist: HashSet, + yanked_whitelist: &HashSet, ) -> CargoResult> { debug!("loading: {}", id); let mut name = match self.id2name.get(&id) { @@ -120,8 +120,8 @@ impl<'cfg> SourceConfigMap<'cfg> { ) } } - let new_src = new_id.load(self.config, yanked_whitelist.clone())?; - let old_src = id.load(self.config, yanked_whitelist.clone())?; + let new_src = new_id.load(self.config, yanked_whitelist)?; + let old_src = id.load(self.config, yanked_whitelist)?; if !new_src.supports_checksums() && old_src.supports_checksums() { failure::bail!( "\ diff --git a/src/cargo/sources/registry/mod.rs b/src/cargo/sources/registry/mod.rs index 3c96063e6fa..c2b97e1dd9b 100644 --- a/src/cargo/sources/registry/mod.rs +++ b/src/cargo/sources/registry/mod.rs @@ -388,7 +388,7 @@ fn short_name(id: SourceId) -> String { impl<'cfg> RegistrySource<'cfg> { pub fn remote( source_id: SourceId, - yanked_whitelist: HashSet, + yanked_whitelist: &HashSet, config: &'cfg Config, ) -> RegistrySource<'cfg> { let name = short_name(source_id); @@ -411,7 +411,7 @@ impl<'cfg> RegistrySource<'cfg> { config, &name, Box::new(ops), - HashSet::new(), + &HashSet::new(), false, ) } @@ -421,7 +421,7 @@ impl<'cfg> RegistrySource<'cfg> { config: &'cfg Config, name: &str, ops: Box, - yanked_whitelist: HashSet, + yanked_whitelist: &HashSet, index_locked: bool, ) -> RegistrySource<'cfg> { RegistrySource { @@ -430,7 +430,7 @@ impl<'cfg> RegistrySource<'cfg> { source_id, updated: false, index: index::RegistryIndex::new(source_id, ops.index_path(), config, index_locked), - yanked_whitelist, + yanked_whitelist: yanked_whitelist.clone(), index_locked, ops, } diff --git a/tests/testsuite/search.rs b/tests/testsuite/search.rs index f150a887302..e0f7f4807ec 100644 --- a/tests/testsuite/search.rs +++ b/tests/testsuite/search.rs @@ -108,7 +108,7 @@ fn not_update() { let sid = SourceId::for_registry(®istry_url()).unwrap(); let cfg = Config::new(Shell::new(), paths::root(), paths::home().join(".cargo")); - let mut regsrc = RegistrySource::remote(sid, HashSet::new(), &cfg); + let mut regsrc = RegistrySource::remote(sid, &HashSet::new(), &cfg); regsrc.update().unwrap(); cargo_process("search postgres") From 6fa66020e52bfc6d27674f06ad630dd149106427 Mon Sep 17 00:00:00 2001 From: Eh2406 Date: Wed, 30 Jan 2019 10:53:04 -0500 Subject: [PATCH 04/10] [WIP] add tests why does this not pass!? --- tests/testsuite/registry.rs | 98 +++++++++++++++++++++++++++++++++++++ 1 file changed, 98 insertions(+) diff --git a/tests/testsuite/registry.rs b/tests/testsuite/registry.rs index 31babf88b75..1a10b6138f9 100644 --- a/tests/testsuite/registry.rs +++ b/tests/testsuite/registry.rs @@ -590,6 +590,104 @@ required by package `foo v0.0.1 ([..])` .run(); } +#[test] +fn yanks_in_lockfiles_are_ok_for_other_update() { + let p = project() + .file( + "Cargo.toml", + r#" + [project] + name = "foo" + version = "0.0.1" + authors = [] + + [dependencies] + bar = "*" + baz = "*" + "#, + ) + .file("src/main.rs", "fn main() {}") + .build(); + + Package::new("bar", "0.0.1").publish(); + Package::new("baz", "0.0.1").publish(); + + p.cargo("build").run(); + + registry_path().join("3").rm_rf(); + + Package::new("bar", "0.0.1").yanked(true).publish(); + Package::new("baz", "0.0.1").publish(); + + p.cargo("build").with_stdout("").run(); + + Package::new("baz", "0.0.2").publish(); + + p.cargo("update") + .with_status(101) + .with_stderr_contains( + "\ +error: no matching package named `bar` found +location searched: registry [..] +required by package `foo v0.0.1 ([..])` +", + ) + .run(); + + p.cargo("update -p baz") + .with_status(101) + .with_stderr_contains( + "\ +[UPDATING] `[..]` index +[UPDATING] baz v0.0.1 -> v0.0.2 +", + ) + .run(); +} + +#[test] +fn yanks_in_lockfiles_are_ok_with_new_dep() { + let p = project() + .file( + "Cargo.toml", + r#" + [project] + name = "foo" + version = "0.0.1" + authors = [] + + [dependencies] + bar = "*" + "#, + ) + .file("src/main.rs", "fn main() {}") + .build(); + + Package::new("bar", "0.0.1").publish(); + + p.cargo("build").run(); + + registry_path().join("3").rm_rf(); + + Package::new("bar", "0.0.1").yanked(true).publish(); + Package::new("baz", "0.0.1").publish(); + + t!(t!(File::create(p.root().join("Cargo.toml"))).write_all( + br#" + [project] + name = "foo" + version = "0.0.1" + authors = [] + + [dependencies] + bar = "*" + baz = "*" + "# + )); + + p.cargo("build").with_stdout("").run(); +} + #[test] fn update_with_lockfile_if_packages_missing() { let p = project() From d611ba16c3419d34ee20c239c9882a47173a0f69 Mon Sep 17 00:00:00 2001 From: Eh2406 Date: Wed, 30 Jan 2019 17:33:25 -0500 Subject: [PATCH 05/10] map_source for ReplacedSource --- src/cargo/core/package_id.rs | 8 ++++++++ src/cargo/sources/config.rs | 9 ++++++++- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/src/cargo/core/package_id.rs b/src/cargo/core/package_id.rs index c25a668e4e9..11c86e9006d 100644 --- a/src/cargo/core/package_id.rs +++ b/src/cargo/core/package_id.rs @@ -155,6 +155,14 @@ impl PackageId { PackageId::pure(self.inner.name, self.inner.version.clone(), source) } + pub fn map_source(self, to_replace: SourceId, replace_with: SourceId) -> Self { + if self.source_id() == to_replace { + self.with_source_id(replace_with) + } else { + self + } + } + pub fn stable_hash(self, workspace: &Path) -> PackageIdStableHash<'_> { PackageIdStableHash(self, workspace) } diff --git a/src/cargo/sources/config.rs b/src/cargo/sources/config.rs index ea873af647b..e1975fc624d 100644 --- a/src/cargo/sources/config.rs +++ b/src/cargo/sources/config.rs @@ -120,7 +120,14 @@ impl<'cfg> SourceConfigMap<'cfg> { ) } } - let new_src = new_id.load(self.config, yanked_whitelist)?; + + let new_src = new_id.load( + self.config, + &yanked_whitelist + .iter() + .map(|p| p.map_source(id, new_id)) + .collect(), + )?; let old_src = id.load(self.config, yanked_whitelist)?; if !new_src.supports_checksums() && old_src.supports_checksums() { failure::bail!( From a539ea345a805e8c04f6473f52c98d7b32c19c99 Mon Sep 17 00:00:00 2001 From: Eh2406 Date: Thu, 31 Jan 2019 11:58:24 -0500 Subject: [PATCH 06/10] Revert "only whitelist packages we are keeping from the lockfile" This reverts commit 4fb53e53 --- src/cargo/core/registry.rs | 5 ++++- src/cargo/ops/resolve.rs | 1 + 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/cargo/core/registry.rs b/src/cargo/core/registry.rs index 348f2b33a7f..f74b507a89b 100644 --- a/src/cargo/core/registry.rs +++ b/src/cargo/core/registry.rs @@ -168,12 +168,15 @@ impl<'cfg> PackageRegistry<'cfg> { self.add_source(source, Kind::Override); } + pub fn add_to_yanked_whitelist(&mut self, iter: impl Iterator) { + self.yanked_whitelist.extend(iter) + } + pub fn register_lock(&mut self, id: PackageId, deps: Vec) { trace!("register_lock: {}", id); for dep in deps.iter() { trace!("\t-> {}", dep); } - self.yanked_whitelist.insert(id); let sub_map = self .locked .entry(id.source_id()) diff --git a/src/cargo/ops/resolve.rs b/src/cargo/ops/resolve.rs index 33f2a2c038c..069670326a3 100644 --- a/src/cargo/ops/resolve.rs +++ b/src/cargo/ops/resolve.rs @@ -471,6 +471,7 @@ fn register_previous_locks( // package's dependencies here as that'll be covered below to poison those // if they changed. let mut avoid_locking = HashSet::new(); + registry.add_to_yanked_whitelist(resolve.iter()); for node in resolve.iter() { if !keep(&node) { add_deps(resolve, node, &mut avoid_locking); From 3b45cf742bb921b7eda35d0c37e730acab332841 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Fri, 1 Feb 2019 10:48:58 -0500 Subject: [PATCH 07/10] Don't add the entire resolve to the yanked whitelist If we're updating a crate then its previous version in the lock file is no longer whitelisted. We may want to perhaps change this in the future! --- src/cargo/ops/resolve.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cargo/ops/resolve.rs b/src/cargo/ops/resolve.rs index 069670326a3..81ce837ad1b 100644 --- a/src/cargo/ops/resolve.rs +++ b/src/cargo/ops/resolve.rs @@ -471,7 +471,7 @@ fn register_previous_locks( // package's dependencies here as that'll be covered below to poison those // if they changed. let mut avoid_locking = HashSet::new(); - registry.add_to_yanked_whitelist(resolve.iter()); + registry.add_to_yanked_whitelist(resolve.iter().filter(keep)); for node in resolve.iter() { if !keep(&node) { add_deps(resolve, node, &mut avoid_locking); From da3028b5a84cd37289c04e80fcea6cd74b945132 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Fri, 1 Feb 2019 10:49:35 -0500 Subject: [PATCH 08/10] Fix a test assertion --- tests/testsuite/registry.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/testsuite/registry.rs b/tests/testsuite/registry.rs index 1a10b6138f9..f742c32a37c 100644 --- a/tests/testsuite/registry.rs +++ b/tests/testsuite/registry.rs @@ -635,7 +635,7 @@ required by package `foo v0.0.1 ([..])` .run(); p.cargo("update -p baz") - .with_status(101) + .with_status(0) .with_stderr_contains( "\ [UPDATING] `[..]` index From 18f7f17bc9ccb48a6fceb0a8372b8d4b856ca618 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Sat, 2 Feb 2019 13:44:19 -0500 Subject: [PATCH 09/10] Update existing sources when whitelist modified When sources already exist in a `PackageRegistry` and the whitelist is updated in the package registry, be sure to update all the existing sources to ensure that everyone gets the same view of the intended whitelist. --- src/cargo/core/registry.rs | 6 +++++- src/cargo/core/source/mod.rs | 13 +++++++++++++ src/cargo/sources/directory.rs | 2 ++ src/cargo/sources/git/source.rs | 2 ++ src/cargo/sources/path.rs | 2 ++ src/cargo/sources/registry/index.rs | 6 +++++- src/cargo/sources/registry/mod.rs | 4 ++++ src/cargo/sources/replaced.rs | 8 ++++++++ 8 files changed, 41 insertions(+), 2 deletions(-) diff --git a/src/cargo/core/registry.rs b/src/cargo/core/registry.rs index f74b507a89b..bc1667071ae 100644 --- a/src/cargo/core/registry.rs +++ b/src/cargo/core/registry.rs @@ -169,7 +169,11 @@ impl<'cfg> PackageRegistry<'cfg> { } pub fn add_to_yanked_whitelist(&mut self, iter: impl Iterator) { - self.yanked_whitelist.extend(iter) + let pkgs = iter.collect::>(); + for (_, source) in self.sources.sources_mut() { + source.add_to_yanked_whitelist(&pkgs); + } + self.yanked_whitelist.extend(pkgs); } pub fn register_lock(&mut self, id: PackageId, deps: Vec) { diff --git a/src/cargo/core/source/mod.rs b/src/cargo/core/source/mod.rs index 62583147c99..50d1c63d869 100644 --- a/src/cargo/core/source/mod.rs +++ b/src/cargo/core/source/mod.rs @@ -83,6 +83,11 @@ pub trait Source { fn is_replaced(&self) -> bool { false } + + /// Add a number of crates that should be whitelisted for showing up during + /// queries, even if they are yanked. Currently only applies to registry + /// sources. + fn add_to_yanked_whitelist(&mut self, pkgs: &[PackageId]); } pub enum MaybePackage { @@ -152,6 +157,10 @@ impl<'a, T: Source + ?Sized + 'a> Source for Box { fn is_replaced(&self) -> bool { (**self).is_replaced() } + + fn add_to_yanked_whitelist(&mut self, pkgs: &[PackageId]) { + (**self).add_to_yanked_whitelist(pkgs); + } } impl<'a, T: Source + ?Sized + 'a> Source for &'a mut T { @@ -206,6 +215,10 @@ impl<'a, T: Source + ?Sized + 'a> Source for &'a mut T { fn is_replaced(&self) -> bool { (**self).is_replaced() } + + fn add_to_yanked_whitelist(&mut self, pkgs: &[PackageId]) { + (**self).add_to_yanked_whitelist(pkgs); + } } /// A `HashMap` of `SourceId` -> `Box` diff --git a/src/cargo/sources/directory.rs b/src/cargo/sources/directory.rs index c1a4b4dc771..8009a149dc0 100644 --- a/src/cargo/sources/directory.rs +++ b/src/cargo/sources/directory.rs @@ -214,4 +214,6 @@ impl<'cfg> Source for DirectorySource<'cfg> { fn describe(&self) -> String { format!("directory source `{}`", self.root.display()) } + + fn add_to_yanked_whitelist(&mut self, _pkgs: &[PackageId]) {} } diff --git a/src/cargo/sources/git/source.rs b/src/cargo/sources/git/source.rs index 9b96ede0308..ff60990254f 100644 --- a/src/cargo/sources/git/source.rs +++ b/src/cargo/sources/git/source.rs @@ -238,6 +238,8 @@ impl<'cfg> Source for GitSource<'cfg> { fn describe(&self) -> String { format!("git repository {}", self.source_id) } + + fn add_to_yanked_whitelist(&mut self, _pkgs: &[PackageId]) {} } #[cfg(test)] diff --git a/src/cargo/sources/path.rs b/src/cargo/sources/path.rs index e95ccd7bf67..9ec74851233 100644 --- a/src/cargo/sources/path.rs +++ b/src/cargo/sources/path.rs @@ -569,4 +569,6 @@ impl<'cfg> Source for PathSource<'cfg> { Err(_) => self.source_id.to_string(), } } + + fn add_to_yanked_whitelist(&mut self, _pkgs: &[PackageId]) {} } diff --git a/src/cargo/sources/registry/index.rs b/src/cargo/sources/registry/index.rs index 51b39c5ae99..249eb65f79a 100644 --- a/src/cargo/sources/registry/index.rs +++ b/src/cargo/sources/registry/index.rs @@ -282,7 +282,11 @@ impl<'cfg> RegistryIndex<'cfg> { let summaries = summaries .iter() .filter(|&(summary, yanked)| { - !yanked || yanked_whitelist.contains(&summary.package_id()) + !yanked || { + log::debug!("{:?}", yanked_whitelist); + log::debug!("{:?}", summary.package_id()); + yanked_whitelist.contains(&summary.package_id()) + } }) .map(|s| s.0.clone()); diff --git a/src/cargo/sources/registry/mod.rs b/src/cargo/sources/registry/mod.rs index c2b97e1dd9b..363e4b18355 100644 --- a/src/cargo/sources/registry/mod.rs +++ b/src/cargo/sources/registry/mod.rs @@ -606,4 +606,8 @@ impl<'cfg> Source for RegistrySource<'cfg> { fn describe(&self) -> String { self.source_id.display_registry() } + + fn add_to_yanked_whitelist(&mut self, pkgs: &[PackageId]) { + self.yanked_whitelist.extend(pkgs); + } } diff --git a/src/cargo/sources/replaced.rs b/src/cargo/sources/replaced.rs index 4a4de90a419..1a8b1885530 100644 --- a/src/cargo/sources/replaced.rs +++ b/src/cargo/sources/replaced.rs @@ -113,4 +113,12 @@ impl<'cfg> Source for ReplacedSource<'cfg> { fn is_replaced(&self) -> bool { true } + + fn add_to_yanked_whitelist(&mut self, pkgs: &[PackageId]) { + let pkgs = pkgs + .iter() + .map(|id| id.with_source_id(self.replace_with)) + .collect::>(); + self.inner.add_to_yanked_whitelist(&pkgs); + } } From 63412be12bddef17e88a003f18e9747a8fa5978f Mon Sep 17 00:00:00 2001 From: Pietro Albini Date: Sun, 10 Feb 2019 13:14:03 +0100 Subject: [PATCH 10/10] switch from unused_imports to deprecated to test unfixable warnings The unused_imports warning is going to emit fixable suggestions in the near future, but that means parts of the cargo's test suite will break. This commit switches the tests to use the deprecated warning, which *shouldn't* be fixable at all. --- tests/testsuite/fix.rs | 63 +++++++++++++++++++++++++----------------- 1 file changed, 38 insertions(+), 25 deletions(-) diff --git a/tests/testsuite/fix.rs b/tests/testsuite/fix.rs index 89c81fd42a8..c4242afab12 100644 --- a/tests/testsuite/fix.rs +++ b/tests/testsuite/fix.rs @@ -702,11 +702,11 @@ fn fix_features() { #[test] fn shows_warnings() { let p = project() - .file("src/lib.rs", "use std::default::Default; pub fn foo() {}") + .file("src/lib.rs", "#[deprecated] fn bar() {} pub fn foo() { let _ = bar(); }") .build(); p.cargo("fix --allow-no-vcs") - .with_stderr_contains("[..]warning: unused import[..]") + .with_stderr_contains("[..]warning: use of deprecated item[..]") .run(); } @@ -984,20 +984,22 @@ fn shows_warnings_on_second_run_without_changes() { .file( "src/lib.rs", r#" - use std::default::Default; + #[deprecated] + fn bar() {} pub fn foo() { + let _ = bar(); } "#, ) .build(); p.cargo("fix --allow-no-vcs") - .with_stderr_contains("[..]warning: unused import[..]") + .with_stderr_contains("[..]warning: use of deprecated item[..]") .run(); p.cargo("fix --allow-no-vcs") - .with_stderr_contains("[..]warning: unused import[..]") + .with_stderr_contains("[..]warning: use of deprecated item[..]") .run(); } @@ -1007,65 +1009,76 @@ fn shows_warnings_on_second_run_without_changes_on_multiple_targets() { .file( "src/lib.rs", r#" - use std::default::Default; + #[deprecated] + fn bar() {} - pub fn a() -> u32 { 3 } + pub fn foo() { + let _ = bar(); + } "#, ) .file( "src/main.rs", r#" - use std::default::Default; - fn main() { println!("3"); } + #[deprecated] + fn bar() {} + + fn main() { + let _ = bar(); + } "#, ) .file( "tests/foo.rs", r#" - use std::default::Default; + #[deprecated] + fn bar() {} + #[test] fn foo_test() { - println!("3"); + let _ = bar(); } "#, ) .file( "tests/bar.rs", r#" - use std::default::Default; + #[deprecated] + fn bar() {} #[test] fn foo_test() { - println!("3"); + let _ = bar(); } "#, ) .file( "examples/fooxample.rs", r#" - use std::default::Default; + #[deprecated] + fn bar() {} fn main() { - println!("3"); + let _ = bar(); } "#, ) .build(); p.cargo("fix --allow-no-vcs --all-targets") - .with_stderr_contains(" --> examples/fooxample.rs:2:21") - .with_stderr_contains(" --> src/lib.rs:2:21") - .with_stderr_contains(" --> src/main.rs:2:21") - .with_stderr_contains(" --> tests/bar.rs:2:21") - .with_stderr_contains(" --> tests/foo.rs:2:21") + .with_stderr_contains(" --> examples/fooxample.rs:6:29") + .with_stderr_contains(" --> src/lib.rs:6:29") + .with_stderr_contains(" --> src/main.rs:6:29") + .with_stderr_contains(" --> tests/bar.rs:7:29") + .with_stderr_contains(" --> tests/foo.rs:7:29") .run(); p.cargo("fix --allow-no-vcs --all-targets") - .with_stderr_contains(" --> examples/fooxample.rs:2:21") - .with_stderr_contains(" --> src/lib.rs:2:21") - .with_stderr_contains(" --> src/main.rs:2:21") - .with_stderr_contains(" --> tests/bar.rs:2:21") - .with_stderr_contains(" --> tests/foo.rs:2:21") + .with_stderr_contains(" --> examples/fooxample.rs:6:29") + .with_stderr_contains(" --> src/lib.rs:6:29") + .with_stderr_contains(" --> src/main.rs:6:29") + .with_stderr_contains(" --> tests/bar.rs:7:29") + .with_stderr_contains(" --> tests/foo.rs:7:29") .run(); }