Auto merge of #4836 - sfackler:dl-template, r=withoutboats

Template a registry's dl field

Previously, crate files were always downloaded from
`/{crate}/{version}/download`. However, if the backing crate store for a
custom registry is a raw file server rather than an API endpoint that
requires every file to be named `download` which is a bit weird. Now a
registry's dl URL can be templated with `{crate}` and `{version}` to
have more control over the resulting path.

For backwards compatibility, we append the default template suffix onto
the dl URL if neither of the template parameters are present for
backwards compatibility.

r? @alexcrichton

cc @withoutboats

Author: bors

src/cargo/sources/registry/mod.rs

@@ -178,7 +178,9 @@ use util::hex;
 use util::to_url::ToUrl;
 
 const INDEX_LOCK: &'static str = ".cargo-index-lock";
-pub static CRATES_IO: &'static str = "https://github.com/rust-lang/crates.io-index";
+pub const CRATES_IO: &'static str = "https://github.com/rust-lang/crates.io-index";
+const CRATE_TEMPLATE: &'static str = "{crate}";
+const VERSION_TEMPLATE: &'static str = "{version}";
 
 pub struct RegistrySource<'cfg> {
     source_id: SourceId,
@@ -192,9 +194,17 @@ pub struct RegistrySource<'cfg> {
 
 #[derive(Deserialize)]
 pub struct RegistryConfig {
-    /// Download endpoint for all crates. This will be appended with
-    /// `/<crate>/<version>/download` and then will be hit with an HTTP GET
-    /// request to download the tarball for a crate.
+    /// Download endpoint for all crates.
+    ///
+    /// The string is a template which will generate the download URL for the
+    /// tarball of a specific version of a crate. The substrings `{crate}` and
+    /// `{version}` will be replaced with the crate's name and version
+    /// respectively.
+    ///
+    /// For backwards compatibility, if the string does not contain `{crate}` or
+    /// `{version}`, it will be extended with `/{crate}/{version}/download` to
+    /// support registries like crates.io which were crated before the
+    /// templating setup was created.
     pub dl: String,
 
     /// API endpoint for the registry. This is what's actually hit to perform

src/cargo/sources/registry/remote.rs

@@ -1,4 +1,5 @@
 use std::cell::{RefCell, Ref, Cell};
+use std::fmt::Write as FmtWrite;
 use std::io::SeekFrom;
 use std::io::prelude::*;
 use std::mem;
@@ -11,7 +12,7 @@ use serde_json;
 
 use core::{PackageId, SourceId};
 use sources::git;
-use sources::registry::{RegistryData, RegistryConfig, INDEX_LOCK};
+use sources::registry::{RegistryData, RegistryConfig, INDEX_LOCK, CRATE_TEMPLATE, VERSION_TEMPLATE};
 use util::network;
 use util::{FileLock, Filesystem, LazyCell};
 use util::{Config, Sha256, ToUrl, Progress};
@@ -203,11 +204,14 @@ impl<'cfg> RegistryData for RemoteRegistry<'cfg> {
         self.config.shell().status("Downloading", pkg)?;
 
         let config = self.config()?.unwrap();
-        let mut url = config.dl.to_url()?;
-        url.path_segments_mut().unwrap()
-            .push(pkg.name())
-            .push(&pkg.version().to_string())
-            .push("download");
+        let mut url = config.dl.clone();
+        if !url.contains(CRATE_TEMPLATE) && !url.contains(VERSION_TEMPLATE) {
+            write!(url, "/{}/{}/download", CRATE_TEMPLATE, VERSION_TEMPLATE).unwrap();
+        }
+        let url = url
+            .replace(CRATE_TEMPLATE, pkg.name())
+            .replace(VERSION_TEMPLATE, &pkg.version().to_string())
+            .to_url()?;
 
         // TODO: don't download into memory, but ensure that if we ctrl-c a
         //       download we should resume either from the start or the middle

tests/alt-registry.rs

@@ -2,7 +2,7 @@ extern crate cargotest;
 extern crate hamcrest;
 
 use cargotest::ChannelChanger;
-use cargotest::support::registry::{self, Package, alt_dl_path};
+use cargotest::support::registry::{self, Package, alt_api_path};
 use cargotest::support::{project, execs};
 use hamcrest::assert_that;
 
@@ -374,7 +374,7 @@ fn publish_to_alt_registry() {
                 execs().with_status(0));
 
     // Ensure that the crate is uploaded
-    assert!(alt_dl_path().join("api/v1/crates/new").exists());
+    assert!(alt_api_path().join("api/v1/crates/new").exists());
 }
 
 #[test]

tests/cargotest/support/registry.rs

@@ -21,7 +21,12 @@ pub fn dl_url() -> Url { Url::from_file_path(&*dl_path()).ok().unwrap() }
 pub fn alt_registry_path() -> PathBuf { paths::root().join("alternative-registry") }
 pub fn alt_registry() -> Url { Url::from_file_path(&*alt_registry_path()).ok().unwrap() }
 pub fn alt_dl_path() -> PathBuf { paths::root().join("alt_dl") }
-pub fn alt_dl_url() -> Url { Url::from_file_path(&*alt_dl_path()).ok().unwrap() }
+pub fn alt_dl_url() -> String {
+    let base = Url::from_file_path(&*alt_dl_path()).ok().unwrap();
+    format!("{}/{{crate}}/{{version}}/{{crate}}-{{version}}.crate", base)
+}
+pub fn alt_api_path() -> PathBuf { paths::root().join("alt_api") }
+pub fn alt_api_url() -> Url { Url::from_file_path(&*alt_api_path()).ok().unwrap() }
 
 pub struct Package {
     name: String,
@@ -76,10 +81,10 @@ pub fn init() {
     // Init an alt registry
     repo(&alt_registry_path())
         .file("config.json", &format!(r#"
-            {{"dl":"{0}","api":"{0}"}}
-        "#, alt_dl_url()))
+            {{"dl":"{}","api":"{}"}}
+        "#, alt_dl_url(), alt_api_url()))
         .build();
-    fs::create_dir_all(alt_dl_path().join("api/v1/crates")).unwrap();
+    fs::create_dir_all(alt_api_path().join("api/v1/crates")).unwrap();
 }
 
 impl Package {
@@ -300,9 +305,13 @@ impl Package {
         if self.local {
             registry_path().join(format!("{}-{}.crate", self.name,
                                          self.vers))
+        } else if self.alternative {
+            alt_dl_path()
+                .join(&self.name)
+                .join(&self.vers)
+                .join(&format!("{}-{}.crate", self.name, self.vers))
         } else {
-            let dl_path = if self.alternative { alt_dl_path() } else { dl_path() };
-            dl_path.join(&self.name).join(&self.vers).join("download")
+            dl_path().join(&self.name).join(&self.vers).join("download")
         }
     }
 }