[release-0.13.x] fix(CI): Add `Cargo.lock.msrv` for CI tests

[ijackson]

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:

• The lockfile is committed under a different name, Cargo.lock.msrv, and used only for CI tests.
• I have clarified the big commit message to make it clear that the runes given, that I used for creating the lockfile, aren't a reuseable recipe.
• The commit to bump the Tokio dependency is gone, but its ghost remains in the lockfile. (Since the recipe has already rotted I decided to leave it that way. The lockfile isn't wrong.)
• Fixed typos in the commit message.

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.lock in 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

[matthiasbeyer]

I have a question regarding this: So as far as I can see, we're always using the msrv-locked Cargo.lock for CI tests after this was merged.

This does imply that we never test against newer dependencies, is that correct? If yes, could this be an issue at some point?

[ijackson]

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.

[matthiasbeyer]

I think just running it on pushed master would be sufficient, wouldn't it? It still would need to be checked manually from time to time.

[ijackson]

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.

[polarathene]

This does imply that we never test against newer dependencies, is that correct? If yes, could this be an issue at some point?

Potentially, but whom that affects would be difficult to say.

• The intent was to verify that config-rs builds and passes test at the given MSRV.
• This requires pinning versions explicitly otherwise it'll fail to work for the MSRV (at least for dev-dependencies).
• If this branch is run on more recent toolchain (notable changes to resolved deps after Rust 1.60 and more from 1.63), this will also change what patch/minor versions are resolved, which is more notable with implicit deps such as ordered-multimap from rust-ini where it's point releases update to minor (pre-1.0.0 major) releases of hashbrown (which itself does the same for it's deps like ahash).

You can't really get that sort of coverage across toolchains, which is also time dependent.

• We can run CI and pass, even release and the resolver outcome can change without any involvement from config-rs.
• Some features will pass CI just fine if reviewed and merged, but could fail just a day later when the resolution would change but may not be caught until next CI run that could be some time away?

Lockfile is serving it's purpose. If there are issues downstream, users will raise an issue (hopefully), or you'll catch them eventually.

• You could use a scheduled CI job to generate the lock file for the MSRV as documented in #495 and when that fails CI, fix it, merge it, repeat.
• You can additionally have CI run the same checks / tests on more recent toolchain like stable where a lock file shouldn't be necessary, or a separate one Cargo.lock can be used and maintained. Cargo.lock.msrv is specifically maintained a separate lock file since it requires explicit pinning to support the MSRV (-Z msrv-policy would fail due to rust-ini implicit deps).

---

I agree with @ijackson suggestions. I am familiar with Github Actions CI, but bit short on time lately.

• There is scheduled jobs using cron syntax. You can pair it with an action to raise a PR or issue based on outcome.
• For the warning only, you can allow the job to continue after a failed step, then have a separate action react to that by adding a comment. Alternatively as a separate job it can fail but not be required to pass for check suite status, allowing merging anyway.

I have experience with each suggestion and can provide direction, otherwise maybe later this month I can contribute the preferred approach.

---

I think just running it on pushed master would be sufficient, wouldn't it?

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.

[matthiasbeyer]

There is scheduled jobs using cron syntax. You can pair it with an action to raise a PR or issue based on outcome.

I think this would be a good idea.

I have experience with each suggestion and can provide direction, otherwise maybe later this month I can contribute the preferred approach.

I think there is no hurry!

I don't follow this suggestion? This release branch is not in sync with master?

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 :laughing: