From 1b4fbfb20ac12be139bad26544784092c503f191 Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Fri, 6 Oct 2023 03:09:49 +0000 Subject: [PATCH 01/12] chore(deps): update latest msrv to v1.73 --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 950a80cca70..bda07248e61 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -11,7 +11,7 @@ exclude = [ ] [workspace.package] -rust-version = "1.72.0" +rust-version = "1.73" edition = "2021" license = "MIT OR Apache-2.0" From 914119940b7c23cc9afe402b75aeae09a0928ec8 Mon Sep 17 00:00:00 2001 From: hi-rustin Date: Fri, 6 Oct 2023 16:16:35 +0800 Subject: [PATCH 02/12] Add test for unsupported short config flag --- tests/testsuite/build.rs | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/tests/testsuite/build.rs b/tests/testsuite/build.rs index 1afa83918f0..62d37a910c5 100644 --- a/tests/testsuite/build.rs +++ b/tests/testsuite/build.rs @@ -158,6 +158,27 @@ For more information, try '--help'. .run(); } +#[cargo_test] +fn cargo_compile_with_unsupported_short_config_flag() { + let p = project() + .file("Cargo.toml", &basic_bin_manifest("foo")) + .file("src/foo.rs", &main_file(r#""i am foo""#, &[])) + .build(); + + p.cargo("build -c net.git-fetch-with-cli=true") + .with_stderr( + "\ +error: unexpected argument '-c' found + +Usage: cargo[EXE] build [OPTIONS] + +For more information, try '--help'. +", + ) + .with_status(1) + .run(); +} + #[cargo_test] fn cargo_compile_with_workspace_excluded() { let p = project().file("src/main.rs", "fn main() {}").build(); From 2400e42652b2280e053cad8691622e18cc809888 Mon Sep 17 00:00:00 2001 From: hi-rustin Date: Fri, 6 Oct 2023 16:27:31 +0800 Subject: [PATCH 03/12] add unsupported short suggestion for `--config` flag Signed-off-by: hi-rustin --- src/bin/cargo/cli.rs | 2 +- src/cargo/util/command_prelude.rs | 15 +++++++++++++++ tests/testsuite/build.rs | 2 ++ 3 files changed, 18 insertions(+), 1 deletion(-) diff --git a/src/bin/cargo/cli.rs b/src/bin/cargo/cli.rs index 06b4a20e231..7c0e3b5ee85 100644 --- a/src/bin/cargo/cli.rs +++ b/src/bin/cargo/cli.rs @@ -618,7 +618,7 @@ See 'cargo help <>' for more information on a sp .help_heading(heading::MANIFEST_OPTIONS) .global(true), ) - .arg(multi_opt("config", "KEY=VALUE", "Override a configuration value").global(true)) + .arg_config() .arg( Arg::new("unstable-features") .help("Unstable (nightly-only) flags to Cargo, see 'cargo -Z help' for details") diff --git a/src/cargo/util/command_prelude.rs b/src/cargo/util/command_prelude.rs index a8b8e31c7eb..bfc2c45cd05 100644 --- a/src/cargo/util/command_prelude.rs +++ b/src/cargo/util/command_prelude.rs @@ -378,6 +378,21 @@ pub trait CommandExt: Sized { ) ._arg(unsupported_short_arg) } + + fn arg_config(self) -> Self { + let unsupported_short_arg = { + let value_parser = UnknownArgumentValueParser::suggest_arg("--config"); + Arg::new("unsupported-short-config-flag") + .help("") + .short('c') + .value_parser(value_parser) + .action(ArgAction::SetTrue) + .global(true) + .hide(true) + }; + self._arg(unsupported_short_arg) + ._arg(multi_opt("config", "KEY=VALUE", "Override a configuration value").global(true)) + } } impl CommandExt for Command { diff --git a/tests/testsuite/build.rs b/tests/testsuite/build.rs index 62d37a910c5..bb6404692bc 100644 --- a/tests/testsuite/build.rs +++ b/tests/testsuite/build.rs @@ -170,6 +170,8 @@ fn cargo_compile_with_unsupported_short_config_flag() { "\ error: unexpected argument '-c' found + tip: a similar argument exists: '--config' + Usage: cargo[EXE] build [OPTIONS] For more information, try '--help'. From e287f430064276c4645736de6b3b27fa929e8b6a Mon Sep 17 00:00:00 2001 From: Tobias Bieniek Date: Fri, 6 Oct 2023 14:03:48 +0200 Subject: [PATCH 04/12] crates-io: Add doc comment for `NewCrate` struct --- crates/crates-io/lib.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/crates/crates-io/lib.rs b/crates/crates-io/lib.rs index 757241fd344..1764ce527de 100644 --- a/crates/crates-io/lib.rs +++ b/crates/crates-io/lib.rs @@ -38,6 +38,10 @@ pub struct Crate { pub max_version: String, } +/// This struct is serialized as JSON and sent as metadata ahead of the crate +/// tarball when publishing crates to a crate registry like crates.io. +/// +/// see #[derive(Serialize, Deserialize)] pub struct NewCrate { pub name: String, From 4aa1a957144c99ab46107e5c6031b1d7870e2a21 Mon Sep 17 00:00:00 2001 From: David Rheinsberg Date: Fri, 6 Oct 2023 13:49:38 +0200 Subject: [PATCH 05/12] cargo/targets: fix error-message typo Fix typo: "with with" -> "with" --- src/cargo/util/toml/targets.rs | 2 +- tests/testsuite/build.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/cargo/util/toml/targets.rs b/src/cargo/util/toml/targets.rs index afc4f258f69..8455ae95b3e 100644 --- a/src/cargo/util/toml/targets.rs +++ b/src/cargo/util/toml/targets.rs @@ -309,7 +309,7 @@ fn clean_bins( if restricted_names::is_conflicting_artifact_name(&name) { anyhow::bail!( "the binary target name `{}` is forbidden, \ - it conflicts with with cargo's build directory names", + it conflicts with cargo's build directory names", name ) } diff --git a/tests/testsuite/build.rs b/tests/testsuite/build.rs index 1afa83918f0..0466ff452c4 100644 --- a/tests/testsuite/build.rs +++ b/tests/testsuite/build.rs @@ -465,7 +465,7 @@ fn cargo_compile_with_forbidden_bin_target_name() { [ERROR] failed to parse manifest at `[..]` Caused by: - the binary target name `build` is forbidden, it conflicts with with cargo's build directory names + the binary target name `build` is forbidden, it conflicts with cargo's build directory names ", ) .run(); From 9e5dac18b9845c9f5b6e750b2124386f74902d7c Mon Sep 17 00:00:00 2001 From: Tobias Bieniek Date: Fri, 6 Oct 2023 16:09:10 +0200 Subject: [PATCH 06/12] crates.io: Bump version to 0.39.1 --- Cargo.lock | 2 +- crates/crates-io/Cargo.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index ac7e02a6b76..0becc6c67bb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -582,7 +582,7 @@ dependencies = [ [[package]] name = "crates-io" -version = "0.39.0" +version = "0.39.1" dependencies = [ "curl", "percent-encoding", diff --git a/crates/crates-io/Cargo.toml b/crates/crates-io/Cargo.toml index d06dacdfa43..f1b92602e9d 100644 --- a/crates/crates-io/Cargo.toml +++ b/crates/crates-io/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "crates-io" -version = "0.39.0" +version = "0.39.1" rust-version.workspace = true edition.workspace = true license.workspace = true From 24ffed4aceb79fa3fab107379d6818eb413c3952 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Wed, 4 Oct 2023 13:00:37 -0500 Subject: [PATCH 07/12] test(toml): Verify existing version-less behavior --- tests/testsuite/check.rs | 29 +++++++++++++++++++ tests/testsuite/metadata.rs | 55 +++++++++++++++++++++++++++++++++++++ tests/testsuite/package.rs | 30 ++++++++++++++++++++ tests/testsuite/publish.rs | 30 ++++++++++++++++++++ 4 files changed, 144 insertions(+) diff --git a/tests/testsuite/check.rs b/tests/testsuite/check.rs index b74bd620996..8391e8eb587 100644 --- a/tests/testsuite/check.rs +++ b/tests/testsuite/check.rs @@ -1496,3 +1496,32 @@ fn check_unused_manifest_keys() { ) .run(); } + +#[cargo_test] +fn versionless_package() { + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + description = "foo" + "#, + ) + .file("src/lib.rs", "") + .build(); + p.cargo("check") + .with_stderr( + r#"error: failed to parse manifest at `[CWD]/Cargo.toml` + +Caused by: + TOML parse error at line 2, column 17 + | + 2 | [package] + | ^^^^^^^^^^^^^^^^^^^^^^^^^ + missing field `version` +"#, + ) + .with_status(101) + .run(); +} diff --git a/tests/testsuite/metadata.rs b/tests/testsuite/metadata.rs index fbead4dea97..659514b3a1c 100644 --- a/tests/testsuite/metadata.rs +++ b/tests/testsuite/metadata.rs @@ -4257,3 +4257,58 @@ fn workspace_metadata_with_dependencies_no_deps_artifact() { ) .run(); } + +#[cargo_test] +fn versionless_packages() { + let p = project() + .file( + "Cargo.toml", + r#" + [workspace] + members = ["bar", "baz"] + "#, + ) + .file( + "bar/Cargo.toml", + r#" + [package] + name = "bar" + + [dependencies] + foobar = "0.0.1" + baz = { path = "../baz/" } + "#, + ) + .file("bar/src/lib.rs", "") + .file( + "baz/Cargo.toml", + r#" + [package] + name = "baz" + + [dependencies] + foobar = "0.0.1" + "#, + ) + .file("baz/src/lib.rs", "") + .build(); + Package::new("foobar", "0.0.1").publish(); + + p.cargo("metadata -q --format-version 1") + .with_stderr( + r#"error: failed to load manifest for workspace member `[CWD]/bar` + +Caused by: + failed to parse manifest at `[CWD]/bar/Cargo.toml` + +Caused by: + TOML parse error at line 2, column 17 + | + 2 | [package] + | ^^^^^^^^^^^^^^^^^^^^^^^^^ + missing field `version` +"#, + ) + .with_status(101) + .run(); +} diff --git a/tests/testsuite/package.rs b/tests/testsuite/package.rs index 8a7334d890d..fd4f1a20a93 100644 --- a/tests/testsuite/package.rs +++ b/tests/testsuite/package.rs @@ -3095,3 +3095,33 @@ src/main.rs &[], ); } + +#[cargo_test] +fn versionless_package() { + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + description = "foo" + "#, + ) + .file("src/main.rs", r#"fn main() { println!("hello"); }"#) + .build(); + + p.cargo("package") + .with_stderr( + r#"error: failed to parse manifest at `[CWD]/Cargo.toml` + +Caused by: + TOML parse error at line 2, column 17 + | + 2 | [package] + | ^^^^^^^^^^^^^^^^^^^^^^^^^ + missing field `version` +"#, + ) + .with_status(101) + .run(); +} diff --git a/tests/testsuite/publish.rs b/tests/testsuite/publish.rs index 67569bf3b6a..585a3bb6fe2 100644 --- a/tests/testsuite/publish.rs +++ b/tests/testsuite/publish.rs @@ -3004,3 +3004,33 @@ Caused by: .with_status(101) .run(); } + +#[cargo_test] +fn versionless_package() { + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + description = "foo" + "#, + ) + .file("src/main.rs", r#"fn main() { println!("hello"); }"#) + .build(); + + p.cargo("publish") + .with_stderr( + r#"error: failed to parse manifest at `[CWD]/Cargo.toml` + +Caused by: + TOML parse error at line 2, column 17 + | + 2 | [package] + | ^^^^^^^^^^^^^^^^^^^^^^^^^ + missing field `version` +"#, + ) + .with_status(101) + .run(); +} From 4bedf9b83122cb8008f4a2602530877250bb5510 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Wed, 4 Oct 2023 12:23:46 -0500 Subject: [PATCH 08/12] refactor(toml): Use explicit deserialize Unsure if we want to generalize the trim policy but this will make it much easier to support an optional version field in future commits since we can just wrap the type in `Option` Strangely, `UntaggedEnumDeserializer` isn't causing bad error messages, so we can keep using it. --- src/cargo/util/toml/mod.rs | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index 08e8f4e31d4..ceb755646f6 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -1660,7 +1660,6 @@ pub struct TomlPackage { edition: Option, rust_version: Option, name: InternedString, - #[serde(deserialize_with = "version_trim_whitespace")] version: MaybeWorkspaceSemverVersion, authors: Option, build: Option, @@ -1710,22 +1709,6 @@ impl TomlPackage { } } -fn version_trim_whitespace<'de, D>(deserializer: D) -> Result -where - D: de::Deserializer<'de>, -{ - UntaggedEnumVisitor::new() - .expecting("SemVer version") - .string( - |value| match value.trim().parse().map_err(de::Error::custom) { - Ok(parsed) => Ok(MaybeWorkspace::Defined(parsed)), - Err(e) => Err(e), - }, - ) - .map(|value| value.deserialize().map(MaybeWorkspace::Workspace)) - .deserialize(deserializer) -} - /// This Trait exists to make [`MaybeWorkspace::Workspace`] generic. It makes deserialization of /// [`MaybeWorkspace`] much easier, as well as making error messages for /// [`MaybeWorkspace::resolve`] much nicer @@ -1794,6 +1777,23 @@ impl MaybeWorkspace { //. This already has a `Deserialize` impl from version_trim_whitespace type MaybeWorkspaceSemverVersion = MaybeWorkspace; +impl<'de> de::Deserialize<'de> for MaybeWorkspaceSemverVersion { + fn deserialize(d: D) -> Result + where + D: de::Deserializer<'de>, + { + UntaggedEnumVisitor::new() + .expecting("SemVer version") + .string( + |value| match value.trim().parse().map_err(de::Error::custom) { + Ok(parsed) => Ok(MaybeWorkspace::Defined(parsed)), + Err(e) => Err(e), + }, + ) + .map(|value| value.deserialize().map(MaybeWorkspace::Workspace)) + .deserialize(d) + } +} type MaybeWorkspaceString = MaybeWorkspace; impl<'de> de::Deserialize<'de> for MaybeWorkspaceString { From 48af530be47462ac3aa386c1dd0bb039d02e477b Mon Sep 17 00:00:00 2001 From: Ed Page Date: Wed, 4 Oct 2023 12:33:35 -0500 Subject: [PATCH 09/12] feat(toml): Allow versionless packages This defaults the version to `0.0.0` for most of cargo. It is an error to lack a version and have a package publishable. That means you have to add `publish = false`. --- src/cargo/util/toml/mod.rs | 18 ++- src/doc/src/reference/manifest.md | 2 + tests/testsuite/check.rs | 15 +- tests/testsuite/metadata.rs | 255 ++++++++++++++++++++++++++++-- tests/testsuite/package.rs | 28 ++-- tests/testsuite/publish.rs | 17 +- 6 files changed, 290 insertions(+), 45 deletions(-) diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index ceb755646f6..2043a1b260e 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -550,11 +550,17 @@ impl TomlManifest { let version = package .version .clone() - .resolve("version", || inherit()?.version())?; + .map(|version| version.resolve("version", || inherit()?.version())) + .transpose()?; - package.version = MaybeWorkspace::Defined(version.clone()); + package.version = version.clone().map(MaybeWorkspace::Defined); - let pkgid = package.to_package_id(source_id, version)?; + let pkgid = package.to_package_id( + source_id, + version + .clone() + .unwrap_or_else(|| semver::Version::new(0, 0, 0)), + )?; let edition = if let Some(edition) = package.edition.clone() { let edition: Edition = edition @@ -1009,6 +1015,10 @@ impl TomlManifest { None | Some(VecStringOrBool::Bool(true)) => None, }; + if version.is_none() && publish != Some(vec![]) { + bail!("`package.version` must be specified unless `package.publish = false`"); + } + if summary.features().contains_key("default-features") { warnings.push( "`default-features = [\"..\"]` was found in [features]. \ @@ -1660,7 +1670,7 @@ pub struct TomlPackage { edition: Option, rust_version: Option, name: InternedString, - version: MaybeWorkspaceSemverVersion, + version: Option, authors: Option, build: Option, metabuild: Option, diff --git a/src/doc/src/reference/manifest.md b/src/doc/src/reference/manifest.md index 5ecbe511749..49e59ecc642 100644 --- a/src/doc/src/reference/manifest.md +++ b/src/doc/src/reference/manifest.md @@ -109,6 +109,8 @@ resolve dependencies, and for guidelines on setting your own version. See the [SemVer compatibility] chapter for more details on exactly what constitutes a breaking change. +This field is optional and defaults to `0.0.0`. + [Resolver]: resolver.md [SemVer compatibility]: semver.md diff --git a/tests/testsuite/check.rs b/tests/testsuite/check.rs index 8391e8eb587..f7c865dedba 100644 --- a/tests/testsuite/check.rs +++ b/tests/testsuite/check.rs @@ -1506,22 +1506,17 @@ fn versionless_package() { [package] name = "foo" description = "foo" + publish = false "#, ) .file("src/lib.rs", "") .build(); p.cargo("check") .with_stderr( - r#"error: failed to parse manifest at `[CWD]/Cargo.toml` - -Caused by: - TOML parse error at line 2, column 17 - | - 2 | [package] - | ^^^^^^^^^^^^^^^^^^^^^^^^^ - missing field `version` -"#, + "\ +[CHECKING] foo v0.0.0 ([CWD]) +[FINISHED] dev [unoptimized + debuginfo] target(s) in [..] +", ) - .with_status(101) .run(); } diff --git a/tests/testsuite/metadata.rs b/tests/testsuite/metadata.rs index 659514b3a1c..223a8f70784 100644 --- a/tests/testsuite/metadata.rs +++ b/tests/testsuite/metadata.rs @@ -4273,6 +4273,7 @@ fn versionless_packages() { r#" [package] name = "bar" + publish = false [dependencies] foobar = "0.0.1" @@ -4285,6 +4286,7 @@ fn versionless_packages() { r#" [package] name = "baz" + publish = false [dependencies] foobar = "0.0.1" @@ -4295,20 +4297,247 @@ fn versionless_packages() { Package::new("foobar", "0.0.1").publish(); p.cargo("metadata -q --format-version 1") - .with_stderr( - r#"error: failed to load manifest for workspace member `[CWD]/bar` - -Caused by: - failed to parse manifest at `[CWD]/bar/Cargo.toml` - -Caused by: - TOML parse error at line 2, column 17 - | - 2 | [package] - | ^^^^^^^^^^^^^^^^^^^^^^^^^ - missing field `version` + .with_json( + r#" +{ + "packages": [ + { + "name": "bar", + "version": "0.0.0", + "id": "bar 0.0.0 [..]", + "license": null, + "license_file": null, + "description": null, + "source": null, + "dependencies": [ + { + "name": "baz", + "source": null, + "req": "*", + "kind": null, + "rename": null, + "optional": false, + "uses_default_features": true, + "features": [], + "target": null, + "registry": null, + "path": "[..]/baz" + }, + { + "name": "foobar", + "source": "registry+https://github.com/rust-lang/crates.io-index", + "req": "^0.0.1", + "kind": null, + "rename": null, + "optional": false, + "uses_default_features": true, + "features": [], + "target": null, + "registry": null + } + ], + "targets": [ + { + "kind": [ + "lib" + ], + "crate_types": [ + "lib" + ], + "name": "bar", + "src_path": "[..]/bar/src/lib.rs", + "edition": "2015", + "doc": true, + "doctest": true, + "test": true + } + ], + "features": {}, + "manifest_path": "[..]/bar/Cargo.toml", + "metadata": null, + "publish": [], + "authors": [], + "categories": [], + "keywords": [], + "readme": null, + "repository": null, + "homepage": null, + "documentation": null, + "edition": "2015", + "links": null, + "default_run": null, + "rust_version": null + }, + { + "name": "baz", + "version": "0.0.0", + "id": "baz 0.0.0 [..]", + "license": null, + "license_file": null, + "description": null, + "source": null, + "dependencies": [ + { + "name": "foobar", + "source": "registry+https://github.com/rust-lang/crates.io-index", + "req": "^0.0.1", + "kind": null, + "rename": null, + "optional": false, + "uses_default_features": true, + "features": [], + "target": null, + "registry": null + } + ], + "targets": [ + { + "kind": [ + "lib" + ], + "crate_types": [ + "lib" + ], + "name": "baz", + "src_path": "[..]/baz/src/lib.rs", + "edition": "2015", + "doc": true, + "doctest": true, + "test": true + } + ], + "features": {}, + "manifest_path": "[..]/baz/Cargo.toml", + "metadata": null, + "publish": [], + "authors": [], + "categories": [], + "keywords": [], + "readme": null, + "repository": null, + "homepage": null, + "documentation": null, + "edition": "2015", + "links": null, + "default_run": null, + "rust_version": null + }, + { + "name": "foobar", + "version": "0.0.1", + "id": "foobar 0.0.1 [..]", + "license": null, + "license_file": null, + "description": null, + "source": "registry+https://github.com/rust-lang/crates.io-index", + "dependencies": [], + "targets": [ + { + "kind": [ + "lib" + ], + "crate_types": [ + "lib" + ], + "name": "foobar", + "src_path": "[..]/foobar-0.0.1/src/lib.rs", + "edition": "2015", + "doc": true, + "doctest": true, + "test": true + } + ], + "features": {}, + "manifest_path": "[..]/foobar-0.0.1/Cargo.toml", + "metadata": null, + "publish": null, + "authors": [], + "categories": [], + "keywords": [], + "readme": null, + "repository": null, + "homepage": null, + "documentation": null, + "edition": "2015", + "links": null, + "default_run": null, + "rust_version": null + } + ], + "workspace_members": [ + "bar 0.0.0 [..]", + "baz 0.0.0 [..]" + ], + "workspace_default_members": [ + "bar 0.0.0 [..]", + "baz 0.0.0 [..]" + ], + "resolve": { + "nodes": [ + { + "id": "bar 0.0.0 [..]", + "dependencies": [ + "baz 0.0.0 [..]", + "foobar 0.0.1 [..]" + ], + "deps": [ + { + "name": "baz", + "pkg": "baz 0.0.0 [..]", + "dep_kinds": [ + { + "kind": null, + "target": null + } + ] + }, + { + "name": "foobar", + "pkg": "foobar 0.0.1 [..]", + "dep_kinds": [ + { + "kind": null, + "target": null + } + ] + } + ], + "features": [] + }, + { + "id": "baz 0.0.0 [..]", + "dependencies": [ + "foobar 0.0.1 [..]" + ], + "deps": [ + { + "name": "foobar", + "pkg": "foobar 0.0.1 [..]", + "dep_kinds": [ + { + "kind": null, + "target": null + } + ] + } + ], + "features": [] + }, + { + "id": "foobar 0.0.1 [..]", + "dependencies": [], + "deps": [], + "features": [] + } + ], + "root": null + }, + "target_directory": "[..]/foo/target", + "version": 1, + "workspace_root": "[..]", + "metadata": null +} "#, ) - .with_status(101) .run(); } diff --git a/tests/testsuite/package.rs b/tests/testsuite/package.rs index fd4f1a20a93..efaaa01ff61 100644 --- a/tests/testsuite/package.rs +++ b/tests/testsuite/package.rs @@ -3105,6 +3105,7 @@ fn versionless_package() { [package] name = "foo" description = "foo" + publish = false "#, ) .file("src/main.rs", r#"fn main() { println!("hello"); }"#) @@ -3112,16 +3113,23 @@ fn versionless_package() { p.cargo("package") .with_stderr( - r#"error: failed to parse manifest at `[CWD]/Cargo.toml` - -Caused by: - TOML parse error at line 2, column 17 - | - 2 | [package] - | ^^^^^^^^^^^^^^^^^^^^^^^^^ - missing field `version` -"#, + "\ +warning: manifest has no license, license-file, documentation, homepage or repository. +See https://doc.rust-lang.org/cargo/reference/manifest.html#package-metadata for more info. + Packaging foo v0.0.0 ([CWD]) + Verifying foo v0.0.0 ([CWD]) + Compiling foo v0.0.0 ([CWD]/target/package/foo-0.0.0) + Finished dev [unoptimized + debuginfo] target(s) in [..]s + Packaged 4 files, [..]B ([..]B compressed) +", ) - .with_status(101) .run(); + + let f = File::open(&p.root().join("target/package/foo-0.0.0.crate")).unwrap(); + validate_crate_contents( + f, + "foo-0.0.0.crate", + &["Cargo.lock", "Cargo.toml", "Cargo.toml.orig", "src/main.rs"], + &[], + ); } diff --git a/tests/testsuite/publish.rs b/tests/testsuite/publish.rs index 585a3bb6fe2..a459e4e2430 100644 --- a/tests/testsuite/publish.rs +++ b/tests/testsuite/publish.rs @@ -3007,6 +3007,9 @@ Caused by: #[cargo_test] fn versionless_package() { + // Use local registry for faster test times since no publish will occur + let registry = registry::init(); + let p = project() .file( "Cargo.toml", @@ -3020,17 +3023,15 @@ fn versionless_package() { .build(); p.cargo("publish") + .replace_crates_io(registry.index_url()) + .with_status(101) .with_stderr( - r#"error: failed to parse manifest at `[CWD]/Cargo.toml` + "\ +error: failed to parse manifest at `[CWD]/Cargo.toml` Caused by: - TOML parse error at line 2, column 17 - | - 2 | [package] - | ^^^^^^^^^^^^^^^^^^^^^^^^^ - missing field `version` -"#, + `package.version` must be specified unless `package.publish = false` +", ) - .with_status(101) .run(); } From b70f23d12a4b60ce13330d105863414979b4c8ea Mon Sep 17 00:00:00 2001 From: Ed Page Date: Fri, 6 Oct 2023 12:30:52 -0500 Subject: [PATCH 10/12] fix(embedded): Rely on new defaulted version field --- src/cargo/util/toml/embedded.rs | 6 ------ src/doc/src/reference/unstable.md | 1 - 2 files changed, 7 deletions(-) diff --git a/src/cargo/util/toml/embedded.rs b/src/cargo/util/toml/embedded.rs index 482268923fc..ca587007557 100644 --- a/src/cargo/util/toml/embedded.rs +++ b/src/cargo/util/toml/embedded.rs @@ -6,7 +6,6 @@ use crate::Config; const DEFAULT_EDITION: crate::core::features::Edition = crate::core::features::Edition::LATEST_STABLE; -const DEFAULT_VERSION: &str = "0.0.0"; const DEFAULT_PUBLISH: bool = false; const AUTO_FIELDS: &[&str] = &["autobins", "autoexamples", "autotests", "autobenches"]; @@ -123,9 +122,6 @@ fn expand_manifest_( package .entry("name".to_owned()) .or_insert(toml::Value::String(name)); - package - .entry("version".to_owned()) - .or_insert_with(|| toml::Value::String(DEFAULT_VERSION.to_owned())); package.entry("edition".to_owned()).or_insert_with(|| { let _ = config.shell().warn(format_args!( "`package.edition` is unspecified, defaulting to `{}`", @@ -622,7 +618,6 @@ build = false edition = "2021" name = "test-" publish = false -version = "0.0.0" [profile.release] strip = true @@ -652,7 +647,6 @@ build = false edition = "2021" name = "test-" publish = false -version = "0.0.0" [profile.release] strip = true diff --git a/src/doc/src/reference/unstable.md b/src/doc/src/reference/unstable.md index e2c4d7a6a6f..fb9a89f4ae5 100644 --- a/src/doc/src/reference/unstable.md +++ b/src/doc/src/reference/unstable.md @@ -1230,7 +1230,6 @@ at the start of the infostring at the top of the file. Inferred / defaulted manifest fields: - `package.name = ` -- `package.version = "0.0.0"` to [call attention to this crate being used in unexpected places](https://matklad.github.io/2021/08/22/large-rust-workspaces.html#Smaller-Tips) - `package.publish = false` to avoid accidental publishes, particularly if we later add support for including them in a workspace. - `package.edition = ` to avoid always having to add an embedded From 6e2b3436c62ba56247c6c06c66aab7e49daf37dc Mon Sep 17 00:00:00 2001 From: Ed Page Date: Fri, 6 Oct 2023 13:01:58 -0500 Subject: [PATCH 11/12] fix(toml): Default package.publish based on presence of package.version Before the default was hardcoded to `true`. The problem was that means that to remove the `package.version` boilerplate, you had to add `package.publish = false` boilerplate. To make the errors easier to understand in this situation, I err on the side of encouraging people to put `publish = true` in their manifests. By making this change, we also unblock "cargo script" / `Cargo.toml` unifying the handling of `package.publish`. --- src/cargo/ops/registry/publish.rs | 2 +- src/cargo/util/toml/mod.rs | 5 +++-- src/doc/src/reference/manifest.md | 19 +++++++++---------- tests/testsuite/check.rs | 1 - tests/testsuite/metadata.rs | 2 -- tests/testsuite/package.rs | 1 - tests/testsuite/publish.rs | 12 +++++------- 7 files changed, 18 insertions(+), 24 deletions(-) diff --git a/src/cargo/ops/registry/publish.rs b/src/cargo/ops/registry/publish.rs index a88a0a30f0a..e4bf0d00fbe 100644 --- a/src/cargo/ops/registry/publish.rs +++ b/src/cargo/ops/registry/publish.rs @@ -102,7 +102,7 @@ pub fn publish(ws: &Workspace<'_>, opts: &PublishOpts<'_>) -> CargoResult<()> { if allowed_registries.is_empty() { bail!( "`{}` cannot be published.\n\ - `package.publish` is set to `false` or an empty list in Cargo.toml and prevents publishing.", + `package.publish` is must be set to `true` or a non-empty list in Cargo.toml to publish.", pkg.name(), ); } else if !allowed_registries.contains(®_name) { diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index 2043a1b260e..14909958c5e 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -1012,11 +1012,12 @@ impl TomlManifest { let publish = match publish { Some(VecStringOrBool::VecString(ref vecstring)) => Some(vecstring.clone()), Some(VecStringOrBool::Bool(false)) => Some(vec![]), - None | Some(VecStringOrBool::Bool(true)) => None, + Some(VecStringOrBool::Bool(true)) => None, + None => version.is_none().then_some(vec![]), }; if version.is_none() && publish != Some(vec![]) { - bail!("`package.version` must be specified unless `package.publish = false`"); + bail!("`package.version` must be specified if `package.publish = true`"); } if summary.features().contains_key("default-features") { diff --git a/src/doc/src/reference/manifest.md b/src/doc/src/reference/manifest.md index 49e59ecc642..66d21d7e047 100644 --- a/src/doc/src/reference/manifest.md +++ b/src/doc/src/reference/manifest.md @@ -109,7 +109,7 @@ resolve dependencies, and for guidelines on setting your own version. See the [SemVer compatibility] chapter for more details on exactly what constitutes a breaking change. -This field is optional and defaults to `0.0.0`. +This field is optional and defaults to `0.0.0`. The field is required for publishing packages. [Resolver]: resolver.md [SemVer compatibility]: semver.md @@ -472,23 +472,22 @@ if any of those files change. ### The `publish` field -The `publish` field can be used to prevent a package from being published to a -package registry (like *crates.io*) by mistake, for instance to keep a package -private in a company. - +The `publish` field can be used to control which registries names the package +may be published to: ```toml [package] # ... -publish = false +publish = ["some-registry-name"] ``` -The value may also be an array of strings which are registry names that are -allowed to be published to. - +To prevent a package from being published to a registry (like crates.io) by mistake, +for instance to keep a package private in a company, +you can leave off the [`version`](#the-version-field) field. +If you'd like to be more explicit, you can disable publishing: ```toml [package] # ... -publish = ["some-registry-name"] +publish = false ``` If publish array contains a single registry, `cargo publish` command will use diff --git a/tests/testsuite/check.rs b/tests/testsuite/check.rs index f7c865dedba..03611ae67e7 100644 --- a/tests/testsuite/check.rs +++ b/tests/testsuite/check.rs @@ -1506,7 +1506,6 @@ fn versionless_package() { [package] name = "foo" description = "foo" - publish = false "#, ) .file("src/lib.rs", "") diff --git a/tests/testsuite/metadata.rs b/tests/testsuite/metadata.rs index 223a8f70784..888cdce8c0e 100644 --- a/tests/testsuite/metadata.rs +++ b/tests/testsuite/metadata.rs @@ -4273,7 +4273,6 @@ fn versionless_packages() { r#" [package] name = "bar" - publish = false [dependencies] foobar = "0.0.1" @@ -4286,7 +4285,6 @@ fn versionless_packages() { r#" [package] name = "baz" - publish = false [dependencies] foobar = "0.0.1" diff --git a/tests/testsuite/package.rs b/tests/testsuite/package.rs index efaaa01ff61..4ec4fc0d6fc 100644 --- a/tests/testsuite/package.rs +++ b/tests/testsuite/package.rs @@ -3105,7 +3105,6 @@ fn versionless_package() { [package] name = "foo" description = "foo" - publish = false "#, ) .file("src/main.rs", r#"fn main() { println!("hello"); }"#) diff --git a/tests/testsuite/publish.rs b/tests/testsuite/publish.rs index a459e4e2430..29691c78ce0 100644 --- a/tests/testsuite/publish.rs +++ b/tests/testsuite/publish.rs @@ -420,7 +420,7 @@ fn unpublishable_crate() { .with_stderr( "\ [ERROR] `foo` cannot be published. -`package.publish` is set to `false` or an empty list in Cargo.toml and prevents publishing. +`package.publish` is must be set to `true` or a non-empty list in Cargo.toml to publish. ", ) .run(); @@ -794,7 +794,7 @@ fn publish_empty_list() { .with_stderr( "\ [ERROR] `foo` cannot be published. -`package.publish` is set to `false` or an empty list in Cargo.toml and prevents publishing. +`package.publish` is must be set to `true` or a non-empty list in Cargo.toml to publish. ", ) .run(); @@ -1020,7 +1020,7 @@ fn block_publish_no_registry() { .with_stderr( "\ [ERROR] `foo` cannot be published. -`package.publish` is set to `false` or an empty list in Cargo.toml and prevents publishing. +`package.publish` is must be set to `true` or a non-empty list in Cargo.toml to publish. ", ) .run(); @@ -3027,10 +3027,8 @@ fn versionless_package() { .with_status(101) .with_stderr( "\ -error: failed to parse manifest at `[CWD]/Cargo.toml` - -Caused by: - `package.version` must be specified unless `package.publish = false` +error: `foo` cannot be published. +`package.publish` is must be set to `true` or a non-empty list in Cargo.toml to publish. ", ) .run(); From 0a58fce653c5580246246478c1240c4f1063e580 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Fri, 6 Oct 2023 13:54:13 -0500 Subject: [PATCH 12/12] fix(embedded): Rely on package.publish default for scripts --- src/cargo/util/toml/embedded.rs | 6 ------ src/doc/src/reference/unstable.md | 2 -- 2 files changed, 8 deletions(-) diff --git a/src/cargo/util/toml/embedded.rs b/src/cargo/util/toml/embedded.rs index ca587007557..8e3912288bf 100644 --- a/src/cargo/util/toml/embedded.rs +++ b/src/cargo/util/toml/embedded.rs @@ -6,7 +6,6 @@ use crate::Config; const DEFAULT_EDITION: crate::core::features::Edition = crate::core::features::Edition::LATEST_STABLE; -const DEFAULT_PUBLISH: bool = false; const AUTO_FIELDS: &[&str] = &["autobins", "autoexamples", "autotests", "autobenches"]; pub fn expand_manifest( @@ -132,9 +131,6 @@ fn expand_manifest_( package .entry("build".to_owned()) .or_insert_with(|| toml::Value::Boolean(false)); - package - .entry("publish".to_owned()) - .or_insert_with(|| toml::Value::Boolean(DEFAULT_PUBLISH)); for field in AUTO_FIELDS { package .entry(field.to_owned()) @@ -617,7 +613,6 @@ autotests = false build = false edition = "2021" name = "test-" -publish = false [profile.release] strip = true @@ -646,7 +641,6 @@ autotests = false build = false edition = "2021" name = "test-" -publish = false [profile.release] strip = true diff --git a/src/doc/src/reference/unstable.md b/src/doc/src/reference/unstable.md index fb9a89f4ae5..b5bbeae8402 100644 --- a/src/doc/src/reference/unstable.md +++ b/src/doc/src/reference/unstable.md @@ -1230,8 +1230,6 @@ at the start of the infostring at the top of the file. Inferred / defaulted manifest fields: - `package.name = ` -- `package.publish = false` to avoid accidental publishes, particularly if we - later add support for including them in a workspace. - `package.edition = ` to avoid always having to add an embedded manifest at the cost of potentially breaking scripts on rust upgrades - Warn when `edition` is unspecified to raise awareness of this