Is it really a good idea for Cargo to silently choose out-of-date versions of crates to avoid pulling in multiple compatible versions of another crate?

tools and infrastructure cargo
21

My PRs have usually stated why the minimum version is needed (e.g., "libc 0.2.45 added function frob used in module blah") which goes over well enough. My issues have mainly been in getting any attention at all followed by a new release if it gets that far. I don't think I've heard "I don't care about your problem" so much, but I also don't ask about yanking.

1 Like
22

Yeah, I'd say that yanking old deps is probably going too far -- it creates massive churn (to the first approximation, we'd have to yank everything that's currently on crates.io, invalidating any single Cargo.lock) for no direct benefit. I'd imagine that ecosystem-wide minver correctness is achieved by just publishing new minor versions which are minver correct.

That is, if you depend on foo 1.2.0 and it itself is not minver correct, you work with foo's author to publish foo 1.3.0 which is minver correct, and then publish a new version of your crate, which depends on foo 1.3.0.

My interpretation is that crate authors are hesitant to proactively maintain minver correctness simply because the tooling to check it is currently unstable. I am willing to bet that, if --minimal-versions is stabilized at Rust X, most of the maintained crates will become minver correct a year after that, or by the time they upgrade MSRV to X, whichever comes last. Even more so, crates won't become minver correct until we stabilize --minimal-versions.

23

So, crates are not minimal-version compatible because the flag is unstable, and the flag is unstable because crates are not minimal-version compatible? That sounds... not great.^^

2 Likes
24

There is benefit to yanking, though. (The problem is that such benefit is even more niche than just the idea of minimal version correctness.)

Specifically, let's consider a hypothetical library magic. magic 1.0.0 is not minimal versions correct. Despite this, it becomes popular, and is now a dependency of many other foundational crates, and it's easy to have a dependency on magic transitively through your dependency tree.

Now, magic 1.0.1 is released, and it is now minimal versions correct. Now, there are three ways forward to fix minimal version correctness for crates which use magic:

  • Every crate that transitively depends on magic and cares about minimal version correctness adds a new magic ^1.0.1 dependency. This forces the included magic version to be the minimal versions correct version, but also adds a "false dependency" just for the purpose of fixing minimal versions. (Number of publishes: O(transitive rdeps), concurrently.)
  • Every crate which directly depends on magic releases a new version which is now minimal versions correct. Each of their direct users release a new version which is now minimal versions correct. Each of their direct users ... (Number of publishes: O(transitive rdeps), serially.)
  • magic 1.0.0 is yanked. A magic ^1.0.0 dependency will now resolve to magic 1.0.0 (unless a lockfile already uses magic 1.0.0, in which case the yanked version is allowed) and now magic is no longer the cause of any package being minimal versions incorrect. (Number of publishes: 1, plus yanking any semver compatible but minimal versions incorrect version (O(1) work w.r.t. crates-io as a whole).)

There is concrete benefit to yanking minimal versions incorrect packages -- but unlike minimal version correctness in the first place, which can be hit on stable by time-delayed cargo add or cargo update -p with a lockfile, yanking minimal versions incorrect packages only helps those using the --minimal-versions lockfile resolution algorithm. (Or IOW, has no impact on stable.)

However, yanking old versions with incorrect metadata actually reduces the amount of version churn, because more packages will resolve to be minimal versions correct (even if they aren't strictly correct in the face of using yanked versions, which imho doesn't need to be considered, since the yanked versions are in fact broken (in metadata only)).

3 Likes
25

I donā€™t this this is quite correct analysis, as it doesnā€™t account for amortization. It certainly holds if magic is the single non-minimal-version correct library in the ecosystem. But I think our situation is closer to the case where every package has to do a minver minor release for one way or any other anyway. That is, I think the total amount of work we need to do is O(len(crates.io)).

26

That's my suspicion as well. Would someone more knowledgeable about GHA be willing to help get some minvers workflow template up and running? Once fixing a crate's minvers situation, it'd be nice to know we are at least raising the bar instead of just throwing another ball into the air to juggle.

FWIW, I have GitLab-CI jobsĀ¹ for my projects, but I couldn't figure out how to nicely translate to GHA when I looked last (soon after it launched before caching and inter-workflow artifacts were well-established).

Ā¹ Following the anchors used there and searching for "mindeps" will find the rest of the bits.

27

This is the job I've been using:

  cargo-test-msrv:
    name: Tests (1.41.0)
    runs-on: ubuntu-latest
    steps:

      - name: Checkout repository
        uses: actions/checkout@v2

      - name: Install arbitrary nightly toolchain
        uses: actions-rs/toolchain@v1
        with:
          toolchain: nightly-2020-01-01
          profile: minimal

      - name: Install msrv toolchain
        uses: actions-rs/toolchain@v1
        with:
          toolchain: 1.41.0
          profile: minimal
          override: true

      - name: Generate minimal-versions lockfile
        uses: actions-rs/cargo@v1
        with:
          command: +nightly-2020-01-01 # installed nightly
          args: -Z minimal-versions generate-lockfile

      - name: Enable caching
        uses: Swatinem/rust-cache@v1.3.0

      - name: Compile
        uses: actions-rs/cargo@v1
        with:
          command: test
          args: --locked --all --all-targets --examples --no-run

      - name: Run tests
        uses: actions-rs/cargo@v1
        with:
          command: test
          args: --locked --all --all-targets --examples

This combines minimal versions with the MSRV test, which keeps MSRV resilient against deps bumping MSRV as well. (example)

2 Likes
28

I will note that the nightly date is important if the MSRV is old enough to not support the new lockfile format (which is not supported in new enough Rust versions).

1 Like
29

This is a good question. I think the best way forward would be to allow cargo authors to retroactively specify old versions as broken. That way when crate version 0.22.2 is published, it (the version 0.22.2) would include metadata that version 0.22.1 (or any other older version) is broken with a message "Security vulnerability: RUSTSEC-2021-0119". This would allow the users that depend on this crate to automatically get signal about old version without the old package being modified manually or the users doing anything. And the metadata from the latest crate would be used always so that errors in metadata can be fixed with newer crates (let's not pretend that metadata cannot be incorrect).

If Cargo cannot automatically find a suitable collection of crate versions that are not broken and are compatible according to version requirements, then it should complain to the user. Not using the latest version of any given crate should be okay as long as the old crate is not broken (e.g. newer variant only adds some extra that's not needed).

30

This is basically what yanking is for, although it'd be nice if authors who are yanking crates could include a reason for yanking, e.g. a security vulnerability

6 Likes
31

I think your RFC looks pretty good. The part I failed to understand is that who keeps the list of known bad versions in the future and how it's distributed to the users? The way I think it should be handled is that the latest crate contains list of all known bad historical versions and I'd also make the reason for each bad package mandatory. However, the syntax could be something simple like

[RUSTSEC-2021-0119]
status=broken
minversion=1.2.15
maxversion=2.2.1
reason=Security vulnerability, RUSTSEC-2021-0119

Where minversion and maxversion create inclusive range. And the crate metadata would have to preserve that data forever. And if it's later found that 1.2.15 was not in fact vulnerable, the minversion can be fixed in later release.

And note that you could have more than one section with breakage metadata so multiple ranges or multiple different bugs could be easily handled.

32

For yanking, the data needs to be associated with each release of a crate. That's just how it works.

What you're describing is fairly close to how the RustSec advisory format already works:

33

This topic was automatically closed 540 days after the last reply. New replies are no longer allowed.