Evaluating Cargo's change in handling default git branches

Hello all, the Cargo team would like to cast a wider net in asking for feedback about a recent change in rust-lang/cargo#8364. Historically Cargo has treated these two dependency directives as exactly equivalent (the first desugars to the second):

[dependencies]
foo = { git = "https://example.org/foo" }
foo = { git = "https://example.org/foo", branch = "master" }

Unfortunately this means though that repositories whose default branch is not called master are not supported well, you have historically needed to type branch = "other-name" to actually depend on that git repository.

In rust-lang/cargo#8364 Cargo's behavior changed to interpret the two dependency directives above as distinct, the first being "whatever HEAD is" and the latter is "specifically just the master branch". This change, prior to landing, was known to cause breakage. After landing rust-lang/cargo#8468 has also been reported with breakage we did not expect. Specifically the following scenarios break:

  • If you previously did not mention branch, then if the git repository you're depending on has both a master branch and a default non-master branch, then this will break.
  • Due to other changes in how Cargo fetches git commits if you check out a repository with a lock file then cargo build will fail if the default branch is not actually called master. (Cargo only fetches the HEAD reference now, not refs/heads/master)
  • From rust-lang/cargo#8468, if you use branch = "master" in your Cargo.toml then the lock file is no longer compatible across Cargo before and after this change.

We're a bit unsure in the Cargo team about how best to handle this change. This is something we'd like to land eventually no matter what. It is possible for us to land this without the third point of breakage around lock files, but that would take quite a long time to deploy since it requires changing the lock file format in one way or another.

This is where we're asking for your help! Have you been broken recently by this change? Is this something you were easily able to work around? Are there other forms of breakage that we're not aware of? We're hoping to get some feedback about the practical impact of this change to help us inform what we should be doing here. Any input is greatly appreciated!

11 Likes

My projects were not broken by this change, but possible solution to avoid breakage would be to gate the change to the next edition: when a crate is edition = "2018" or lower the current behavior is kept, and starting from the next edition the default is HEAD. To aid the transition rustfix could also be changed to add branch = "master" to all existing git dependencies.

8 Likes

This does seem like exactly the sort of transition for which Editions are a good match.

As I see it { git = "https://example.org/foo" } should have the semantics of git clone that url and then path= depend on the result. That is normatively what it should be like. That feels like the least astonishment behavior to me. As such the current behavior of assuming branch = "master" is a bug and just wrong.

So I am reading this as a bug fix that breaks users. That does not tell me how we roll out the fix nor how fast.

5 Likes

An independent issue related to vendoring has been discovered as a regression of this Cargo PR as well. That's enough that I believe we need to revert this change, so I'm going to figure out a different way to roll out this change.

1 Like

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.