Merge pull request #88 from brson/fix

Basic URL canonicalization. Fixes #84

Author: brson

src/cargo/sources/git/source.rs

@@ -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() == "" {
@@ -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> {
@@ -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);
+    }
 
     fn url(s: &str) -> Url {
         url::from_str(s).unwrap()