Clarify Implications of Cargo Yank by tustvold · Pull Request #11071 · rust-lang/cargo

tustvold · 2022-09-10T18:13:16Z

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.

rust-highfive · 2022-09-10T18:13:18Z

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @ehuss (or someone else) soon.

Please see the contribution instructions for more information.

Mingun · 2022-09-10T18:38:02Z

I think it would be useful to add an example here

adamreichold · 2022-09-10T18:46:00Z

If the implications are discussed, I think mentioning the usual course of action (publishing a semver-compatible release before yanking) would be nice as well.

weihanglo

The first sentence is too long for me to get it at the first glimpse. Is it possible to make it simpler?

Besides, I believe we should follow the step here to generate docs.

src/doc/man/cargo-yank.md

alamb · 2022-09-11T11:25:50Z

src/doc/man/cargo-yank.md

+However, yanking a release will prevent cargo from selecting that version
+when determining the version of a dependency to use. If there are no longer
+any compatible versions that haven't been yanked, cargo will return an error.
+
+The only exception to this is crates locked to a specific version by a lockfile,
+these will still be able to download the yanked version to use it.


Suggested change

However, yanking a release will prevent cargo from selecting that version

when determining the version of a dependency to use. If there are no longer

any compatible versions that haven't been yanked, cargo will return an error.

The only exception to this is crates locked to a specific version by a lockfile,

these will still be able to download the yanked version to use it.

Best practice is to yank a release *only* when it contains a serious flaw, such as

a security vulnerability, and when there is a newer semver compatible release

published. You do not need to yank a version to suggest users of your crate upgrade.

Cargo will not use a yanked version for any new project or checkout without a

pre existing lockfile, and will generate an error if there are no longer

any compatible versions for your crate.

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

declared a dependency on version `foo = 0.22`. Now `foo` releases a new, but

not semver compatibile, version `0.23.0`, and finds a critical issue with `0.22.0`.

If `0.22.0` is yanked, no new project or checkout without an existing lockfile will be

able to use crate `bar` as it relies on `0.22`

In this case, the maintainers of `foo` should first publish a semver compatible version

such as `0.22.1` prior to yanking `0.22.0` so that `bar` and all projects that depend

on `bar` will continue to work

Cargo will not use a yanked version for any new project

I think, that the concept of "new" requires clarification. When project is considered as "new"?

Practically speaking, I think "new" means any rust project that does not already have a Cargo.lock file.

I believe the basic guidance is that "libraries" (e.g. stuff published to crates.io that other applications can depend on) should not have a Cargo.lock: https://doc.rust-lang.org/cargo/faq.html#why-do-binaries-have-cargolock-in-version-control-but-not-libraries

such as a security vulnerability

I think accidentally breaking semver compatibility might deserve mention there as well it can wreak havoc in downstream projects.

Best practice is to yank a release only when it contains a serious flaw, such as
a security vulnerability, and when there is a newer semver compatible release
published.

Even for security flaws: publish a new patch version and if you're really concerned, file a rustsec warning. No yanking required. The downstream user can then decide if they are even affected (e.g. for flaws that only affect certain platforms) and -- depending on how critical the flaw is -- update in a cadence that matches THEIR workflow. Remember: yanking a crate is a choice that the downstream user CANNOT make.

A good example that justifies yanking are license/copyright issues (e.g. you accidentally included code that cannot be used under the crate's license; or you've included personally identifiable information in some tests). Apart from that, I cannot think of a good reason to yank a crate. There's semver and patch releases provide a clear path forward and are THE tool the use in nearly every case.

Probably that explanation also should appear when you try to click "Yank" button on crates.io or run cargo yank

Updated with a slightly tweaked version of this, PTAL

src/doc/man/cargo-yank.md

weihanglo

Thank you for posting this. Looks nice to me so far!

src/doc/man/cargo-yank.md

ehuss · 2022-09-26T21:58:59Z

r? @weihanglo

weihanglo · 2022-09-27T15:52:27Z

src/doc/man/cargo-yank.md

+Crates should only be yanked in exceptional circumstances, for example, license/copyright issues, accidental
+inclusion of [PII](https://en.wikipedia.org/wiki/Personal_data), credentials, etc... In the case of security
+vulnerabilities, [RustSec](https://rustsec.org/) is typically a less disruptive mechanism to inform users
+and encourage them to upgrade, and avoids the possibility of significant downstream disruption irrespective
+of susceptibility to the vulnerability in question.


How do you feel putting them in the end of this DESCRIPTION section, under a subsection heading like ### When to yank or something better?

Just trying to make it look less lengthy and more organized. The content itself is really well-written! Thank you!

weihanglo · 2022-09-27T16:15:22Z

src/doc/man/cargo-yank.md

+In this case, the maintainers of `foo` should first publish a semver compatible version
+such as `0.22.1` prior to yanking `0.22.0` so that `bar` and all projects that depend
+on `bar` will continue to work.


This paragraph concludes the example really well, thought it feels like somewhat a duplicate of line 49-50. A challenge in documentation is putting the right amount. Could we find a way to merge them and make it more concise? It is not a blocker on merging this PR though.

weihanglo · 2023-02-09T11:31:53Z

Friendly ping @tustvold. Just checking in to see if you are still interested in working on this, or if you had any questions. Thank you :)

weihanglo · 2023-03-16T15:02:06Z

This PR is indeed valuable, and I want it to move forward!!
I posted a PR #11862 with only moving paragraphs around. Thanks everyone for participating the discussion and mostly @tustvold for writing all these down. Close in favor of #11862.

Update cargo-yank.md

9065c87

rust-highfive assigned ehuss Sep 10, 2022

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 10, 2022

tustvold changed the title ~~Update cargo-yank.md~~ Clarify Implications of Cargo Yank Sep 10, 2022

tustvold mentioned this pull request Sep 10, 2022

Release 0.24.1 tafia/quick-xml#475

Closed

weihanglo reviewed Sep 10, 2022

View reviewed changes

tustvold added 3 commits September 10, 2022 20:28

Review feedback

17c711c

Tweak wording

aa1f590

Remove empty files

e48b16f

adamreichold reviewed Sep 10, 2022

View reviewed changes

src/doc/man/cargo-yank.md Outdated Show resolved Hide resolved

Fix copypasta

af6b722

alamb reviewed Sep 11, 2022

View reviewed changes

weihanglo reviewed Sep 13, 2022

View reviewed changes

src/doc/man/cargo-yank.md Outdated Show resolved Hide resolved

src/doc/man/cargo-yank.md Outdated Show resolved Hide resolved

src/doc/man/cargo-yank.md Outdated Show resolved Hide resolved

weihanglo added A-documenting-cargo-itself Area: Cargo's documentation Command-yank labels Sep 13, 2022

rust-highfive assigned weihanglo and unassigned ehuss Sep 26, 2022

tustvold added 2 commits September 27, 2022 16:17

Review feedback

d227ad6

Fix typo

1cba8b7

weihanglo reviewed Sep 27, 2022

View reviewed changes

weihanglo added S-waiting-on-author Status: The marked PR is awaiting some action (such as code changes) from the PR author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 24, 2022

weihanglo mentioned this pull request Mar 16, 2023

doc: clarify implications of cargo-yank #11862

Merged

weihanglo closed this Mar 16, 2023

-However, yanking a release will prevent cargo from selecting that version
-when determining the version of a dependency to use. If there are no longer
-any compatible versions that haven't been yanked, cargo will return an error.
-The only exception to this is crates locked to a specific version by a lockfile,
-these will still be able to download the yanked version to use it.
+Best practice is to yank a release *only* when it contains a serious flaw, such as
+a security vulnerability, and when there is a newer semver compatible release
+published. You do not need to yank a version to suggest users of your crate upgrade.
+Cargo will not use a yanked version for any new project or checkout without a
+pre existing lockfile, and will generate an error if there are no longer
+any compatible versions for your crate.
+For example, the `foo` crate published version `0.22.0` and another crate `bar`
+declared a dependency on version `foo = 0.22`. Now `foo` releases a new, but
+not semver compatibile, version `0.23.0`, and finds a critical issue with `0.22.0`.
+If `0.22.0` is yanked, no new project or checkout without an existing lockfile will be
+able to use crate `bar` as it relies on `0.22`
+In this case, the maintainers of `foo` should first publish a semver compatible version
+such as `0.22.1` prior to yanking `0.22.0` so that `bar` and all projects that depend
+on `bar` will continue to work

Conversation

tustvold commented Sep 10, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR try to resolve?

Uh oh!

rust-highfive commented Sep 10, 2022

Uh oh!

Mingun commented Sep 10, 2022

Uh oh!

adamreichold commented Sep 10, 2022

Uh oh!

weihanglo left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

alamb Sep 11, 2022

Choose a reason for hiding this comment

Uh oh!

Mingun Sep 11, 2022

Choose a reason for hiding this comment

Uh oh!

alamb Sep 11, 2022

Choose a reason for hiding this comment

Uh oh!

adamreichold Sep 11, 2022

Choose a reason for hiding this comment

Uh oh!

crepererum Sep 12, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Mingun Sep 12, 2022

Choose a reason for hiding this comment

Uh oh!

tustvold Sep 27, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

weihanglo left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ehuss commented Sep 26, 2022

Uh oh!

weihanglo Sep 27, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

weihanglo Sep 27, 2022

Choose a reason for hiding this comment

Uh oh!

weihanglo commented Feb 9, 2023

Uh oh!

weihanglo commented Mar 16, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

tustvold commented Sep 10, 2022 •

edited

Loading

weihanglo left a comment •

edited

Loading

crepererum Sep 12, 2022 •

edited

Loading

weihanglo Sep 27, 2022 •

edited

Loading