Skip to content

Commit

Permalink
fix(toml): Convert warnings that license and readme files do not exis…
Browse files Browse the repository at this point in the history 
…t into errors
  • Loading branch information
@heisen-li
heisen-li committed May 16, 2024
1 parent 2f17770 commit 5c35e29
Show file tree
Hide file tree
Showing 4 changed files with 49 additions and 106 deletions.
37 changes: 23 additions & 14 deletions src/cargo/ops/cargo_package.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,8 @@ fn build_ar_list(
});
}

let mut invalid_manifest_field: Vec<String> = 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);
Expand All @@ -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 {
Expand All @@ -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()
Expand Down Expand Up @@ -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<String>,
) {
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(
Expand Down Expand Up @@ -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(", ");
Expand Down
66 changes: 24 additions & 42 deletions tests/testsuite/package.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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])
Expand Down Expand Up @@ -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",
Expand All @@ -1963,22 +1961,19 @@ 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();
}

#[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",
Expand All @@ -1997,22 +1992,19 @@ 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();
}

#[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",
Expand All @@ -2031,25 +2023,21 @@ 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();
}

#[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",
Expand All @@ -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();
Expand All @@ -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();
Expand Down Expand Up @@ -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])
Expand Down Expand Up @@ -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[..]
Expand Down
50 changes: 1 addition & 49 deletions tests/testsuite/publish.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand Down Expand Up @@ -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() {
Expand Down
2 changes: 1 addition & 1 deletion tests/testsuite/registry.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}

Expand Down

0 comments on commit 5c35e29

Please sign in to comment.