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

Add Cargo.lock to the repo to fix flaky CI #69

Merged
merged 1 commit into from
Feb 20, 2024

Conversation

Swatinem
Copy link
Contributor

Previously, CI was breaking "randomly" every time a dependency moved its MSRV beyond whatever is used to run in CI. To fix that, just commit the lockfile to git so that CI runs with a fixed set of dependencies. Also adds my own rust-cache for good measure :-)


Lets hope that raising this to 1.63 is actually far enough. Personally, I would just take #64 and bump MSRV as far as is comfortable. It is a bit weird that a much higher Rust version is used in CI than specified in MSRV, so there is zero guarantee that the MSRV is actually correct 🤷🏻‍♂️

@Amanieu
Copy link
Owner

Amanieu commented Feb 20, 2024

It's generally a bad pattern to commit Cargo.lock for library crates: it is normally only done for binaries.

CI is still failing because of regex-automata, which requires 1.65. At this point I would just bump the CI to 1.72 (which should be enough for most of our dev-dependencies) while keeping the MSRV in Cargo.toml at 1.60.

@Swatinem
Copy link
Contributor Author

At this point I would just bump the CI to 1.72

done

It's generally a bad pattern to commit Cargo.lock for library crates

Not any more: https://blog.rust-lang.org/2023/08/29/committing-lockfiles.html
IMO committing it for library crates is good for a bunch of reasons:

  • It avoids CI randomly breaking because of updated dependencies (exactly what happens here)
  • It makes CI caching effective
  • Similar to CI, but even more importantly, it does not break things for new contributors. SemVer should ideally avoid that, but I still had that happen to myself often enough.

But either way, I can also remove it again from the PR if you prefer that.

@Amanieu
Copy link
Owner

Amanieu commented Feb 20, 2024

CI is still failing, it seems clap requires 1.74. Just bump CI up to that (and continue bumping as needed).

Previously, CI was breaking "randomly" every time a dependency moved its MSRV beyond whatever is used to run in CI. To fix that, just commit the lockfile to git so that CI runs with a fixed set of dependencies. Also adds my own `rust-cache` for good measure :-)
@Swatinem
Copy link
Contributor Author

CI is still failing, it seems clap requires 1.74.

Done. Lets hope this is enough ;-)

@Amanieu Amanieu merged commit b285630 into Amanieu:master Feb 20, 2024
1 check passed
@Amanieu
Copy link
Owner

Amanieu commented Feb 20, 2024

Great! Can you rebase your other PRs so they pass CI.

@Amanieu
Copy link
Owner

Amanieu commented Feb 20, 2024

Nevermind, I went ahead and rebased them myself.

@Swatinem Swatinem deleted the lockfile branch February 20, 2024 17:08
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