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

doc: clarify implications of cargo-yank #11862

Merged
merged 6 commits into from
May 3, 2023
Merged

Conversation

weihanglo
Copy link
Member

A continuation of #11071. Below are copied from there.

r? @Muscraft if you have time to review

What does this PR try to resolve?

I found the documentation for cargo yank was not especially clear on the implications of yanking a crate, and I have seen this causing confusion within the community - tafia/quick-xml#475.

On a somewhat related note, I have been observing lots more crates getting yanked recently and this is resulting in a fair amount of dependency upgrade busywork. I think/hope part of this is a documentation issue.

@rustbot rustbot added A-cli-help Area: built-in command-line help A-documenting-cargo-itself Area: Cargo's documentation S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 16, 2023
Comment on lines 54 to 55
license/copyright issues, accidental inclusion of
[PII](https://en.wikipedia.org/wiki/Personal_data), credentials, etc...
Copy link
Contributor

Choose a reason for hiding this comment

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

I might caution against this recommendation. Copyright, PII, and credential leakage should email [email protected] (or whichever registry you published to) to permanently delete the affected versions.

Other examples of when to yank might be:

  • An accidental publish.
  • An unintentional semver breakage.
  • Egregious breakage (marginally unusable).

(I imagine it would be good to brainstorm other examples to recommend.)

Copy link
Member Author

Choose a reason for hiding this comment

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

Nice catch. Updated.

Copyright, PII, and credential leakage should email [email protected] (or whichever registry you published to) to permanently delete the affected versions.

Do you think it should also mention this? I feel it is a bit much.

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 it would be good to mention. I think it helps provide more information on what actions someone should take in those situations (instead of leaving them to guess if they should use yank or not). Another thought is to also maybe point them at https://crates.io/policies?

I'd also maybe consider removing "credential leakage", and instead say that if you have leaked credentials, the recommended process is to revoke those credentials. Once it is published, it is too late to assume they haven't been copied. I'm not even sure if crates.io will remove a crate in that circumstance (their removal criteria is fairly limited).

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated. Let me know if it is too much :)


This command requires you to be authenticated with either the `--token` option
or using {{man "cargo-login" 1}}.

If the crate name is not specified, it will use the package name from the
current directory.

### How yank works

For example, the `foo` crate published version `0.22.0` and another crate `bar`
Copy link
Contributor

Choose a reason for hiding this comment

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

Just a minor note. I'm not sure if you picked 0.x versions for these examples on purpose, but I generally try to avoid them since people can be confused or can normalize their behavior. (It's not too important, just something I want to caution about.)

Copy link
Member Author

Choose a reason for hiding this comment

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

Agree with you on that. Changed them to 1.5.x (just a random pick).

@weihanglo
Copy link
Member Author

@bors try

@bors
Copy link
Contributor

bors commented Mar 23, 2023

⌛ Trying commit 5e55cfc with merge 602f1a5...

bors added a commit that referenced this pull request Mar 23, 2023
doc: clarify implications of `cargo-yank`

### What does this PR try to resolve?

I found the documentation for `cargo yank` was not especially clear on the implications of yanking a crate, and I have seen this causing confusion within the community - tafia/quick-xml#475.

On a somewhat related note, I have been observing lots more crates getting yanked recently and this is resulting in a fair amount of dependency upgrade busywork. I think/hope part of this is a documentation issue.
@weihanglo
Copy link
Member Author

I think this is ready for another round of review :)

@ehuss
Copy link
Contributor

ehuss commented May 1, 2023

@Muscraft Just checking if you'll have a chance to review this. It looks good to me, but I think it is good to have new perspectives on the documentation.

Copy link
Member

@Muscraft Muscraft left a comment

Choose a reason for hiding this comment

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

Looks good to me, Thanks!

@Muscraft
Copy link
Member

Muscraft commented May 3, 2023

@bors r

@bors
Copy link
Contributor

bors commented May 3, 2023

📌 Commit 2aea4d0 has been approved by Muscraft

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 May 3, 2023
@bors
Copy link
Contributor

bors commented May 3, 2023

⌛ Testing commit 2aea4d0 with merge 9984c88...

@bors
Copy link
Contributor

bors commented May 3, 2023

☀️ Test successful - checks-actions
Approved by: Muscraft
Pushing 9984c88 to master...

@bors bors merged commit 9984c88 into rust-lang:master May 3, 2023
bors added a commit to rust-lang-ci/rust that referenced this pull request May 5, 2023
Update cargo

10 commits in ac84010322a31f4a581dafe26258aa4ac8dea9cd..569b648b5831ae8a515e90c80843a5287c3304ef
2023-05-02 13:41:16  0000 to 2023-05-05 15:49:44  0000
- xtask-unpublished: output a markdown table (rust-lang/cargo#12085)
- fix: hack around `libsysroot` instead of `libtest` (rust-lang/cargo#12088)
- Optimize usage under rustup. (rust-lang/cargo#11917)
- Update lock to normalize `home` dep (rust-lang/cargo#12084)
- fix:  doc-test failures (rust-lang/cargo#12055)
- feat(cargo-metadata): add `workspace_default_members` (rust-lang/cargo#11978)
- doc: clarify implications of `cargo-yank` (rust-lang/cargo#11862)
- chore: Use `[workspace.dependencies]` (rust-lang/cargo#12057)
- support for shallow clones and fetches with `gitoxide` (rust-lang/cargo#11840)
- Build by PackageIdSpec, not name, to avoid ambiguity (rust-lang/cargo#12015)

r? `@ghost`
@ehuss ehuss added this to the 1.71.0 milestone May 5, 2023
@weihanglo weihanglo deleted the doc-yank branch May 10, 2023 20:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-cli-help Area: built-in command-line help A-documenting-cargo-itself Area: Cargo's documentation 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.

6 participants