CI: build with --locked, and add one job for stable build without --locked
#195
Conversation
| - name: Cargo update | ||
| run: cargo update --locked |
There was a problem hiding this comment.
I think we wouldn't want to have cargo update anywhere except in the check-notlocked job.
My understanding is using cargo update with --locked like this will start to exit non-zero if there's an update available with a message like "error: the lock file xxxxxx/Cargo.lock needs to be updated but --locked was passed to prevent this" and that's not what we want.
Based on that I think CI is green right now because we just landed an up-to-date Cargo.lock but it will start to fail soon.
Am I misunderstanding anything?
There was a problem hiding this comment.
I wasn't sure, so I tried to look other repositories and it seems they do not run cargo update. However, I'm wondering if this is necessary or not (for ex, when installing MSRV, cargo update also updates the database of known packages).
I thought it would be harmless - I'm looking at the documentation for an answer
There was a problem hiding this comment.
Based on the cargo book, my understanding is that it is necessary.
It will raise an error if the dependency resolution changed and cause a change in the lock file. If I'm correct, his should not happen except if a dependency was yanked, or changed its own dependencies in an incompatible way.
Thew cargo book also explicitly says (cargo update page, doc for option --locked):
It may be used in environments where deterministic builds are desired, such as in CI pipelines.
There was a problem hiding this comment.
I don't believe it to be necessary (even in the MSRV case) but I'm open to learning more if I'm wrong!
I think the intent is that using --locked throughout CI with a checked-in Cargo.lock means we build with the exact dependency versions specified there in a reliable fashion.
A cargo update is only necessary for the new extra CI task that tests that versions newer than those in the checked-in Cargo.lock work, and we don't need cargo update --locked in CI at all (since it would just verify whether the checked-in Cargo.lock matches the newest available versions for deps).
There was a problem hiding this comment.
Well, it doesn't seem very useful.
I'll send and updated PR.
There was a problem hiding this comment.
Thew cargo book also explicitly says (cargo update page, doc for option --locked):
It may be used in environments where deterministic builds are desired, such as in CI pipelines.
That's an interesting find, thanks for linking. I have a feeling this isn't the right advice for this particular subcommand (it makes more sense for, e.g., cargo build --locked, but not for update IMO). I will try and open an upstream PR to suggest different documentation and see if the cargo folks agree with me or not.
|
Superseded by #196 |
Changes for branch: master
--locked--locked