diff --git a/src/libextra/url.rs b/src/libextra/url.rs index 7570d26901c95..0f2f8697c4fe4 100644 --- a/src/libextra/url.rs +++ b/src/libextra/url.rs @@ -573,7 +573,7 @@ fn get_authority(rawurl: &str) -> // returns the path and unparsed part of url, or an error -fn get_path(rawurl: &str, authority: bool) -> +pub fn get_path(rawurl: &str, authority: bool) -> Result<(~str, ~str), ~str> { let len = rawurl.len(); let mut end = len; diff --git a/src/librustpkg/package_id.rs b/src/librustpkg/package_id.rs index 0da343a27bfca..68c7c812ed8e0 100644 --- a/src/librustpkg/package_id.rs +++ b/src/librustpkg/package_id.rs @@ -8,10 +8,12 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. +use extra::url; use version::{try_getting_version, try_getting_local_version, Version, NoVersion, split_version}; use std::hash::Streaming; use std::hash; +use messages::error; /// Path-fragment identifier of a package such as /// 'github.com/graydon/test'; path must be a relative @@ -41,10 +43,60 @@ impl Eq for PkgId { } } +/// Parses `s` as a URL and, if the parse was successful, returns the +/// portion of the URL without the scheme +/// for example: git://github.com/catamorphism/foo => github.com/catamorphism/foo +fn drop_url_scheme<'a>(s: &'a str) -> Option<~str> { + match url::get_scheme(s) { + Ok((_, rest)) => { + // get_scheme leaves in the leading // + let len = rest.len(); + let path_to_use = if len > 2 { + let no_slashes = rest.slice(2, len); + // Don't display the .git extension, since it's not part of the actual + // package ID + if no_slashes.ends_with(".git") { + no_slashes.slice(0, len - 6).to_owned() + } + else { + no_slashes.to_owned() + } + } else { + rest + }; + Some(path_to_use) + } + Err(_) => None + } +} + +// Fails if this is not a legal package ID, +// after printing a hint +fn ensure_legal_package_id(s: &str) { + // url::get_path checks that the string contains characters + // that are legal in a URL ('#' is legal, so we're good there) + let maybe_intended_path = drop_url_scheme(s); + let legal = maybe_intended_path.is_none() + && url::get_path(s, false).is_ok(); + if !legal { + for maybe_package_id in maybe_intended_path.iter() { + error(format!("rustpkg operates on package IDs; did you mean to write \ + `{}` instead of `{}`?", + *maybe_package_id, + s)); + } + fail!("Can't parse {} as a package ID", s); + } +} + impl PkgId { pub fn new(s: &str) -> PkgId { use conditions::bad_pkg_id::cond; + // Make sure the path is a legal package ID -- it might not even + // be a legal path, so we do this first + ensure_legal_package_id(s); + let mut given_version = None; // Did the user request a specific version? diff --git a/src/librustpkg/tests.rs b/src/librustpkg/tests.rs index bf62e7068f325..24934836dd467 100644 --- a/src/librustpkg/tests.rs +++ b/src/librustpkg/tests.rs @@ -15,8 +15,6 @@ use std::{os, run, str, task}; use std::io; use std::io::fs; use std::io::File; -use std::io::process; -use std::io::process::ProcessExit; use extra::arc::Arc; use extra::arc::RWArc; use extra::tempfile::TempDir; @@ -294,7 +292,7 @@ fn command_line_test_with_env(args: &[~str], cwd: &Path, env: Option<~[(~str, ~s output.status); if !output.status.success() { debug!("Command {} {:?} failed with exit code {:?}; its output was --- {} ---", - cmd, args, output.status, + cmd, args, output.status, str::from_utf8(output.output) + str::from_utf8(output.error)); Fail(output) } @@ -615,7 +613,6 @@ fn test_install_valid() { } #[test] -#[ignore] fn test_install_invalid() { let sysroot = test_sysroot(); let pkgid = fake_pkg(); @@ -631,8 +628,11 @@ fn test_install_invalid() { pkgid.clone()); ctxt.install(pkg_src, &WhatToBuild::new(MaybeCustom, Everything)); }; - assert!(result.unwrap_err() - .to_str().contains("supplied path for package dir does not exist")); + let x = result.unwrap_err(); + assert!(x.is::<~str>()); + let error_string = *x.move::<~str>().unwrap(); + debug!("result error = {}", error_string); + assert!(error_string.contains("supplied path for package dir does not exist")); } #[test] @@ -663,7 +663,6 @@ fn test_install_valid_external() { } #[test] -#[ignore(reason = "9994")] fn test_install_invalid_external() { let cwd = os::getcwd(); command_line_test_expect_fail([~"install", ~"foo"], @@ -2442,6 +2441,21 @@ fn correct_error_dependency() { } } +#[test] +fn test_bad_package_id_url() { + use str::{is_utf8, from_utf8}; + + match command_line_test_partial([~"install", ~"git://github.com/mozilla/servo.git"], + &os::getcwd()) { + Fail(ProcessOutput{ error: _, output: output, _}) => { + assert!(is_utf8(output)); + assert!(from_utf8(output).contains("rustpkg operates on package IDs; did you mean to \ + write `github.com/mozilla/servo` instead")); + } + Success(*) => fail!("test_bad_package_id_url: succeeded but should have failed") + } +} + /// Returns true if p exists and is executable fn is_executable(p: &Path) -> bool { p.exists() && p.stat().perm & io::UserExecute == io::UserExecute