Skip to content

Commit

Permalink
Warn on any inconsistency in workspace and package resolver
Browse files Browse the repository at this point in the history 
If the workspace and package specify or default to different resolver
versions then this will cause inconsistencies in build behavior between
a development build within the workspace and the published package. To
avoid this all packages must be configured to the same resolver
setting as the workspace, whether implied by the edition or explicitly
set.
  • Loading branch information
@Nemo157
Nemo157 committed Jul 28, 2022
1 parent 015143c commit 0026431
Show file tree
Hide file tree
Showing 5 changed files with 110 additions and 22 deletions.
17 changes: 17 additions & 0 deletions src/cargo/core/manifest.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -511,6 +511,18 @@ impl Manifest {
self.resolve_behavior
}

/// The style of resolver behavior to use, declared with the `resolver` field or defaulted
/// based on edition.
pub fn defaulted_resolve_behavior(&self) -> ResolveBehavior {
self.resolve_behavior().unwrap_or_else(|| {
if self.edition() >= Edition::Edition2021 {
ResolveBehavior::V2
} else {
ResolveBehavior::V1
}
})
}

pub fn map_source(self, to_replace: SourceId, replace_with: SourceId) -> Manifest {
Manifest {
summary: self.summary.map_source(to_replace, replace_with),
Expand Down Expand Up @@ -632,6 +644,11 @@ impl VirtualManifest {
pub fn resolve_behavior(&self) -> Option<ResolveBehavior> {
self.resolve_behavior
}

/// The style of resolver behavior to use, declared with the `resolver` field or defaulted.
pub fn defaulted_resolve_behavior(&self) -> ResolveBehavior {
self.resolve_behavior().unwrap_or(ResolveBehavior::V1)
}
}

impl Target {
Expand Down
52 changes: 34 additions & 18 deletions src/cargo/core/workspace.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use crate::core::features::Features;
use crate::core::registry::PackageRegistry;
use crate::core::resolver::features::CliFeatures;
use crate::core::resolver::ResolveBehavior;
use crate::core::{Dependency, Edition, FeatureValue, PackageId, PackageIdSpec};
use crate::core::{Dependency, FeatureValue, PackageId, PackageIdSpec};
use crate::core::{EitherManifest, Package, SourceId, VirtualManifest};
use crate::ops;
use crate::sources::{PathSource, CRATES_IO_INDEX, CRATES_IO_REGISTRY};
Expand Down Expand Up @@ -282,21 +282,10 @@ impl<'cfg> Workspace<'cfg> {
}

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(),
MaybePackage::Package(p) => p.manifest().defaulted_resolve_behavior(),
MaybePackage::Virtual(vm) => vm.defaulted_resolve_behavior(),
}
.unwrap_or(ResolveBehavior::V1);
}

/// Returns the current package of this workspace.
Expand Down Expand Up @@ -991,11 +980,38 @@ impl<'cfg> Workspace<'cfg> {
if !manifest.patch().is_empty() {
emit_warning("patch")?;
}
if let Some(behavior) = manifest.resolve_behavior() {
if behavior != self.resolve_behavior {
// Only warn if they don't match.
emit_warning("resolver")?;
if manifest.defaulted_resolve_behavior() != self.resolve_behavior {
let package_reason = manifest
.resolve_behavior()
.is_none()
.then(|| format!(r#" implied by edition = "{}""#, manifest.edition()))
.unwrap_or_default();
let workspace_reason = match self.root_maybe() {
MaybePackage::Package(p) => {
p.manifest().resolve_behavior().is_none().then(|| {
format!(r#" implied by edition = "{}""#, manifest.edition())
})
}
MaybePackage::Virtual(vm) => {
vm.resolve_behavior().is_none().then(|| " (default)".into())
}
}
.unwrap_or_default();
let msg = format!(
"expected resolver for package is overridden by the workspace root:\n\
package: {} expects resolver = \"{}\"{package_reason}\n\
workspace: {} sets resolver = \"{}\"{workspace_reason}",
pkg.manifest_path().display(),
manifest
.defaulted_resolve_behavior()
.to_manifest()
.unwrap_or_else(|| "1".into()),
root_manifest.display(),
self.resolve_behavior
.to_manifest()
.unwrap_or_else(|| "1".into()),
);
self.config.shell().warn(&msg)?;
}
}
}
Expand Down
57 changes: 53 additions & 4 deletions tests/testsuite/features2.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1065,6 +1065,7 @@ fn proc_macro_ws() {
[package]
name = "foo"
version = "0.1.0"
resolver = "2"
[features]
feat1 = []
Expand All @@ -1077,6 +1078,7 @@ fn proc_macro_ws() {
[package]
name = "pm"
version = "0.1.0"
resolver = "2"
[lib]
proc-macro = true
Expand Down Expand Up @@ -1364,7 +1366,7 @@ Caused by:

#[cargo_test]
fn resolver_ws_member() {
// Can't specify `resolver` in a ws member.
// Can't specify a different `resolver` in a ws member.
let p = project()
.file(
"Cargo.toml",
Expand All @@ -1388,9 +1390,9 @@ fn resolver_ws_member() {
p.cargo("check")
.with_stderr(
"\
warning: resolver for the non root package will be ignored, specify resolver at the workspace root:
package: [..]/foo/a/Cargo.toml
workspace: [..]/foo/Cargo.toml
warning: expected resolver for package is overridden by the workspace root:
package: [..]/foo/a/Cargo.toml expects resolver = \"2\"
workspace: [..]/foo/Cargo.toml sets resolver = \"1\" (default)
[CHECKING] a v0.1.0 [..]
[FINISHED] [..]
",
Expand Down Expand Up @@ -1433,6 +1435,41 @@ fn resolver_ws_root_and_member() {
.run();
}

#[cargo_test]
fn implicit_resolver_ws_member() {
let p = project()
.file(
"Cargo.toml",
r#"
[workspace]
members = ["a"]
"#,
)
.file(
"a/Cargo.toml",
r#"
[package]
name = "a"
version = "0.1.0"
edition = "2021" # implies `resolver = "2"`
"#,
)
.file("a/src/lib.rs", "")
.build();

p.cargo("check")
.with_stderr(
"\
warning: expected resolver for package is overridden by the workspace root:
package: [..]/foo/a/Cargo.toml expects resolver = \"2\" implied by edition = \"2021\"
workspace: [..]/foo/Cargo.toml sets resolver = \"1\" (default)
[CHECKING] a v0.1.0 [..]
[FINISHED] [..]
",
)
.run();
}

#[cargo_test]
fn resolver_enables_new_features() {
// resolver="2" enables all the things.
Expand Down Expand Up @@ -1472,6 +1509,7 @@ fn resolver_enables_new_features() {
name = "a"
version = "0.1.0"
edition = "2018"
resolver = "2"
[dependencies]
common = {version = "1.0", features=["normal"]}
Expand Down Expand Up @@ -1510,6 +1548,7 @@ fn resolver_enables_new_features() {
[package]
name = "b"
version = "0.1.0"
resolver = "2"
[features]
ping = []
Expand Down Expand Up @@ -1698,6 +1737,7 @@ fn shared_dep_same_but_dependencies() {
[package]
name = "bin1"
version = "0.1.0"
resolver = "2"
[dependencies]
dep = { path = "../dep" }
Expand All @@ -1710,6 +1750,7 @@ fn shared_dep_same_but_dependencies() {
[package]
name = "bin2"
version = "0.1.0"
resolver = "2"
[build-dependencies]
dep = { path = "../dep" }
Expand All @@ -1724,6 +1765,7 @@ fn shared_dep_same_but_dependencies() {
[package]
name = "dep"
version = "0.1.0"
resolver = "2"
[dependencies]
subdep = { path = "../subdep" }
Expand All @@ -1739,6 +1781,7 @@ fn shared_dep_same_but_dependencies() {
[package]
name = "subdep"
version = "0.1.0"
resolver = "2"
[features]
feat = []
Expand Down Expand Up @@ -1818,6 +1861,7 @@ fn test_proc_macro() {
[package]
name = "the-macro"
version = "0.1.0"
resolver = "2"
[lib]
proc-macro = true
test = false
Expand All @@ -1844,6 +1888,7 @@ fn test_proc_macro() {
[package]
name = "the-macro-support"
version = "0.1.0"
resolver = "2"
[dependencies]
shared = { path = "../shared" }
"#,
Expand All @@ -1860,6 +1905,7 @@ fn test_proc_macro() {
[package]
name = "shared"
version = "0.1.0"
resolver = "2"
[features]
b = []
"#,
Expand Down Expand Up @@ -2177,6 +2223,7 @@ fn pm_with_int_shared() {
name = "foo"
version = "0.1.0"
edition = "2018"
resolver = "2"
[dependencies]
pm = { path = "../pm" }
Expand All @@ -2196,6 +2243,7 @@ fn pm_with_int_shared() {
[package]
name = "pm"
version = "0.1.0"
resolver = "2"
[lib]
proc-macro = true
Expand Down Expand Up @@ -2224,6 +2272,7 @@ fn pm_with_int_shared() {
[package]
name = "shared"
version = "0.1.0"
resolver = "2"
[features]
norm-feat = []
Expand Down
3 changes: 3 additions & 0 deletions tests/testsuite/proc_macro.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -513,6 +513,7 @@ fn proc_macro_built_once() {
[package]
name = "a"
version = "0.1.0"
resolver = "2"
[build-dependencies]
the-macro = { path = '../the-macro' }
Expand All @@ -526,6 +527,7 @@ fn proc_macro_built_once() {
[package]
name = "b"
version = "0.1.0"
resolver = "2"
[dependencies]
the-macro = { path = '../the-macro', features = ['a'] }
Expand All @@ -538,6 +540,7 @@ fn proc_macro_built_once() {
[package]
name = "the-macro"
version = "0.1.0"
resolver = "2"
[lib]
proc_macro = true
Expand Down
3 changes: 3 additions & 0 deletions tests/testsuite/publish.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1708,6 +1708,7 @@ fn in_package_workspace() {
version = "0.0.1"
description = "li"
license = "MIT"
resolver = "2"
"#,
)
.file("li/src/main.rs", "fn main() {}")
Expand Down Expand Up @@ -1754,6 +1755,7 @@ fn with_duplicate_spec_in_members() {
version = "0.0.1"
description = "li"
license = "MIT"
resolver = "2"
"#,
)
.file("li/src/main.rs", "fn main() {}")
Expand All @@ -1765,6 +1767,7 @@ fn with_duplicate_spec_in_members() {
version = "0.0.1"
description = "bar"
license = "MIT"
resolver = "2"
"#,
)
.file("bar/src/main.rs", "fn main() {}")
Expand Down

0 comments on commit 0026431

Please sign in to comment.