Skip to content

Commit

Permalink
feat(resolver): -Zdirect-minimal-versions
Browse files Browse the repository at this point in the history
This is an alternative to `-Zminimal-versions` as discussed in #5657.

The problem with `-Zminimal-versions` is it requires the root most
dependencies to verify it and we then percolate that up the stack.  This
requires a massive level of cooperation to accomplish and so far there
have been mixed results with it to the point that cargo's unstable
documentation discourages its use.

`-Zdirect-minimal-versions` instead only applies this rule to your
direct dependencies, allowing anyone in the stack to immediately adopt
it, independent of everyone else.

Special notes
- Living up to the name and the existing design, this ignores yanked
  crates.  This makes sense for `^1.1` version requirements but might
  look weird for `^1.1.1` version requirements as it could select
  `1.1.2`.
- This will error if an indirect dependency requires a newer version.
  Your version requirement will need to capture what you use **and** all
  of you dependencies.  An alternative design would have tried to merge
  the result of minimum versions for direct dependencies and maximum
  versions for indirect dependencies.  This would have been complex and
  led to weird corner cases, making it harder to predict.  I also suspect
  the value gained would be relatively low as you can't verify that
  version requirement in any other way.
  - The error could be improved to call out that this was from minimal
    versions but I felt getting this out now and starting to collect
    feedback was more important.
  • Loading branch information
epage committed Feb 7, 2023
1 parent 28a33de commit ad61a32
Show file tree
Hide file tree
Showing 6 changed files with 134 additions and 80 deletions.
2 changes: 2 additions & 0 deletions src/cargo/core/features.rs
Original file line number Diff line number Diff line change
Expand Up @@ -718,6 +718,7 @@ unstable_cli_options!(
features: Option<Vec<String>> = (HIDDEN),
jobserver_per_rustc: bool = (HIDDEN),
minimal_versions: bool = ("Resolve minimal dependency versions instead of maximum"),
direct_minimal_versions: bool = ("Resolve minimal dependency versions instead of maximum (direct dependencies only)"),
mtime_on_use: bool = ("Configure Cargo to update the mtime of used files"),
no_index_update: bool = ("Do not update the registry index even if the cache is outdated"),
panic_abort_tests: bool = ("Enable support to run tests with -Cpanic=abort"),
Expand Down Expand Up @@ -948,6 +949,7 @@ impl CliUnstable {
"no-index-update" => self.no_index_update = parse_empty(k, v)?,
"avoid-dev-deps" => self.avoid_dev_deps = parse_empty(k, v)?,
"minimal-versions" => self.minimal_versions = parse_empty(k, v)?,
"direct-minimal-versions" => self.direct_minimal_versions = parse_empty(k, v)?,
"advanced-env" => self.advanced_env = parse_empty(k, v)?,
"config-include" => self.config_include = parse_empty(k, v)?,
"check-cfg" => {
Expand Down
57 changes: 32 additions & 25 deletions src/cargo/core/resolver/dep_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,11 @@ impl<'a> RegistryQueryer<'a> {
/// any candidates are returned which match an override then the override is
/// applied by performing a second query for what the override should
/// return.
pub fn query(&mut self, dep: &Dependency) -> Poll<CargoResult<Rc<Vec<Summary>>>> {
pub fn query(
&mut self,
dep: &Dependency,
first_minimal_version: bool,
) -> Poll<CargoResult<Rc<Vec<Summary>>>> {
if let Some(out) = self.registry_cache.get(dep).cloned() {
return out.map(Result::Ok);
}
Expand Down Expand Up @@ -192,14 +196,14 @@ impl<'a> RegistryQueryer<'a> {

// When we attempt versions for a package we'll want to do so in a sorted fashion to pick
// the "best candidates" first. VersionPreferences implements this notion.
self.version_prefs.sort_summaries(
&mut ret,
if self.minimal_versions {
VersionOrdering::MinimumVersionsFirst
} else {
VersionOrdering::MaximumVersionsFirst
},
);
let ordering = if first_minimal_version || self.minimal_versions {
VersionOrdering::MinimumVersionsFirst
} else {
VersionOrdering::MaximumVersionsFirst
};
let first_version = first_minimal_version;
self.version_prefs
.sort_summaries(&mut ret, ordering, first_version);

let out = Poll::Ready(Rc::new(ret));

Expand All @@ -218,6 +222,7 @@ impl<'a> RegistryQueryer<'a> {
parent: Option<PackageId>,
candidate: &Summary,
opts: &ResolveOpts,
first_minimal_version: bool,
) -> ActivateResult<Rc<(HashSet<InternedString>, Rc<Vec<DepInfo>>)>> {
// if we have calculated a result before, then we can just return it,
// as it is a "pure" query of its arguments.
Expand All @@ -237,22 +242,24 @@ impl<'a> RegistryQueryer<'a> {
let mut all_ready = true;
let mut deps = deps
.into_iter()
.filter_map(|(dep, features)| match self.query(&dep) {
Poll::Ready(Ok(candidates)) => Some(Ok((dep, candidates, features))),
Poll::Pending => {
all_ready = false;
// we can ignore Pending deps, resolve will be repeatedly called
// until there are none to ignore
None
}
Poll::Ready(Err(e)) => Some(Err(e).with_context(|| {
format!(
"failed to get `{}` as a dependency of {}",
dep.package_name(),
describe_path_in_context(cx, &candidate.package_id()),
)
})),
})
.filter_map(
|(dep, features)| match self.query(&dep, first_minimal_version) {
Poll::Ready(Ok(candidates)) => Some(Ok((dep, candidates, features))),
Poll::Pending => {
all_ready = false;
// we can ignore Pending deps, resolve will be repeatedly called
// until there are none to ignore
None
}
Poll::Ready(Err(e)) => Some(Err(e).with_context(|| {
format!(
"failed to get `{}` as a dependency of {}",
dep.package_name(),
describe_path_in_context(cx, &candidate.package_id()),
)
})),
},
)
.collect::<CargoResult<Vec<DepInfo>>>()?;

// Attempt to resolve dependencies with fewer candidates before trying
Expand Down
45 changes: 39 additions & 6 deletions src/cargo/core/resolver/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -133,11 +133,21 @@ pub fn resolve(
Some(config) => config.cli_unstable().minimal_versions,
None => false,
};
let direct_minimal_versions = match config {
Some(config) => config.cli_unstable().direct_minimal_versions,
None => false,
};
let mut registry =
RegistryQueryer::new(registry, replacements, version_prefs, minimal_versions);
let cx = loop {
let cx = Context::new(check_public_visible_dependencies);
let cx = activate_deps_loop(cx, &mut registry, summaries, config)?;
let cx = activate_deps_loop(
cx,
&mut registry,
summaries,
direct_minimal_versions,
config,
)?;
if registry.reset_pending() {
break cx;
} else {
Expand Down Expand Up @@ -189,6 +199,7 @@ fn activate_deps_loop(
mut cx: Context,
registry: &mut RegistryQueryer<'_>,
summaries: &[(Summary, ResolveOpts)],
direct_minimal_versions: bool,
config: Option<&Config>,
) -> CargoResult<Context> {
let mut backtrack_stack = Vec::new();
Expand All @@ -201,7 +212,14 @@ fn activate_deps_loop(
// Activate all the initial summaries to kick off some work.
for &(ref summary, ref opts) in summaries {
debug!("initial activation: {}", summary.package_id());
let res = activate(&mut cx, registry, None, summary.clone(), opts);
let res = activate(
&mut cx,
registry,
None,
summary.clone(),
direct_minimal_versions,
opts,
);
match res {
Ok(Some((frame, _))) => remaining_deps.push(frame),
Ok(None) => (),
Expand Down Expand Up @@ -399,7 +417,15 @@ fn activate_deps_loop(
dep.package_name(),
candidate.version()
);
let res = activate(&mut cx, registry, Some((&parent, &dep)), candidate, &opts);
let direct_minimal_version = false; // this is an indirect dependency
let res = activate(
&mut cx,
registry,
Some((&parent, &dep)),
candidate,
direct_minimal_version,
&opts,
);

let successfully_activated = match res {
// Success! We've now activated our `candidate` in our context
Expand Down Expand Up @@ -611,6 +637,7 @@ fn activate(
registry: &mut RegistryQueryer<'_>,
parent: Option<(&Summary, &Dependency)>,
candidate: Summary,
first_minimal_version: bool,
opts: &ResolveOpts,
) -> ActivateResult<Option<(DepsFrame, Duration)>> {
let candidate_pid = candidate.package_id();
Expand Down Expand Up @@ -662,8 +689,13 @@ fn activate(
};

let now = Instant::now();
let (used_features, deps) =
&*registry.build_deps(cx, parent.map(|p| p.0.package_id()), &candidate, opts)?;
let (used_features, deps) = &*registry.build_deps(
cx,
parent.map(|p| p.0.package_id()),
&candidate,
opts,
first_minimal_version,
)?;

// Record what list of features is active for this package.
if !used_features.is_empty() {
Expand Down Expand Up @@ -859,8 +891,9 @@ fn generalize_conflicting(
// A dep is equivalent to one of the things it can resolve to.
// Thus, if all the things it can resolve to have already ben determined
// to be conflicting, then we can just say that we conflict with the parent.
let first_minimal_version = false;
if let Some(others) = registry
.query(critical_parents_dep)
.query(critical_parents_dep, first_minimal_version)
.expect("an already used dep now error!?")
.expect("an already used dep now pending!?")
.iter()
Expand Down
22 changes: 15 additions & 7 deletions src/cargo/core/resolver/version_prefs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,12 @@ impl VersionPreferences {
/// Sort the given vector of summaries in-place, with all summaries presumed to be for
/// the same package. Preferred versions appear first in the result, sorted by
/// `version_ordering`, followed by non-preferred versions sorted the same way.
pub fn sort_summaries(&self, summaries: &mut Vec<Summary>, version_ordering: VersionOrdering) {
pub fn sort_summaries(
&self,
summaries: &mut Vec<Summary>,
version_ordering: VersionOrdering,
first_version: bool,
) {
let should_prefer = |pkg_id: &PackageId| {
self.try_to_use.contains(pkg_id)
|| self
Expand All @@ -66,6 +71,9 @@ impl VersionPreferences {
_ => previous_cmp,
}
});
if first_version {
let _ = summaries.split_off(1);
}
}
}

Expand Down Expand Up @@ -115,13 +123,13 @@ mod test {
summ("foo", "1.0.9"),
];

vp.sort_summaries(&mut summaries, VersionOrdering::MaximumVersionsFirst);
vp.sort_summaries(&mut summaries, VersionOrdering::MaximumVersionsFirst, false);
assert_eq!(
describe(&summaries),
"foo/1.2.3, foo/1.2.4, foo/1.1.0, foo/1.0.9".to_string()
);

vp.sort_summaries(&mut summaries, VersionOrdering::MinimumVersionsFirst);
vp.sort_summaries(&mut summaries, VersionOrdering::MinimumVersionsFirst, false);
assert_eq!(
describe(&summaries),
"foo/1.2.3, foo/1.0.9, foo/1.1.0, foo/1.2.4".to_string()
Expand All @@ -140,13 +148,13 @@ mod test {
summ("foo", "1.0.9"),
];

vp.sort_summaries(&mut summaries, VersionOrdering::MaximumVersionsFirst);
vp.sort_summaries(&mut summaries, VersionOrdering::MaximumVersionsFirst, false);
assert_eq!(
describe(&summaries),
"foo/1.2.3, foo/1.2.4, foo/1.1.0, foo/1.0.9".to_string()
);

vp.sort_summaries(&mut summaries, VersionOrdering::MinimumVersionsFirst);
vp.sort_summaries(&mut summaries, VersionOrdering::MinimumVersionsFirst, false);
assert_eq!(
describe(&summaries),
"foo/1.2.3, foo/1.0.9, foo/1.1.0, foo/1.2.4".to_string()
Expand All @@ -166,13 +174,13 @@ mod test {
summ("foo", "1.0.9"),
];

vp.sort_summaries(&mut summaries, VersionOrdering::MaximumVersionsFirst);
vp.sort_summaries(&mut summaries, VersionOrdering::MaximumVersionsFirst, false);
assert_eq!(
describe(&summaries),
"foo/1.2.3, foo/1.1.0, foo/1.2.4, foo/1.0.9".to_string()
);

vp.sort_summaries(&mut summaries, VersionOrdering::MinimumVersionsFirst);
vp.sort_summaries(&mut summaries, VersionOrdering::MinimumVersionsFirst, false);
assert_eq!(
describe(&summaries),
"foo/1.1.0, foo/1.2.3, foo/1.0.9, foo/1.2.4".to_string()
Expand Down
18 changes: 18 additions & 0 deletions src/doc/src/reference/unstable.md
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ Each new feature described below should explain how to use it.
* [no-index-update](#no-index-update) — Prevents cargo from updating the index cache.
* [avoid-dev-deps](#avoid-dev-deps) — Prevents the resolver from including dev-dependencies during resolution.
* [minimal-versions](#minimal-versions) — Forces the resolver to use the lowest compatible version instead of the highest.
* [direct-minimal-versions](#direct-minimal-versions) — Forces the resolver to use the lowest compatible version instead of the highest.
* [public-dependency](#public-dependency) — Allows dependencies to be classified as either public or private.
* Output behavior
* [out-dir](#out-dir) — Adds a directory where artifacts are copied to.
Expand Down Expand Up @@ -176,6 +177,23 @@ minimum versions that you are actually using. That is, if Cargo.toml says
`foo = "1.0.0"` that you don't accidentally depend on features added only in
`foo 1.5.0`.

### direct-minimal-versions
* Original Issue: [#4100](https://github.com/rust-lang/cargo/issues/4100)
* Tracking Issue: [#5657](https://github.com/rust-lang/cargo/issues/5657)

When a `Cargo.lock` file is generated, the `-Z direct-minimal-versions` flag will
resolve the dependencies to the minimum SemVer version that will satisfy the
requirements (instead of the greatest version) for direct dependencies only.

The intended use-case of this flag is to check, during continuous integration,
that the versions specified in Cargo.toml are a correct reflection of the
minimum versions that you are actually using. That is, if Cargo.toml says
`foo = "1.0.0"` that you don't accidentally depend on features added only in
`foo 1.5.0`.

Indirect dependencies are resolved as normal so as not to be blocked on their
minimal version validation.

### out-dir
* Original Issue: [#4875](https://github.com/rust-lang/cargo/issues/4875)
* Tracking Issue: [#6790](https://github.com/rust-lang/cargo/issues/6790)
Expand Down
Loading

0 comments on commit ad61a32

Please sign in to comment.