Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 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
2 changes: 2 additions & 0 deletions src/cargo/core/features.rs
Original file line number Diff line number Diff line change
Expand Up @@ -285,6 +285,7 @@ pub struct CliUnstable {
pub offline: bool,
pub no_index_update: bool,
pub avoid_dev_deps: bool,
pub minimal_versions: bool,
}

impl CliUnstable {
Expand Down Expand Up @@ -317,6 +318,7 @@ impl CliUnstable {
"offline" => self.offline = true,
"no-index-update" => self.no_index_update = true,
"avoid-dev-deps" => self.avoid_dev_deps = true,
"minimal-versions" => self.minimal_versions = true,
_ => bail!("unknown `-Z` flag specified: {}", k),
}

Expand Down
17 changes: 15 additions & 2 deletions src/cargo/core/resolver/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -430,7 +430,11 @@ pub fn resolve(
warnings: RcList::new(),
};
let _p = profile::start("resolving");
let mut registry = RegistryQueryer::new(registry, replacements, try_to_use);
let minimal_versions = match config {
Some(config) => config.cli_unstable().minimal_versions,
None => false,
};
let mut registry = RegistryQueryer::new(registry, replacements, try_to_use, minimal_versions);
let cx = activate_deps_loop(cx, &mut registry, summaries, config)?;

let mut resolve = Resolve {
Expand Down Expand Up @@ -683,19 +687,25 @@ struct RegistryQueryer<'a> {
try_to_use: &'a HashSet<&'a PackageId>,
// TODO: with nll the Rc can be removed
cache: HashMap<Dependency, Rc<Vec<Candidate>>>,
// If set the list of dependency candidates will be sorted by minimal
// versions first. That allows `cargo update -Z minimal-versions` which will
// specify minimum depedency versions to be used.
minimal_versions: bool,
}

impl<'a> RegistryQueryer<'a> {
fn new(
registry: &'a mut Registry,
replacements: &'a [(PackageIdSpec, Dependency)],
try_to_use: &'a HashSet<&'a PackageId>,
minimal_versions: bool,
) -> Self {
RegistryQueryer {
registry,
replacements,
cache: HashMap::new(),
try_to_use,
minimal_versions,
}
}

Expand Down Expand Up @@ -797,7 +807,10 @@ impl<'a> RegistryQueryer<'a> {
let b_in_previous = self.try_to_use.contains(b.summary.package_id());
let a = (a_in_previous, a.summary.version());
let b = (b_in_previous, b.summary.version());
a.cmp(&b).reverse()
match self.minimal_versions {
true => a.cmp(&b),
false => a.cmp(&b).reverse(),
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This needs your code from #5200 (comment). Also match on a bool is somtimes better as an if, and the .then_with( method may be helpfull. I leave it to your judgment.

});

let out = Rc::new(ret);
Expand Down
83 changes: 82 additions & 1 deletion tests/testsuite/resolve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use hamcrest::{assert_that, contains, is_not};
use cargo::core::source::{GitReference, SourceId};
use cargo::core::dependency::Kind::{self, Development};
use cargo::core::{Dependency, PackageId, Registry, Summary};
use cargo::util::{CargoResult, ToUrl};
use cargo::util::{CargoResult, Config, ToUrl};
use cargo::core::resolver::{self, Method};

fn resolve(
Expand Down Expand Up @@ -45,6 +45,45 @@ fn resolve(
Ok(res)
}

// Clone of resolve() from above that also passes down a Config instance.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I am not fond of the copy/paste code duplication, but if @klausi thinks that it is the best solution I am ok with it.
As an alternative thought if this takes an Option<&Config> then resolve can just call resolve_with_config(...,None).

fn resolve_with_config(
pkg: &PackageId,
deps: Vec<Dependency>,
registry: &[Summary],
config: &Config,
) -> CargoResult<Vec<PackageId>> {
struct MyRegistry<'a>(&'a [Summary]);
impl<'a> Registry for MyRegistry<'a> {
fn query(&mut self, dep: &Dependency, f: &mut FnMut(Summary)) -> CargoResult<()> {
for summary in self.0.iter() {
if dep.matches(summary) {
f(summary.clone());
}
}
Ok(())
}
fn supports_checksums(&self) -> bool {
false
}
fn requires_precise(&self) -> bool {
false
}
}
let mut registry = MyRegistry(registry);
let summary = Summary::new(pkg.clone(), deps, BTreeMap::new(), None).unwrap();
let method = Method::Everything;
let resolve = resolver::resolve(
&[(summary, method)],
&[],
&mut registry,
&HashSet::new(),
Some(config),
false,
)?;
let res = resolve.iter().cloned().collect();
Ok(res)
}

trait ToDep {
fn to_dep(self) -> Dependency;
}
Expand Down Expand Up @@ -320,6 +359,48 @@ fn test_resolving_maximum_version_with_transitive_deps() {
assert_that(&res, is_not(contains(names(&[("util", "1.1.1")]))));
}

#[test]
fn test_resolving_minimum_version_with_transitive_deps() {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I really like this test, it concisely demonstrates both properties of minimal-versions, that it chooses the smallest version while meeting all the requirements. Mabey a comment at the beginning explaining that for the next reader. @alexcrichton has been trying to teach me to leave more comments.

Also @alexcrichton, do you think this needs a full integration test that calls cargo -Z minimal-versions or is this sufficient?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Oh sorry I missed this. Yeah @klausi mind adding a small integration test to ensure that the flag is plumbed to the right location?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done!

let reg = registry(vec![
pkg!(("util", "1.2.2")),
pkg!(("util", "1.0.0")),
pkg!(("util", "1.1.1")),
pkg!("foo" => [dep_req("util", "1.0.0")]),
pkg!("bar" => [dep_req("util", ">=1.0.1")]),
]);

let mut config = Config::default().unwrap();
config
.configure(
1,
None,
&None,
false,
false,
&["minimal-versions".to_string()],
)
.unwrap();

let res = resolve_with_config(
&pkg_id("root"),
vec![dep_req("foo", "1.0.0"), dep_req("bar", "1.0.0")],
&reg,
&config,
).unwrap();

assert_that(
&res,
contains(names(&[
("root", "1.0.0"),
("foo", "1.0.0"),
("bar", "1.0.0"),
("util", "1.1.1"),
])),
);
assert_that(&res, is_not(contains(names(&[("util", "1.2.2")]))));
assert_that(&res, is_not(contains(names(&[("util", "1.0.0")]))));
}

#[test]
fn resolving_incompat_versions() {
let reg = registry(vec![
Expand Down