auto merge of rust-lang#72 : Arcterus/cargo/master, r=alexcrichton

Now Cargo should give a normal error message rather than a task failure when encountering an SSH URL as a dependency.
alexcrichton · Jun 27, 2014 · c993a54 · c993a54
2 parents c919f2f + 856f370
commit c993a54
Show file tree

Hide file tree

Showing 3 changed files with 46 additions and 25 deletions.
diff --git a/src/bin/cargo-git-checkout.rs b/src/bin/cargo-git-checkout.rs
@@ -10,7 +10,7 @@ extern crate hammer;
 
 use cargo::{execute_main_without_stdin};
 use cargo::core::MultiShell;
-use cargo::core::source::{Source,SourceId};
+use cargo::core::source::{Source, SourceId};
 use cargo::sources::git::{GitSource};
 use cargo::util::{Config, CliResult, CliError, Require, human};
 use url::Url;

diff --git a/src/cargo/util/toml.rs b/src/cargo/util/toml.rs
@@ -2,12 +2,11 @@ use serialize::Decodable;
 use std::collections::HashMap;
 use std::str;
 use toml;
-use url;
 
 use core::{SourceId, GitKind};
 use core::manifest::{LibKind, Lib, Profile};
 use core::{Summary, Manifest, Target, Dependency, PackageId};
-use core::source::{Location, Local, Remote};
+use core::source::Location;
 use util::{CargoResult, Require, human};
 
 pub fn to_manifest(contents: &[u8],
@@ -118,15 +117,6 @@ impl TomlManifest {
 
         let mut deps = Vec::new();
 
-        fn to_location(s: &str) -> Location {
-            if s.starts_with("file:") {
-                Local(Path::new(s.slice_from(5)))
-            } else {
-                // TODO: Don't unwrap here
-                Remote(url::from_str(s).unwrap())
-            }
-        }
-
         // Collect the deps
         match self.dependencies {
             Some(ref dependencies) => {
@@ -141,19 +131,22 @@ impl TomlManifest {
                                 .or_else(|| details.rev.as_ref().map(|t| t.clone()))
                                 .unwrap_or_else(|| "master".to_str());
 
-                            let new_source_id = details.git.as_ref().map(|git| {
-                                let kind = GitKind(reference.clone());
-                                let loc = to_location(git.as_slice());
-                                let source_id = SourceId::new(kind, loc);
-                                // TODO: Don't do this for path
-                                sources.push(source_id.clone());
-                                source_id
-                            }).or_else(|| {
-                                details.path.as_ref().map(|path| {
-                                    nested_paths.push(Path::new(path.as_slice()));
-                                    source_id.clone()
-                                })
-                            }).unwrap_or(SourceId::for_central());
+                            let new_source_id = match details.git {
+                                Some(ref git) => {
+                                    let kind = GitKind(reference.clone());
+                                    let loc = try!(Location::parse(git.as_slice()));
+                                    let source_id = SourceId::new(kind, loc);
+                                    // TODO: Don't do this for path
+                                    sources.push(source_id.clone());
+                                    Some(source_id)
+                                }
+                                None => {
+                                    details.path.as_ref().map(|path| {
+                                        nested_paths.push(Path::new(path.as_slice()));
+                                        source_id.clone()
+                                    })
+                                }
+                            }.unwrap_or(SourceId::for_central());
 
                             (details.version.clone(), new_source_id)
                         }

diff --git a/tests/test_cargo_compile_git_deps.rs b/tests/test_cargo_compile_git_deps.rs
@@ -302,6 +302,34 @@ test!(cargo_compile_with_nested_paths {
       execs().with_stdout("hello world\n"));
 })
 
+test!(cargo_compile_with_short_ssh_git {
+    let url = "git@github.com:a/dep";
+
+    let project = project("project")
+        .file("Cargo.toml", format!(r#"
+            [project]
+
+            name = "foo"
+            version = "0.5.0"
+            authors = ["wycats@example.com"]
+
+            [dependencies.dep]
+
+            git = "{}"
+
+            [[bin]]
+
+            name = "foo"
+        "#, url))
+        .file("src/foo.rs", main_file(r#""{}", dep1::hello()"#, ["dep1"]));
+
+    assert_that(project.cargo_process("cargo-build"),
+        execs()
+        .with_stdout("")
+        .with_stderr(format!("Cargo.toml is not a valid manifest\n\n\
+                              invalid url `{}`: `url: Invalid character in scheme.\n", url)));
+})
+
 test!(recompilation {
     let git_project = git_repo("bar", |project| {
         project