Skip to content

Commit 7c86025

Browse files
borsbors
committed
Auto merge of #12036 - kylematsuda:empty-readme-license-file, r=epage
Warn instead of error in `cargo package` on empty `readme` or `license-file` in manifest ### What does this PR try to resolve? Fixes #11522. The cause of the problem is that `cargo package` and `cargo publish` fail if `readme = ""` or `license-file = ""` in the manifest. This PR changes the `build_ar_list` function to check that the `readme` and `license-file` paths point to a file (instead of checking if the paths simply exist), and emits a warning if not. This should also catch the related failure case when either value is a valid path to a directory (e.g., `readme = "./src"`). Changes to warnings: - `license-file`: extends the existing warning when `license-file` points to a non-existent path to also cover the case where it points to a directory - `readme`: new warning when `readme` is not a path to a file ### How should we test and review this PR? Please see the new integration tests in `testsuite/package.rs`. Thanks!
2 parents be33892 + 90c7d3b commit 7c86025

File tree

3 files changed

+165
-15
lines changed

3 files changed

+165
-15
lines changed

‎src/cargo/ops/cargo_package.rs

Lines changed: 26 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -281,7 +281,7 @@ fn build_ar_list(
281281
if let Some(license_file) = &pkg.manifest().metadata().license_file {
282282
let license_path = Path::new(license_file);
283283
let abs_file_path = paths::normalize_path(&pkg.root().join(license_path));
284-
if abs_file_path.exists() {
284+
if abs_file_path.is_file() {
285285
check_for_file_and_add(
286286
"license-file",
287287
license_path,
@@ -291,26 +291,16 @@ fn build_ar_list(
291291
ws,
292292
)?;
293293
} else {
294-
let rel_msg = if license_path.is_absolute() {
295-
"".to_string()
296-
} else {
297-
format!(" (relative to `{}`)", pkg.root().display())
298-
};
299-
ws.config().shell().warn(&format!(
300-
"license-file `{}` does not appear to exist{}.\n\
301-
Please update the license-file setting in the manifest at `{}`\n\
302-
This may become a hard error in the future.",
303-
license_path.display(),
304-
rel_msg,
305-
pkg.manifest_path().display()
306-
))?;
294+
warn_on_nonexistent_file(&pkg, &license_path, "license-file", &ws)?;
307295
}
308296
}
309297
if let Some(readme) = &pkg.manifest().metadata().readme {
310298
let readme_path = Path::new(readme);
311299
let abs_file_path = paths::normalize_path(&pkg.root().join(readme_path));
312-
if abs_file_path.exists() {
300+
if abs_file_path.is_file() {
313301
check_for_file_and_add("readme", readme_path, abs_file_path, pkg, &mut result, ws)?;
302+
} else {
303+
warn_on_nonexistent_file(&pkg, &readme_path, "readme", &ws)?;
314304
}
315305
}
316306
result.sort_unstable_by(|a, b| a.rel_path.cmp(&b.rel_path));
@@ -369,6 +359,27 @@ fn check_for_file_and_add(
369359
Ok(())
370360
}
371361

362+
fn warn_on_nonexistent_file(
363+
pkg: &Package,
364+
path: &Path,
365+
manifest_key_name: &'static str,
366+
ws: &Workspace<'_>,
367+
) -> CargoResult<()> {
368+
let rel_msg = if path.is_absolute() {
369+
"".to_string()
370+
} else {
371+
format!(" (relative to `{}`)", pkg.root().display())
372+
};
373+
ws.config().shell().warn(&format!(
374+
"{manifest_key_name} `{}` does not appear to exist{}.\n\
375+
Please update the {manifest_key_name} setting in the manifest at `{}`\n\
376+
This may become a hard error in the future.",
377+
path.display(),
378+
rel_msg,
379+
pkg.manifest_path().display()
380+
))
381+
}
382+
372383
/// Construct `Cargo.lock` for the package to be published.
373384
fn build_lock(ws: &Workspace<'_>, orig_pkg: &Package) -> CargoResult<String> {
374385
let config = ws.config();

‎tests/testsuite/package.rs

Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1774,6 +1774,142 @@ fn exclude_dot_files_and_directories_by_default() {
17741774
);
17751775
}
17761776

1777+
#[cargo_test]
1778+
fn empty_readme_path() {
1779+
// Warn but don't fail if `readme` is empty.
1780+
// Issue #11522.
1781+
let p = project()
1782+
.file(
1783+
"Cargo.toml",
1784+
r#"
1785+
[package]
1786+
name = "foo"
1787+
version = "1.0.0"
1788+
readme = ""
1789+
license = "MIT"
1790+
description = "foo"
1791+
homepage = "foo"
1792+
"#,
1793+
)
1794+
.file("src/lib.rs", "")
1795+
.build();
1796+
1797+
p.cargo("package --no-verify")
1798+
.with_stderr(
1799+
"\
1800+
[WARNING] readme `` does not appear to exist (relative to `[..]/foo`).
1801+
Please update the readme setting in the manifest at `[..]/foo/Cargo.toml`
1802+
This may become a hard error in the future.
1803+
[PACKAGING] foo v1.0.0 ([..]/foo)
1804+
[PACKAGED] [..] files, [..] ([..] compressed)
1805+
",
1806+
)
1807+
.run();
1808+
}
1809+
1810+
#[cargo_test]
1811+
fn invalid_readme_path() {
1812+
// Warn but don't fail if `readme` path is invalid.
1813+
// Issue #11522.
1814+
let p = project()
1815+
.file(
1816+
"Cargo.toml",
1817+
r#"
1818+
[package]
1819+
name = "foo"
1820+
version = "1.0.0"
1821+
readme = "DOES-NOT-EXIST"
1822+
license = "MIT"
1823+
description = "foo"
1824+
homepage = "foo"
1825+
"#,
1826+
)
1827+
.file("src/lib.rs", "")
1828+
.build();
1829+
1830+
p.cargo("package --no-verify")
1831+
.with_stderr(
1832+
"\
1833+
[WARNING] readme `DOES-NOT-EXIST` does not appear to exist (relative to `[..]/foo`).
1834+
Please update the readme setting in the manifest at `[..]/foo/Cargo.toml`
1835+
This may become a hard error in the future.
1836+
[PACKAGING] foo v1.0.0 ([..]/foo)
1837+
[PACKAGED] [..] files, [..] ([..] compressed)
1838+
",
1839+
)
1840+
.run();
1841+
}
1842+
1843+
#[cargo_test]
1844+
fn readme_or_license_file_is_dir() {
1845+
// Test warning when `readme` or `license-file` is a directory, not a file.
1846+
// Issue #11522.
1847+
let p = project()
1848+
.file(
1849+
"Cargo.toml",
1850+
r#"
1851+
[package]
1852+
name = "foo"
1853+
version = "1.0.0"
1854+
readme = "./src"
1855+
license-file = "./src"
1856+
description = "foo"
1857+
homepage = "foo"
1858+
"#,
1859+
)
1860+
.file("src/lib.rs", "")
1861+
.build();
1862+
1863+
p.cargo("package --no-verify")
1864+
.with_stderr(
1865+
"\
1866+
[WARNING] license-file `./src` does not appear to exist (relative to `[..]/foo`).
1867+
Please update the license-file setting in the manifest at `[..]/foo/Cargo.toml`
1868+
This may become a hard error in the future.
1869+
[WARNING] readme `./src` does not appear to exist (relative to `[..]/foo`).
1870+
Please update the readme setting in the manifest at `[..]/foo/Cargo.toml`
1871+
This may become a hard error in the future.
1872+
[PACKAGING] foo v1.0.0 ([..]/foo)
1873+
[PACKAGED] [..] files, [..] ([..] compressed)
1874+
",
1875+
)
1876+
.run();
1877+
}
1878+
1879+
#[cargo_test]
1880+
fn empty_license_file_path() {
1881+
// Warn but don't fail if license-file is empty.
1882+
// Issue #11522.
1883+
let p = project()
1884+
.file(
1885+
"Cargo.toml",
1886+
r#"
1887+
[package]
1888+
name = "foo"
1889+
version = "1.0.0"
1890+
license-file = ""
1891+
description = "foo"
1892+
homepage = "foo"
1893+
"#,
1894+
)
1895+
.file("src/lib.rs", "")
1896+
.build();
1897+
1898+
p.cargo("package --no-verify")
1899+
.with_stderr(
1900+
"\
1901+
[WARNING] manifest has no license or license-file.
1902+
See https://doc.rust-lang.org/cargo/reference/manifest.html#package-metadata for more info.
1903+
[WARNING] license-file `` does not appear to exist (relative to `[..]/foo`).
1904+
Please update the license-file setting in the manifest at `[..]/foo/Cargo.toml`
1905+
This may become a hard error in the future.
1906+
[PACKAGING] foo v1.0.0 ([..]/foo)
1907+
[PACKAGED] [..] files, [..] ([..] compressed)
1908+
",
1909+
)
1910+
.run();
1911+
}
1912+
17771913
#[cargo_test]
17781914
fn invalid_license_file_path() {
17791915
// Test warning when license-file points to a non-existent file.

‎tests/testsuite/publish.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1756,6 +1756,9 @@ fn publish_with_missing_readme() {
17561756
.with_stderr(&format!(
17571757
"\
17581758
[UPDATING] [..]
1759+
[WARNING] readme `foo.md` does not appear to exist (relative to `[..]/foo`).
1760+
Please update the readme setting in the manifest at `[..]/foo/Cargo.toml`
1761+
This may become a hard error in the future.
17591762
[PACKAGING] foo v0.1.0 [..]
17601763
[PACKAGED] [..] files, [..] ([..] compressed)
17611764
[UPLOADING] foo v0.1.0 [..]

0 commit comments

Comments
 (0)