Auto merge of #6611 - Eh2406:conservative-updates, r=alexcrichton

keep track of crates that are whitelisted to be used even if yanked This is a start on #6609. It definitely needs tests that the bug is fixed, and to reduce the clones. But for now let's see what CI thinks.
rust-lang · Feb 4, 2019 · 37ad03f · 37ad03f
2 parents 91e610a + 4a2f810
commit 37ad03f
Show file tree

Hide file tree

Showing 16 changed files with 234 additions and 37 deletions.
diff --git 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/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<SourceId, (SourceId, Kind)>,
 
     locked: LockedMap,
+    yanked_whitelist: HashSet<PackageId>,
     source_config: SourceConfigMap<'cfg>,
 
     patches: HashMap<Url, Vec<Summary>>,
@@ -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,14 @@ impl<'cfg> PackageRegistry<'cfg> {
         self.add_source(source, Kind::Override);
     }
 
+    pub fn add_to_yanked_whitelist(&mut self, iter: impl Iterator<Item = PackageId>) {
+        let pkgs = iter.collect::<Vec<_>>();
+        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<PackageId>) {
         trace!("register_lock: {}", id);
         for dep in deps.iter() {
@@ -301,7 +311,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)?;
+            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/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<T> {
     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<Source>`

diff --git a/src/cargo/core/source/source_id.rs b/src/cargo/core/source/source_id.rs
@@ -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<Box<dyn super::Source + 'a>> {
+    pub fn load<'a>(
+        self,
+        config: &'a Config,
+        yanked_whitelist: &HashSet<PackageId>,
+    ) -> CargoResult<Box<dyn super::Source + 'a>> {
         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
@@ -1,4 +1,4 @@
-use std::collections::{BTreeMap, BTreeSet};
+use std::collections::{BTreeMap, BTreeSet, HashSet};
 use std::path::{Path, PathBuf};
 use std::sync::Arc;
 use std::{env, fs};
@@ -181,7 +181,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
@@ -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
@@ -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().filter(keep));
     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
@@ -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<Box<dyn Source + 'cfg>> {
+    pub fn load(
+        &self,
+        id: SourceId,
+        yanked_whitelist: &HashSet<PackageId>,
+    ) -> CargoResult<Box<dyn Source + 'cfg>> {
         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,15 @@ 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
+                .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!(
                 "\

diff --git 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
@@ -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
@@ -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
@@ -1,4 +1,4 @@
-use std::collections::HashMap;
+use std::collections::{HashMap, HashSet};
 use std::path::Path;
 use std::str;
 
@@ -273,14 +273,21 @@ impl<'cfg> RegistryIndex<'cfg> {
         &mut self,
         dep: &Dependency,
         load: &mut dyn RegistryData,
+        yanked_whitelist: &HashSet<PackageId>,
         f: &mut dyn FnMut(Summary),
     ) -> CargoResult<()> {
         let source_id = self.source_id;
         let name = dep.package_name().as_str();
         let summaries = self.summaries(name, load)?;
         let summaries = summaries
             .iter()
-            .filter(|&&(_, yanked)| dep.source_id().precise().is_some() || !yanked)
+            .filter(|&(summary, yanked)| {
+                !yanked || {
+                    log::debug!("{:?}", yanked_whitelist);
+                    log::debug!("{:?}", summary.package_id());
+                    yanked_whitelist.contains(&summary.package_id())
+                }
+            })
             .map(|s| s.0.clone());
 
         // Handle `cargo update --precise` here. If specified, our own source