From 5f856f72ea71c84c38a4172e27e35ab620842245 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Mon, 16 Oct 2023 11:03:07 -0500 Subject: [PATCH 1/8] test(spec): Make room for more tests --- src/cargo/core/package_id_spec.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/cargo/core/package_id_spec.rs b/src/cargo/core/package_id_spec.rs index 37a6f5e7b3c..313bcf6a4a5 100644 --- a/src/cargo/core/package_id_spec.rs +++ b/src/cargo/core/package_id_spec.rs @@ -444,11 +444,10 @@ mod tests { fn matching() { let url = Url::parse("https://example.com").unwrap(); let sid = SourceId::for_registry(&url).unwrap(); - let foo = PackageId::new("foo", "1.2.3", sid).unwrap(); - let bar = PackageId::new("bar", "1.2.3", sid).unwrap(); + let foo = PackageId::new("foo", "1.2.3", sid).unwrap(); assert!(PackageIdSpec::parse("foo").unwrap().matches(foo)); - assert!(!PackageIdSpec::parse("foo").unwrap().matches(bar)); + assert!(!PackageIdSpec::parse("bar").unwrap().matches(foo)); assert!(PackageIdSpec::parse("foo:1.2.3").unwrap().matches(foo)); assert!(!PackageIdSpec::parse("foo:1.2.2").unwrap().matches(foo)); assert!(PackageIdSpec::parse("foo@1.2.3").unwrap().matches(foo)); From ca6a57fbdd91857e2f032661a9f7a52d56fd3319 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Mon, 16 Oct 2023 11:08:22 -0500 Subject: [PATCH 2/8] test(spec): Cover meta/pre cases for partial versions We already tested with missing minor/patch --- src/cargo/core/package_id_spec.rs | 36 +++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/src/cargo/core/package_id_spec.rs b/src/cargo/core/package_id_spec.rs index 313bcf6a4a5..11cecb2d9f6 100644 --- a/src/cargo/core/package_id_spec.rs +++ b/src/cargo/core/package_id_spec.rs @@ -453,5 +453,41 @@ mod tests { assert!(PackageIdSpec::parse("foo@1.2.3").unwrap().matches(foo)); assert!(!PackageIdSpec::parse("foo@1.2.2").unwrap().matches(foo)); assert!(PackageIdSpec::parse("foo@1.2").unwrap().matches(foo)); + + let meta = PackageId::new("meta", "1.2.3+hello", sid).unwrap(); + assert!(PackageIdSpec::parse("meta").unwrap().matches(meta)); + assert!(PackageIdSpec::parse("meta@1").unwrap().matches(meta)); + assert!(PackageIdSpec::parse("meta@1.2").unwrap().matches(meta)); + assert!(PackageIdSpec::parse("meta@1.2.3").unwrap().matches(meta)); + assert!(!PackageIdSpec::parse("meta@1.2.3-alpha.0") + .unwrap() + .matches(meta)); + assert!(PackageIdSpec::parse("meta@1.2.3+hello") + .unwrap() + .matches(meta)); + assert!(PackageIdSpec::parse("meta@1.2.3+bye") + .unwrap() + .matches(meta)); + + let pre = PackageId::new("pre", "1.2.3-alpha.0", sid).unwrap(); + assert!(PackageIdSpec::parse("pre").unwrap().matches(pre)); + assert!(!PackageIdSpec::parse("pre@1").unwrap().matches(pre)); + assert!(!PackageIdSpec::parse("pre@1.2").unwrap().matches(pre)); + assert!(!PackageIdSpec::parse("pre@1.2.3").unwrap().matches(pre)); + assert!(PackageIdSpec::parse("pre@1.2.3-alpha.0") + .unwrap() + .matches(pre)); + assert!(!PackageIdSpec::parse("pre@1.2.3-alpha.1") + .unwrap() + .matches(pre)); + assert!(!PackageIdSpec::parse("pre@1.2.3-beta.0") + .unwrap() + .matches(pre)); + assert!(!PackageIdSpec::parse("pre@1.2.3+hello") + .unwrap() + .matches(pre)); + assert!(PackageIdSpec::parse("pre@1.2.3-alpha.0+hello") + .unwrap() + .matches(pre)); } } From fb1075be4f361826dccccade4e0353c3ffa3a185 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Tue, 10 Oct 2023 16:18:56 -0500 Subject: [PATCH 3/8] test(replace): Show behavior post-#12614 PR #12614 - Changed ignored replaces to not-ignored - Has bugs in matching replaces --- tests/testsuite/replace.rs | 144 +++++++++++++++++++++++++++++++++++++ 1 file changed, 144 insertions(+) diff --git a/tests/testsuite/replace.rs b/tests/testsuite/replace.rs index b583de7b7d5..15df0f9feeb 100644 --- a/tests/testsuite/replace.rs +++ b/tests/testsuite/replace.rs @@ -1298,3 +1298,147 @@ fn override_plus_dep() { .with_stderr_contains("error: cyclic package dependency: [..]") .run(); } + +#[cargo_test] +fn override_different_metadata() { + Package::new("bar", "0.1.0+a").publish(); + + let bar = git::repo(&paths::root().join("override")) + .file("Cargo.toml", &basic_manifest("bar", "0.1.0")) + .file("src/lib.rs", "pub fn bar() {}") + .build(); + + let p = project() + .file( + "Cargo.toml", + &format!( + r#" + [package] + name = "foo" + version = "0.0.1" + authors = [] + + [dependencies] + bar = "0.1.0" + + [replace] + "bar:0.1.0" = {{ git = '{}' }} + "#, + bar.url() + ), + ) + .file( + "src/lib.rs", + "extern crate bar; pub fn foo() { bar::bar(); }", + ) + .build(); + + p.cargo("check") + .with_stderr( + "\ +[UPDATING] `dummy-registry` index +[UPDATING] git repository `[..]` +thread 'main' panicked at src/cargo/core/resolver/dep_cache.rs:179:13: +assertion `left == right` failed + left: Version { major: 0, minor: 1, patch: 0 } + right: Version { major: 0, minor: 1, patch: 0, build: BuildMetadata(\"a\") } +note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace +", + ) + .with_status(101) + .run(); +} + +#[cargo_test] +fn override_different_metadata_2() { + Package::new("bar", "0.1.0+a").publish(); + + let bar = git::repo(&paths::root().join("override")) + .file("Cargo.toml", &basic_manifest("bar", "0.1.0+a")) + .file("src/lib.rs", "pub fn bar() {}") + .build(); + + let p = project() + .file( + "Cargo.toml", + &format!( + r#" + [package] + name = "foo" + version = "0.0.1" + authors = [] + + [dependencies] + bar = "0.1.0" + + [replace] + "bar:0.1.0+notTheBuild" = {{ git = '{}' }} + "#, + bar.url() + ), + ) + .file( + "src/lib.rs", + "extern crate bar; pub fn foo() { bar::bar(); }", + ) + .build(); + + p.cargo("check") + .with_stderr( + "\ +[UPDATING] `dummy-registry` index +[UPDATING] git repository `[..]` +[CHECKING] bar v0.1.0+a (file://[..]) +[CHECKING] foo v0.0.1 ([CWD]) +[FINISHED] dev [unoptimized + debuginfo] target(s) in [..] +", + ) + .run(); +} + +#[cargo_test] +fn override_different_metadata_3() { + Package::new("bar", "0.1.0+a").publish(); + + let bar = git::repo(&paths::root().join("override")) + .file("Cargo.toml", &basic_manifest("bar", "0.1.0+a")) + .file("src/lib.rs", "pub fn bar() {}") + .build(); + + let p = project() + .file( + "Cargo.toml", + &format!( + r#" + [package] + name = "foo" + version = "0.0.1" + authors = [] + + [dependencies] + bar = "0.1.0" + + [replace] + "bar:0.1.0" = {{ git = '{}' }} + "#, + bar.url() + ), + ) + .file( + "src/lib.rs", + "extern crate bar; pub fn foo() { bar::bar(); }", + ) + .build(); + + p.cargo("check") + .with_stderr( + "\ +[UPDATING] `dummy-registry` index +[UPDATING] git repository `[..]` +[CHECKING] bar v0.1.0+a (file://[..]) +[CHECKING] foo v0.0.1 ([CWD]) +[FINISHED] dev [unoptimized + debuginfo] target(s) in [..] +", + ) + .run(); +} From 2f8ab44b8185f9ce75f93f32c4a93244c45e31f1 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Tue, 10 Oct 2023 16:39:46 -0500 Subject: [PATCH 4/8] fix(spec): Ensure PackageIdSpec respects version 'build' field --- src/cargo/core/package_id_spec.rs | 13 ++++++------- src/cargo/util/semver_ext.rs | 24 ++++++++++++++---------- tests/testsuite/replace.rs | 20 +++++++++++++++----- 3 files changed, 35 insertions(+), 22 deletions(-) diff --git a/src/cargo/core/package_id_spec.rs b/src/cargo/core/package_id_spec.rs index 11cecb2d9f6..901c56ae19c 100644 --- a/src/cargo/core/package_id_spec.rs +++ b/src/cargo/core/package_id_spec.rs @@ -176,8 +176,7 @@ impl PackageIdSpec { } if let Some(ref v) = self.version { - let req = v.exact_req(); - if !req.matches(package_id.version()) { + if !v.matches(package_id.version()) { return false; } } @@ -465,15 +464,15 @@ mod tests { assert!(PackageIdSpec::parse("meta@1.2.3+hello") .unwrap() .matches(meta)); - assert!(PackageIdSpec::parse("meta@1.2.3+bye") + assert!(!PackageIdSpec::parse("meta@1.2.3+bye") .unwrap() .matches(meta)); let pre = PackageId::new("pre", "1.2.3-alpha.0", sid).unwrap(); assert!(PackageIdSpec::parse("pre").unwrap().matches(pre)); - assert!(!PackageIdSpec::parse("pre@1").unwrap().matches(pre)); - assert!(!PackageIdSpec::parse("pre@1.2").unwrap().matches(pre)); - assert!(!PackageIdSpec::parse("pre@1.2.3").unwrap().matches(pre)); + assert!(PackageIdSpec::parse("pre@1").unwrap().matches(pre)); + assert!(PackageIdSpec::parse("pre@1.2").unwrap().matches(pre)); + assert!(PackageIdSpec::parse("pre@1.2.3").unwrap().matches(pre)); assert!(PackageIdSpec::parse("pre@1.2.3-alpha.0") .unwrap() .matches(pre)); @@ -486,7 +485,7 @@ mod tests { assert!(!PackageIdSpec::parse("pre@1.2.3+hello") .unwrap() .matches(pre)); - assert!(PackageIdSpec::parse("pre@1.2.3-alpha.0+hello") + assert!(!PackageIdSpec::parse("pre@1.2.3-alpha.0+hello") .unwrap() .matches(pre)); } diff --git a/src/cargo/util/semver_ext.rs b/src/cargo/util/semver_ext.rs index bee3b2da3eb..abb56778db0 100644 --- a/src/cargo/util/semver_ext.rs +++ b/src/cargo/util/semver_ext.rs @@ -186,16 +186,20 @@ impl PartialVersion { } } - pub fn exact_req(&self) -> VersionReq { - VersionReq { - comparators: vec![Comparator { - op: semver::Op::Exact, - major: self.major, - minor: self.minor, - patch: self.patch, - pre: self.pre.as_ref().cloned().unwrap_or_default(), - }], - } + /// Check if this matches a version, including build metadata + /// + /// Build metadata does not affect version precedence but may be necessary for uniquely + /// identifying a package. + pub fn matches(&self, version: &Version) -> bool { + self.major == version.major + && self.minor.map(|f| f == version.minor).unwrap_or(true) + && self.patch.map(|f| f == version.patch).unwrap_or(true) + && self.pre.as_ref().map(|f| f == &version.pre).unwrap_or(true) + && self + .build + .as_ref() + .map(|f| f == &version.build) + .unwrap_or(true) } } diff --git a/tests/testsuite/replace.rs b/tests/testsuite/replace.rs index 15df0f9feeb..ee2314fd797 100644 --- a/tests/testsuite/replace.rs +++ b/tests/testsuite/replace.rs @@ -1350,7 +1350,7 @@ note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace } #[cargo_test] -fn override_different_metadata_2() { +fn override_respects_spec_metadata() { Package::new("bar", "0.1.0+a").publish(); let bar = git::repo(&paths::root().join("override")) @@ -1387,12 +1387,22 @@ fn override_different_metadata_2() { .with_stderr( "\ [UPDATING] `dummy-registry` index -[UPDATING] git repository `[..]` -[CHECKING] bar v0.1.0+a (file://[..]) -[CHECKING] foo v0.0.1 ([CWD]) -[FINISHED] dev [unoptimized + debuginfo] target(s) in [..] +[WARNING] package replacement is not used: https://github.com/rust-lang/crates.io-index#bar@0.1.0+notTheBuild +[DOWNLOADING] crates ... +[DOWNLOADED] bar v0.1.0+a (registry `dummy-registry`) +[CHECKING] bar v0.1.0+a +[CHECKING] foo v0.0.1 ([..]/foo) +[..] +[..] +[..] +[..] +[..] +[..] +[..] +error: could not compile `foo` (lib) due to previous error ", ) + .with_status(101) .run(); } From 50434851af0ae86b1ff2ee2c16d71a7e58b764c5 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Tue, 17 Oct 2023 10:09:12 -0500 Subject: [PATCH 5/8] fix(spec): Require opt-in for pre-release --- src/cargo/core/package_id_spec.rs | 6 +++--- src/cargo/util/semver_ext.rs | 5 +++++ 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/src/cargo/core/package_id_spec.rs b/src/cargo/core/package_id_spec.rs index 901c56ae19c..53d99b84ba7 100644 --- a/src/cargo/core/package_id_spec.rs +++ b/src/cargo/core/package_id_spec.rs @@ -470,9 +470,9 @@ mod tests { let pre = PackageId::new("pre", "1.2.3-alpha.0", sid).unwrap(); assert!(PackageIdSpec::parse("pre").unwrap().matches(pre)); - assert!(PackageIdSpec::parse("pre@1").unwrap().matches(pre)); - assert!(PackageIdSpec::parse("pre@1.2").unwrap().matches(pre)); - assert!(PackageIdSpec::parse("pre@1.2.3").unwrap().matches(pre)); + assert!(!PackageIdSpec::parse("pre@1").unwrap().matches(pre)); + assert!(!PackageIdSpec::parse("pre@1.2").unwrap().matches(pre)); + assert!(!PackageIdSpec::parse("pre@1.2.3").unwrap().matches(pre)); assert!(PackageIdSpec::parse("pre@1.2.3-alpha.0") .unwrap() .matches(pre)); diff --git a/src/cargo/util/semver_ext.rs b/src/cargo/util/semver_ext.rs index abb56778db0..5839d85d23f 100644 --- a/src/cargo/util/semver_ext.rs +++ b/src/cargo/util/semver_ext.rs @@ -191,6 +191,11 @@ impl PartialVersion { /// Build metadata does not affect version precedence but may be necessary for uniquely /// identifying a package. pub fn matches(&self, version: &Version) -> bool { + if !version.pre.is_empty() && self.pre.is_none() { + // Pre-release versions must be explicitly opted into, if for no other reason than to + // give us room to figure out and define the semantics + return false; + } self.major == version.major && self.minor.map(|f| f == version.minor).unwrap_or(true) && self.patch.map(|f| f == version.patch).unwrap_or(true) From 04aba58c8509623605c3668adacf0821c2eae785 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Tue, 10 Oct 2023 21:31:31 -0500 Subject: [PATCH 6/8] fix(replace): Error, rather than assert, on version mismatch --- src/cargo/core/resolver/dep_cache.rs | 19 ++++++++++++++----- tests/testsuite/replace.rs | 12 ++++++------ 2 files changed, 20 insertions(+), 11 deletions(-) diff --git a/src/cargo/core/resolver/dep_cache.rs b/src/cargo/core/resolver/dep_cache.rs index 7b6e0661f17..9041c5b0f9e 100644 --- a/src/cargo/core/resolver/dep_cache.rs +++ b/src/cargo/core/resolver/dep_cache.rs @@ -173,11 +173,20 @@ impl<'a> RegistryQueryer<'a> { ))); } - // The dependency should be hard-coded to have the same name and an - // exact version requirement, so both of these assertions should - // never fail. - assert_eq!(s.version(), summary.version()); - assert_eq!(s.name(), summary.name()); + assert_eq!( + s.name(), + summary.name(), + "dependency should be hard coded to have the same name" + ); + if s.version() != summary.version() { + return Poll::Ready(Err(anyhow::anyhow!( + "replacement specification `{}` matched {} and tried to override it with {}\n\ + avoid matching unrelated packages by being more specific", + spec, + summary.version(), + s.version(), + ))); + } let replace = if s.source_id() == summary.source_id() { debug!("Preventing\n{:?}\nfrom replacing\n{:?}", summary, s); diff --git a/tests/testsuite/replace.rs b/tests/testsuite/replace.rs index ee2314fd797..a0bce4e2489 100644 --- a/tests/testsuite/replace.rs +++ b/tests/testsuite/replace.rs @@ -1300,7 +1300,7 @@ fn override_plus_dep() { } #[cargo_test] -fn override_different_metadata() { +fn override_generic_matching_other_versions() { Package::new("bar", "0.1.0+a").publish(); let bar = git::repo(&paths::root().join("override")) @@ -1338,11 +1338,11 @@ fn override_different_metadata() { "\ [UPDATING] `dummy-registry` index [UPDATING] git repository `[..]` -thread 'main' panicked at src/cargo/core/resolver/dep_cache.rs:179:13: -assertion `left == right` failed - left: Version { major: 0, minor: 1, patch: 0 } - right: Version { major: 0, minor: 1, patch: 0, build: BuildMetadata(\"a\") } -note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace +[ERROR] failed to get `bar` as a dependency of package `foo v0.0.1 ([..]/foo)` + +Caused by: + replacement specification `https://github.com/rust-lang/crates.io-index#bar@0.1.0` matched 0.1.0+a and tried to override it with 0.1.0 + avoid matching unrelated packages by being more specific ", ) .with_status(101) From 162a73f005e9e9ccea7ea1b3c04538d3f4eac253 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Tue, 10 Oct 2023 21:35:02 -0500 Subject: [PATCH 7/8] test(replace): Clarify name for remaining new test --- tests/testsuite/replace.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/testsuite/replace.rs b/tests/testsuite/replace.rs index a0bce4e2489..b9de51d2fe6 100644 --- a/tests/testsuite/replace.rs +++ b/tests/testsuite/replace.rs @@ -1407,7 +1407,7 @@ error: could not compile `foo` (lib) due to previous error } #[cargo_test] -fn override_different_metadata_3() { +fn override_spec_metadata_is_optional() { Package::new("bar", "0.1.0+a").publish(); let bar = git::repo(&paths::root().join("override")) From 01a543efdd4d90e0a70064b3fe32e563a5aac6f3 Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Tue, 10 Oct 2023 10:13:16 -0700 Subject: [PATCH 8/8] rustdoc: remove the word "Version" from test cases Needed for https://github.com/rust-lang/rust/pull/115948 to merge. --- tests/testsuite/doc.rs | 4 ++-- tests/testsuite/rustdocflags.rs | 8 +++++--- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/tests/testsuite/doc.rs b/tests/testsuite/doc.rs index 481df859045..a1698091246 100644 --- a/tests/testsuite/doc.rs +++ b/tests/testsuite/doc.rs @@ -2004,7 +2004,7 @@ fn crate_versions() { let output_path = p.root().join("target/doc/foo/index.html"); let output_documentation = fs::read_to_string(&output_path).unwrap(); - assert!(output_documentation.contains("Version 1.2.4")); + assert!(output_documentation.contains("1.2.4")); } #[cargo_test] @@ -2028,7 +2028,7 @@ fn crate_versions_flag_is_overridden() { }; let asserts = |html: String| { assert!(!html.contains("1.2.4")); - assert!(html.contains("Version 2.0.3")); + assert!(html.contains("2.0.3")); }; p.cargo("doc") diff --git a/tests/testsuite/rustdocflags.rs b/tests/testsuite/rustdocflags.rs index 6992961ce39..c37d5a8266b 100644 --- a/tests/testsuite/rustdocflags.rs +++ b/tests/testsuite/rustdocflags.rs @@ -110,17 +110,19 @@ fn whitespace() { .with_status(101) .run(); - const SPACED_VERSION: &str = "a\nb\tc\u{00a0}d"; p.cargo("doc") .env_remove("__CARGO_TEST_FORCE_ARGFILE") // Not applicable for argfile. .env( "RUSTDOCFLAGS", - format!("--crate-version {}", SPACED_VERSION), + "--crate-version 1111\n2222\t3333\u{00a0}4444", ) .run(); let contents = p.read_file("target/doc/foo/index.html"); - assert!(contents.contains(SPACED_VERSION)); + assert!(contents.contains("1111")); + assert!(contents.contains("2222")); + assert!(contents.contains("3333")); + assert!(contents.contains("4444")); } #[cargo_test]