Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

keep track of crates that are whitelisted to be used even if yanked #6611

Merged
merged 9 commits into from
Feb 4, 2019
Next Next commit
keep track of crates that are whitelisted to be used even if yanked
  • Loading branch information
Eh2406 committed Jan 29, 2019
commit 4400514638a1a99a3df17d224f7eb936e314bea7
12 changes: 10 additions & 2 deletions src/cargo/core/registry.rs
Original file line number Diff line number Diff line change
@@ -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,10 @@ impl<'cfg> PackageRegistry<'cfg> {
self.add_source(source, Kind::Override);
}

pub fn add_to_yanked_whitelist(&mut self, iter: impl Iterator<Item = PackageId>) {
self.yanked_whitelist.extend(iter)
alexcrichton marked this conversation as resolved.
Show resolved Hide resolved
}

pub fn register_lock(&mut self, id: PackageId, deps: Vec<PackageId>) {
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 {
13 changes: 11 additions & 2 deletions src/cargo/core/source/source_id.rs
Original file line number Diff line number Diff line change
@@ -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>,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might help clean things up to plumb through &HashSet<PackageId> mostly and then only clone when necessary at the bottom

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have been experimenting but plumbing thure the lifetime can be annoying. I will try again.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh and to be clear I wouldn't connect the lifetime to anything, only passing around a borrow and then when it needs to be stored cloning it

) -> 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,
4 changes: 2 additions & 2 deletions src/cargo/ops/cargo_install.rs
Original file line number Diff line number Diff line change
@@ -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,
6 changes: 3 additions & 3 deletions src/cargo/ops/registry.rs
Original file line number Diff line number Diff line change
@@ -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())
}
}
1 change: 1 addition & 0 deletions src/cargo/ops/resolve.rs
Original file line number Diff line number Diff line change
@@ -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);
18 changes: 11 additions & 7 deletions src/cargo/sources/config.rs
Original file line number Diff line number Diff line change
@@ -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,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!(
"\
7 changes: 5 additions & 2 deletions src/cargo/sources/registry/index.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use std::collections::HashMap;
use std::collections::{HashMap, HashSet};
use std::path::Path;
use std::str;

@@ -273,14 +273,17 @@ 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 || yanked_whitelist.contains(&summary.package_id())
})
.map(|s| s.0.clone());

// Handle `cargo update --precise` here. If specified, our own source
59 changes: 41 additions & 18 deletions src/cargo/sources/registry/mod.rs
Original file line number Diff line number Diff line change
@@ -160,6 +160,7 @@

use std::borrow::Cow;
use std::collections::BTreeMap;
use std::collections::HashSet;
use std::io::Write;
use std::path::{Path, PathBuf};

@@ -192,6 +193,7 @@ pub struct RegistrySource<'cfg> {
updated: bool,
ops: Box<dyn RegistryData + 'cfg>,
index: index::RegistryIndex<'cfg>,
yanked_whitelist: HashSet<PackageId>,
index_locked: bool,
}

@@ -385,23 +387,42 @@ 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<PackageId>,
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(
source_id: SourceId,
config: &'cfg Config,
name: &str,
ops: Box<dyn RegistryData + 'cfg>,
yanked_whitelist: HashSet<PackageId>,
index_locked: bool,
) -> RegistrySource<'cfg> {
RegistrySource {
@@ -410,6 +431,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,
}
@@ -431,9 +453,7 @@ impl<'cfg> RegistrySource<'cfg> {
// unpacked and to lock the directory for unpacking.
let mut ok = {
let package_dir = format!("{}-{}", pkg.name(), pkg.version());
let dst = self
.src_path
.join(&package_dir);
let dst = self.src_path.join(&package_dir);
dst.create_dir()?;

// Attempt to open a read-only copy first to avoid an exclusive write
@@ -526,12 +546,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 {
@@ -540,15 +561,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 {
3 changes: 2 additions & 1 deletion tests/testsuite/search.rs
Original file line number Diff line number Diff line change
@@ -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(&registry_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")