Skip to content

Conversation

@SteveLauC
Copy link
Member

@SteveLauC SteveLauC commented Nov 3, 2025

What does this PR do

This commit:

  1. Pins indirect dependencies

    • parking_lot_core to 0.9.11
    • lock_api to 0.4.13

    by adding them to our "dev-dependencies" list. Newer versions of them
    require MSRV 1.71, which is higher than our current one 1.69.0

    parking_lot_core 0.9.12 link
    lock_api 0.4.14 link

  2. Bumps bitflags to 2.4, which is required after doing 1, or we have a
    dependency conflict

  3. Resolves Clippy warning "clippy::unnecessary_unwrap"

Checklist:

  • I have read CONTRIBUTING.md
  • I have written necessary tests and rustdoc comments
    not needed
  • A change log has been added if this PR modifies nix's API
    not needed

This commit:

1. Pins **indirect** dependencies

   * parking_lot_core to 0.9.11
   * lock_api to 0.4.13

   by adding them to our "dev-dependencies" list. Newer versions of them
   require MSRV 1.71 [1][2], which is higher than our current one 1.69.0

2. Bumps bitflags to 2.4, which is required after doing 1, or we have a
   dependency conflict

3. Resolves Clippy warning "clippy::unnecessary_unwrap" [3]

[1]: https://crates.io/crates/parking_lot_core/0.9.12
[2]: https://crates.io/crates/lock_api/0.4.14
[3]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_unwrap
@SteveLauC SteveLauC changed the title chore: pin parking_lot to 0.12.4 Pin parking_lot_core/lock_api & resolve clippy::unnecessary_unwrap Nov 3, 2025
@SteveLauC SteveLauC marked this pull request as ready for review November 3, 2025 15:08
@SteveLauC SteveLauC requested a review from Copilot November 3, 2025 15:08
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors error handling in the Flock drop implementation and adds dependency version constraints to maintain MSRV (Minimum Supported Rust Version) compatibility.

  • Improved error handling pattern in Flock::drop using idiomatic Rust if let pattern
  • Updated bitflags dependency from 2.3.3 to 2.4
  • Added explicit version constraints for indirect dependencies to maintain MSRV 1.69 compatibility

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
src/fcntl.rs Refactored error handling in Flock::drop to use if let Err(e) pattern instead of chaining is_err() and unwrap_err()
Cargo.toml Updated bitflags to 2.4, added version-pinned dev-dependencies for parking_lot_core and lock_api to maintain MSRV 1.69 compatibility

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@SteveLauC SteveLauC mentioned this pull request Nov 3, 2025
@SteveLauC SteveLauC enabled auto-merge November 3, 2025 15:10
@SteveLauC SteveLauC added this pull request to the merge queue Nov 3, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Nov 3, 2025
@asomers
Copy link
Member

asomers commented Nov 3, 2025

Hi @SteveLauC . Welcome back! BTW, did you see #2869 ? That was my attempt to fix this same problem in a different way. Unfortunately, it triggered new test failures in CI that I was never able to solve.

@SteveLauC
Copy link
Member Author

Hi @SteveLauC . Welcome back!

Sorry for leaving for such a long time 🫥

did you see #2869

Sorry for didn't noticing #2689


Recently a transitive dev-dependency published a new version that is incompatible with Nix's MSRV, causing CI to break. That's annoying, because there's really no reason why a crate's dev-dependencies ought to respect the MSRV.

Considering this, I think your approch is better, this PR is just a workaround.

I just checked that PR in details, these commits are valuable so I think we should merge them regardless of how we solve this MSRV issue:

@SteveLauC
Copy link
Member Author

Closing this PR in favor of #2689

@SteveLauC SteveLauC closed this Nov 4, 2025
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

Successfully merging this pull request may close these issues.

2 participants