Skip to content

Commit dc1567b

Browse files
committed
feat(resolver): -Zdirect-minimal-versions
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.
1 parent a3a42da commit dc1567b

File tree

6 files changed

+149
-93
lines changed

6 files changed

+149
-93
lines changed

src/cargo/core/features.rs

+2
Original file line numberDiff line numberDiff line change
@@ -718,6 +718,7 @@ unstable_cli_options!(
718718
features: Option<Vec<String>> = (HIDDEN),
719719
jobserver_per_rustc: bool = (HIDDEN),
720720
minimal_versions: bool = ("Resolve minimal dependency versions instead of maximum"),
721+
direct_minimal_versions: bool = ("Resolve minimal dependency versions instead of maximum (direct dependencies only)"),
721722
mtime_on_use: bool = ("Configure Cargo to update the mtime of used files"),
722723
no_index_update: bool = ("Do not update the registry index even if the cache is outdated"),
723724
panic_abort_tests: bool = ("Enable support to run tests with -Cpanic=abort"),
@@ -948,6 +949,7 @@ impl CliUnstable {
948949
"no-index-update" => self.no_index_update = parse_empty(k, v)?,
949950
"avoid-dev-deps" => self.avoid_dev_deps = parse_empty(k, v)?,
950951
"minimal-versions" => self.minimal_versions = parse_empty(k, v)?,
952+
"direct-minimal-versions" => self.direct_minimal_versions = parse_empty(k, v)?,
951953
"advanced-env" => self.advanced_env = parse_empty(k, v)?,
952954
"config-include" => self.config_include = parse_empty(k, v)?,
953955
"check-cfg" => {

src/cargo/core/resolver/dep_cache.rs

+32-25
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,11 @@ impl<'a> RegistryQueryer<'a> {
9696
/// any candidates are returned which match an override then the override is
9797
/// applied by performing a second query for what the override should
9898
/// return.
99-
pub fn query(&mut self, dep: &Dependency) -> Poll<CargoResult<Rc<Vec<Summary>>>> {
99+
pub fn query(
100+
&mut self,
101+
dep: &Dependency,
102+
first_minimal_version: bool,
103+
) -> Poll<CargoResult<Rc<Vec<Summary>>>> {
100104
if let Some(out) = self.registry_cache.get(dep).cloned() {
101105
return out.map(Result::Ok);
102106
}
@@ -192,14 +196,14 @@ impl<'a> RegistryQueryer<'a> {
192196

193197
// When we attempt versions for a package we'll want to do so in a sorted fashion to pick
194198
// the "best candidates" first. VersionPreferences implements this notion.
195-
self.version_prefs.sort_summaries(
196-
&mut ret,
197-
if self.minimal_versions {
198-
VersionOrdering::MinimumVersionsFirst
199-
} else {
200-
VersionOrdering::MaximumVersionsFirst
201-
},
202-
);
199+
let ordering = if first_minimal_version || self.minimal_versions {
200+
VersionOrdering::MinimumVersionsFirst
201+
} else {
202+
VersionOrdering::MaximumVersionsFirst
203+
};
204+
let first_version = first_minimal_version;
205+
self.version_prefs
206+
.sort_summaries(&mut ret, ordering, first_version);
203207

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

@@ -218,6 +222,7 @@ impl<'a> RegistryQueryer<'a> {
218222
parent: Option<PackageId>,
219223
candidate: &Summary,
220224
opts: &ResolveOpts,
225+
first_minimal_version: bool,
221226
) -> ActivateResult<Rc<(HashSet<InternedString>, Rc<Vec<DepInfo>>)>> {
222227
// if we have calculated a result before, then we can just return it,
223228
// as it is a "pure" query of its arguments.
@@ -237,22 +242,24 @@ impl<'a> RegistryQueryer<'a> {
237242
let mut all_ready = true;
238243
let mut deps = deps
239244
.into_iter()
240-
.filter_map(|(dep, features)| match self.query(&dep) {
241-
Poll::Ready(Ok(candidates)) => Some(Ok((dep, candidates, features))),
242-
Poll::Pending => {
243-
all_ready = false;
244-
// we can ignore Pending deps, resolve will be repeatedly called
245-
// until there are none to ignore
246-
None
247-
}
248-
Poll::Ready(Err(e)) => Some(Err(e).with_context(|| {
249-
format!(
250-
"failed to get `{}` as a dependency of {}",
251-
dep.package_name(),
252-
describe_path_in_context(cx, &candidate.package_id()),
253-
)
254-
})),
255-
})
245+
.filter_map(
246+
|(dep, features)| match self.query(&dep, first_minimal_version) {
247+
Poll::Ready(Ok(candidates)) => Some(Ok((dep, candidates, features))),
248+
Poll::Pending => {
249+
all_ready = false;
250+
// we can ignore Pending deps, resolve will be repeatedly called
251+
// until there are none to ignore
252+
None
253+
}
254+
Poll::Ready(Err(e)) => Some(Err(e).with_context(|| {
255+
format!(
256+
"failed to get `{}` as a dependency of {}",
257+
dep.package_name(),
258+
describe_path_in_context(cx, &candidate.package_id()),
259+
)
260+
})),
261+
},
262+
)
256263
.collect::<CargoResult<Vec<DepInfo>>>()?;
257264

258265
// Attempt to resolve dependencies with fewer candidates before trying

src/cargo/core/resolver/mod.rs

+39-6
Original file line numberDiff line numberDiff line change
@@ -133,11 +133,21 @@ pub fn resolve(
133133
Some(config) => config.cli_unstable().minimal_versions,
134134
None => false,
135135
};
136+
let direct_minimal_versions = match config {
137+
Some(config) => config.cli_unstable().direct_minimal_versions,
138+
None => false,
139+
};
136140
let mut registry =
137141
RegistryQueryer::new(registry, replacements, version_prefs, minimal_versions);
138142
let cx = loop {
139143
let cx = Context::new(check_public_visible_dependencies);
140-
let cx = activate_deps_loop(cx, &mut registry, summaries, config)?;
144+
let cx = activate_deps_loop(
145+
cx,
146+
&mut registry,
147+
summaries,
148+
direct_minimal_versions,
149+
config,
150+
)?;
141151
if registry.reset_pending() {
142152
break cx;
143153
} else {
@@ -189,6 +199,7 @@ fn activate_deps_loop(
189199
mut cx: Context,
190200
registry: &mut RegistryQueryer<'_>,
191201
summaries: &[(Summary, ResolveOpts)],
202+
direct_minimal_versions: bool,
192203
config: Option<&Config>,
193204
) -> CargoResult<Context> {
194205
let mut backtrack_stack = Vec::new();
@@ -201,7 +212,14 @@ fn activate_deps_loop(
201212
// Activate all the initial summaries to kick off some work.
202213
for &(ref summary, ref opts) in summaries {
203214
debug!("initial activation: {}", summary.package_id());
204-
let res = activate(&mut cx, registry, None, summary.clone(), opts);
215+
let res = activate(
216+
&mut cx,
217+
registry,
218+
None,
219+
summary.clone(),
220+
direct_minimal_versions,
221+
opts,
222+
);
205223
match res {
206224
Ok(Some((frame, _))) => remaining_deps.push(frame),
207225
Ok(None) => (),
@@ -399,7 +417,15 @@ fn activate_deps_loop(
399417
dep.package_name(),
400418
candidate.version()
401419
);
402-
let res = activate(&mut cx, registry, Some((&parent, &dep)), candidate, &opts);
420+
let direct_minimal_version = false; // this is an indirect dependency
421+
let res = activate(
422+
&mut cx,
423+
registry,
424+
Some((&parent, &dep)),
425+
candidate,
426+
direct_minimal_version,
427+
&opts,
428+
);
403429

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

664691
let now = Instant::now();
665-
let (used_features, deps) =
666-
&*registry.build_deps(cx, parent.map(|p| p.0.package_id()), &candidate, opts)?;
692+
let (used_features, deps) = &*registry.build_deps(
693+
cx,
694+
parent.map(|p| p.0.package_id()),
695+
&candidate,
696+
opts,
697+
first_minimal_version,
698+
)?;
667699

668700
// Record what list of features is active for this package.
669701
if !used_features.is_empty() {
@@ -859,8 +891,9 @@ fn generalize_conflicting(
859891
// A dep is equivalent to one of the things it can resolve to.
860892
// Thus, if all the things it can resolve to have already ben determined
861893
// to be conflicting, then we can just say that we conflict with the parent.
894+
let first_minimal_version = false;
862895
if let Some(others) = registry
863-
.query(critical_parents_dep)
896+
.query(critical_parents_dep, first_minimal_version)
864897
.expect("an already used dep now error!?")
865898
.expect("an already used dep now pending!?")
866899
.iter()

src/cargo/core/resolver/version_prefs.rs

+15-7
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,12 @@ impl VersionPreferences {
4242
/// Sort the given vector of summaries in-place, with all summaries presumed to be for
4343
/// the same package. Preferred versions appear first in the result, sorted by
4444
/// `version_ordering`, followed by non-preferred versions sorted the same way.
45-
pub fn sort_summaries(&self, summaries: &mut Vec<Summary>, version_ordering: VersionOrdering) {
45+
pub fn sort_summaries(
46+
&self,
47+
summaries: &mut Vec<Summary>,
48+
version_ordering: VersionOrdering,
49+
first_version: bool,
50+
) {
4651
let should_prefer = |pkg_id: &PackageId| {
4752
self.try_to_use.contains(pkg_id)
4853
|| self
@@ -66,6 +71,9 @@ impl VersionPreferences {
6671
_ => previous_cmp,
6772
}
6873
});
74+
if first_version {
75+
let _ = summaries.split_off(1);
76+
}
6977
}
7078
}
7179

@@ -115,13 +123,13 @@ mod test {
115123
summ("foo", "1.0.9"),
116124
];
117125

118-
vp.sort_summaries(&mut summaries, VersionOrdering::MaximumVersionsFirst);
126+
vp.sort_summaries(&mut summaries, VersionOrdering::MaximumVersionsFirst, false);
119127
assert_eq!(
120128
describe(&summaries),
121129
"foo/1.2.3, foo/1.2.4, foo/1.1.0, foo/1.0.9".to_string()
122130
);
123131

124-
vp.sort_summaries(&mut summaries, VersionOrdering::MinimumVersionsFirst);
132+
vp.sort_summaries(&mut summaries, VersionOrdering::MinimumVersionsFirst, false);
125133
assert_eq!(
126134
describe(&summaries),
127135
"foo/1.2.3, foo/1.0.9, foo/1.1.0, foo/1.2.4".to_string()
@@ -140,13 +148,13 @@ mod test {
140148
summ("foo", "1.0.9"),
141149
];
142150

143-
vp.sort_summaries(&mut summaries, VersionOrdering::MaximumVersionsFirst);
151+
vp.sort_summaries(&mut summaries, VersionOrdering::MaximumVersionsFirst, false);
144152
assert_eq!(
145153
describe(&summaries),
146154
"foo/1.2.3, foo/1.2.4, foo/1.1.0, foo/1.0.9".to_string()
147155
);
148156

149-
vp.sort_summaries(&mut summaries, VersionOrdering::MinimumVersionsFirst);
157+
vp.sort_summaries(&mut summaries, VersionOrdering::MinimumVersionsFirst, false);
150158
assert_eq!(
151159
describe(&summaries),
152160
"foo/1.2.3, foo/1.0.9, foo/1.1.0, foo/1.2.4".to_string()
@@ -166,13 +174,13 @@ mod test {
166174
summ("foo", "1.0.9"),
167175
];
168176

169-
vp.sort_summaries(&mut summaries, VersionOrdering::MaximumVersionsFirst);
177+
vp.sort_summaries(&mut summaries, VersionOrdering::MaximumVersionsFirst, false);
170178
assert_eq!(
171179
describe(&summaries),
172180
"foo/1.2.3, foo/1.1.0, foo/1.2.4, foo/1.0.9".to_string()
173181
);
174182

175-
vp.sort_summaries(&mut summaries, VersionOrdering::MinimumVersionsFirst);
183+
vp.sort_summaries(&mut summaries, VersionOrdering::MinimumVersionsFirst, false);
176184
assert_eq!(
177185
describe(&summaries),
178186
"foo/1.1.0, foo/1.2.3, foo/1.0.9, foo/1.2.4".to_string()

src/doc/src/reference/unstable.md

+18
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ Each new feature described below should explain how to use it.
6767
* [no-index-update](#no-index-update) — Prevents cargo from updating the index cache.
6868
* [avoid-dev-deps](#avoid-dev-deps) — Prevents the resolver from including dev-dependencies during resolution.
6969
* [minimal-versions](#minimal-versions) — Forces the resolver to use the lowest compatible version instead of the highest.
70+
* [direct-minimal-versions](#direct-minimal-versions) — Forces the resolver to use the lowest compatible version instead of the highest.
7071
* [public-dependency](#public-dependency) — Allows dependencies to be classified as either public or private.
7172
* Output behavior
7273
* [out-dir](#out-dir) — Adds a directory where artifacts are copied to.
@@ -176,6 +177,23 @@ minimum versions that you are actually using. That is, if Cargo.toml says
176177
`foo = "1.0.0"` that you don't accidentally depend on features added only in
177178
`foo 1.5.0`.
178179

180+
### direct-minimal-versions
181+
* Original Issue: [#4100](https://github.com/rust-lang/cargo/issues/4100)
182+
* Tracking Issue: [#5657](https://github.com/rust-lang/cargo/issues/5657)
183+
184+
When a `Cargo.lock` file is generated, the `-Z direct-minimal-versions` flag will
185+
resolve the dependencies to the minimum SemVer version that will satisfy the
186+
requirements (instead of the greatest version) for direct dependencies only.
187+
188+
The intended use-case of this flag is to check, during continuous integration,
189+
that the versions specified in Cargo.toml are a correct reflection of the
190+
minimum versions that you are actually using. That is, if Cargo.toml says
191+
`foo = "1.0.0"` that you don't accidentally depend on features added only in
192+
`foo 1.5.0`.
193+
194+
Indirect dependencies are resolved as normal so as not to be blocked on their
195+
minimal version validation.
196+
179197
### out-dir
180198
* Original Issue: [#4875](https://github.com/rust-lang/cargo/issues/4875)
181199
* Tracking Issue: [#6790](https://github.com/rust-lang/cargo/issues/6790)

0 commit comments

Comments
 (0)