Suggestion: cargo yank is a misfeature and should be deprecated and eventually removed

Maybe we could introduce a finer mechanism so the owner of a crate can mark some versions as Vulnerable, Broken_API, Major_issue, Unmaintained, ... with an issue id (like a rustsec id) and a comment. A warning would be raised if a marked crate is used. I'd advocate that crates with vulnerabilities and broken api might still be treated as currently yanked crates. You would still be able to ignore an issue with a parameter like ignore="RUSTSEC-2023-0666"

But right now, yanking is the only mechanism available to reach the users than does not use external tools when there is a problem. If we want to deprecate yanking, we need to introduce a better feature as replacement first.

5 Likes

It doesn't make sense to put "ignore yanking" options in Cargo.toml. The point is that you don't know which crates should ignore yanking until they're yanked, and you need it most on legacy and historic code anyway, where you can't change the Cargo.toml.

It should be a flag on cargo call itself. Perhaps writing a special cargo tool to ignore yanking is the best solution. It doesn't look like we'll get any compassion around here.

4 Likes

I think you have an opportunity to focus your argument here better.

Thank you for this comment --

I want to try to give a more concrete example, or series of examples, where I think that yanking can cause cargo to do something undesirable and where the amount of stuff I have to do to get it not to do that grows larger and larger.

I haven't had time to put together such an example in detail, but I think I will have time later this week. Thanks to everyone who has commented for their interest in this issue.

3 Likes

Giving an overridable error on an attempt to yank the final available version of a semver bracket is a good idea, I think. Something like this:

error: attempt to yank the final remaining version of 0.13
help: publish a new 0.13.7 version first, or run with --ignore-semver

And similarly for a gross publishing error, which is what I would argue yank is intended for:

error: attempt to yank the final remaining version of 999999

"ignore semver" is clearly a sensible response to being told people won't be able to use version 999999 anymore.

4 Likes

A patch for this is trivial btw, I've made one years ago. The tough part is convincing the cargo team to accept it. And yes, this can 100% be an external tool... maybe I should make one.

5 Likes

(This is the epitome of bikeshedding the naming of a small part of something where the whole design is in question, but IMO it should be --allow-yanked since "ignoring yanked crates during version resolution" is the default (and for that reasoning I used it in cargo-dl)).

3 Likes

Did this writeup happen? I am firmly in camp "always commit your lockfile, for lib and bin crates alike" (for the reasons stated above: reproducing earlier builds, git bisect), so I would be curious what your reasoning is here.

As has been said before: it causes no such problems when lockfiles are committed. And when lockfiles are not committed, these problems arise even without yanking, since cargo will pick up newer semver-compatible versions.

8 Likes

Here's what I find confusing about your proposal to deprecate cargo yank: Yanking a crate can be useful. It has some problems, such as:

  • Some people use it "incorrectly" when it isn't needed
  • Yanking crates unnecessarily disrupts the ecosystem

But your solution is to just remove the feature, instead of trying to address these problems. There are many ways how cargo yank can be improved; but to deprecate it, you must clearly demonstrate that

  1. it isn't useful, or its downsides far outweigh its benefits
  2. it's not feasible to improve the feature to make it useful

In particular, the argument that too many people are using the feature incorrectly is very weak:

  • It only has anecdotal evidence, not actual numbers
  • Whenever something is used incorrectly, better communication (like documentation improvements or a big red warning when yanking) help
  • Using a crate means that you have to trust its author; it would be inconsistent to trust that the author has written secure and high-quality code that you can use in your project, but not trust their decision to yank a version of the crate. If the author is trustworthy, I assume that they make good decisions, and if they're not, I don't use their code.

If (and that's a big "if") a lot of maintainers yanked versions unnecessarily, even though they were competent and acting in good faith, that would indicate bad documentation, which can be improved.

Scenario for those that want cargo yank to be removed (eventually):

  • crate is uploaded with --no-verify because CI is passing on the precise commit
  • for whatever reason, Cargo.toml is not included in the package (despite being present locally)

It is impossible to compile the crate, and all reverse dependencies are broken as a result. What action would you like the crate author to take?

While this scenario, particularly the second step, seems impossible, this has actually happened to me when publishing time. I promptly yanked the affected version.

5 Likes

I've also encountered scenarios where crates are unbuildable.

Misspecified version requirements can allow a crate to build for a time, but if you accidentally do e.g. >=1 when you meant ^1/1, the crate will pull in all future versions of the dependency, even ones with SemVer breaking changes. However, it will work fine until you release v2 of the dependency.

I recently yanked some crates which had such a defect, providing a SemVer-compatible fix which corrected the version requirement.

Just uploading a new version seems like a perfectly workable alternative to yanking in this scenario.

1 Like

If you made such a release today which was completely unbuildable, would you yank it? I certainly would, immediately.

It's also possible you might need to debug something before your next release. Perhaps your release automations are broken and you need to fix them to cut another release. In the meantime your latest release is broken and people are going to start trying to use it, complaining that it's broken.

5 Likes

Since yanking exists, using it in this scenario is indeed pretty obvious. But the point is that if yanking didn't exist, doing a new release is really perfectly viable here.

I think this type of thing is really the exception that proves the rule. Yes of course, if you somehow manage to break your release infrastructure soon after releasing a broken version, and debugging that is somehow hard, the availability of a parallel "yank" infrastructure certainly is convenient. But regardless, you're going to need to fix your release infrastructure, and so I don't think this is a strong argument for the "normality" of yanking.

1 Like

If your release infrastructure generates a broken release, you're going to have to debug it before you can put out a good release.

Your "somehow" seems to be discounting this case. There's no coincidence that needs to happen here. A broken release infrastructure can generate the unbuildable release, and the only other option than fixing it is to put together a "break glass" release by hand.

I work on crates with >100,000 downloads a day. If I put out an unbuildable release, yanking is invaluable for incident response.

Figuring out what went wrong might take time, and in that intervening time it's nice to make the bad release go away rather than be constantly bugged about it by people trying to upgrade.

8 Likes

But your solution is to just remove the feature, instead of trying to address these problems. There are many ways how cargo yank can be improved; but to deprecate it, you must clearly demonstrate that

  • it isn't useful, or its downsides far outweigh its benefits
  • it's not feasible to improve the feature to make it useful

I suppose I take an alternative point of view.

My point of view is, options are bad, from a UX perspective. Each feature that you add to the software is more stuff you have to learn, have to teach, have to maintain. It's more cognitive overload for the person who actually has to make the decision. When there's only one obvious way to handle a situation, it leads to better outcomes and happier users.

Each feature has to justify its own existence by solving problems that can't be solved another way, without creating more problems of its own.

An example that comes to mind: The consensus in the rust community agrees that "life before main" as it exists in C++ was a misfeature. It's a misfeature because, while it maybe lets you save some lines of code in some simple programs, it creates a problem called static initialization order fiasco, and makes it very difficult to reason about what order things actually happen in when you program starts up. There was a consensus view that this should not be added to rust, and we explicitly chose not to add it.

If we applied your criteria to this feature:

Here's what I find confusing about your proposal: Static initialization can be useful. It has some problems, such as:

  • Some people use it "incorrectly" when it isn't needed
  • Static initialization unnecessarily disrupts the ecosystem

But your solution is to just remove the feature, instead of trying to address the problems.

Indeed, sometimes removing a feature is exactly the right thing to do, rather than trying to change it. As has been done repeatedly in the history of rust. (See also core::mem::uninitialized)

My argument right now is that, yanking is never necessary.

  • If you made a release that doesn't build, you don't have to yank it. You can make a new patch release that fixes the problem. This is a fine solution because, anyone cargo that is selecting your crate according to semver rules will pick the patched version. No one is going to have the bad version in their cargo.lock, because their code would never have built in the first place.
  • If you made a release that has security problems, it is better to make a security advisory than to yank it -- you can yank it, but it's more helpful to downstream if you make a security advisory.

Bascule had some interesting comments:

It's also possible you might need to debug something before your next release. Perhaps your release automations are broken and you need to fix them to cut another release. In the meantime your latest release is broken and people are going to start trying to use it, complaining that it's broken.

So, in the case that you make a release that doesn't build, maybe fixing the build and making another release is unattractive because this is time-sensitive -- other people's cargos will start selecting the bad release in real-time if you don't yank it, and it may be too hard for you to fix the build on a short time frame.

This is a good reason in my mind for yank to continue to exist in some form. But it would be nice if it were time limited to a few days or weeks, similar to npm. This would make it more clear how it is intended to be used (which would help people to all use it in the same way, and for downstream to make sense of what it means), and encourage people to use security advisories instead of yanking when security problems are discovered.


I have not made time to build the concrete example that I describe earlier, but I will soon.

4 Likes

I had to look this up, just for reference this is Unpublishing packages from the registry | npm Docs and indeed I think something like this would be a way better baseline.

In my case I got burned by a crate maintainer yanking all old versions of a package unnecessarily; it wouldn't even need to be an enforced "policy" so much as detecting when you're yanking a crate that is older than 72 hours and just redirecting to documentation that describes why it may not be a good idea to yank in this scenario and asking for extra confirmation.

2 Likes

That's a different situation though: it was never deprecated in Rust, because it was never added. Removing/deprecating something must be better justified than a decision not to add it.

Note that the cargo yank command can be easily deprecated, but existing yanked versions must be supported forever to not break backwards compatibility.

mem::uninitialized didn't just have minor problems, it was literally impossible to use without triggering UB, and there was no way to fix it. However, a better alternative (MaybeUninit) was added before mem::uninitialized was deprecated.

That sounds reasonable to me. Another way to minimize breakage is that after a certain time has passed, a crate may only be yanked if a newer semver-compatible version exists. For example, version 0.3.1 can be yanked once 0.3.2 has been published, but 0.3.2 can't be yanked after a certain time has passed, if it is the latest 0.3.x version.

One other thing that would help is making it clearer to people when they're using a yanked crate due to a Cargo.lock file. Right now, unless you remove Cargo.lock, you won't get an error, and you won't get a warning about the yanked crate until you attempt a cargo publish - but this is a warning, not an error, and does not block your attempt to publish the crate.

Not only that, but I can't find an obvious flag or option that would tell cargo publish to fail if I depend on a yanked crate; this is a foot-gun, since if I run cargo clean && cargo clippy -- --deny warnings && cargo test && cargo publish && cargo bump intending to only publish if my code is in a good state, I can accidentally publish a crate that depends on a yanked version, and thus cannot be used by downstream consumers.

Ideally, you'd get a build warning as soon as Cargo is aware that the crate is yanked (which might not happen until you next add a dependency, because it may need network access if you've chosen a version that's been yanked recently), and you'd be unable to publish a crate that depends on a yanked crate without a CLI option to cargo publish to tell Cargo that you know what you're doing.

Reproduction steps

First, create a new and valid project, which depends on a crate with yanked versions, and build it.

cargo new test-yanking
cargo add jimblandy-yank-test
mv src/main.rs src/lib.rs # to make this into a library crate
cargo build

Then, edit Cargo.lock to point at the yanked version, not the unyanked version:

# This file is automatically @generated by Cargo.
# It is not intended for manual editing.
version = 3

[[package]]
name = "jimblandy-yank-test"
version = "0.1.1"
source = "registry+https://github.com/rust-lang/crates.io-index"

[[package]]
name = "test-yanking"
version = "0.1.0"
dependencies = [
 "jimblandy-yank-test",
]

Building this works without complaint.

Change Cargo.toml so that I now depend on the yanked version in Cargo.lock:

[package]
name = "test-yanking"
version = "0.1.0"
edition = "2021"

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
jimblandy-yank-test = "0.1.1"

Do a clean build:

cargo clean
cargo build

Note the lack of any complaint when you do this. Now, remove Cargo.lock and retry the build, and see the complaint about the yanked crate:

$ cargo build
    Updating crates.io index
error: failed to select a version for the requirement `jimblandy-yank-test = "^0.1.1"`
candidate versions found which didn't match: 0.1.0
location searched: crates.io index
required by package `test-yanking v0.1.0 (/home/farnz/test-yanking)`

This isn't a lack of complaint because Cargo doesn't know that I'm depending on a yanked version, since the yanking took place years before I did the experiment - hence the index downloaded when I first did cargo build tells Cargo that the crate is yanked. Instead, it's a footgun waiting for me - if I ever remove the Cargo.lock file, things are going to break.

I also tried cargo add clap && cargo b to see if that caused a complaint - it doesn't, even though Cargo has to redo depsolving at this point.

I try to publish with a dry-run, and get a warning, not an error:

$ cargo publish --dry-run
    Updating crates.io index
warning: manifest has no description, license, license-file, documentation, homepage or repository.
See https://doc.rust-lang.org/cargo/reference/manifest.html#package-metadata for more info.
   Packaging test-yanking v0.1.0 (/home/farnz/test-yanking)
warning: package `jimblandy-yank-test v0.1.1` in Cargo.lock is yanked in registry `crates-io`, consider updating to a version that is not yanked
   Verifying test-yanking v0.1.0 (/home/farnz/test-yanking)
   Compiling libc v0.2.141
   Compiling io-lifetimes v1.0.10
   Compiling rustix v0.37.11
   Compiling bitflags v1.3.2
   Compiling linux-raw-sys v0.3.1
   Compiling utf8parse v0.2.1
   Compiling concolor-override v1.0.0
   Compiling concolor-query v0.3.3
   Compiling anstyle v0.3.5
   Compiling strsim v0.10.0
   Compiling clap_lex v0.4.1
   Compiling jimblandy-yank-test v0.1.1
   Compiling anstyle-parse v0.1.1
   Compiling is-terminal v0.4.7
   Compiling anstream v0.2.6
   Compiling clap_builder v4.2.1
   Compiling clap v4.2.1
   Compiling test-yanking v0.1.0 (/home/farnz/test-yanking/target/package/test-yanking-0.1.0)
    Finished dev [unoptimized + debuginfo] target(s) in 2.82s
    Packaged 6 files, 10.2KiB (3.2KiB compressed)
   Uploading test-yanking v0.1.0 (/home/farnz/test-yanking)
warning: aborting upload due to dry run

But if I use this crate as a path dependency from another crate, it doesn't work: a Cargo.toml with the following contents in a peer directory results in an error:

[package]
name = "test-dep-yanking"
version = "0.1.0"
edition = "2021"

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
test-yanking = { path = "../test-yanking" }

When built:

$ cargo b
    Updating crates.io index
error: failed to select a version for the requirement `jimblandy-yank-test = "^0.1.1"`
candidate versions found which didn't match: 0.1.0
location searched: crates.io index
required by package `test-yanking v0.1.0 (/home/farnz/test-yanking)`
    ... which satisfies path dependency `test-yanking` of package `test-dep-yanking v0.1.0 (/home/farnz/test-dep-yanking)`

and yet if I go into test-yanking, I can still build because of the Cargo.lock file.

3 Likes

Following this and other discussions over the years, I opened a PR for the cargo book to change the recommendation to always commit Cargo.lock files.

I'm sharing it here for visibility as it's related to yanking. I hope to see this change merged as I believe that it's a better default (no information loss). Maybe you'd want to post there @zackw.

3 Likes

This is a terrible practice for any project that uses CI (so, terrible for any half-serious project, really). Effective CI needs reproducibility. And Cargo.lock is what provides reproducibility. Especially for library crates!