Skip to content

Fix caching features across backtracking #2484

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

Merged
merged 1 commit into from
Mar 17, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 18 additions & 11 deletions src/cargo/core/resolver/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,7 @@ struct BacktrackFrame {
remaining_candidates: RcVecIter<Rc<Summary>>,
parent: Rc<Summary>,
dep: Dependency,
features: Vec<String>,
}

/// Recursively activates the dependencies for `top`, in depth-first order,
Expand Down Expand Up @@ -319,15 +320,9 @@ fn activate_deps_loop(mut cx: Context,
}
None => continue,
};
let (mut parent, (mut cur, (mut dep, candidates, features))) = frame;
let (mut parent, (mut cur, (mut dep, candidates, mut features))) = frame;
assert!(!remaining_deps.is_empty());

let method = Method::Required {
dev_deps: false,
features: &features,
uses_default_features: dep.uses_default_features(),
};

let my_candidates = {
let prev_active = cx.prev_active(&dep);
trace!("{}[{}]>{} {} candidates", parent.name(), cur, dep.name(),
Expand Down Expand Up @@ -373,6 +368,7 @@ fn activate_deps_loop(mut cx: Context,
remaining_candidates: remaining_candidates,
parent: parent.clone(),
dep: dep.clone(),
features: features.clone(),
});
candidate
}
Expand All @@ -383,9 +379,13 @@ fn activate_deps_loop(mut cx: Context,
// their state at the found level of the `backtrack_stack`.
trace!("{}[{}]>{} -- no candidates", parent.name(), cur,
dep.name());
match find_candidate(&mut backtrack_stack, &mut cx,
&mut remaining_deps, &mut parent, &mut cur,
&mut dep) {
match find_candidate(&mut backtrack_stack,
&mut cx,
&mut remaining_deps,
&mut parent,
&mut cur,
&mut dep,
&mut features) {
None => return Err(activation_error(&cx, registry, &parent,
&dep,
&cx.prev_active(&dep),
Expand All @@ -395,6 +395,11 @@ fn activate_deps_loop(mut cx: Context,
}
};

let method = Method::Required {
dev_deps: false,
features: &features,
uses_default_features: dep.uses_default_features(),
};
trace!("{}[{}]>{} trying {}", parent.name(), cur, dep.name(),
candidate.version());
cx.resolve.graph.link(parent.package_id().clone(),
Expand All @@ -414,14 +419,16 @@ fn find_candidate(backtrack_stack: &mut Vec<BacktrackFrame>,
remaining_deps: &mut BinaryHeap<DepsFrame>,
parent: &mut Rc<Summary>,
cur: &mut usize,
dep: &mut Dependency) -> Option<Rc<Summary>> {
dep: &mut Dependency,
features: &mut Vec<String>) -> Option<Rc<Summary>> {
while let Some(mut frame) = backtrack_stack.pop() {
if let Some((_, candidate)) = frame.remaining_candidates.next() {
*cx = frame.context_backup.clone();
*remaining_deps = frame.deps_backup.clone();
*parent = frame.parent.clone();
*cur = remaining_deps.peek().unwrap().remaining_siblings.cur_index();
*dep = frame.dep.clone();
*features = frame.features.clone();
backtrack_stack.push(frame);
return Some(candidate)
}
Expand Down
93 changes: 64 additions & 29 deletions tests/support/registry.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use std::collections::HashMap;
use std::fs::{self, File};
use std::io::prelude::*;
use std::path::{PathBuf, Path};
Expand All @@ -6,6 +7,7 @@ use flate2::Compression::Default;
use flate2::write::GzEncoder;
use git2;
use rustc_serialize::hex::ToHex;
use rustc_serialize::json::ToJson;
use tar::{Builder, Header};
use url::Url;

Expand All @@ -21,9 +23,18 @@ pub fn dl_url() -> Url { Url::from_file_path(&*dl_path()).ok().unwrap() }
pub struct Package {
name: String,
vers: String,
deps: Vec<(String, String, &'static str, String)>,
deps: Vec<Dependency>,
files: Vec<(String, String)>,
yanked: bool,
features: HashMap<String, Vec<String>>,
}

struct Dependency {
name: String,
vers: String,
kind: String,
target: Option<String>,
features: Vec<String>,
}

fn init() {
Expand Down Expand Up @@ -55,6 +66,7 @@ impl Package {
deps: Vec::new(),
files: Vec::new(),
yanked: false,
features: HashMap::new(),
}
}

Expand All @@ -64,23 +76,40 @@ impl Package {
}

pub fn dep(&mut self, name: &str, vers: &str) -> &mut Package {
self.deps.push((name.to_string(), vers.to_string(), "normal",
"null".to_string()));
self
self.full_dep(name, vers, None, "normal", &[])
}

pub fn feature_dep(&mut self,
name: &str,
vers: &str,
features: &[&str]) -> &mut Package {
self.full_dep(name, vers, None, "normal", features)
}

pub fn target_dep(&mut self,
name: &str,
vers: &str,
target: &str) -> &mut Package {
self.deps.push((name.to_string(), vers.to_string(), "normal",
format!("\"{}\"", target)));
self
self.full_dep(name, vers, Some(target), "normal", &[])
}

pub fn dev_dep(&mut self, name: &str, vers: &str) -> &mut Package {
self.deps.push((name.to_string(), vers.to_string(), "dev",
"null".to_string()));
self.full_dep(name, vers, None, "dev", &[])
}

fn full_dep(&mut self,
name: &str,
vers: &str,
target: Option<&str>,
kind: &str,
features: &[&str]) -> &mut Package {
self.deps.push(Dependency {
name: name.to_string(),
vers: vers.to_string(),
kind: kind.to_string(),
target: target.map(|s| s.to_string()),
features: features.iter().map(|s| s.to_string()).collect(),
});
self
}

Expand All @@ -89,30 +118,36 @@ impl Package {
self
}

#[allow(deprecated)] // connect => join in 1.3
pub fn publish(&self) {
self.make_archive();

// Figure out what we're going to write into the index
let deps = self.deps.iter().map(|&(ref name, ref req, ref kind, ref target)| {
format!("{{\"name\":\"{}\",\
\"req\":\"{}\",\
\"features\":[],\
\"default_features\":false,\
\"target\":{},\
\"optional\":false,\
\"kind\":\"{}\"}}", name, req, target, kind)
}).collect::<Vec<_>>().connect(",");
let deps = self.deps.iter().map(|dep| {
let mut map = HashMap::new();
map.insert("name".to_string(), dep.name.to_json());
map.insert("req".to_string(), dep.vers.to_json());
map.insert("features".to_string(), dep.features.to_json());
map.insert("default_features".to_string(), false.to_json());
map.insert("target".to_string(), dep.target.to_json());
map.insert("optional".to_string(), false.to_json());
map.insert("kind".to_string(), dep.kind.to_json());
map
}).collect::<Vec<_>>();
let cksum = {
let mut c = Vec::new();
File::open(&self.archive_dst()).unwrap()
.read_to_end(&mut c).unwrap();
cksum(&c)
};
let line = format!("{{\"name\":\"{}\",\"vers\":\"{}\",\
\"deps\":[{}],\"cksum\":\"{}\",\"features\":{{}},\
\"yanked\":{}}}",
self.name, self.vers, deps, cksum, self.yanked);
let mut dep = HashMap::new();
dep.insert("name".to_string(), self.name.to_json());
dep.insert("vers".to_string(), self.vers.to_json());
dep.insert("deps".to_string(), deps.to_json());
dep.insert("cksum".to_string(), cksum.to_json());
dep.insert("features".to_string(), self.features.to_json());
dep.insert("yanked".to_string(), self.yanked.to_json());
let line = dep.to_json().to_string();

let file = match self.name.len() {
1 => format!("1/{}", self.name),
2 => format!("2/{}", self.name),
Expand Down Expand Up @@ -152,20 +187,20 @@ impl Package {
version = "{}"
authors = []
"#, self.name, self.vers);
for &(ref dep, ref req, kind, ref target) in self.deps.iter() {
let target = match &target[..] {
"null" => String::new(),
t => format!("target.{}.", t),
for dep in self.deps.iter() {
let target = match dep.target {
None => String::new(),
Some(ref s) => format!("target.{}.", s),
};
let kind = match kind {
let kind = match &dep.kind[..] {
"build" => "build-",
"dev" => "dev-",
_ => ""
};
manifest.push_str(&format!(r#"
[{}{}dependencies.{}]
version = "{}"
"#, target, kind, dep, req));
"#, target, kind, dep.name, dep.vers));
}

let dst = self.archive_dst();
Expand Down
4 changes: 2 additions & 2 deletions tests/test_cargo_cfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -194,8 +194,8 @@ test!(works_through_the_registry {

Package::new("foo", "0.1.0").publish();
Package::new("bar", "0.1.0")
.target_dep("foo", "0.1.0", "cfg(unix)")
.target_dep("foo", "0.1.0", "cfg(windows)")
.target_dep("foo", "0.1.0", "'cfg(unix)'")
.target_dep("foo", "0.1.0", "'cfg(windows)'")
.publish();

let p = project("a")
Expand Down
23 changes: 23 additions & 0 deletions tests/test_cargo_registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1015,3 +1015,26 @@ test!(only_download_relevant {
{compiling} bar v0.5.0 ([..])
", downloading = DOWNLOADING, compiling = COMPILING, updating = UPDATING)));
});

test!(resolve_and_backtracking {
let p = project("foo")
.file("Cargo.toml", r#"
[project]
name = "bar"
version = "0.5.0"
authors = []

[dependencies]
foo = "*"
"#)
.file("src/main.rs", "fn main() {}");
p.build();

Package::new("foo", "0.1.1")
.feature_dep("bar", "0.1", &["a", "b"])
.publish();
Package::new("foo", "0.1.0").publish();

assert_that(p.cargo("build"),
execs().with_status(0));
});