No longer ignore Cargo.lock

[matthiasbeyer]

This is a proposal to no longer ignore the lockfile for this repository.

Previously there was a guideline to ignore the lockfile for library crate projects (like this). This guideline was since removed. There is some discussion around the subject whether to checkin the lockfile or not.

I want to have this PR as a starting point of a discussion whether we want to change this and what we might want to do in terms of CI. Maybe we want more (less?) jobs for testing with dependencies? Updating dependencies in CI and test against newer versions?

I'll ping some contributors here for opinions.

@ijackson @polarathene

[ijackson]

+1 from me. I think lockfiles should be committed even for library crates.

[ijackson]

And yes you might want a periodic job that tests without the lockfile, but having normal CI test using the locked versions means that your work isn't interrupted by random weather.

[polarathene]

I am mostly familiar with JS dependency management where libraries lockfiles are not relevant to package resolution.

Projects however would always want a lockfile committed and used for builds, since package resolution isn't usually deterministic depending when you build. That seems to be the case with Rust too https://github.com/mehcode/config-rs/pull/478

If the lockfile in a lib doesn't affect downstream projects using config-rs as a dependency, I don't see the harm?

I just recall that it was not good in JS land if dependencies had lockfiles since it was better to just align with semver for more common dependency versions which the projects lockfile could then pin.

[polarathene]

If the ordered-multimap crate patch release was a little later, I assume that this PR could have passed and merged, and without the lockfile, config-rs would start failing in CI runs? (there was also recently this one with ahash, as an indirect dependency via indexmap)

The lockfile would prevent that issue right? But might not for consumers of config-rs?

If so, is the lockfile helping with maintenance of the crate, or would it mislead maintainers and still cause problems?

---

EDIT: I read the nightly FAQ reference, which is also referenced from this Aug 2023 Rust blogpost (while as mentioned, prior advice was not to commit for libs and notes that downstream ignores the Cargo.lock from each crate).

There seems to be good benefits to having the lock file, and scenarios like with rust-ini are perhaps rare?

[matthiasbeyer]

The lockfile would prevent that issue right? But might not for consumers of config-rs?

If so, is the lockfile helping with maintenance of the crate, or would it mislead maintainers and still cause problems?

Yes, that's why I think we need at least one CI job that does not call cargo with --locked, to ensure that it tries to update dependencies as allowed by our Cargo.toml. For the test runs and clippy linting, running with --locked should be totally fine and benefit from caching, but we need one job that does not care about the lockfile.