Skip to content

Update to rust-url 1.0 #2428

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
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
47 changes: 17 additions & 30 deletions src/cargo/core/package_id_spec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use std::collections::HashMap;
use std::fmt;

use semver::Version;
use url::{self, Url, UrlParser};
use url::Url;

use core::PackageId;
use util::{CargoResult, ToUrl, human, ToSemver, ChainError};
Expand All @@ -22,7 +22,7 @@ impl PackageIdSpec {
Err(..) => {}
}
if !spec.contains("://") {
match url(&format!("cargo://{}", spec)) {
match Url::parse(&format!("cargo://{}", spec)) {
Ok(url) => return PackageIdSpec::from_url(url),
Err(..) => {}
}
Expand Down Expand Up @@ -64,15 +64,16 @@ impl PackageIdSpec {
}

fn from_url(mut url: Url) -> CargoResult<PackageIdSpec> {
if url.query.is_some() {
if url.query().is_some() {
bail!("cannot have a query string in a pkgid: {}", url)
}
let frag = url.fragment.take();
let frag = url.fragment().map(|s| s.to_owned());
url.set_fragment(None);
let (name, version) = {
let path = try!(url.path().chain_error(|| {
let mut path = try!(url.path_segments().chain_error(|| {
human(format!("pkgid urls must have a path: {}", url))
}));
let path_name = try!(path.last().chain_error(|| {
let path_name = try!(path.next_back().chain_error(|| {
human(format!("pkgid urls must have at least one path \
component: {}", url))
}));
Expand Down Expand Up @@ -171,31 +172,17 @@ impl PackageIdSpec {
}
}

fn url(s: &str) -> url::ParseResult<Url> {
return UrlParser::new().scheme_type_mapper(mapper).parse(s);

fn mapper(scheme: &str) -> url::SchemeType {
if scheme == "cargo" {
url::SchemeType::Relative(1)
} else {
url::whatwg_scheme_type_mapper(scheme)
}
}

}

impl fmt::Display for PackageIdSpec {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
let mut printed_name = false;
match self.url {
Some(ref url) => {
if url.scheme == "cargo" {
try!(write!(f, "{}/{}", url.host().unwrap(),
url.path().unwrap().join("/")));
if url.scheme() == "cargo" {
try!(write!(f, "{}{}", url.host().unwrap(), url.path()));
} else {
try!(write!(f, "{}", url));
}
if url.path().unwrap().last().unwrap() != &self.name {
if url.path_segments().unwrap().next_back().unwrap() != &self.name {
printed_name = true;
try!(write!(f, "#{}", self.name));
}
Expand All @@ -215,7 +202,7 @@ impl fmt::Display for PackageIdSpec {
#[cfg(test)]
mod tests {
use core::{PackageId, SourceId};
use super::{PackageIdSpec, url};
use super::PackageIdSpec;
use url::Url;
use semver::Version;

Expand All @@ -230,32 +217,32 @@ mod tests {
ok("http://crates.io/foo#1.2.3", PackageIdSpec {
name: "foo".to_string(),
version: Some(Version::parse("1.2.3").unwrap()),
url: Some(url("http://crates.io/foo").unwrap()),
url: Some(Url::parse("http://crates.io/foo").unwrap()),
});
ok("http://crates.io/foo#bar:1.2.3", PackageIdSpec {
name: "bar".to_string(),
version: Some(Version::parse("1.2.3").unwrap()),
url: Some(url("http://crates.io/foo").unwrap()),
url: Some(Url::parse("http://crates.io/foo").unwrap()),
});
ok("crates.io/foo", PackageIdSpec {
name: "foo".to_string(),
version: None,
url: Some(url("cargo://crates.io/foo").unwrap()),
url: Some(Url::parse("cargo://crates.io/foo").unwrap()),
});
ok("crates.io/foo#1.2.3", PackageIdSpec {
name: "foo".to_string(),
version: Some(Version::parse("1.2.3").unwrap()),
url: Some(url("cargo://crates.io/foo").unwrap()),
url: Some(Url::parse("cargo://crates.io/foo").unwrap()),
});
ok("crates.io/foo#bar", PackageIdSpec {
name: "bar".to_string(),
version: None,
url: Some(url("cargo://crates.io/foo").unwrap()),
url: Some(Url::parse("cargo://crates.io/foo").unwrap()),
});
ok("crates.io/foo#bar:1.2.3", PackageIdSpec {
name: "bar".to_string(),
version: Some(Version::parse("1.2.3").unwrap()),
url: Some(url("cargo://crates.io/foo").unwrap()),
url: Some(Url::parse("cargo://crates.io/foo").unwrap()),
});
ok("foo", PackageIdSpec {
name: "foo".to_string(),
Expand Down
6 changes: 3 additions & 3 deletions src/cargo/core/source.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ use std::cmp::{self, Ordering};
use std::collections::hash_map::{HashMap, Values, IterMut};
use std::fmt::{self, Formatter};
use std::hash;
use std::mem;
use std::path::Path;
use std::sync::Arc;
use rustc_serialize::{Decodable, Decoder, Encodable, Encoder};
Expand Down Expand Up @@ -113,8 +112,9 @@ impl SourceId {
_ => {}
}
}
url.query = None;
let precise = mem::replace(&mut url.fragment, None);
let precise = url.fragment().map(|s| s.to_owned());
url.set_fragment(None);
url.set_query(None);
SourceId::for_git(&url, reference).with_precise(precise)
}
"registry" => {
Expand Down
47 changes: 17 additions & 30 deletions src/cargo/sources/git/source.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
use std::fmt::{self, Debug, Formatter};
use std::hash::{Hash, Hasher, SipHasher};
use std::mem;

use url::{self, Url};
use url::Url;

use core::source::{Source, SourceId};
use core::GitReference;
Expand Down Expand Up @@ -62,15 +61,11 @@ impl<'cfg> GitSource<'cfg> {
fn ident(url: &Url) -> String {
let mut hasher = SipHasher::new_with_keys(0,0);

// 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 url = canonicalize_url(url);
let ident = url.path().unwrap_or(&[])
.last().map(|a| a.clone()).unwrap_or(String::new());
let ident = url.path_segments().and_then(|mut s| s.next_back()).unwrap_or("");

let ident = if ident == "" {
"_empty".to_string()
"_empty"
} else {
ident
};
Expand All @@ -84,39 +79,31 @@ pub fn canonicalize_url(url: &Url) -> Url {
let mut url = url.clone();

// Strip a trailing slash
if let url::SchemeData::Relative(ref mut rel) = url.scheme_data {
if rel.path.last().map(|s| s.is_empty()).unwrap_or(false) {
rel.path.pop();
}
if url.path_segments().unwrap().next_back().unwrap().is_empty() {
url.pop_path_segment().unwrap();
}

// HACKHACK: For github URL's specifically just lowercase
// everything. GitHub treats 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)
if url.domain() == Some("github.com") {
url.scheme = "https".to_string();
if let url::SchemeData::Relative(ref mut rel) = url.scheme_data {
rel.port = Some(443);
rel.default_port = Some(443);
let path = mem::replace(&mut rel.path, Vec::new());
rel.path = path.into_iter().map(|s| {
s.chars().flat_map(|c| c.to_lowercase()).collect()
}).collect();
}
if url.host_str() == Some("github.com") {
url.set_scheme("https").unwrap();
url.set_port(Some(443)).unwrap();
let path = url.path().to_lowercase();
url.set_path(&path);
}

// Repos generally can be accessed with or w/o '.git'
if let url::SchemeData::Relative(ref mut rel) = url.scheme_data {
let needs_chopping = {
let last = rel.path.last().map(|s| &s[..]).unwrap_or("");
last.ends_with(".git")
let needs_chopping = url.path().ends_with(".git");
if needs_chopping {
let last = {
let last = url.path_segments().unwrap().next_back().unwrap();
last[..last.len() - 4].to_owned()
};
if needs_chopping {
let last = rel.path.pop().unwrap();
rel.path.push(last[..last.len() - 4].to_string())
}
url.pop_path_segment().unwrap();
url.push_path_segment(&last).unwrap();
}

url
Expand Down
8 changes: 4 additions & 4 deletions src/cargo/sources/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ impl<'cfg> RegistrySource<'cfg> {
pub fn new(source_id: &SourceId,
config: &'cfg Config) -> RegistrySource<'cfg> {
let hash = hex::short_hash(source_id);
let ident = source_id.url().host().unwrap().to_string();
let ident = source_id.url().host_str().unwrap_or("").to_string();
let part = format!("{}-{}", ident, hash);
RegistrySource {
checkout_path: config.registry_index_path().join(&part),
Expand Down Expand Up @@ -558,9 +558,9 @@ impl<'cfg> Source for RegistrySource<'cfg> {
let config = try!(self.config());
let url = try!(config.dl.to_url().map_err(internal));
let mut url = url.clone();
url.path_mut().unwrap().push(package.name().to_string());
url.path_mut().unwrap().push(package.version().to_string());
url.path_mut().unwrap().push("download".to_string());
url.push_path_segment(package.name()).unwrap();
url.push_path_segment(&package.version().to_string()).unwrap();
url.push_path_segment("download").unwrap();
let krate = try!(self.download_package(package, &url).chain_error(|| {
internal(format!("failed to download package `{}` from {}",
package, url))
Expand Down
12 changes: 2 additions & 10 deletions src/cargo/util/to_url.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use url::{self, Url, UrlParser};
use url::Url;
use std::path::Path;

pub trait ToUrl {
Expand All @@ -19,7 +19,7 @@ impl<'a> ToUrl for &'a Url {

impl<'a> ToUrl for &'a str {
fn to_url(self) -> Result<Url, String> {
UrlParser::new().scheme_type_mapper(mapper).parse(self).map_err(|s| {
Url::parse(self).map_err(|s| {
format!("invalid url `{}`: {}", self, s)
})
}
Expand All @@ -32,11 +32,3 @@ impl<'a> ToUrl for &'a Path {
})
}
}

fn mapper(s: &str) -> url::SchemeType {
match s {
"git" => url::SchemeType::Relative(9418),
"ssh" => url::SchemeType::Relative(22),
s => url::whatwg_scheme_type_mapper(s),
}
}