Remove the hokey add_lockfile_sources

This method shouldn't be necessary as it should be possible to simply add all
sources known in the lockfile to a package registry, and
core::{resolve, registry} should take care of weeding them out if necessary.

In the process of doing so, this actually ends up fixing a problem with
rewriting a dependency to a new one which shares transitive deps. Previously all
the transitive deps were updated, but now the transitive deps remain locked at
where they were previously locked.
This commit is contained in:
Alex Crichton 2014-10-22 15:05:49 -07:00
parent 9fba127e4f
commit ebd3c1da77
2 changed files with 82 additions and 67 deletions

View file

@ -1,6 +1,4 @@
use std::collections::{HashSet, HashMap};
use core::{MultiShell, Package, PackageId, Summary};
use core::{MultiShell, Package};
use core::registry::PackageRegistry;
use core::resolver::{mod, Resolve};
use core::source::Source;
@ -34,9 +32,10 @@ pub fn resolve_and_fetch(registry: &mut PackageRegistry, package: &Package)
let source_id = package.get_package_id().get_source_id();
let previous_resolve = try!(ops::load_lockfile(&lockfile, source_id));
match previous_resolve {
Some(ref r) => try!(add_lockfile_sources(registry, package, r)),
None => try!(registry.add_sources(package.get_source_ids())),
}
Some(ref r) => r.iter().map(|p| p.get_source_id().clone()).collect(),
None => package.get_source_ids(),
};
try!(registry.add_sources(sources));
let mut resolved = try!(resolver::resolve(package.get_summary(),
resolver::ResolveEverything,
@ -48,64 +47,3 @@ pub fn resolve_and_fetch(registry: &mut PackageRegistry, package: &Package)
try!(ops::write_resolve(package, &resolved));
Ok(resolved)
}
/// When a lockfile is present, we want to keep as many dependencies at their
/// original revision as possible. We need to account, however, for
/// modifications to the manifest in terms of modifying, adding, or deleting
/// dependencies.
///
/// This method will add any appropriate sources from the lockfile into the
/// registry, and add all other sources from the root package to the registry.
/// Any dependency which has not been modified has its source added to the
/// registry (to retain the precise field if possible). Any dependency which
/// *has* changed has its source id listed in the manifest added and all of its
/// transitive dependencies are blacklisted to not be added from the lockfile.
///
/// TODO: this won't work too well for registry-based packages, but we don't
/// have many of those anyway so we should be ok for now.
fn add_lockfile_sources(registry: &mut PackageRegistry,
root: &Package,
resolve: &Resolve) -> CargoResult<()> {
let deps = resolve.deps(root.get_package_id()).into_iter().flat_map(|deps| {
deps.map(|d| (d.get_name(), d))
}).collect::<HashMap<_, &PackageId>>();
let mut sources = vec![root.get_package_id().get_source_id().clone()];
let mut to_avoid = HashSet::new();
let mut to_add = HashSet::new();
for dep in root.get_dependencies().iter() {
match deps.find(&dep.get_name()) {
Some(&lockfile_dep) => {
let summary = Summary::new(lockfile_dep.clone(), Vec::new(),
HashMap::new()).unwrap();
if dep.matches(&summary) {
fill_with_deps(resolve, lockfile_dep, &mut to_add);
} else {
fill_with_deps(resolve, lockfile_dep, &mut to_avoid);
sources.push(dep.get_source_id().clone());
}
}
None => sources.push(dep.get_source_id().clone()),
}
}
// Only afterward once we know the entire blacklist are the lockfile
// sources added.
for addition in to_add.iter() {
if !to_avoid.contains(addition) {
sources.push(addition.get_source_id().clone());
}
}
return registry.add_sources(sources);
fn fill_with_deps<'a>(resolve: &'a Resolve, dep: &'a PackageId,
set: &mut HashSet<&'a PackageId>) {
if !set.insert(dep) { return }
for mut deps in resolve.deps(dep).into_iter() {
for dep in deps {
fill_with_deps(resolve, dep, set);
}
}
}
}

View file

@ -1427,3 +1427,80 @@ test!(update_one_dep_in_repo_with_many_deps {
Updating git repository `{}`
", foo.url())));
})
test!(switch_deps_does_not_update_transitive {
let transitive = git_repo("transitive", |project| {
project.file("Cargo.toml", r#"
[package]
name = "transitive"
version = "0.5.0"
authors = ["wycats@example.com"]
"#)
.file("src/lib.rs", "")
}).assert();
let dep1 = git_repo("dep1", |project| {
project.file("Cargo.toml", format!(r#"
[package]
name = "dep"
version = "0.5.0"
authors = ["wycats@example.com"]
[dependencies.transitive]
git = '{}'
"#, transitive.url()).as_slice())
.file("src/lib.rs", "")
}).assert();
let dep2 = git_repo("dep2", |project| {
project.file("Cargo.toml", format!(r#"
[package]
name = "dep"
version = "0.5.0"
authors = ["wycats@example.com"]
[dependencies.transitive]
git = '{}'
"#, transitive.url()).as_slice())
.file("src/lib.rs", "")
}).assert();
let p = project("project")
.file("Cargo.toml", format!(r#"
[project]
name = "project"
version = "0.5.0"
authors = []
[dependencies.dep]
git = '{}'
"#, dep1.url()).as_slice())
.file("src/main.rs", "fn main() {}");
p.build();
assert_that(p.process(cargo_dir().join("cargo")).arg("build"),
execs().with_status(0)
.with_stdout(format!("\
Updating git repository `{}`
Updating git repository `{}`
{compiling} transitive [..]
{compiling} dep [..]
{compiling} project [..]
", dep1.url(), transitive.url(), compiling = COMPILING)));
// Update the dependency to point to the second repository, but this
// shouldn't update the transitive dependency which is the same.
File::create(&p.root().join("Cargo.toml")).write_str(format!(r#"
[project]
name = "project"
version = "0.5.0"
authors = []
[dependencies.dep]
git = '{}'
"#, dep2.url()).as_slice()).unwrap();
assert_that(p.process(cargo_dir().join("cargo")).arg("build"),
execs().with_status(0)
.with_stdout(format!("\
Updating git repository `{}`
{compiling} dep [..]
{compiling} project [..]
", dep2.url(), compiling = COMPILING)));
})