Skip to content

Commit

Permalink
Change how workspace resolve behavior is calculated.
Browse files Browse the repository at this point in the history
This fixes an issue where warnings would be printed for packages
migrating to 2021 in a workspace (that the "resolver" field is ignored,
which is wrong).

This also places the default resolver logic in one place, and should
make it easier to update later.
  • Loading branch information
ehuss committed Feb 19, 2021
1 parent c303213 commit a82a23c
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 22 deletions.
45 changes: 30 additions & 15 deletions src/cargo/core/workspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use crate::core::features::Features;
use crate::core::registry::PackageRegistry;
use crate::core::resolver::features::RequestedFeatures;
use crate::core::resolver::ResolveBehavior;
use crate::core::{Dependency, PackageId, PackageIdSpec};
use crate::core::{Dependency, Edition, PackageId, PackageIdSpec};
use crate::core::{EitherManifest, Package, SourceId, VirtualManifest};
use crate::ops;
use crate::sources::PathSource;
Expand Down Expand Up @@ -88,7 +88,7 @@ pub struct Workspace<'cfg> {
ignore_lock: bool,

/// The resolver behavior specified with the `resolver` field.
resolve_behavior: Option<ResolveBehavior>,
resolve_behavior: ResolveBehavior,

/// Workspace-level custom metadata
custom_metadata: Option<toml::Value>,
Expand Down Expand Up @@ -164,10 +164,7 @@ impl<'cfg> Workspace<'cfg> {
.load_workspace_config()?
.and_then(|cfg| cfg.custom_metadata);
ws.find_members()?;
ws.resolve_behavior = match ws.root_maybe() {
MaybePackage::Package(p) => p.manifest().resolve_behavior(),
MaybePackage::Virtual(vm) => vm.resolve_behavior(),
};
ws.set_resolve_behavior();
ws.validate()?;
Ok(ws)
}
Expand All @@ -189,7 +186,7 @@ impl<'cfg> Workspace<'cfg> {
require_optional_deps: true,
loaded_packages: RefCell::new(HashMap::new()),
ignore_lock: false,
resolve_behavior: None,
resolve_behavior: ResolveBehavior::V1,
custom_metadata: None,
}
}
Expand All @@ -203,11 +200,11 @@ impl<'cfg> Workspace<'cfg> {
let mut ws = Workspace::new_default(current_manifest, config);
ws.root_manifest = Some(root_path.join("Cargo.toml"));
ws.target_dir = config.target_dir()?;
ws.resolve_behavior = manifest.resolve_behavior();
ws.packages
.packages
.insert(root_path, MaybePackage::Virtual(manifest));
ws.find_members()?;
ws.set_resolve_behavior();
// TODO: validation does not work because it walks up the directory
// tree looking for the root which is a fake file that doesn't exist.
Ok(ws)
Expand All @@ -231,7 +228,6 @@ impl<'cfg> Workspace<'cfg> {
let mut ws = Workspace::new_default(package.manifest_path().to_path_buf(), config);
ws.is_ephemeral = true;
ws.require_optional_deps = require_optional_deps;
ws.resolve_behavior = package.manifest().resolve_behavior();
let key = ws.current_manifest.parent().unwrap();
let id = package.package_id();
let package = MaybePackage::Package(package);
Expand All @@ -244,9 +240,28 @@ impl<'cfg> Workspace<'cfg> {
ws.members.push(ws.current_manifest.clone());
ws.member_ids.insert(id);
ws.default_members.push(ws.current_manifest.clone());
ws.set_resolve_behavior();
Ok(ws)
}

fn set_resolve_behavior(&mut self) {
// - If resolver is specified in the workspace definition, use that.
// - If the root package specifies the resolver, use that.
// - If the root package specifies edition 2021, use v2.
// - Otherwise, use the default v1.
self.resolve_behavior = match self.root_maybe() {
MaybePackage::Package(p) => p.manifest().resolve_behavior().or_else(|| {
if p.manifest().edition() >= Edition::Edition2021 {
Some(ResolveBehavior::V2)
} else {
None
}
}),
MaybePackage::Virtual(vm) => vm.resolve_behavior(),
}
.unwrap_or(ResolveBehavior::V1);
}

/// Returns the current package of this workspace.
///
/// Note that this can return an error if it the current manifest is
Expand Down Expand Up @@ -634,7 +649,7 @@ impl<'cfg> Workspace<'cfg> {
}

pub fn resolve_behavior(&self) -> ResolveBehavior {
self.resolve_behavior.unwrap_or(ResolveBehavior::V1)
self.resolve_behavior
}

/// Returns `true` if this workspace uses the new CLI features behavior.
Expand Down Expand Up @@ -843,11 +858,11 @@ impl<'cfg> Workspace<'cfg> {
if !manifest.patch().is_empty() {
emit_warning("patch")?;
}
if manifest.resolve_behavior().is_some()
&& manifest.resolve_behavior() != self.resolve_behavior
{
// Only warn if they don't match.
emit_warning("resolver")?;
if let Some(behavior) = manifest.resolve_behavior() {
if behavior != self.resolve_behavior {
// Only warn if they don't match.
emit_warning("resolver")?;
}
}
}
}
Expand Down
8 changes: 1 addition & 7 deletions src/cargo/util/toml/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1132,13 +1132,7 @@ impl TomlManifest {
project.resolver.as_ref(),
me.workspace.as_ref().and_then(|ws| ws.resolver.as_ref()),
) {
(None, None) => {
if edition == Edition::Edition2021 {
Some(ResolveBehavior::V2)
} else {
None
}
}
(None, None) => None,
(Some(s), None) | (None, Some(s)) => Some(ResolveBehavior::from_manifest(s)?),
(Some(_), Some(_)) => {
bail!("cannot specify `resolver` field in both `[workspace]` and `[package]`")
Expand Down

0 comments on commit a82a23c

Please sign in to comment.