Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 65 additions & 2 deletions src/cargo/sources/git/source.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,10 @@ fn ident(location: &Location) -> String {
let last = path.components().last().unwrap();
str::from_utf8(last).unwrap().to_str()
}
Remote(ref url) => url.path.as_slice().split('/').last().unwrap().to_str()
Remote(ref url) => {
let path = canonicalize_url(url.path.as_slice());
path.as_slice().split('/').last().unwrap().to_str()
}
};

let ident = if ident.as_slice() == "" {
Expand All @@ -76,7 +79,47 @@ fn ident(location: &Location) -> String {
ident
};

format!("{}-{}", ident, to_hex(hasher.hash(&location.to_str())))
let location = canonicalize_url(location.to_str().as_slice());

format!("{}-{}", ident, to_hex(hasher.hash(&location.as_slice())))
}

fn strip_trailing_slash<'a>(path: &'a str) -> &'a str {
// Remove the trailing '/' so that 'split' doesn't give us
// an empty string, making '../foo/' and '../foo' both
// result in the name 'foo' (#84)
if path.as_bytes().last() != Some(&('/' as u8)) {
path.clone()
} else {
path.slice(0, path.len() - 1)
}
}

// Some hacks and heuristics for making equivalent URLs hash the same
fn canonicalize_url(url: &str) -> String {
let url = strip_trailing_slash(url);

// HACKHACK: For github URL's specifically just lowercase
// everything. GitHub traits both the same, but they hash
// differently, and we're gonna be hashing them. This wants a more
// general solution, and also we're almost certainly not using the
// same case conversion rules that GitHub does. (#84)

let lower_url = url.chars().map(|c|c.to_lowercase()).collect::<String>();
let url = if lower_url.as_slice().contains("github.com") {
lower_url
} else {
url.to_string()
};

// Repos generally can be accessed with or w/o '.git'
let url = if !url.as_slice().ends_with(".git") {
url
} else {
url.as_slice().slice(0, url.len() - 4).to_string()
};

return url;
}

impl<'a, 'b> Show for GitSource<'a, 'b> {
Expand Down Expand Up @@ -150,6 +193,26 @@ mod test {
assert_eq!(ident.as_slice(), "_empty-fc065c9b6b16fc00");
}

#[test]
fn test_canonicalize_idents_by_stripping_trailing_url_slash() {
let ident1 = ident(&Remote(url("https://github.com/PistonDevelopers/piston/")));
let ident2 = ident(&Remote(url("https://github.com/PistonDevelopers/piston")));
assert_eq!(ident1, ident2);
}

#[test]
fn test_canonicalize_idents_by_lowercasing_github_urls() {
let ident1 = ident(&Remote(url("https://github.com/PistonDevelopers/piston")));
let ident2 = ident(&Remote(url("https://github.com/pistondevelopers/piston")));
assert_eq!(ident1, ident2);
}

#[test]
fn test_canonicalize_idents_by_stripping_dot_git() {
let ident1 = ident(&Remote(url("https://github.com/PistonDevelopers/piston")));
let ident2 = ident(&Remote(url("https://github.com/PistonDevelopers/piston.git")));
assert_eq!(ident1, ident2);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just in case we change the hashing algorithm in the future, could this test just test equality of all the urls when hashed? (don't actually look at the specific hash).

Could you also add a test for case insensitivity? The one above just calculates one hash but it's not clear from the test that the hash was generated from a lowercase url.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will, but this is following the existing pattern.


fn url(s: &str) -> Url {
url::from_str(s).unwrap()
Expand Down