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

MSRV of 1.65 is broken #49

Open
epage opened this issue Nov 1, 2024 · 2 comments
Open

MSRV of 1.65 is broken #49

epage opened this issue Nov 1, 2024 · 2 comments

Comments

@epage
Copy link

epage commented Nov 1, 2024

To reproduce, I had to do:

$ CARGO_RESOLVER_INCOMPATIBLE_RUST_VERSIONS=fallback cargo +nightly generate-lockfile
$ cargo +1.65 check
error[E0277]: `Sender<(Outcome, TestInfo)>` cannot be shared between threads safely
   --> src/lib.rs:521:29
    |
521 |                   scope.spawn(|| {
    |  _______________________-----_^
    | |                       |
    | |                       required by a bound introduced by this call
522 | |                     loop {
523 | |                         // Get next test to process from the iterator.
524 | |                         let Some(trial) = iter.lock().unwrap().next() else {
...   |
539 | |                     }
540 | |                 });
    | |_________________^ `Sender<(Outcome, TestInfo)>` cannot be shared between threads safely
    |
    = help: the trait `Sync` is not implemented for `Sender<(Outcome, TestInfo)>`
    = note: required for `&Sender<(Outcome, TestInfo)>` to implement `Send`
note: required because it's used within this closure
   --> src/lib.rs:521:29
    |
521 |                 scope.spawn(|| {
    |                             ^^
note: required by a bound in `Scope::<'scope, 'env>::spawn`

Not seeing any MSRV verification in CI. Highly recommend cargo hack for this as it let's you single-source your MSRV, e.g. https://github.com/epage/_rust/blob/e121dd6ef9e11dfa818a813bbaffb12a16cd174e/.github/workflows/ci.yml#L54-L67

@LukasKalbertodt
Copy link
Owner

Hi, thanks for the ping. So there is 08e76d3 from some time ago, where I removed the MSRV CI check since it broke due to a new clap version. As you can see in the commit message, I don't really know how I, as a library maintainer, should deal with this. Do I simply state the MSRV of my code without dependencies? Do I state the MSRV of my code and of the oldest supported versions of my dependencies (which, in case of that commit, would be clap 4.0.x)? Because tracking the MSRV of the newest versions of all of my dependencies, that seems like exhausting and annoying work.

I suppose that Cargo can chose compatible dependency versions, also considering rust-version. I'm not sure if that was already the case when I created that commit. I do know that CI just broke due to a new clap version.

So yeah: can you tell me about best practices?

(Note: cargo hack seems really cool, not only for MSRV. I didn't know that, thanks!)

@epage
Copy link
Author

epage commented Nov 2, 2024

While not yet on the stable version of the Cargo book, the Cargo team has posted guidance on MSRV that includes this topic, see https://doc.rust-lang.org/nightly/cargo/reference/rust-version.html#support-expectations

For myself, I have a Cargo.lock committed that is compatible with my MSRV. You can jump start that on nightly using the MSRV-aware resolver, see https://doc.rust-lang.org/nightly/cargo/reference/config.html#resolverincompatible-rust-versions

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants