Move from all-URL to Path/URL enum

On windows a path cannot be represented as a file:// URL because of the
backslashes and colons in the file name. This causes all of the tests which rely
on git to fail on windows. This commit changes the representation of the
location of a package to be an enum, Location, with two variants: Remote and
Local.

When parsing Cargo.toml, all locations which begin with the string "file:" have
that prefix stripped and are then interpreted as Local packages. Everything else
is parsed as a URL and used as a Remote package.
This commit is contained in:
Alex Crichton 2014-06-24 22:02:21 -07:00
parent df3b3c0b34
commit 1a7a48844c
7 changed files with 130 additions and 105 deletions

View file

@ -123,7 +123,7 @@ impl Package {
// Sort the sources just to make sure we have a consistent fingerprint.
sources.sort_by(|a, b| {
cmp::lexical_ordering(a.kind.cmp(&b.kind),
a.url.to_str().cmp(&b.url.to_str()))
a.location.to_str().cmp(&b.location.to_str()))
});
let sources = sources.iter().map(|source_id| {
source_id.load(config)

View file

@ -10,7 +10,8 @@ use serialize::{
Decoder
};
use util::{CargoResult, CargoError};
use util::{CargoResult, CargoError, FromError};
use core::source::Location;
trait ToVersion {
fn to_version(self) -> Result<semver::Version, String>;
@ -57,7 +58,7 @@ impl<'a> ToUrl for &'a Url {
pub struct PackageId {
name: String,
version: semver::Version,
namespace: Url
namespace: Location,
}
#[deriving(Clone, Show, PartialEq)]
@ -77,14 +78,13 @@ impl CargoError for PackageIdError {
}
impl PackageId {
pub fn new<T: ToVersion, U: ToUrl>(name: &str, version: T,
namespace: U) -> CargoResult<PackageId> {
pub fn new<T: ToVersion>(name: &str, version: T,
ns: &Location) -> CargoResult<PackageId> {
let v = try!(version.to_version().map_err(InvalidVersion));
let ns = try!(namespace.to_url().map_err(InvalidNamespace));
Ok(PackageId {
name: name.to_str(),
version: v,
namespace: ns
namespace: ns.clone()
})
}
@ -96,7 +96,7 @@ impl PackageId {
&self.version
}
pub fn get_namespace<'a>(&'a self) -> &'a Url {
pub fn get_namespace<'a>(&'a self) -> &'a Location {
&self.namespace
}
}
@ -125,7 +125,7 @@ impl<D: Decoder<Box<CargoError>>> Decodable<D,Box<CargoError>> for PackageId {
PackageId::new(
vector.get(0).as_slice(),
vector.get(1).as_slice(),
vector.get(2).as_slice())
&Location::parse(vector.get(2).as_slice()).unwrap())
}
}

View file

@ -57,56 +57,43 @@ pub fn resolve<R: Registry>(deps: &[Dependency],
#[cfg(test)]
mod test {
use url;
use hamcrest::{assert_that, equal_to, contains};
use hamcrest::{
assert_that,
equal_to,
contains
};
use core::source::{
SourceId,
RegistryKind
};
use core::{
Dependency,
PackageId,
Summary
};
use super::{
resolve
};
use core::source::{SourceId, RegistryKind, Location, Remote};
use core::{Dependency, PackageId, Summary};
use super::resolve;
macro_rules! pkg(
($name:expr => $($deps:expr),+) => (
{
let url = url::from_str("http://example.com").unwrap();
let source_id = SourceId::new(RegistryKind, url);
let source_id = SourceId::new(RegistryKind, Remote(url));
let d: Vec<Dependency> = vec!($($deps),+).iter().map(|s| {
Dependency::parse(*s, Some("1.0.0"), &source_id).unwrap()
}).collect();
Summary::new(&PackageId::new($name, "1.0.0",
"http://www.example.com/").unwrap(),
Summary::new(&PackageId::new($name, "1.0.0", &registry_loc()).unwrap(),
d.as_slice())
}
);
($name:expr) => (
Summary::new(&PackageId::new($name, "1.0.0",
"http://www.example.com/").unwrap(), [])
Summary::new(&PackageId::new($name, "1.0.0", &registry_loc()).unwrap(),
[])
)
)
fn registry_loc() -> Location {
Location::parse("http://www.example.com/").unwrap()
}
fn pkg(name: &str) -> Summary {
Summary::new(&PackageId::new(name, "1.0.0", "http://www.example.com/").unwrap(),
Summary::new(&PackageId::new(name, "1.0.0", &registry_loc()).unwrap(),
&[])
}
fn dep(name: &str) -> Dependency {
let url = url::from_str("http://example.com").unwrap();
let source_id = SourceId::new(RegistryKind, url);
let source_id = SourceId::new(RegistryKind, Remote(url));
Dependency::parse(name, Some("1.0.0"), &source_id).unwrap()
}
@ -116,7 +103,7 @@ mod test {
fn names(names: &[&'static str]) -> Vec<PackageId> {
names.iter()
.map(|name| PackageId::new(*name, "1.0.0", "http://www.example.com/").unwrap())
.map(|name| PackageId::new(*name, "1.0.0", &registry_loc()).unwrap())
.collect()
}

View file

@ -4,9 +4,10 @@ use std::fmt::{Show, Formatter};
use url;
use url::Url;
use core::{Summary,Package,PackageId};
use sources::{PathSource,GitSource};
use util::{Config,CargoResult};
use core::{Summary, Package, PackageId};
use sources::{PathSource, GitSource};
use util::{Config, CargoResult};
use util::errors::human;
/// A Source finds and downloads remote packages based on names and
/// versions.
@ -51,20 +52,47 @@ pub enum SourceKind {
RegistryKind
}
#[deriving(Clone, PartialEq, Eq)]
pub enum Location {
Local(Path),
Remote(Url),
}
#[deriving(Clone,PartialEq)]
pub struct SourceId {
pub kind: SourceKind,
pub url: Url
pub location: Location,
}
impl Show for Location {
fn fmt(&self, f: &mut Formatter) -> fmt::Result {
match *self {
Local(ref p) => write!(f, "file:{}", p.display()),
Remote(ref u) => write!(f, "{}", u),
}
}
}
impl Location {
pub fn parse(s: &str) -> CargoResult<Location> {
if s.starts_with("file:") {
Ok(Local(Path::new(s.slice_from(5))))
} else {
url::from_str(s).map(Remote).map_err(|e| {
human(format!("invalid url `{}`: `{}", s, e))
})
}
}
}
impl Show for SourceId {
fn fmt(&self, f: &mut Formatter) -> fmt::Result {
match *self {
SourceId { kind: PathKind, ref url } => {
try!(write!(f, "{}", url))
SourceId { kind: PathKind, ref location } => {
try!(write!(f, "{}", location))
},
SourceId { kind: GitKind(ref reference), ref url } => {
try!(write!(f, "{}", url));
SourceId { kind: GitKind(ref reference), ref location } => {
try!(write!(f, "{}", location));
if reference.as_slice() != "master" {
try!(write!(f, " (ref={})", reference));
}
@ -80,33 +108,26 @@ impl Show for SourceId {
}
impl SourceId {
pub fn new(kind: SourceKind, url: Url) -> SourceId {
SourceId { kind: kind, url: url }
pub fn new(kind: SourceKind, location: Location) -> SourceId {
SourceId { kind: kind, location: location }
}
// Pass absolute path
pub fn for_path(path: &Path) -> SourceId {
// TODO: use proper path -> URL
let url = if cfg!(windows) {
let path = path.display().to_str();
format!("file://{}", path.as_slice().replace("\\", "/"))
} else {
format!("file://{}", path.display())
};
SourceId::new(PathKind, url::from_str(url.as_slice()).unwrap())
SourceId::new(PathKind, Local(path.clone()))
}
pub fn for_git(url: &Url, reference: &str) -> SourceId {
SourceId::new(GitKind(reference.to_str()), url.clone())
SourceId::new(GitKind(reference.to_str()), Remote(url.clone()))
}
pub fn for_central() -> SourceId {
SourceId::new(RegistryKind,
url::from_str("https://example.com").unwrap())
Remote(url::from_str("https://example.com").unwrap()))
}
pub fn get_url<'a>(&'a self) -> &'a Url {
&self.url
pub fn get_location<'a>(&'a self) -> &'a Location {
&self.location
}
pub fn is_path(&self) -> bool {
@ -124,12 +145,11 @@ impl SourceId {
match self.kind {
GitKind(..) => box GitSource::new(self, config) as Box<Source>,
PathKind => {
let mut path = self.url.path.clone();
if cfg!(windows) {
path = path.replace("/", "\\");
}
let path = Path::new(path);
box PathSource::new(&path, self) as Box<Source>
let path = match self.location {
Local(ref p) => p,
Remote(..) => fail!("path sources cannot be remote"),
};
box PathSource::new(path, self) as Box<Source>
},
RegistryKind => unimplemented!()
}

View file

@ -1,13 +1,12 @@
use std::fmt;
use std::hash::sip::SipHasher;
use std::hash::Hasher;
use std::fmt::{Show,Formatter};
use std::fmt;
use std::hash::Hasher;
use std::hash::sip::SipHasher;
use std::io::MemWriter;
use std::str;
use serialize::hex::ToHex;
use url;
use url::Url;
use core::source::{Source,SourceId,GitKind};
use core::source::{Source, SourceId, GitKind, Location, Remote, Local};
use core::{Package,PackageId,Summary};
use util::{CargoResult,Config};
use sources::PathSource;
@ -33,8 +32,8 @@ impl<'a, 'b> GitSource<'a, 'b> {
_ => fail!("Not a git source; id={}", source_id)
};
let remote = GitRemote::new(source_id.get_url());
let ident = ident(&source_id.url);
let remote = GitRemote::new(source_id.get_location());
let ident = ident(source_id.get_location());
let db_path = config.git_db_path()
.join(ident.as_slice());
@ -54,23 +53,32 @@ impl<'a, 'b> GitSource<'a, 'b> {
}
}
pub fn get_namespace<'a>(&'a self) -> &'a url::Url {
self.remote.get_url()
pub fn get_namespace<'a>(&'a self) -> &'a Location {
self.remote.get_location()
}
}
fn ident(url: &Url) -> String {
fn ident(location: &Location) -> String {
let hasher = SipHasher::new_with_keys(0,0);
let mut ident = url.path.as_slice().split('/').last().unwrap();
// FIXME: this really should be able to not use to_str() everywhere, but the
// compiler seems to currently ask for static lifetimes spuriously.
// Perhaps related to rust-lang/rust#15144
let ident = match *location {
Local(ref path) => {
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()
};
ident = if ident == "" {
"_empty"
let ident = if ident.as_slice() == "" {
"_empty".to_string()
} else {
ident
};
format!("{}-{}", ident, to_hex(hasher.hash(&url.to_str())))
format!("{}-{}", ident, to_hex(hasher.hash(&location.to_str())))
}
fn to_hex(num: u64) -> String {
@ -81,7 +89,7 @@ fn to_hex(num: u64) -> String {
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()));
try!(write!(f, "git repo at {}", self.remote.get_location()));
match self.reference {
Master => Ok(()),
@ -98,7 +106,7 @@ impl<'a, 'b> Source for GitSource<'a, 'b> {
let repo = if should_update {
try!(self.config.shell().status("Updating",
format!("git repository `{}`", self.remote.get_url())));
format!("git repository `{}`", self.remote.get_location())));
log!(5, "updating git source `{}`", self.remote);
try!(self.remote.checkout(&self.db_path))
@ -135,17 +143,18 @@ impl<'a, 'b> Source for GitSource<'a, 'b> {
mod test {
use url;
use url::Url;
use core::source::Remote;
use super::ident;
#[test]
pub fn test_url_to_path_ident_with_path() {
let ident = ident(&url("https://github.com/carlhuda/cargo"));
let ident = ident(&Remote(url("https://github.com/carlhuda/cargo")));
assert_eq!(ident.as_slice(), "cargo-0eed735c8ffd7c88");
}
#[test]
pub fn test_url_to_path_ident_without_path() {
let ident = ident(&url("https://github.com"));
let ident = ident(&Remote(url("https://github.com")));
assert_eq!(ident.as_slice(), "_empty-fc065c9b6b16fc00");
}

View file

@ -1,5 +1,3 @@
use url::Url;
use util::{CargoResult, ChainError, ProcessBuilder, process, human};
use std::fmt;
use std::fmt::{Show,Formatter};
use std::str;
@ -7,6 +5,9 @@ use std::io::{UserDir,AllPermissions};
use std::io::fs::{mkdir_recursive,rmdir_recursive,chmod};
use serialize::{Encodable,Encoder};
use core::source::{Location, Local, Remote};
use util::{CargoResult, ChainError, ProcessBuilder, process, human};
#[deriving(PartialEq,Clone,Encodable)]
pub enum GitReference {
Master,
@ -67,18 +68,18 @@ macro_rules! errln(
/// GitDatabase.
#[deriving(PartialEq,Clone,Show)]
pub struct GitRemote {
url: Url,
location: Location,
}
#[deriving(PartialEq,Clone,Encodable)]
struct EncodableGitRemote {
url: String,
location: String,
}
impl<E, S: Encoder<E>> Encodable<S, E> for GitRemote {
fn encode(&self, s: &mut S) -> Result<(), E> {
EncodableGitRemote {
url: self.url.to_str()
location: self.location.to_str()
}.encode(s)
}
}
@ -138,12 +139,12 @@ impl<E, S: Encoder<E>> Encodable<S, E> for GitCheckout {
// Implementations
impl GitRemote {
pub fn new(url: &Url) -> GitRemote {
GitRemote { url: url.clone() }
pub fn new(location: &Location) -> GitRemote {
GitRemote { location: location.clone() }
}
pub fn get_url<'a>(&'a self) -> &'a Url {
&self.url
pub fn get_location<'a>(&'a self) -> &'a Location {
&self.location
}
pub fn has_ref<S: Str>(&self, path: &Path, reference: S) -> CargoResult<()> {
@ -180,9 +181,9 @@ impl GitRemote {
}
fn fetch_location(&self) -> String {
match self.url.scheme.as_slice() {
"file" => self.url.path.clone(),
_ => self.url.to_str()
match self.location {
Local(ref p) => p.display().to_str(),
Remote(ref u) => u.to_str(),
}
}
}

View file

@ -2,12 +2,12 @@ use serialize::Decodable;
use std::collections::HashMap;
use std::str;
use toml;
use url::Url;
use url;
use core::{SourceId,GitKind};
use core::manifest::{LibKind,Lib};
use core::{Summary,Manifest,Target,Dependency,PackageId};
use core::{SourceId, GitKind};
use core::manifest::{LibKind, Lib};
use core::{Summary, Manifest, Target, Dependency, PackageId};
use core::source::{Location, Local, Remote};
use util::{CargoResult, Require, human};
pub fn to_manifest(contents: &[u8],
@ -95,7 +95,7 @@ pub struct TomlProject {
}
impl TomlProject {
pub fn to_package_id(&self, namespace: &Url) -> CargoResult<PackageId> {
pub fn to_package_id(&self, namespace: &Location) -> CargoResult<PackageId> {
PackageId::new(self.name.as_slice(), self.version.as_slice(), namespace)
}
}
@ -117,6 +117,15 @@ 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) => {
@ -132,10 +141,9 @@ impl TomlManifest {
.unwrap_or_else(|| "master".to_str());
let new_source_id = details.git.as_ref().map(|git| {
// TODO: Don't unwrap here
let kind = GitKind(reference.clone());
let url = url::from_str(git.as_slice()).unwrap();
let source_id = SourceId::new(kind, url);
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
@ -162,7 +170,7 @@ impl TomlManifest {
let project = try!(project.require(|| human("No `package` or `project` section found.")));
Ok((Manifest::new(
&Summary::new(&try!(project.to_package_id(source_id.get_url())),
&Summary::new(&try!(project.to_package_id(source_id.get_location())),
deps.as_slice()),
targets.as_slice(),
&Path::new("target"),