Skip to content

Commit

Permalink
Effectively revert rust-lang#8364
Browse files Browse the repository at this point in the history
This commit is intended to be an effective but not literal revert
of rust-lang#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 rust-lang#8468.
  • Loading branch information
alexcrichton committed Jul 23, 2020
1 parent aa68721 commit 538fb1b
Show file tree
Hide file tree
Showing 3 changed files with 216 additions and 18 deletions.
80 changes: 77 additions & 3 deletions src/cargo/core/source/source_id.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use std::cmp::{self, Ordering};
use std::collections::HashSet;
use std::fmt::{self, Formatter};
use std::hash::{self, Hash};
use std::hash::{self, Hash, Hasher};
use std::path::Path;
use std::ptr;
use std::sync::Mutex;
Expand Down Expand Up @@ -59,7 +59,7 @@ enum SourceKind {
}

/// Information to find a specific commit in a Git repository.
#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
#[derive(Debug, Clone)]
pub enum GitReference {
/// From a tag.
Tag(String),
Expand Down Expand Up @@ -553,8 +553,11 @@ impl GitReference {
/// Returns a `Display`able view of this git reference, or None if using
/// the head of the default branch
pub fn pretty_ref(&self) -> Option<PrettyRef<'_>> {
match *self {
match self {
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 }),
}
}
Expand All @@ -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)]
mod tests {
use super::{GitReference, SourceId, SourceKind};
Expand Down
19 changes: 13 additions & 6 deletions src/cargo/sources/git/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -217,12 +217,17 @@ impl GitReference {
.target()
.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 => {
let refname = "refs/remotes/origin/HEAD";
let id = repo.refname_to_id(refname)?;
let obj = repo.find_object(id, None)?;
let obj = obj.peel(ObjectType::Commit)?;
obj.id()
let master = repo
.find_branch("origin/master", git2::BranchType::Remote)
.chain_err(|| "failed to find branch `master`")?;
let master = master
.get()
.target()
.ok_or_else(|| anyhow::format_err!("branch `master` did not have a target"))?;
master
}

GitReference::Rev(s) => {
Expand Down Expand Up @@ -757,6 +762,7 @@ pub fn fetch(
}

GitReference::DefaultBranch => {
refspecs.push(format!("refs/heads/master:refs/remotes/origin/master"));
refspecs.push(String::from("HEAD:refs/remotes/origin/HEAD"));
}

Expand Down Expand Up @@ -984,7 +990,8 @@ fn github_up_to_date(
let github_branch_name = match reference {
GitReference::Branch(branch) => branch,
GitReference::Tag(tag) => tag,
GitReference::DefaultBranch => "HEAD",
// See the module docs for why we're using `master` here.
GitReference::DefaultBranch => "master",
GitReference::Rev(_) => {
debug!("can't use github fast path with `rev`");
return Ok(false);
Expand Down
135 changes: 126 additions & 9 deletions tests/testsuite/git.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use std::fs;
use std::io::prelude::*;
use std::net::{TcpListener, TcpStream};
use std::path::Path;
use std::str;
use std::sync::atomic::{AtomicBool, Ordering};
use std::sync::Arc;
use std::thread;
Expand Down Expand Up @@ -2789,19 +2790,23 @@ to proceed despite [..]
fn default_not_master() {
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| {
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();
let head = repo.find_commit(head_id).unwrap();
repo.branch("main", &head, false).unwrap();
repo.set_head("refs/heads/main").unwrap();

// Then create a `main` branch with actual code, and set the head of the
// repository (default branch) to `main`.
git_project.change_file("src/lib.rs", "pub fn foo() {}");
// Then create a commit on the new `main` branch so `master` and `main`
// differ.
git_project.change_file("src/lib.rs", "");
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();
git::commit(&repo);

let project = project
.file(
Expand Down Expand Up @@ -2832,3 +2837,115 @@ fn default_not_master() {
)
.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();
}

0 comments on commit 538fb1b

Please sign in to comment.