Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Warn instead of error in cargo package on empty readme or license-file in manifest #12036

Merged
merged 3 commits into from
Apr 25, 2023

Conversation

kylematsuda
Copy link
Contributor

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!

@rustbot
Copy link
Collaborator

rustbot commented Apr 25, 2023

r? @epage

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added Command-package S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 25, 2023
"\
[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.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the wording here, I just copied the existing warning when license-file doesn't exist. Not sure if it's appropriate for readme or if it needs to be tweaked. If I understand correctly, one of license or license-file is required for crates.io, but it's okay for the readme to be missing, so not sure that the "hard error" caution is accurate here

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think a hard error caution is warranted because the user told us to do something and we couldn't do it.

@epage
Copy link
Contributor

epage commented Apr 25, 2023

@bors r+

@bors
Copy link
Collaborator

bors commented Apr 25, 2023

📌 Commit 90c7d3b has been approved by epage

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 25, 2023
@bors
Copy link
Collaborator

bors commented Apr 25, 2023

⌛ Testing commit 90c7d3b with merge 7c86025...

@bors
Copy link
Collaborator

bors commented Apr 25, 2023

☀️ Test successful - checks-actions
Approved by: epage
Pushing 7c86025 to master...

@bors bors merged commit 7c86025 into rust-lang:master Apr 25, 2023
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 26, 2023
Update cargo

17 commits in de80432f04da61d98dcbbc1572598071718ccfd2..9e586fbd8b931494067144623b76c37d213b1ab6
2023-04-21 13:18:32 +0000 to 2023-04-25 22:09:11 +0000
- Update home dependency (rust-lang/cargo#12037)
- Warn instead of error in `cargo package` on empty `readme` or `license-file` in manifest (rust-lang/cargo#12036)
- Clarify documentation around test target setting. (rust-lang/cargo#12032)
- fix: apply `[env]` to target info discovery rustc (rust-lang/cargo#12029)
- CI: ensure intra links for all members are checked (rust-lang/cargo#12025)
- chore: make credential dependencies platform-specific (rust-lang/cargo#12027)
- CI: use `-p` to specify workspace members instead of `--manifest-path` (rust-lang/cargo#12024)
- ci: requires `test_gitoxide` and `lockfile` for both bors success and failure (rust-lang/cargo#12026)
- Update windows-sys (rust-lang/cargo#12021)
- Bump libc to 0.2.142 (rust-lang/cargo#12014)
- Update openssl-src to 111.25.3+1.1.1t (rust-lang/cargo#12005)
- Improve error message for empty dep (rust-lang/cargo#12001)
- Remove wrong url in benchsuite manifest. (rust-lang/cargo#12020)
- Bump versions of local crates (rust-lang/cargo#12019)
- Add the Win32_System_Console feature since it is used (rust-lang/cargo#12016)
- Update outdated crates.io URLs in publishing guide (rust-lang/cargo#12018)
- Allow named debuginfo options in Cargo.toml (rust-lang/cargo#11958)

r? `@ghost`
@ehuss ehuss added this to the 1.71.0 milestone May 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Command-package S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Impossible to publish: failed to prepare local package for uploading
5 participants