Effectively revert #8364

This commit is intended to be an effective but not literal revert
of #8364. Internally Cargo will still distinguish between
`DefaultBranch` and `Branch("master")` when reading `Cargo.toml` files,
but for almost all purposes the two are equivalent. This will namely fix
the issue we have with lock file encodings where both are encoded with
no `branch` (and without a branch it's parsed from a lock file as
`DefaultBranch`).

This will preserve the change that `cargo vendor` will not print out
`branch = "master"` annotations but that desugars to match the lock file
on the other end, so it should continue to work.

Tests have been added in this commit for the regressions found on #8468.
This commit is contained in:
Alex Crichton 2020-07-20 08:47:57 -07:00
parent aa6872140a
commit 538fb1b4ed
3 changed files with 217 additions and 19 deletions

View File

@ -1,7 +1,7 @@
use std::cmp::{self, Ordering}; use std::cmp::{self, Ordering};
use std::collections::HashSet; use std::collections::HashSet;
use std::fmt::{self, Formatter}; use std::fmt::{self, Formatter};
use std::hash::{self, Hash}; use std::hash::{self, Hash, Hasher};
use std::path::Path; use std::path::Path;
use std::ptr; use std::ptr;
use std::sync::Mutex; use std::sync::Mutex;
@ -59,7 +59,7 @@ enum SourceKind {
} }
/// Information to find a specific commit in a Git repository. /// Information to find a specific commit in a Git repository.
#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] #[derive(Debug, Clone)]
pub enum GitReference { pub enum GitReference {
/// From a tag. /// From a tag.
Tag(String), Tag(String),
@ -553,8 +553,11 @@ impl GitReference {
/// Returns a `Display`able view of this git reference, or None if using /// Returns a `Display`able view of this git reference, or None if using
/// the head of the default branch /// the head of the default branch
pub fn pretty_ref(&self) -> Option<PrettyRef<'_>> { pub fn pretty_ref(&self) -> Option<PrettyRef<'_>> {
match *self { match self {
GitReference::DefaultBranch => None, GitReference::DefaultBranch => None,
// See module comments in src/cargo/sources/git/utils.rs for why
// `DefaultBranch` is treated specially here.
GitReference::Branch(m) if m == "master" => None,
_ => Some(PrettyRef { inner: self }), _ => Some(PrettyRef { inner: self }),
} }
} }
@ -576,6 +579,77 @@ impl<'a> fmt::Display for PrettyRef<'a> {
} }
} }
// See module comments in src/cargo/sources/git/utils.rs for why `DefaultBranch`
// is treated specially here.
impl PartialEq for GitReference {
fn eq(&self, other: &GitReference) -> bool {
match (self, other) {
(GitReference::Tag(a), GitReference::Tag(b)) => a == b,
(GitReference::Rev(a), GitReference::Rev(b)) => a == b,
(GitReference::Branch(b), GitReference::DefaultBranch)
| (GitReference::DefaultBranch, GitReference::Branch(b)) => b == "master",
(GitReference::DefaultBranch, GitReference::DefaultBranch) => true,
(GitReference::Branch(a), GitReference::Branch(b)) => a == b,
_ => false,
}
}
}
impl Eq for GitReference {}
// See module comments in src/cargo/sources/git/utils.rs for why `DefaultBranch`
// is treated specially here.
impl Hash for GitReference {
fn hash<H: Hasher>(&self, hasher: &mut H) {
match self {
GitReference::Tag(a) => {
0u8.hash(hasher);
a.hash(hasher);
}
GitReference::Rev(a) => {
1u8.hash(hasher);
a.hash(hasher);
}
GitReference::Branch(a) => {
2u8.hash(hasher);
a.hash(hasher);
}
GitReference::DefaultBranch => {
2u8.hash(hasher);
"master".hash(hasher);
}
}
}
}
impl PartialOrd for GitReference {
fn partial_cmp(&self, other: &GitReference) -> Option<Ordering> {
Some(self.cmp(other))
}
}
// See module comments in src/cargo/sources/git/utils.rs for why `DefaultBranch`
// is treated specially here.
impl Ord for GitReference {
fn cmp(&self, other: &GitReference) -> Ordering {
use GitReference::*;
match (self, other) {
(Tag(a), Tag(b)) => a.cmp(b),
(Tag(_), _) => Ordering::Less,
(_, Tag(_)) => Ordering::Greater,
(Rev(a), Rev(b)) => a.cmp(b),
(Rev(_), _) => Ordering::Less,
(_, Rev(_)) => Ordering::Greater,
(Branch(b), DefaultBranch) => b.as_str().cmp("master"),
(DefaultBranch, Branch(b)) => "master".cmp(b),
(Branch(a), Branch(b)) => a.cmp(b),
(DefaultBranch, DefaultBranch) => Ordering::Equal,
}
}
}
#[cfg(test)] #[cfg(test)]
mod tests { mod tests {
use super::{GitReference, SourceId, SourceKind}; use super::{GitReference, SourceId, SourceKind};

View File

@ -217,12 +217,17 @@ impl GitReference {
.target() .target()
.ok_or_else(|| anyhow::format_err!("branch `{}` did not have a target", s))? .ok_or_else(|| anyhow::format_err!("branch `{}` did not have a target", s))?
} }
// See the module docs for why we're using `master` here.
GitReference::DefaultBranch => { GitReference::DefaultBranch => {
let refname = "refs/remotes/origin/HEAD"; let master = repo
let id = repo.refname_to_id(refname)?; .find_branch("origin/master", git2::BranchType::Remote)
let obj = repo.find_object(id, None)?; .chain_err(|| "failed to find branch `master`")?;
let obj = obj.peel(ObjectType::Commit)?; let master = master
obj.id() .get()
.target()
.ok_or_else(|| anyhow::format_err!("branch `master` did not have a target"))?;
master
} }
GitReference::Rev(s) => { GitReference::Rev(s) => {
@ -757,6 +762,7 @@ pub fn fetch(
} }
GitReference::DefaultBranch => { GitReference::DefaultBranch => {
refspecs.push(format!("refs/heads/master:refs/remotes/origin/master"));
refspecs.push(String::from("HEAD:refs/remotes/origin/HEAD")); refspecs.push(String::from("HEAD:refs/remotes/origin/HEAD"));
} }
@ -984,7 +990,8 @@ fn github_up_to_date(
let github_branch_name = match reference { let github_branch_name = match reference {
GitReference::Branch(branch) => branch, GitReference::Branch(branch) => branch,
GitReference::Tag(tag) => tag, GitReference::Tag(tag) => tag,
GitReference::DefaultBranch => "HEAD", // See the module docs for why we're using `master` here.
GitReference::DefaultBranch => "master",
GitReference::Rev(_) => { GitReference::Rev(_) => {
debug!("can't use github fast path with `rev`"); debug!("can't use github fast path with `rev`");
return Ok(false); return Ok(false);

View File

@ -5,6 +5,7 @@ use std::fs;
use std::io::prelude::*; use std::io::prelude::*;
use std::net::{TcpListener, TcpStream}; use std::net::{TcpListener, TcpStream};
use std::path::Path; use std::path::Path;
use std::str;
use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::atomic::{AtomicBool, Ordering};
use std::sync::Arc; use std::sync::Arc;
use std::thread; use std::thread;
@ -2789,20 +2790,24 @@ to proceed despite [..]
fn default_not_master() { fn default_not_master() {
let project = project(); let project = project();
// Create a repository with a `master` branch ... // Create a repository with a `master` branch, but switch the head to a
// branch called `main` at the same time.
let (git_project, repo) = git::new_repo("dep1", |project| { let (git_project, repo) = git::new_repo("dep1", |project| {
project.file("Cargo.toml", &basic_lib_manifest("dep1")) project
.file("Cargo.toml", &basic_lib_manifest("dep1"))
.file("src/lib.rs", "pub fn foo() {}")
}); });
let head_id = repo.head().unwrap().target().unwrap();
// Then create a `main` branch with actual code, and set the head of the let head = repo.find_commit(head_id).unwrap();
// repository (default branch) to `main`. repo.branch("main", &head, false).unwrap();
git_project.change_file("src/lib.rs", "pub fn foo() {}");
git::add(&repo);
let rev = git::commit(&repo);
let commit = repo.find_commit(rev).unwrap();
repo.branch("main", &commit, false).unwrap();
repo.set_head("refs/heads/main").unwrap(); repo.set_head("refs/heads/main").unwrap();
// Then create a commit on the new `main` branch so `master` and `main`
// differ.
git_project.change_file("src/lib.rs", "");
git::add(&repo);
git::commit(&repo);
let project = project let project = project
.file( .file(
"Cargo.toml", "Cargo.toml",
@ -2832,3 +2837,115 @@ fn default_not_master() {
) )
.run(); .run();
} }
#[cargo_test]
fn historical_lockfile_works() {
let project = project();
let (git_project, repo) = git::new_repo("dep1", |project| {
project
.file("Cargo.toml", &basic_lib_manifest("dep1"))
.file("src/lib.rs", "")
});
let head_id = repo.head().unwrap().target().unwrap();
let project = project
.file(
"Cargo.toml",
&format!(
r#"
[project]
name = "foo"
version = "0.5.0"
[dependencies]
dep1 = {{ git = '{}', branch = 'master' }}
"#,
git_project.url()
),
)
.file("src/lib.rs", "")
.build();
project.cargo("build").run();
project.change_file(
"Cargo.lock",
&format!(
r#"# This file is automatically @generated by Cargo.
# It is not intended for manual editing.
[[package]]
name = "dep1"
version = "0.5.0"
source = "git+{}#{}"
[[package]]
name = "foo"
version = "0.5.0"
dependencies = [
"dep1",
]
"#,
git_project.url(),
head_id
),
);
project
.cargo("build")
.with_stderr("[FINISHED] [..]\n")
.run();
}
#[cargo_test]
fn historical_lockfile_works_with_vendor() {
let project = project();
let (git_project, repo) = git::new_repo("dep1", |project| {
project
.file("Cargo.toml", &basic_lib_manifest("dep1"))
.file("src/lib.rs", "")
});
let head_id = repo.head().unwrap().target().unwrap();
let project = project
.file(
"Cargo.toml",
&format!(
r#"
[project]
name = "foo"
version = "0.5.0"
[dependencies]
dep1 = {{ git = '{}', branch = 'master' }}
"#,
git_project.url()
),
)
.file("src/lib.rs", "")
.build();
let output = project.cargo("vendor").exec_with_output().unwrap();
project.change_file(".cargo/config", str::from_utf8(&output.stdout).unwrap());
project.change_file(
"Cargo.lock",
&format!(
r#"# This file is automatically @generated by Cargo.
# It is not intended for manual editing.
[[package]]
name = "dep1"
version = "0.5.0"
source = "git+{}#{}"
[[package]]
name = "foo"
version = "0.5.0"
dependencies = [
"dep1",
]
"#,
git_project.url(),
head_id
),
);
project.cargo("build").run();
}