Skip to content
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

feat(resolver): **Very** preliminary MSRV resolver support #12560

Merged
merged 3 commits into from
Aug 28, 2023
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
4 changes: 4 additions & 0 deletions benches/benchsuite/benches/resolve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ fn do_resolve<'cfg>(config: &'cfg Config, ws_root: &Path) -> ResolveInfo<'cfg> {
let specs = pkgs.to_package_id_specs(&ws).unwrap();
let has_dev_units = HasDevUnits::Yes;
let force_all_targets = ForceAllTargets::No;
let max_rust_version = None;
// Do an initial run to download anything necessary so that it does
// not confuse criterion's warmup.
let ws_resolve = cargo::ops::resolve_ws_with_opts(
Expand All @@ -41,6 +42,7 @@ fn do_resolve<'cfg>(config: &'cfg Config, ws_root: &Path) -> ResolveInfo<'cfg> {
&specs,
has_dev_units,
force_all_targets,
max_rust_version,
)
.unwrap();
ResolveInfo {
Expand Down Expand Up @@ -82,6 +84,7 @@ fn resolve_ws(c: &mut Criterion) {
force_all_targets,
..
} = lazy_info.get_or_insert_with(|| do_resolve(&config, &ws_root));
let max_rust_version = None;
b.iter(|| {
cargo::ops::resolve_ws_with_opts(
ws,
Expand All @@ -91,6 +94,7 @@ fn resolve_ws(c: &mut Criterion) {
specs,
*has_dev_units,
*force_all_targets,
max_rust_version,
)
.unwrap();
})
Expand Down
2 changes: 2 additions & 0 deletions crates/resolver-tests/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -188,13 +188,15 @@ pub fn resolve_with_config_raw(
.unwrap();
let opts = ResolveOpts::everything();
let start = Instant::now();
let max_rust_version = None;
let resolve = resolver::resolve(
&[(summary, opts)],
&[],
&mut registry,
&VersionPreferences::default(),
Some(config),
true,
max_rust_version,
);

// The largest test in our suite takes less then 30 sec.
Expand Down
2 changes: 2 additions & 0 deletions src/cargo/core/compiler/standard_lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,7 @@ pub fn resolve_std<'cfg>(
let cli_features = CliFeatures::from_command_line(
&features, /*all_features*/ false, /*uses_default_features*/ false,
)?;
let max_rust_version = ws.rust_version();
let resolve = ops::resolve_ws_with_opts(
&std_ws,
target_data,
Expand All @@ -153,6 +154,7 @@ pub fn resolve_std<'cfg>(
&specs,
HasDevUnits::No,
crate::core::resolver::features::ForceAllTargets::No,
max_rust_version,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is one case I'm not entirely sure of but since its another unstable feature, I figure doing a best effort isn't the end of the world

Copy link
Contributor

Choose a reason for hiding this comment

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

I did not follow the point you were trying to make here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm assuming this is build-std and we do a separate resolve pass for that. What should we consider max_rust_version to be in this case?

)?;
Ok((
resolve.pkg_set,
Expand Down
8 changes: 7 additions & 1 deletion src/cargo/core/resolver/dep_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ use crate::core::{
};
use crate::util::errors::CargoResult;
use crate::util::interning::InternedString;
use crate::util::PartialVersion;

use anyhow::Context as _;
use std::collections::{BTreeSet, HashMap, HashSet};
Expand All @@ -36,6 +37,7 @@ pub struct RegistryQueryer<'a> {
/// versions first. That allows `cargo update -Z minimal-versions` which will
/// specify minimum dependency versions to be used.
minimal_versions: bool,
max_rust_version: Option<PartialVersion>,
/// a cache of `Candidate`s that fulfil a `Dependency` (and whether `first_minimal_version`)
registry_cache: HashMap<(Dependency, bool), Poll<Rc<Vec<Summary>>>>,
/// a cache of `Dependency`s that are required for a `Summary`
Expand All @@ -57,12 +59,14 @@ impl<'a> RegistryQueryer<'a> {
replacements: &'a [(PackageIdSpec, Dependency)],
version_prefs: &'a VersionPreferences,
minimal_versions: bool,
max_rust_version: Option<PartialVersion>,
) -> Self {
RegistryQueryer {
registry,
replacements,
version_prefs,
minimal_versions,
max_rust_version,
registry_cache: HashMap::new(),
summary_cache: HashMap::new(),
used_replacements: HashMap::new(),
Expand Down Expand Up @@ -112,7 +116,9 @@ impl<'a> RegistryQueryer<'a> {

let mut ret = Vec::new();
let ready = self.registry.query(dep, QueryKind::Exact, &mut |s| {
ret.push(s);
if self.max_rust_version.is_none() || s.rust_version() <= self.max_rust_version {
ret.push(s);
}
})?;
if ready.is_pending() {
self.registry_cache
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 @@ -71,6 +71,7 @@ use crate::util::config::Config;
use crate::util::errors::CargoResult;
use crate::util::network::PollExt;
use crate::util::profile;
use crate::util::PartialVersion;

use self::context::Context;
use self::dep_cache::RegistryQueryer;
Expand Down Expand Up @@ -138,6 +139,7 @@ pub fn resolve(
version_prefs: &VersionPreferences,
config: Option<&Config>,
check_public_visible_dependencies: bool,
mut max_rust_version: Option<PartialVersion>,
) -> CargoResult<Resolve> {
let _p = profile::start("resolving");
let minimal_versions = match config {
Expand All @@ -148,8 +150,19 @@ pub fn resolve(
Some(config) => config.cli_unstable().direct_minimal_versions,
None => false,
};
let mut registry =
RegistryQueryer::new(registry, replacements, version_prefs, minimal_versions);
if !config
.map(|c| c.cli_unstable().msrv_policy)
.unwrap_or(false)
{
max_rust_version = None;
}
Comment on lines +153 to +158
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I centralized the feature check to this one spot

I am somewhat tempted to do something similar for honor_rust_version and have the config initialized with it and just check the config here.

let mut registry = RegistryQueryer::new(
registry,
replacements,
version_prefs,
minimal_versions,
max_rust_version,
);
let cx = loop {
let cx = Context::new(check_public_visible_dependencies);
let cx = activate_deps_loop(
Expand Down
7 changes: 7 additions & 0 deletions src/cargo/core/workspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ use crate::util::edit_distance;
use crate::util::errors::{CargoResult, ManifestError};
use crate::util::interning::InternedString;
use crate::util::toml::{read_manifest, InheritableFields, TomlDependency, TomlProfiles};
use crate::util::PartialVersion;
use crate::util::{config::ConfigRelativePath, Config, Filesystem, IntoUrl};
use cargo_util::paths;
use cargo_util::paths::normalize_path;
Expand Down Expand Up @@ -595,6 +596,12 @@ impl<'cfg> Workspace<'cfg> {
self
}

/// Get the lowest-common denominator `package.rust-version` within the workspace, if specified
/// anywhere
pub fn rust_version(&self) -> Option<PartialVersion> {
self.members().filter_map(|pkg| pkg.rust_version()).min()
}

pub fn custom_metadata(&self) -> Option<&toml::Value> {
self.custom_metadata.as_ref()
}
Expand Down
2 changes: 2 additions & 0 deletions src/cargo/ops/cargo_compile/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,7 @@ pub fn create_bcx<'a, 'cfg>(
HasDevUnits::No
}
};
let max_rust_version = ws.rust_version();
let resolve = ops::resolve_ws_with_opts(
ws,
&mut target_data,
Expand All @@ -269,6 +270,7 @@ pub fn create_bcx<'a, 'cfg>(
&specs,
has_dev_units,
crate::core::resolver::features::ForceAllTargets::No,
max_rust_version,
)?;
let WorkspaceResolve {
mut pkg_set,
Expand Down
6 changes: 6 additions & 0 deletions src/cargo/ops/cargo_generate_lockfile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ pub struct UpdateOptions<'a> {

pub fn generate_lockfile(ws: &Workspace<'_>) -> CargoResult<()> {
let mut registry = PackageRegistry::new(ws.config())?;
let max_rust_version = ws.rust_version();
let mut resolve = ops::resolve_with_previous(
&mut registry,
ws,
Expand All @@ -30,6 +31,7 @@ pub fn generate_lockfile(ws: &Workspace<'_>) -> CargoResult<()> {
None,
&[],
true,
max_rust_version,
)?;
ops::write_pkg_lockfile(ws, &mut resolve)?;
Ok(())
Expand All @@ -48,6 +50,8 @@ pub fn update_lockfile(ws: &Workspace<'_>, opts: &UpdateOptions<'_>) -> CargoRes
// that we're synchronized against other Cargos.
let _lock = ws.config().acquire_package_cache_lock()?;

let max_rust_version = ws.rust_version();

let previous_resolve = match ops::load_pkg_lockfile(ws)? {
Some(resolve) => resolve,
None => {
Expand All @@ -67,6 +71,7 @@ pub fn update_lockfile(ws: &Workspace<'_>, opts: &UpdateOptions<'_>) -> CargoRes
None,
&[],
true,
max_rust_version,
)?
}
}
Expand Down Expand Up @@ -125,6 +130,7 @@ pub fn update_lockfile(ws: &Workspace<'_>, opts: &UpdateOptions<'_>) -> CargoRes
Some(&to_avoid),
&[],
true,
max_rust_version,
)?;

// Summarize what is changing for the user.
Expand Down
3 changes: 3 additions & 0 deletions src/cargo/ops/cargo_output_metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,8 @@ fn build_resolve_graph(
crate::core::resolver::features::ForceAllTargets::No
};

let max_rust_version = ws.rust_version();

// Note that even with --filter-platform we end up downloading host dependencies as well,
// as that is the behavior of download_accessible.
let ws_resolve = ops::resolve_ws_with_opts(
Expand All @@ -145,6 +147,7 @@ fn build_resolve_graph(
&specs,
HasDevUnits::Yes,
force_all,
max_rust_version,
)?;

let package_map: BTreeMap<PackageId, Package> = ws_resolve
Expand Down
3 changes: 3 additions & 0 deletions src/cargo/ops/cargo_package.rs
Original file line number Diff line number Diff line change
Expand Up @@ -423,6 +423,8 @@ fn build_lock(ws: &Workspace<'_>, orig_pkg: &Package) -> CargoResult<String> {
TomlManifest::to_real_manifest(&toml_manifest, false, source_id, package_root, config)?;
let new_pkg = Package::new(manifest, orig_pkg.manifest_path());

let max_rust_version = new_pkg.rust_version();

// Regenerate Cargo.lock using the old one as a guide.
let tmp_ws = Workspace::ephemeral(new_pkg, ws.config(), None, true)?;
let mut tmp_reg = PackageRegistry::new(ws.config())?;
Expand All @@ -435,6 +437,7 @@ fn build_lock(ws: &Workspace<'_>, orig_pkg: &Package) -> CargoResult<String> {
None,
&[],
true,
max_rust_version,
)?;
let pkg_set = ops::get_resolved_packages(&new_resolve, tmp_reg)?;

Expand Down
3 changes: 3 additions & 0 deletions src/cargo/ops/fix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,8 @@ fn check_resolver_change(ws: &Workspace<'_>, opts: &FixOptions) -> CargoResult<(
let mut target_data =
RustcTargetData::new(ws, &opts.compile_opts.build_config.requested_kinds)?;
let mut resolve_differences = |has_dev_units| -> CargoResult<(WorkspaceResolve<'_>, DiffMap)> {
let max_rust_version = ws.rust_version();

let ws_resolve = ops::resolve_ws_with_opts(
ws,
&mut target_data,
Expand All @@ -254,6 +256,7 @@ fn check_resolver_change(ws: &Workspace<'_>, opts: &FixOptions) -> CargoResult<(
&specs,
has_dev_units,
crate::core::resolver::features::ForceAllTargets::No,
max_rust_version,
)?;

let feature_opts = FeatureOpts::new_behavior(ResolveBehavior::V2, has_dev_units);
Expand Down
13 changes: 11 additions & 2 deletions src/cargo/ops/resolve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ use crate::core::{GitReference, PackageId, PackageIdSpec, PackageSet, SourceId,
use crate::ops;
use crate::sources::PathSource;
use crate::util::errors::CargoResult;
use crate::util::PartialVersion;
use crate::util::{profile, CanonicalUrl};
use anyhow::Context as _;
use std::collections::{HashMap, HashSet};
Expand Down Expand Up @@ -106,8 +107,10 @@ version. This may also occur with an optional dependency that is not enabled.";
/// This is a simple interface used by commands like `clean`, `fetch`, and
/// `package`, which don't specify any options or features.
pub fn resolve_ws<'a>(ws: &Workspace<'a>) -> CargoResult<(PackageSet<'a>, Resolve)> {
let max_rust_version = ws.rust_version();

let mut registry = PackageRegistry::new(ws.config())?;
let resolve = resolve_with_registry(ws, &mut registry)?;
let resolve = resolve_with_registry(ws, &mut registry, max_rust_version)?;
let packages = get_resolved_packages(&resolve, registry)?;
Ok((packages, resolve))
}
Expand All @@ -130,6 +133,7 @@ pub fn resolve_ws_with_opts<'cfg>(
specs: &[PackageIdSpec],
has_dev_units: HasDevUnits,
force_all_targets: ForceAllTargets,
max_rust_version: Option<PartialVersion>,
) -> CargoResult<WorkspaceResolve<'cfg>> {
let mut registry = PackageRegistry::new(ws.config())?;
let mut add_patches = true;
Expand All @@ -138,7 +142,7 @@ pub fn resolve_ws_with_opts<'cfg>(
} else if ws.require_optional_deps() {
// First, resolve the root_package's *listed* dependencies, as well as
// downloading and updating all remotes and such.
let resolve = resolve_with_registry(ws, &mut registry)?;
let resolve = resolve_with_registry(ws, &mut registry, max_rust_version)?;
// No need to add patches again, `resolve_with_registry` has done it.
add_patches = false;

Expand Down Expand Up @@ -184,6 +188,7 @@ pub fn resolve_ws_with_opts<'cfg>(
None,
specs,
add_patches,
max_rust_version,
)?;

let pkg_set = get_resolved_packages(&resolved_with_overrides, registry)?;
Expand Down Expand Up @@ -235,6 +240,7 @@ pub fn resolve_ws_with_opts<'cfg>(
fn resolve_with_registry<'cfg>(
ws: &Workspace<'cfg>,
registry: &mut PackageRegistry<'cfg>,
max_rust_version: Option<PartialVersion>,
) -> CargoResult<Resolve> {
let prev = ops::load_pkg_lockfile(ws)?;
let mut resolve = resolve_with_previous(
Expand All @@ -246,6 +252,7 @@ fn resolve_with_registry<'cfg>(
None,
&[],
true,
max_rust_version,
)?;

if !ws.is_ephemeral() && ws.require_optional_deps() {
Expand Down Expand Up @@ -278,6 +285,7 @@ pub fn resolve_with_previous<'cfg>(
to_avoid: Option<&HashSet<PackageId>>,
specs: &[PackageIdSpec],
register_patches: bool,
max_rust_version: Option<PartialVersion>,
) -> CargoResult<Resolve> {
// We only want one Cargo at a time resolving a crate graph since this can
// involve a lot of frobbing of the global caches.
Expand Down Expand Up @@ -505,6 +513,7 @@ pub fn resolve_with_previous<'cfg>(
ws.unstable_features()
.require(Feature::public_dependency())
.is_ok(),
max_rust_version,
)?;
let patches: Vec<_> = registry
.patches()
Expand Down
2 changes: 2 additions & 0 deletions src/cargo/ops/tree/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,7 @@ pub fn build_and_print(ws: &Workspace<'_>, opts: &TreeOptions) -> CargoResult<()
} else {
ForceAllTargets::No
};
let max_rust_version = ws.rust_version();
let ws_resolve = ops::resolve_ws_with_opts(
ws,
&mut target_data,
Expand All @@ -158,6 +159,7 @@ pub fn build_and_print(ws: &Workspace<'_>, opts: &TreeOptions) -> CargoResult<()
&specs,
has_dev,
force_all,
max_rust_version,
)?;

let package_map: HashMap<PackageId, &Package> = ws_resolve
Expand Down
2 changes: 1 addition & 1 deletion src/cargo/util/semver_ext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ impl From<VersionReq> for OptVersionReq {
}
}

#[derive(PartialEq, Eq, Hash, Copy, Clone, Debug)]
#[derive(PartialEq, Eq, PartialOrd, Ord, Hash, Copy, Clone, Debug)]
pub struct PartialVersion {
pub major: u64,
pub minor: Option<u64>,
Expand Down
2 changes: 1 addition & 1 deletion tests/testsuite/cargo_add/rust_version_ignore/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ fn case() {
.current_dir(cwd)
.masquerade_as_nightly_cargo(&["msrv-policy"])
.assert()
.success()
.code(101)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This will be undone when we get --ignore-rust-version working

.stdout_matches_path(curr_dir!().join("stdout.log"))
.stderr_matches_path(curr_dir!().join("stderr.log"));

Expand Down
5 changes: 5 additions & 0 deletions tests/testsuite/cargo_add/rust_version_ignore/stderr.log
Original file line number Diff line number Diff line change
@@ -1,2 +1,7 @@
Updating `dummy-registry` index
Adding rust-version-user v0.2.1 to dependencies.
error: failed to select a version for the requirement `rust-version-user = "^0.2.1"`
candidate versions found which didn't match: 0.2.1, 0.1.0
location searched: `dummy-registry` index (which is replacing registry `crates-io`)
required by package `cargo-list-test-fixture v0.0.0 ([ROOT]/case)`
perhaps a crate was updated and forgotten to be re-vendored?
Loading