diff --git a/src/cargo/ops/cargo_package.rs b/src/cargo/ops/cargo_package.rs index 533852551c2..afe4d6c6008 100644 --- a/src/cargo/ops/cargo_package.rs +++ b/src/cargo/ops/cargo_package.rs @@ -310,6 +310,8 @@ fn build_ar_list( }); } + let mut invalid_manifest_field: Vec = vec![]; + let mut result = result.into_values().flatten().collect(); if let Some(license_file) = &pkg.manifest().metadata().license_file { let license_path = Path::new(license_file); @@ -324,7 +326,12 @@ fn build_ar_list( ws, )?; } else { - warn_on_nonexistent_file(&pkg, &license_path, "license-file", &ws)?; + error_on_nonexistent_file( + &pkg, + &license_path, + "license-file", + &mut invalid_manifest_field, + ); } } if let Some(readme) = &pkg.manifest().metadata().readme { @@ -333,10 +340,14 @@ fn build_ar_list( if abs_file_path.is_file() { check_for_file_and_add("readme", readme_path, abs_file_path, pkg, &mut result, ws)?; } else { - warn_on_nonexistent_file(&pkg, &readme_path, "readme", &ws)?; + error_on_nonexistent_file(&pkg, &readme_path, "readme", &mut invalid_manifest_field); } } + if !invalid_manifest_field.is_empty() { + return Err(anyhow::anyhow!(invalid_manifest_field.join("\n"))); + } + for t in pkg .manifest() .targets() @@ -406,25 +417,27 @@ fn check_for_file_and_add( Ok(()) } -fn warn_on_nonexistent_file( +fn error_on_nonexistent_file( pkg: &Package, path: &Path, manifest_key_name: &'static str, - ws: &Workspace<'_>, -) -> CargoResult<()> { + invalid: &mut Vec, +) { let rel_msg = if path.is_absolute() { "".to_string() } else { format!(" (relative to `{}`)", pkg.root().display()) }; - ws.gctx().shell().warn(&format!( + + let msg = format!( "{manifest_key_name} `{}` does not appear to exist{}.\n\ - Please update the {manifest_key_name} setting in the manifest at `{}`\n\ - This may become a hard error in the future.", + Please update the {manifest_key_name} setting in the manifest at `{}`.", path.display(), rel_msg, pkg.manifest_path().display() - )) + ); + + invalid.push(msg); } fn error_custom_build_file_not_in_package( @@ -500,11 +513,7 @@ fn check_metadata(pkg: &Package, gctx: &GlobalContext) -> CargoResult<()> { )* }} } - lacking!( - description, - license || license_file, - documentation || homepage || repository - ); + lacking!(description, documentation || homepage || repository); if !missing.is_empty() { let mut things = missing[..missing.len() - 1].join(", "); diff --git a/tests/testsuite/package.rs b/tests/testsuite/package.rs index 850fe9c50b9..2d227ef32b3 100644 --- a/tests/testsuite/package.rs +++ b/tests/testsuite/package.rs @@ -85,8 +85,7 @@ fn metadata_warning() { p.cargo("package") .with_stderr( "\ -warning: manifest has no description, license, license-file, documentation, \ -homepage or repository. +warning: manifest has no description, documentation, homepage or repository. See https://doc.rust-lang.org/cargo/reference/manifest.html#package-metadata for more info. [PACKAGING] foo v0.0.1 ([CWD]) [VERIFYING] foo v0.0.1 ([CWD]) @@ -1943,8 +1942,7 @@ fn exclude_dot_files_and_directories_by_default() { #[cargo_test] fn empty_readme_path() { - // Warn but don't fail if `readme` is empty. - // Issue #11522. + // fail if `readme` is empty. let p = project() .file( "Cargo.toml", @@ -1963,13 +1961,11 @@ fn empty_readme_path() { .build(); p.cargo("package --no-verify") + .with_status(101) .with_stderr( "\ -[WARNING] readme `` does not appear to exist (relative to `[..]/foo`). -Please update the readme setting in the manifest at `[..]/foo/Cargo.toml` -This may become a hard error in the future. -[PACKAGING] foo v1.0.0 ([..]/foo) -[PACKAGED] [..] files, [..] ([..] compressed) +[ERROR] readme `` does not appear to exist (relative to `[..]/foo`). +Please update the readme setting in the manifest at `[..]/foo/Cargo.toml`. ", ) .run(); @@ -1977,8 +1973,7 @@ This may become a hard error in the future. #[cargo_test] fn invalid_readme_path() { - // Warn but don't fail if `readme` path is invalid. - // Issue #11522. + // fail if `readme` path is invalid. let p = project() .file( "Cargo.toml", @@ -1997,13 +1992,11 @@ fn invalid_readme_path() { .build(); p.cargo("package --no-verify") + .with_status(101) .with_stderr( "\ -[WARNING] readme `DOES-NOT-EXIST` does not appear to exist (relative to `[..]/foo`). -Please update the readme setting in the manifest at `[..]/foo/Cargo.toml` -This may become a hard error in the future. -[PACKAGING] foo v1.0.0 ([..]/foo) -[PACKAGED] [..] files, [..] ([..] compressed) +[ERROR] readme `DOES-NOT-EXIST` does not appear to exist (relative to `[..]/foo`). +Please update the readme setting in the manifest at `[..]/foo/Cargo.toml`. ", ) .run(); @@ -2011,8 +2004,7 @@ This may become a hard error in the future. #[cargo_test] fn readme_or_license_file_is_dir() { - // Test warning when `readme` or `license-file` is a directory, not a file. - // Issue #11522. + // Test error when `readme` or `license-file` is a directory, not a file. let p = project() .file( "Cargo.toml", @@ -2031,16 +2023,13 @@ fn readme_or_license_file_is_dir() { .build(); p.cargo("package --no-verify") + .with_status(101) .with_stderr( "\ -[WARNING] license-file `./src` does not appear to exist (relative to `[..]/foo`). -Please update the license-file setting in the manifest at `[..]/foo/Cargo.toml` -This may become a hard error in the future. -[WARNING] readme `./src` does not appear to exist (relative to `[..]/foo`). -Please update the readme setting in the manifest at `[..]/foo/Cargo.toml` -This may become a hard error in the future. -[PACKAGING] foo v1.0.0 ([..]/foo) -[PACKAGED] [..] files, [..] ([..] compressed) +[ERROR] license-file `./src` does not appear to exist (relative to `[..]/foo`). +Please update the license-file setting in the manifest at `[..]/foo/Cargo.toml`. +readme `./src` does not appear to exist (relative to `[..]/foo`). +Please update the readme setting in the manifest at `[..]/foo/Cargo.toml`. ", ) .run(); @@ -2048,8 +2037,7 @@ This may become a hard error in the future. #[cargo_test] fn empty_license_file_path() { - // Warn but don't fail if license-file is empty. - // Issue #11522. + // fail if license-file is empty. let p = project() .file( "Cargo.toml", @@ -2067,15 +2055,11 @@ fn empty_license_file_path() { .build(); p.cargo("package --no-verify") + .with_status(101) .with_stderr( "\ -[WARNING] manifest has no license or license-file. -See https://doc.rust-lang.org/cargo/reference/manifest.html#package-metadata for more info. -[WARNING] license-file `` does not appear to exist (relative to `[..]/foo`). -Please update the license-file setting in the manifest at `[..]/foo/Cargo.toml` -This may become a hard error in the future. -[PACKAGING] foo v1.0.0 ([..]/foo) -[PACKAGED] [..] files, [..] ([..] compressed) +[ERROR] license-file `` does not appear to exist (relative to `[..]/foo`). +Please update the license-file setting in the manifest at `[..]/foo/Cargo.toml`. ", ) .run(); @@ -2101,13 +2085,11 @@ fn invalid_license_file_path() { .build(); p.cargo("package --no-verify") + .with_status(101) .with_stderr( "\ -[WARNING] license-file `does-not-exist` does not appear to exist (relative to `[..]/foo`). -Please update the license-file setting in the manifest at `[..]/foo/Cargo.toml` -This may become a hard error in the future. -[PACKAGING] foo v1.0.0 ([..]/foo) -[PACKAGED] [..] files, [..] ([..] compressed) +[ERROR] license-file `does-not-exist` does not appear to exist (relative to `[..]/foo`). +Please update the license-file setting in the manifest at `[..]/foo/Cargo.toml`. ", ) .run(); @@ -3522,7 +3504,7 @@ fn versionless_package() { p.cargo("package") .with_stderr( "\ -warning: manifest has no license, license-file, documentation, homepage or repository. +warning: manifest has no 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]) @@ -3728,7 +3710,7 @@ fn symlink_manifest_path() { .arg(foo_symlink.join("Cargo.toml")) .with_stderr( "\ -warning: manifest has no description, license, license-file, documentation, homepage or repository. +warning: manifest has no description, documentation, homepage or repository. See https://doc.rust-lang.org/cargo/reference/manifest.html#package-metadata for more info. [PACKAGING] foo v1.0.0 ([..]foo-symlink) [PACKAGED] 6 files[..] diff --git a/tests/testsuite/publish.rs b/tests/testsuite/publish.rs index 38577bbcfdd..c182b508749 100644 --- a/tests/testsuite/publish.rs +++ b/tests/testsuite/publish.rs @@ -3,7 +3,7 @@ use cargo_test_support::git::{self, repo}; use cargo_test_support::paths; use cargo_test_support::registry::{self, Package, RegistryBuilder, Response}; -use cargo_test_support::{basic_manifest, no_such_file_err_msg, project, publish}; +use cargo_test_support::{basic_manifest, project, publish}; use std::fs; use std::sync::{Arc, Mutex}; @@ -2137,54 +2137,6 @@ include `--registry dummy-registry` or `--registry crates-io` .run(); } -#[cargo_test] -fn publish_with_missing_readme() { - // Use local registry for faster test times since no publish will occur - let registry = registry::init(); - - let p = project() - .file( - "Cargo.toml", - r#" - [package] - name = "foo" - version = "0.1.0" - edition = "2015" - authors = [] - license = "MIT" - description = "foo" - homepage = "https://example.com/" - readme = "foo.md" - "#, - ) - .file("src/lib.rs", "") - .build(); - - p.cargo("publish --no-verify") - .replace_crates_io(registry.index_url()) - .with_status(101) - .with_stderr(&format!( - "\ -[UPDATING] [..] -[WARNING] readme `foo.md` does not appear to exist (relative to `[..]/foo`). -Please update the readme setting in the manifest at `[..]/foo/Cargo.toml` -This may become a hard error in the future. -[PACKAGING] foo v0.1.0 [..] -[PACKAGED] [..] files, [..] ([..] compressed) -[UPLOADING] foo v0.1.0 [..] -[ERROR] failed to read `readme` file for package `foo v0.1.0 ([ROOT]/foo)` - -Caused by: - failed to read `[ROOT]/foo/foo.md` - -Caused by: - {} -", - no_such_file_err_msg() - )) - .run(); -} - // Registry returns an API error. #[cargo_test] fn api_error_json() { diff --git a/tests/testsuite/registry.rs b/tests/testsuite/registry.rs index 24c253c5504..0475e8f6d7f 100644 --- a/tests/testsuite/registry.rs +++ b/tests/testsuite/registry.rs @@ -1146,7 +1146,7 @@ fn bad_license_file(registry: &TestRegistry) { p.cargo("publish -v") .replace_crates_io(registry.index_url()) .with_status(101) - .with_stderr_contains("[ERROR] the license file `foo` does not exist") + .with_stderr_contains("[ERROR] license-file `foo` does not appear to exist ([..]).") .run(); }