From ebba8ea7815a2ee27aeb63f50cbab6ac3abce100 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Karol=20Bary=C5=82a?= Date: Sun, 14 Jan 2024 19:57:02 +0100 Subject: [PATCH 1/2] Add info about Cargo.lock.msrv to Contributing.md This is so that contributors (external or internal) are able to find information about how to deal with failures in min_rust CI job. --- CONTRIBUTING.md | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 174c11cb8b..2ebf45c46b 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -54,6 +54,38 @@ Starting a cluster without running any test is possible with `make up`. Before sending a pull request, it is a good idea to run `make ci` locally (or `make dockerized-ci` if on macOS). It will perform a format check, `cargo check`, linter check (clippy), build and `cargo test`. +### min_rust workflow and Cargo.lock.msrv + +There is min_rust job defined in rust.yml workflow that checks if the driver compiles with our current MSRV. +Bumping MSRV is generally not considered a breaking change, so our dependencies are free to do it, +and do it regularly. This resulted in failures in this job. +We could pin versions of problematic crates in `Cargo.toml`, but it would affect version selection +for client applications. It would also force people that use non-ancient versions of Rust to use older +versions of dependencies, which is not desirable. + +We opted for a different approach. There is `Cargo.lock.msrv` file in repository, which is used only by min_rust job - +it is renamed to `Cargo.lock` before building the driver with `--locked` flag. + +This solution is good for us (because we don't have to fix breakage so often) and for users of the driver on modern versions +of Rust - for them the driver will just work. +Users on old versions of Rust will potentially have to pin indirect dependencies to specific versions - but that shouldn't be +that much of a problem, it's just few `cargo update` commands. + +If your PR added / removed / updated a dependency, you will need to also update `Cargo.lock.msrv` file. +There are a few scenarios: + - If you just bumped version of one of crates in the workspace: rename `Cargo.lock.msrv` to `Cargo.lock`, + run `cargo check --all-targets --all-features --offline`, rename the file back. + - If you added / removed / updated dependency in one of `Cargo.toml` files it should be enough to + rename `Cargo.lock.msrv` to `Cargo.lock`, run `cargo check --all-targets --all-features` + and rename the file back. + - If previous methods didn't work or you want to recreate the file (which we should do once in a while): + 1. Switch to MSRV version (e.g. `rustup install && rustup default `). Check `README.md` file for current MSRV. + 2. Remove your `Cargo.lock` + 3. Run `cargo check --all-targets --all-features` + 4. If you got an error about one of dependencies not working with this version of Rust, update it's version in `Cargo.lock`, + using command like `cargo update -p toml_datetime --precise 0.6.3` and go back to step 3. + 5. Rename `Cargo.lock` to `Cargo.lock.msrv`. + ## Contributing to the book The documentation book is written using [mdbook](https://github.com/rust-lang/mdBook)\ From b6c813fe1f8ef01cff1a0c5aebc38458f0e5573f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Karol=20Bary=C5=82a?= Date: Mon, 22 Jan 2024 18:11:06 +0100 Subject: [PATCH 2/2] CONTRIBUTING.MD: Remove redundant backtick This backtick is unnecessary, but breaks syntax highlighting in VSCode. --- CONTRIBUTING.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 2ebf45c46b..7674fe08f7 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -42,7 +42,7 @@ When on non-Linux machine, however, it can be impossible to connect to container If you are using macOS, we provide a `dockerized-test` make target for running tests inside another Docker container: ```bash make dockerized-test -```` +``` If working on Windows, run tests in WSL. The above commands will leave a running ScyllaDB cluster in the background.