Refactor git rev handling infrastructure

This commit unifies the notion of a "git revision" between a SourceId and the
GitSource. This pushes the request of a branch, tag, or revision all the way
down into a GitSource so special care can be taken for each case.

This primarily was discovered by #1069 where a git tag's id is different from
the commit that it points at, and we need to push the knowledge of whether it's
a tag or not all the way down to the point where we resolve what revision we
want (and perform appropriate operations to find the commit we want).

Closes #1069
This commit is contained in:
Alex Crichton 2014-12-08 10:46:07 -08:00
parent b46015585b
commit aa256ffddc
8 changed files with 133 additions and 107 deletions

View file

@ -1,5 +1,5 @@
use cargo::core::MultiShell;
use cargo::core::source::{Source, SourceId};
use cargo::core::source::{Source, SourceId, GitReference};
use cargo::sources::git::{GitSource};
use cargo::util::{Config, CliResult, CliError, human, ToUrl};
@ -30,7 +30,8 @@ pub fn execute(options: Options, shell: &mut MultiShell) -> CliResult<Option<()>
})
.map_err(|e| CliError::from_boxed(e, 1)));
let source_id = SourceId::for_git(&url, reference.as_slice());
let reference = GitReference::Branch(reference.to_string());
let source_id = SourceId::for_git(&url, reference);
let mut config = try!(Config::new(shell, None, None).map_err(|e| {
CliError::from_boxed(e, 1)

View file

@ -6,7 +6,7 @@ pub use self::package_id_spec::PackageIdSpec;
pub use self::registry::Registry;
pub use self::resolver::Resolve;
pub use self::shell::{Shell, MultiShell, ShellConfig};
pub use self::source::{Source, SourceId, SourceMap, SourceSet};
pub use self::source::{Source, SourceId, SourceMap, SourceSet, GitReference};
pub use self::summary::Summary;
pub mod source;

View file

@ -42,16 +42,23 @@ pub trait Source: Registry {
fn fingerprint(&self, pkg: &Package) -> CargoResult<String>;
}
#[deriving(RustcEncodable, RustcDecodable, Show, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
#[deriving(Show, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
enum Kind {
/// Kind::Git(<git reference>) represents a git repository
Git(String),
Git(GitReference),
/// represents a local path
Path,
/// represents the central registry
Registry,
}
#[deriving(Show, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
pub enum GitReference {
Tag(String),
Branch(String),
Rev(String),
}
type Error = Box<CargoError + Send>;
/// Unique identifier for a source of packages.
@ -97,16 +104,22 @@ impl SourceId {
match kind {
"git" => {
let mut url = url.to_url().unwrap();
let mut reference = "master".to_string();
let mut reference = GitReference::Branch("master".to_string());
let pairs = url.query_pairs().unwrap_or(Vec::new());
for &(ref k, ref v) in pairs.iter() {
if k.as_slice() == "ref" {
reference = v.clone();
match k.as_slice() {
// map older 'ref' to branch
"branch" |
"ref" => reference = GitReference::Branch(v.clone()),
"rev" => reference = GitReference::Rev(v.clone()),
"tag" => reference = GitReference::Tag(v.clone()),
_ => {}
}
}
url.query = None;
let precise = mem::replace(&mut url.fragment, None);
SourceId::for_git(&url, reference.as_slice())
SourceId::for_git(&url, reference)
.with_precise(precise)
},
"registry" => {
@ -128,11 +141,7 @@ impl SourceId {
SourceIdInner {
kind: Kind::Git(ref reference), ref url, ref precise, ..
} => {
let ref_str = if reference.as_slice() != "master" {
format!("?ref={}", reference)
} else {
"".to_string()
};
let ref_str = url_ref(reference);
let precise_str = if precise.is_some() {
format!("#{}", precise.as_ref().unwrap())
@ -154,8 +163,8 @@ impl SourceId {
Ok(SourceId::new(Kind::Path, url))
}
pub fn for_git(url: &Url, reference: &str) -> SourceId {
SourceId::new(Kind::Git(reference.to_string()), url.clone())
pub fn for_git(url: &Url, reference: GitReference) -> SourceId {
SourceId::new(Kind::Git(reference), url.clone())
}
pub fn for_registry(url: &Url) -> SourceId {
@ -203,9 +212,9 @@ impl SourceId {
self.inner.precise.as_ref().map(|s| s.as_slice())
}
pub fn git_reference(&self) -> Option<&str> {
pub fn git_reference(&self) -> Option<&GitReference> {
match self.inner.kind {
Kind::Git(ref s) => Some(s.as_slice()),
Kind::Git(ref s) => Some(s),
_ => None,
}
}
@ -269,10 +278,7 @@ impl Show for SourceId {
SourceIdInner { kind: Kind::Path, ref url, .. } => url.fmt(f),
SourceIdInner { kind: Kind::Git(ref reference), ref url,
ref precise, .. } => {
try!(write!(f, "{}", url));
if reference.as_slice() != "master" {
try!(write!(f, "?ref={}", reference));
}
try!(write!(f, "{}{}", url, url_ref(reference)));
match *precise {
Some(ref s) => {
@ -319,6 +325,29 @@ impl<S: hash::Writer> hash::Hash<S> for SourceId {
}
}
fn url_ref(r: &GitReference) -> String {
match r.to_ref_string() {
None => "".to_string(),
Some(s) => format!("?{}", s),
}
}
impl GitReference {
pub fn to_ref_string(&self) -> Option<String> {
match *self {
GitReference::Branch(ref s) => {
if s.as_slice() == "master" {
None
} else {
Some(format!("branch={}", s))
}
}
GitReference::Tag(ref s) => Some(format!("tag={}", s)),
GitReference::Rev(ref s) => Some(format!("rev={}", s)),
}
}
}
pub struct SourceMap<'src> {
map: HashMap<SourceId, Box<Source+'src>>
}
@ -446,20 +475,22 @@ impl<'src> Source for SourceSet<'src> {
#[cfg(test)]
mod tests {
use super::{SourceId, Kind};
use super::{SourceId, Kind, GitReference};
use util::ToUrl;
#[test]
fn github_sources_equal() {
let loc = "https://github.com/foo/bar".to_url().unwrap();
let s1 = SourceId::new(Kind::Git("master".to_string()), loc);
let master = Kind::Git(GitReference::Branch("master".to_string()));
let s1 = SourceId::new(master.clone(), loc);
let loc = "git://github.com/foo/bar".to_url().unwrap();
let s2 = SourceId::new(Kind::Git("master".to_string()), loc.clone());
let s2 = SourceId::new(master, loc.clone());
assert_eq!(s1, s2);
let s3 = SourceId::new(Kind::Git("foo".to_string()), loc);
let foo = Kind::Git(GitReference::Branch("foo".to_string()));
let s3 = SourceId::new(foo, loc);
assert!(s1 != s3);
}
}

View file

@ -5,10 +5,11 @@ use std::mem;
use url::{mod, Url};
use core::source::{Source, SourceId};
use core::GitReference;
use core::{Package, PackageId, Summary, Registry, Dependency};
use util::{CargoResult, Config, to_hex};
use sources::PathSource;
use sources::git::utils::{GitReference, GitRemote, GitRevision};
use sources::git::utils::{GitRemote, GitRevision};
/* TODO: Refactor GitSource to delegate to a PathSource
*/
@ -39,17 +40,23 @@ impl<'a, 'b> GitSource<'a, 'b> {
let db_path = config.git_db_path()
.join(ident.as_slice());
let reference_path = match *reference {
GitReference::Branch(ref s) |
GitReference::Tag(ref s) |
GitReference::Rev(ref s) => s.to_string(),
};
let checkout_path = config.git_checkout_path()
.join(ident.as_slice()).join(reference.as_slice());
.join(ident)
.join(reference_path);
let reference = match source_id.get_precise() {
Some(s) => s,
None => reference.as_slice(),
Some(s) => GitReference::Rev(s.to_string()),
None => source_id.git_reference().unwrap().clone(),
};
GitSource {
remote: remote,
reference: GitReference::for_str(reference.as_slice()),
reference: reference,
db_path: db_path,
checkout_path: checkout_path,
source_id: source_id.clone(),
@ -140,9 +147,9 @@ impl<'a, 'b> Show for GitSource<'a, 'b> {
fn fmt(&self, f: &mut Formatter) -> fmt::Result {
try!(write!(f, "git repo at {}", self.remote.get_url()));
match self.reference {
GitReference::Master => Ok(()),
GitReference::Other(ref reference) => write!(f, " ({})", reference)
match self.reference.to_ref_string() {
Some(s) => write!(f, " ({})", s),
None => Ok(())
}
}
}
@ -157,8 +164,7 @@ impl<'a, 'b> Registry for GitSource<'a, 'b> {
impl<'a, 'b> Source for GitSource<'a, 'b> {
fn update(&mut self) -> CargoResult<()> {
let actual_rev = self.remote.rev_for(&self.db_path,
self.reference.as_slice());
let actual_rev = self.remote.rev_for(&self.db_path, &self.reference);
let should_update = actual_rev.is_err() ||
self.source_id.get_precise().is_none();
@ -168,7 +174,7 @@ impl<'a, 'b> Source for GitSource<'a, 'b> {
log!(5, "updating git source `{}`", self.remote);
let repo = try!(self.remote.checkout(&self.db_path));
let rev = try!(repo.rev_for(self.reference.as_slice()));
let rev = try!(repo.rev_for(&self.reference));
(repo, rev)
} else {
(try!(self.remote.db_at(&self.db_path)), actual_rev.unwrap())

View file

@ -5,52 +5,16 @@ use rustc_serialize::{Encodable, Encoder};
use url::Url;
use git2;
use core::GitReference;
use util::{CargoResult, ChainError, human, ToUrl, internal, Require};
#[deriving(PartialEq,Clone,RustcEncodable)]
pub enum GitReference {
Master,
Other(String)
}
#[deriving(PartialEq,Clone,RustcEncodable)]
pub struct GitRevision(String);
impl GitReference {
pub fn for_str<S: Str>(string: S) -> GitReference {
if string.as_slice() == "master" {
GitReference::Master
} else {
GitReference::Other(string.as_slice().to_string())
}
}
}
impl Str for GitReference {
fn as_slice(&self) -> &str {
match *self {
GitReference::Master => "master",
GitReference::Other(ref string) => string.as_slice()
}
}
}
impl Show for GitReference {
fn fmt(&self, f: &mut Formatter) -> fmt::Result {
self.as_slice().fmt(f)
}
}
impl Str for GitRevision {
fn as_slice(&self) -> &str {
let GitRevision(ref me) = *self;
me.as_slice()
}
}
#[deriving(PartialEq, Clone)]
#[allow(missing_copy_implementations)]
pub struct GitRevision(git2::Oid);
impl Show for GitRevision {
fn fmt(&self, f: &mut Formatter) -> fmt::Result {
self.as_slice().fmt(f)
self.0.fmt(f)
}
}
@ -138,8 +102,8 @@ impl GitRemote {
&self.url
}
pub fn rev_for<S: Str>(&self, path: &Path, reference: S)
-> CargoResult<GitRevision> {
pub fn rev_for(&self, path: &Path, reference: &GitReference)
-> CargoResult<GitRevision> {
let db = try!(self.db_at(path));
db.rev_for(reference)
}
@ -215,9 +179,36 @@ impl GitDatabase {
Ok(checkout)
}
pub fn rev_for<S: Str>(&self, reference: S) -> CargoResult<GitRevision> {
let rev = try!(self.repo.revparse_single(reference.as_slice()));
Ok(GitRevision(rev.id().to_string()))
pub fn rev_for(&self, reference: &GitReference) -> CargoResult<GitRevision> {
let id = match *reference {
GitReference::Tag(ref s) => {
try!((|| {
let refname = format!("refs/tags/{}", s);
let id = try!(self.repo.refname_to_id(refname.as_slice()));
let tag = try!(self.repo.find_tag(id));
let obj = try!(tag.peel());
Ok(obj.id())
}).chain_error(|| {
human(format!("failed to find tag `{}`", s))
}))
}
GitReference::Branch(ref s) => {
try!((|| {
let b = try!(self.repo.find_branch(s.as_slice(),
git2::BranchType::Local));
b.get().target().require(|| {
human(format!("branch `{}` did not have a target", s))
})
}).chain_error(|| {
human(format!("failed to find branch `{}`", s))
}))
}
GitReference::Rev(ref s) => {
let obj = try!(self.repo.revparse_single(s.as_slice()));
obj.id()
}
};
Ok(GitRevision(id))
}
pub fn has_ref<S: Str>(&self, reference: S) -> CargoResult<()> {
@ -249,10 +240,6 @@ impl<'a> GitCheckout<'a> {
Ok(checkout)
}
pub fn get_rev(&self) -> &str {
self.revision.as_slice()
}
fn clone_repo(source: &Path, into: &Path) -> CargoResult<git2::Repository> {
let dirname = into.dir_path();
@ -293,10 +280,8 @@ impl<'a> GitCheckout<'a> {
}
fn reset(&self) -> CargoResult<()> {
info!("reset {} to {}", self.repo.path().display(),
self.revision.as_slice());
let oid = try!(git2::Oid::from_str(self.revision.as_slice()));
let object = try!(self.repo.find_object(oid, None));
info!("reset {} to {}", self.repo.path().display(), self.revision);
let object = try!(self.repo.find_object(self.revision.0, None));
try!(self.repo.reset(&object, git2::ResetType::Hard, None, None));
Ok(())
}

View file

@ -11,7 +11,7 @@ use semver;
use rustc_serialize::{Decodable, Decoder};
use core::SourceId;
use core::{Summary, Manifest, Target, Dependency, PackageId};
use core::{Summary, Manifest, Target, Dependency, PackageId, GitReference};
use core::dependency::Kind;
use core::manifest::{LibKind, Profile, ManifestMetadata};
use core::package_id::Metadata;
@ -571,17 +571,17 @@ fn process_dependencies<'a>(cx: &mut Context<'a>,
}
TomlDependency::Detailed(ref details) => details.clone(),
};
let reference = details.branch.clone()
.or_else(|| details.tag.clone())
.or_else(|| details.rev.clone())
.unwrap_or_else(|| "master".to_string());
let reference = details.branch.clone().map(GitReference::Branch)
.or_else(|| details.tag.clone().map(GitReference::Tag))
.or_else(|| details.rev.clone().map(GitReference::Rev))
.unwrap_or_else(|| GitReference::Branch("master".to_string()));
let new_source_id = match details.git {
Some(ref git) => {
let loc = try!(git.as_slice().to_url().map_err(|e| {
human(e)
}));
Some(SourceId::for_git(&loc, reference.as_slice()))
Some(SourceId::for_git(&loc, reference))
}
None => {
details.path.as_ref().map(|path| {

View file

@ -7,7 +7,7 @@ use std::collections::HashMap;
use hamcrest::{assert_that, equal_to, contains};
use cargo::core::source::SourceId;
use cargo::core::source::{SourceId, GitReference};
use cargo::core::dependency::Kind::Development;
use cargo::core::{Dependency, PackageId, Summary, Registry};
use cargo::util::{CargoResult, ToUrl};
@ -85,7 +85,8 @@ fn pkg_id(name: &str) -> PackageId {
fn pkg_id_loc(name: &str, loc: &str) -> PackageId {
let remote = loc.to_url();
let source_id = SourceId::for_git(&remote.unwrap(), "master");
let master = GitReference::Branch("master".to_string());
let source_id = SourceId::for_git(&remote.unwrap(), master);
PackageId::new(name, "1.0.0", &source_id).unwrap()
}
@ -103,7 +104,8 @@ fn dep_req(name: &str, req: &str) -> Dependency {
fn dep_loc(name: &str, location: &str) -> Dependency {
let url = location.to_url().unwrap();
let source_id = SourceId::for_git(&url, "master");
let master = GitReference::Branch("master".to_string());
let source_id = SourceId::for_git(&url, master);
Dependency::parse(name, Some("1.0.0"), &source_id).unwrap()
}

View file

@ -187,7 +187,7 @@ test!(cargo_compile_git_dep_branch {
assert_that(project.cargo_process("build"),
execs()
.with_stdout(format!("{} git repository `{}`\n\
{} dep1 v0.5.0 ({}?ref=branchy#[..])\n\
{} dep1 v0.5.0 ({}?branch=branchy#[..])\n\
{} foo v0.5.0 ({})\n",
UPDATING, path2url(git_root.clone()),
COMPILING, path2url(git_root),
@ -257,18 +257,19 @@ test!(cargo_compile_git_dep_tag {
assert_that(project.cargo_process("build"),
execs()
.with_stdout(format!("{} git repository `{}`\n\
{} dep1 v0.5.0 ({}?ref=v0.1.0#[..])\n\
{} dep1 v0.5.0 ({}?tag=v0.1.0#[..])\n\
{} foo v0.5.0 ({})\n",
UPDATING, path2url(git_root.clone()),
COMPILING, path2url(git_root),
COMPILING, path2url(root)))
.with_stderr(""));
COMPILING, path2url(root))));
assert_that(&project.bin("foo"), existing_file());
assert_that(
cargo::util::process(project.bin("foo")).unwrap(),
execs().with_stdout("hello world\n"));
assert_that(cargo::util::process(project.bin("foo")).unwrap(),
execs().with_stdout("hello world\n"));
assert_that(project.process(cargo_dir().join("cargo")).arg("build"),
execs().with_status(0));
});
test!(cargo_compile_with_nested_paths {