Change git dependencies to use HEAD by default
#9133
Conversation
This commit follows through with work started in rust-lang#8522 to change the default behavior of `git` dependencies where if not branch/tag/etc is listed then `HEAD` is used instead of the `master` branch. This involves also changing the default lock file format, now including a `version` marker at the top of the file notably as well as changing the encoding of `branch=master` directives in `Cargo.toml`. If we did all our work correctly then this will be a seamless change. First released on stable in 1.47.0 (2020-10-08) Cargo has been emitting warnings about situations which may break in the future. This means that if you don't specify `branch = 'master'` but your HEAD branch isn't `master`, you've been getting a warning. Similarly if your dependency graph used both `branch = 'master'` as well as specifying nothing, you were receiving warnings as well. These two situations are broken by this commit, but it's hoped that by giving enough times with warnings we don't actually break anyone in practice.
This commit fixes a bug in Cargo where after `DefaultBranch` and
`Branch("master")` are considered distinct no v2 lockfile would by
default match a dependency specification with the `branch = 'master'`
annotation. A feature of the v2 lockfile, however, is that no mention of
a branch is equivalent to the `master` branch.
The bug here is fixed by updating the code which registers a previous
lock file with our `PackageRegistry`. This code registers nodes, only
with v2 lock files, for both default and `master` branch git
dependencies. This way requests for either one will get matched now that
they're considered distinct.
|
r? @ehuss (rust-highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
|
@rfcbot fcp merge This switches the default format for |
|
Team member @ehuss has proposed to merge this. The next step is review by the rest of the tagged team members: No concerns currently listed. Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! See this document for info about what commands tagged team members can give me. |
rfcbot
added
proposed-final-comment-period
|
🔔 This is now entering its final comment period, as per the review above. 🔔 |
rfcbot
removed
the
proposed-final-comment-period
|
@bors r+ |
|
📌 Commit 9f2ce1f has been approved by |
bors
added
S-waiting-on-bors
|
☀️ Test successful - checks-actions |
Update cargo 5 commits in 34170fcd6e0947808a1ac63ac85ffc0da7dace2f..ab64d1393b5b77c66b6534ef5023a1b89ee7bf64 2021-02-04 15:52:52 +0000 to 2021-02-10 00:19:10 +0000 - Allow `true` and `false` as options for `strip` option (rust-lang/cargo#9153) - Change git dependencies to use `HEAD` by default (rust-lang/cargo#9133) - appendix auth gcm link to new repo (which is xplat) (rust-lang/cargo#9152) - Fix warnings of the new non_fmt_panic lint (rust-lang/cargo#9148) - Fix panic with doc collision orphan. (rust-lang/cargo#9142)
Refs: rust-lang/cargo#9133 Co-authored-by: Taiki Yoshida <yoshida.taiki@bpsinc.jp>
rfcbot
added
finished-final-comment-period
This commit fixes an issue where the order of packages serialized into a lock file differs on stable vs nightly. This is due to a bug introduced in rust-lang#9133 where a manual `Ord` implementation was replaced with a `#[derive]`'d one. This was an unintended consequence of rust-lang#9133 and means that the same lock file produced by two different versions of Cargo only differs in what order items are serialized. With rust-lang#9133 being reverted soon on the current beta channel this is intended to be the nightly fix for rust-lang#9334. This will hopefully mean that those projects which don't build with beta/nightly will remain unaffected, and those affected on beta/nightly will need to switch to the new nightly ordering when it's published (which matches the current stable). The reverted beta will match this ordering as well. Closes rust-lang#9334
[beta] Revert #9133, moving to git HEAD dependencies by default This PR reverts #9133 on the beta branch. The reason for this is that I would like to take some more time to investigate fixing #9352 and #9334. I think those should be relatively easy to fix but I would prefer to not be too rushed with the next release happening soon. We realized today in the cargo meeting that this revert is likely to not have a ton of impact. For any projects using the v3 lock file format they'll continue to use it successfully. For projects on stable still using v2 they remain unaffected. This will ideally simply give some more time to fix these issues on nightly.
Fix disagreement about lockfile ordering on stable/nightly This commit fixes an issue where the order of packages serialized into a lock file differs on stable vs nightly. This is due to a bug introduced in #9133 where a manual `Ord` implementation was replaced with a `#[derive]`'d one. This was an unintended consequence of #9133 and means that the same lock file produced by two different versions of Cargo only differs in what order items are serialized. With #9133 being reverted soon on the current beta channel this is intended to be the nightly fix for #9334. This will hopefully mean that those projects which don't build with beta/nightly will remain unaffected, and those affected on beta/nightly will need to switch to the new nightly ordering when it's published (which matches the current stable). The reverted beta will match this ordering as well. Closes #9334
|
I'm stating an obvious here, but all the issues that have been raised about this seem to concern the use of cargo directly alone. But in reality, cargo is used both directly, and as a crate depended on by other utilities. For example So maybe it should be publicly recommended that users state the branch name explicitly for a while, until the ecosystem is caught up with cargo. |
[beta] Beta rollups - Upgrade expat dependency in riscv64 to newer version. rust-lang#84394 - Fix Self keyword doc URL conflict on case insensitive file systems (until definitely fixed on rustdoc) rust-lang#83678 - Cargo: - Backport "Don't give a hard error when the end-user specifies RUSTC_BOOTSTRAP=crate_name" (rust-lang/cargo#9385) - [beta] Revert rust-lang/cargo#9133, moving to git HEAD dependencies by default (rust-lang/cargo#9383)
|
Consumers of |
|
Since lock files are kept in version control, will catching a "version = 3" marker once a contributor uses an updated cargo have any compatibility implications? (Apologies for asking here, this is the first issue linked from the changelog entry) |
|
Cargos going back a while know how to use a "version = 3" lockfile. So if a new Cargo makes one, they will work with it just fine. I believe older Cargo will see it as a corrupted lockfile and regenerate one it does understand. Which may be annoying to diffs, but will still work. |
This commit follows through with work started in #8522 to change the
default behavior of
gitdependencies where if not branch/tag/etc islisted then
HEADis used instead of themasterbranch. This involvesalso changing the default lock file format, now including a
versionmarker at the top of the file notably as well as changing the encoding
of
branch=masterdirectives inCargo.toml.If we did all our work correctly then this will be a seamless change.
First released on stable in 1.47.0 (2020-10-08) Cargo has been emitting
warnings about situations which may break in the future. This means that
if you don't specify
branch = 'master'but your HEAD branch isn'tmaster, you've been getting a warning. Similarly if your dependencygraph used both
branch = 'master'as well as specifying nothing, youwere receiving warnings as well. These two situations are broken by this
commit, but it's hoped that by giving enough times with warnings we
don't actually break anyone in practice.