Skip to content

Commit ab85225

Browse files
borsbors
committed
Auto merge of #13921 - heisen-li:licence_readme_warning, r=weihanglo
fix(toml): Convert warnings that `licence` and `readme` files do not exist into errors ### What does this PR try to resolve? In this PR: - Changed the warning to a hard error and modified the associated test function; - Removed what should have been a redundant test function：`publish::publish_with_missing_readme`； - Since `cargo publish` is preceded by the execution of `cargo package`, the error message in the test `function bad_license_file` needs to be modified. issue: #13629 (comment). ### Additional information It seems that this is not enough, the current situation is that `cargo package` warns if `package.readme` is an empty string or the wrong file location, but if I cancel `package.readme`, no warning is generated. I'm wondering if I should judge `package.readme&licence` when executing `cargo package` and return an error if it doesn't exist? As this has not been done before, your advice is sought.
2 parents b1feb75 + 193319c commit ab85225

File tree

4 files changed

+45
-95
lines changed

4 files changed

+45
-95
lines changed

‎src/cargo/ops/cargo_package.rs

+22-9
Original file line numberDiff line numberDiff line change
@@ -311,6 +311,8 @@ fn build_ar_list(
311311
});
312312
}
313313

314+
let mut invalid_manifest_field: Vec<String> = vec![];
315+
314316
let mut result = result.into_values().flatten().collect();
315317
if let Some(license_file) = &pkg.manifest().metadata().license_file {
316318
let license_path = Path::new(license_file);
@@ -325,7 +327,12 @@ fn build_ar_list(
325327
ws,
326328
)?;
327329
} else {
328-
warn_on_nonexistent_file(&pkg, &license_path, "license-file", &ws)?;
330+
error_on_nonexistent_file(
331+
&pkg,
332+
&license_path,
333+
"license-file",
334+
&mut invalid_manifest_field,
335+
);
329336
}
330337
}
331338
if let Some(readme) = &pkg.manifest().metadata().readme {
@@ -334,10 +341,14 @@ fn build_ar_list(
334341
if abs_file_path.is_file() {
335342
check_for_file_and_add("readme", readme_path, abs_file_path, pkg, &mut result, ws)?;
336343
} else {
337-
warn_on_nonexistent_file(&pkg, &readme_path, "readme", &ws)?;
344+
error_on_nonexistent_file(&pkg, &readme_path, "readme", &mut invalid_manifest_field);
338345
}
339346
}
340347

348+
if !invalid_manifest_field.is_empty() {
349+
return Err(anyhow::anyhow!(invalid_manifest_field.join("\n")));
350+
}
351+
341352
for t in pkg
342353
.manifest()
343354
.targets()
@@ -407,25 +418,27 @@ fn check_for_file_and_add(
407418
Ok(())
408419
}
409420

410-
fn warn_on_nonexistent_file(
421+
fn error_on_nonexistent_file(
411422
pkg: &Package,
412423
path: &Path,
413424
manifest_key_name: &'static str,
414-
ws: &Workspace<'_>,
415-
) -> CargoResult<()> {
425+
invalid: &mut Vec<String>,
426+
) {
416427
let rel_msg = if path.is_absolute() {
417428
"".to_string()
418429
} else {
419430
format!(" (relative to `{}`)", pkg.root().display())
420431
};
421-
ws.gctx().shell().warn(&format!(
432+
433+
let msg = format!(
422434
"{manifest_key_name} `{}` does not appear to exist{}.\n\
423-
Please update the {manifest_key_name} setting in the manifest at `{}`\n\
424-
This may become a hard error in the future.",
435+
Please update the {manifest_key_name} setting in the manifest at `{}`.",
425436
path.display(),
426437
rel_msg,
427438
pkg.manifest_path().display()
428-
))
439+
);
440+
441+
invalid.push(msg);
429442
}
430443

431444
fn error_custom_build_file_not_in_package(

‎tests/testsuite/package.rs

+21-36
Original file line numberDiff line numberDiff line change
@@ -1943,8 +1943,7 @@ fn exclude_dot_files_and_directories_by_default() {
19431943

19441944
#[cargo_test]
19451945
fn empty_readme_path() {
1946-
// Warn but don't fail if `readme` is empty.
1947-
// Issue #11522.
1946+
// fail if `readme` is empty.
19481947
let p = project()
19491948
.file(
19501949
"Cargo.toml",
@@ -1963,22 +1962,19 @@ fn empty_readme_path() {
19631962
.build();
19641963

19651964
p.cargo("package --no-verify")
1965+
.with_status(101)
19661966
.with_stderr(
19671967
"\
1968-
[WARNING] readme `` does not appear to exist (relative to `[..]/foo`).
1969-
Please update the readme setting in the manifest at `[..]/foo/Cargo.toml`
1970-
This may become a hard error in the future.
1971-
[PACKAGING] foo v1.0.0 ([..]/foo)
1972-
[PACKAGED] [..] files, [..] ([..] compressed)
1968+
[ERROR] readme `` does not appear to exist (relative to `[..]/foo`).
1969+
Please update the readme setting in the manifest at `[..]/foo/Cargo.toml`.
19731970
",
19741971
)
19751972
.run();
19761973
}
19771974

19781975
#[cargo_test]
19791976
fn invalid_readme_path() {
1980-
// Warn but don't fail if `readme` path is invalid.
1981-
// Issue #11522.
1977+
// fail if `readme` path is invalid.
19821978
let p = project()
19831979
.file(
19841980
"Cargo.toml",
@@ -1997,22 +1993,19 @@ fn invalid_readme_path() {
19971993
.build();
19981994

19991995
p.cargo("package --no-verify")
1996+
.with_status(101)
20001997
.with_stderr(
20011998
"\
2002-
[WARNING] readme `DOES-NOT-EXIST` does not appear to exist (relative to `[..]/foo`).
2003-
Please update the readme setting in the manifest at `[..]/foo/Cargo.toml`
2004-
This may become a hard error in the future.
2005-
[PACKAGING] foo v1.0.0 ([..]/foo)
2006-
[PACKAGED] [..] files, [..] ([..] compressed)
1999+
[ERROR] readme `DOES-NOT-EXIST` does not appear to exist (relative to `[..]/foo`).
2000+
Please update the readme setting in the manifest at `[..]/foo/Cargo.toml`.
20072001
",
20082002
)
20092003
.run();
20102004
}
20112005

20122006
#[cargo_test]
20132007
fn readme_or_license_file_is_dir() {
2014-
// Test warning when `readme` or `license-file` is a directory, not a file.
2015-
// Issue #11522.
2008+
// Test error when `readme` or `license-file` is a directory, not a file.
20162009
let p = project()
20172010
.file(
20182011
"Cargo.toml",
@@ -2031,25 +2024,21 @@ fn readme_or_license_file_is_dir() {
20312024
.build();
20322025

20332026
p.cargo("package --no-verify")
2027+
.with_status(101)
20342028
.with_stderr(
20352029
"\
2036-
[WARNING] license-file `./src` does not appear to exist (relative to `[..]/foo`).
2037-
Please update the license-file setting in the manifest at `[..]/foo/Cargo.toml`
2038-
This may become a hard error in the future.
2039-
[WARNING] readme `./src` does not appear to exist (relative to `[..]/foo`).
2040-
Please update the readme setting in the manifest at `[..]/foo/Cargo.toml`
2041-
This may become a hard error in the future.
2042-
[PACKAGING] foo v1.0.0 ([..]/foo)
2043-
[PACKAGED] [..] files, [..] ([..] compressed)
2030+
[ERROR] license-file `./src` does not appear to exist (relative to `[..]/foo`).
2031+
Please update the license-file setting in the manifest at `[..]/foo/Cargo.toml`.
2032+
readme `./src` does not appear to exist (relative to `[..]/foo`).
2033+
Please update the readme setting in the manifest at `[..]/foo/Cargo.toml`.
20442034
",
20452035
)
20462036
.run();
20472037
}
20482038

20492039
#[cargo_test]
20502040
fn empty_license_file_path() {
2051-
// Warn but don't fail if license-file is empty.
2052-
// Issue #11522.
2041+
// fail if license-file is empty.
20532042
let p = project()
20542043
.file(
20552044
"Cargo.toml",
@@ -2067,15 +2056,13 @@ fn empty_license_file_path() {
20672056
.build();
20682057

20692058
p.cargo("package --no-verify")
2059+
.with_status(101)
20702060
.with_stderr(
20712061
"\
20722062
[WARNING] manifest has no license or license-file.
20732063
See https://doc.rust-lang.org/cargo/reference/manifest.html#package-metadata for more info.
2074-
[WARNING] license-file `` does not appear to exist (relative to `[..]/foo`).
2075-
Please update the license-file setting in the manifest at `[..]/foo/Cargo.toml`
2076-
This may become a hard error in the future.
2077-
[PACKAGING] foo v1.0.0 ([..]/foo)
2078-
[PACKAGED] [..] files, [..] ([..] compressed)
2064+
[ERROR] license-file `` does not appear to exist (relative to `[..]/foo`).
2065+
Please update the license-file setting in the manifest at `[..]/foo/Cargo.toml`.
20792066
",
20802067
)
20812068
.run();
@@ -2101,13 +2088,11 @@ fn invalid_license_file_path() {
21012088
.build();
21022089

21032090
p.cargo("package --no-verify")
2091+
.with_status(101)
21042092
.with_stderr(
21052093
"\
2106-
[WARNING] license-file `does-not-exist` does not appear to exist (relative to `[..]/foo`).
2107-
Please update the license-file setting in the manifest at `[..]/foo/Cargo.toml`
2108-
This may become a hard error in the future.
2109-
[PACKAGING] foo v1.0.0 ([..]/foo)
2110-
[PACKAGED] [..] files, [..] ([..] compressed)
2094+
[ERROR] license-file `does-not-exist` does not appear to exist (relative to `[..]/foo`).
2095+
Please update the license-file setting in the manifest at `[..]/foo/Cargo.toml`.
21112096
",
21122097
)
21132098
.run();

‎tests/testsuite/publish.rs

+1-49
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
use cargo_test_support::git::{self, repo};
44
use cargo_test_support::paths;
55
use cargo_test_support::registry::{self, Package, RegistryBuilder, Response};
6-
use cargo_test_support::{basic_manifest, no_such_file_err_msg, project, publish};
6+
use cargo_test_support::{basic_manifest, project, publish};
77
use std::fs;
88
use std::sync::{Arc, Mutex};
99

@@ -2137,54 +2137,6 @@ include `--registry dummy-registry` or `--registry crates-io`
21372137
.run();
21382138
}
21392139

2140-
#[cargo_test]
2141-
fn publish_with_missing_readme() {
2142-
// Use local registry for faster test times since no publish will occur
2143-
let registry = registry::init();
2144-
2145-
let p = project()
2146-
.file(
2147-
"Cargo.toml",
2148-
r#"
2149-
[package]
2150-
name = "foo"
2151-
version = "0.1.0"
2152-
edition = "2015"
2153-
authors = []
2154-
license = "MIT"
2155-
description = "foo"
2156-
homepage = "https://example.com/"
2157-
readme = "foo.md"
2158-
"#,
2159-
)
2160-
.file("src/lib.rs", "")
2161-
.build();
2162-
2163-
p.cargo("publish --no-verify")
2164-
.replace_crates_io(registry.index_url())
2165-
.with_status(101)
2166-
.with_stderr(&format!(
2167-
"\
2168-
[UPDATING] [..]
2169-
[WARNING] readme `foo.md` does not appear to exist (relative to `[..]/foo`).
2170-
Please update the readme setting in the manifest at `[..]/foo/Cargo.toml`
2171-
This may become a hard error in the future.
2172-
[PACKAGING] foo v0.1.0 [..]
2173-
[PACKAGED] [..] files, [..] ([..] compressed)
2174-
[UPLOADING] foo v0.1.0 [..]
2175-
[ERROR] failed to read `readme` file for package `foo v0.1.0 ([ROOT]/foo)`
2176-
2177-
Caused by:
2178-
failed to read `[ROOT]/foo/foo.md`
2179-
2180-
Caused by:
2181-
{}
2182-
",
2183-
no_such_file_err_msg()
2184-
))
2185-
.run();
2186-
}
2187-
21882140
// Registry returns an API error.
21892141
#[cargo_test]
21902142
fn api_error_json() {

‎tests/testsuite/registry.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1146,7 +1146,7 @@ fn bad_license_file(registry: &TestRegistry) {
11461146
p.cargo("publish -v")
11471147
.replace_crates_io(registry.index_url())
11481148
.with_status(101)
1149-
.with_stderr_contains("[ERROR] the license file `foo` does not exist")
1149+
.with_stderr_contains("[ERROR] license-file `foo` does not appear to exist ([..]).")
11501150
.run();
11511151
}
11521152

0 commit comments

Comments
 (0)