Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

include Cargo.lock? #19

Closed
sethp opened this issue Mar 6, 2023 · 2 comments
Closed

include Cargo.lock? #19

sethp opened this issue Mar 6, 2023 · 2 comments

Comments

@sethp
Copy link
Contributor

sethp commented Mar 6, 2023

I've observed that we're currently gitignoring Cargo.lock:

Cargo.lock

As a big fan of deterministic builds, this seems unfortunate to me. To wit:

The purpose of a Cargo.lock lockfile is to describe the state of the world at the time of a successful build. Cargo uses the lockfile to provide deterministic builds on different times and different systems, by ensuring that the exact same dependencies and versions are used as when the Cargo.lock file was originally generated.

https://doc.rust-lang.org/cargo/faq.html#why-do-binaries-have-cargolock-in-version-control-but-not-libraries

Feels like a useful thing to have, so we can be relatively sure about consistently reproducing results. I think we're both big fans of not constantly fighting over noisy changes, but perhaps the frame shift here is "any commit that does not intentionally update a dependency ought not contain a different Cargo.lock from its parent"? Do we know what kinds of nondeterminism we're exposed to, here? Obviously there's gonna be "semver nonsense", but I think the path there would be to pin all of our dependencies that "flux" to a particular version (and implement #18 so they don't get stale).

NB: this might be related to #14 , I'm not sure if that's a source of (this kind of) nondeterminism or not.

sethp added a commit that referenced this issue Mar 14, 2023
Part of addressing issue #19.
@sethp
Copy link
Contributor Author

sethp commented Mar 15, 2023

Ok, so in #24 we've got a check to make it more visible when we unintentionally modify the Cargo.lock, but it also sounds like that's not the expected behavior: Cargo.lock is only supposed to change in response to either Cargo.toml changes or an intentional cargo update. We don't have a way to make implicit cargo update changes explicit, sadly (I asked about this a bit over here: https://users.rust-lang.org/t/inconsistent-cargo-patch-for-transitive-dependencies/90942 ), so we're going to have to find a slightly different strategy for handling those.

Maybe we modify the check to pass if the PR has a certain label1? I suspect it'll come up for #18 because those will likely modify Cargo.lock without touching Cargo.toml, but we'll want the CI to pass them anyway (or, at least, not fail them for that reason).

Footnotes

  1. seems easy enough to invert a check like this one: https://stackoverflow.com/a/74829754/151464

@sethp sethp closed this as completed Mar 15, 2023
@sethp
Copy link
Contributor Author

sethp commented Mar 21, 2023

Three facts about cargo I learned via that discussion thread:

  1. cargo maintains internal dependency forks on major version (e.g. 1.0.0 vs 2.0.0) as entirely separate packages; so you can have both crate v1.0.0 and crate v2.0.0 in a single artifact just like you could have both serde and nom. Including that it would be the same kind of category error to pass a v1.0.0 struct to a v2.0.0 function as it would be to pass a nom struct to a function with a signature matching a serde type.
  2. for 0.x releases, each minor version is treated as a major revision (i.e. v0.2.0 is a major version away from v0.1.0)
  3. a dependency specification like 1.2.3 means ">=1.2.3, <2.0.0" (e.g. 1.2.4 or 1.4.5,or any any other version greater than x.2.3 in the 1.y.z series)

Additionally, Cargo offers two strategies for dependency resolution:

Minimal Versions: pick the "leftmost" end of the range wherein each major version's requirements are satisfied. Optional, can be invoked on a command-by-command basis with -Zminimal-versions.
Maximal Versions: pick the "rightmost" end of the range wherein each major version's requirements are satisfied. The default.

Notably, neither strategy unifies major versions: only two dependencies on the same major version may conflict. If, say, one requires a package version >=1.2.3 and the other requires <1.2.0, cargo will fail to resolve, but >=2.2.3 and <1.2.0 will resolve just fine to two distinct packages. In combination with the special handling for 0.x releases, this implies that two requirements targeting v0.7.0 and v0.8.0 of the "same" dependency will be resolved as two distinct, separately built, non-interoperable packages.1

Finally, Cargo.lock forms a "materialized view" or "snapshot" of a particular resolution for a dependency graph. It's intended to be used in perpetuity as long as it's 1) present and 2) complete. The only way it'll be incomplete is if the requirements changed (e.g. adding or upgrading a dependency), and the requirements can't be satisfied from the existing resolution.2 The lock file usually exists for the "root" package, as cargo build always produces a Cargo.lock file for the package where it's invoked, but is usually unavailable for dependencies. It's recommended that packages with no possible dependents (i.e. binaries3) provide a Cargo.lock, but that any package intended to be used by a downstream consumer should explicitly not provide a Cargo.lock. This snapshot is managed with a dedicated sub-command, cargo update,4 that can be used to re-resolve either the entire graph (or a subgraph) against the latest repository state.

Footnotes

  1. Editorializing a bit, here, but there's a conflict between cargo's model and what appears to be the most common development strategy for a given crate. Most crates seem to follow a "trunk-based development" model, where there is a single current version with a static historical record of releases (i.e. tags), whereas cargo operates as if each major release were its own independently developed long-lived software artifact (i.e. branches).

  2. Changing a requirement to more strictly match the existing resolution, for example, doesn't change Cargo.lock.

  3. Though, confusingly, it seems that the default behavior of cargo install is to ignore Cargo.lock; you have to invoke it as cargo install --locked for the recommendation to apply.

  4. Not to be confused with cargo upgrade, the installable third-party tool that checks Cargo.toml for requirements that might be behind the latest release of their referents. As with brew up{date,grade} or with USB plug orientation, the one you want is always the third one you try.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant