[release-0.13.x] fix(CI): Add Cargo.lock.msrv for CI tests
#496
Conversation
This is not so trivial. In theory,
cargo +nightly update -Z minimal-versions generate-lockfile
ought to work.
However, there are rather many places in our dependency tree where
some crate A depends on crate B, version V, but it actually doesn't
build with version V, but requires something considerably newer.
There seemed to be particular problems in the parts of the dependency
graph near "pest". My attempts at starting with the minimal versions
and upgrading crates as needed, were not successful.
However, I was able to converge reasonably quickly from the other end:
starting with the lockfile generated by 1.59, using recent versions,
and then repeatedly downgrading crates whose (actual, or declared)
MSRV was too high.
I used the following recipe on 2023-10-24 to generates this lockfile:
Bump Tokio dependency to 1.29 in Cargo.toml
cargo +1.59 generate-lockfile
cargo +nightly update -Z minimal-versions -p linux-raw-sys
cargo +nightly update -Z minimal-versions -p tokio
cargo +nightly update -Z minimal-versions -p rustix
cargo +nightly update -Z minimal-versions -p tempfile
cargo +nightly update -Z minimal-versions -p reqwest
cargo +nightly update -Z minimal-versions -p byteorder
cargo +nightly update -Z minimal-versions -p h2
cargo +nightly update -Z minimal-versions -p log
cargo +nightly update -Z minimal-versions -p tokio
cargo +nightly update -Z minimal-versions -p [email protected]
Discard the Tokio dependency update
So that is what we commit here.
This is unprincipled, and the recipe above has already rotted by
2023-11-06. In the future (especially on the mainline) we can
hopefully have a more principled approach.
Signed-off-by: Ian Jackson <[email protected]>
This will prevent our CI breaking due to updates to our dependencies. Because of the way the msrv.yml file is constructed, it was not straightforward to use the committed lockfile only for the MSRV tests. It would be better to use a different lockfile, with latest versions of our dependencies, for non-MSRV tests. Signed-off-by: Ian Jackson <[email protected]>
polarathene
changed the title
Cargo.lock.msrv for CI tests
|
I have a question regarding this: So as far as I can see, we're always using the msrv-locked This does imply that we never test against newer dependencies, is that correct? If yes, could this be an issue at some point? |
Yes, that is correct. And it does mean that we might miss breakage induced by breakage in our dependencies. (Such breakage would be semver violations, but that does happen sometimes.) I think ideally we would detect such breakage so we can respond to it. But I don't think we want such breakage to interfere with our primary work. Specifically, it ought not to block MR work or the work of contributors. I think the best approach would be to run a separate CI job which ignores the lockfile. To avoid it being blocking we could run it periodically, and not on MR branches. Or we could mark it to be only a warning on failure. I'm mostly familiar with gitlab CI so I'm not sure how to do these things with github. |
|
I think just running it on pushed master would be sufficient, wouldn't it? |
Yes, that would do nicely. It wouldn't block anyone's work but it would mean we'd notice. |
Potentially, but whom that affects would be difficult to say.
You can't really get that sort of coverage across toolchains, which is also time dependent.
Lockfile is serving it's purpose. If there are issues downstream, users will raise an issue (hopefully), or you'll catch them eventually.
I agree with @ijackson suggestions. I am familiar with Github Actions CI, but bit short on time lately.
I have experience with each suggestion and can provide direction, otherwise maybe later this month I can contribute the preferred approach.
I don't follow this suggestion? This release branch is not in sync with master? If you want to maintain CI on the main/master branch, and have this branch use those (or only some) workflows for easier maintenance, that's absolutely doable. |
I think this would be a good idea.
I think there is no hurry!
Sorry, I was talking about master here. I think master should be our main concern, the release branch is fine IMO because there will be a 0.14.0 soon (for some definitions of the word) anyways 😆 |
This is an alternative to #492. It is my attempt to do the minimal changes needed to get the 0.13.x release branch CI passing.
Compared to #492:
Cargo.lock.msrv, and used only for CI tests.In some sense this might be considered an alternative to #495. However, #495's CI is failing. It might pass when combined with the
Cargo.lockin this branch; so, it may be that this MR and #495 are complementary.It would be nice to have a more principled way to generate
Cargo.lock.msrv. Ideally we'd have an in-tree script for that. However, so far no-one has been able to produce a working, stable, recipe for generating a working lockfile. If and when such a recipe appears we should use it and commit both the recipe and its result.On the config-rs main branch things may be a little better, because we'll be more able to update and change our dependencies to avoid bugs in our dependency tree. Deciding the approach for MSRV tests in the config-rs mainline is left to the future.
Also, it would be nice to use a different lockfile (or, perhaps, none at all) for non-MSRV tests. That seems less straightforward than what I have done here. Again, I suggest we leave that for a future MR to improve.
CC @polarathene