Elaborate registry names to index URLs when publishing
authorSteven Fackler <sfackler@palantir.com>
Thu, 18 Jan 2018 19:33:26 +0000 (11:33 -0800)
committerSteven Fackler <sfackler@palantir.com>
Wed, 24 Jan 2018 20:54:53 +0000 (12:54 -0800)
This avoids introducing a dependency on the publisher's name for the
registry.

Closes #4880

src/cargo/core/package.rs
src/cargo/ops/cargo_package.rs
src/cargo/util/toml/mod.rs
tests/package.rs

index 7042dd5d0b0939dc2486bd2961152100ce2cc533..6e8e4480679873c4fb34c25d64c7357921586b76 100644 (file)
@@ -134,8 +134,8 @@ impl Package {
         }
     }
 
-    pub fn to_registry_toml(&self) -> CargoResult<String> {
-        let manifest = self.manifest().original().prepare_for_publish();
+    pub fn to_registry_toml(&self, config: &Config) -> CargoResult<String> {
+        let manifest = self.manifest().original().prepare_for_publish(config)?;
         let toml = toml::to_string(&manifest)?;
         Ok(format!("\
             # THIS FILE IS AUTOMATICALLY GENERATED BY CARGO\n\
index bd968d08d45d4a9f48112e1ecfa4efcd65a24d06..9562b7abae5c48cf528d9d32ebc28849d7b612ce 100644 (file)
@@ -249,7 +249,7 @@ fn tar(ws: &Workspace,
             })?;
 
             let mut header = Header::new_ustar();
-            let toml = pkg.to_registry_toml()?;
+            let toml = pkg.to_registry_toml(ws.config())?;
             header.set_path(&path)?;
             header.set_entry_type(EntryType::file());
             header.set_mode(0o644);
index 2515b1c5de461ba64bb4630f461546e304d1a97d..86b87b861216a2b2b2b44abe49a620cefbab2bbd 100644 (file)
@@ -181,6 +181,7 @@ impl<'de> de::Deserialize<'de> for TomlDependency {
 pub struct DetailedTomlDependency {
     version: Option<String>,
     registry: Option<String>,
+    registry_index: Option<String>,
     path: Option<String>,
     git: Option<String>,
     branch: Option<String>,
@@ -471,13 +472,13 @@ struct Context<'a, 'b> {
 }
 
 impl TomlManifest {
-    pub fn prepare_for_publish(&self) -> TomlManifest {
+    pub fn prepare_for_publish(&self, config: &Config) -> CargoResult<TomlManifest> {
         let mut package = self.package.as_ref()
                               .or_else(|| self.project.as_ref())
                               .unwrap()
                               .clone();
         package.workspace = None;
-        return TomlManifest {
+        return Ok(TomlManifest {
             package: Some(package),
             project: None,
             profile: self.profile.clone(),
@@ -486,56 +487,68 @@ impl TomlManifest {
             example: self.example.clone(),
             test: self.test.clone(),
             bench: self.bench.clone(),
-            dependencies: map_deps(self.dependencies.as_ref()),
-            dev_dependencies: map_deps(self.dev_dependencies.as_ref()
-                                         .or_else(|| self.dev_dependencies2.as_ref())),
+            dependencies: map_deps(config, self.dependencies.as_ref())?,
+            dev_dependencies: map_deps(config, self.dev_dependencies.as_ref()
+                                         .or_else(|| self.dev_dependencies2.as_ref()))?,
             dev_dependencies2: None,
-            build_dependencies: map_deps(self.build_dependencies.as_ref()
-                                         .or_else(|| self.build_dependencies2.as_ref())),
+            build_dependencies: map_deps(config, self.build_dependencies.as_ref()
+                                         .or_else(|| self.build_dependencies2.as_ref()))?,
             build_dependencies2: None,
             features: self.features.clone(),
-            target: self.target.as_ref().map(|target_map| {
+            target: match self.target.as_ref().map(|target_map| {
                 target_map.iter().map(|(k, v)| {
-                    (k.clone(), TomlPlatform {
-                        dependencies: map_deps(v.dependencies.as_ref()),
-                        dev_dependencies: map_deps(v.dev_dependencies.as_ref()
-                                                     .or_else(|| v.dev_dependencies2.as_ref())),
+                    Ok((k.clone(), TomlPlatform {
+                        dependencies: map_deps(config, v.dependencies.as_ref())?,
+                        dev_dependencies: map_deps(config, v.dev_dependencies.as_ref()
+                                                     .or_else(|| v.dev_dependencies2.as_ref()))?,
                         dev_dependencies2: None,
-                        build_dependencies: map_deps(v.build_dependencies.as_ref()
-                                                     .or_else(|| v.build_dependencies2.as_ref())),
+                        build_dependencies: map_deps(config, v.build_dependencies.as_ref()
+                                                     .or_else(|| v.build_dependencies2.as_ref()))?,
                         build_dependencies2: None,
-                    })
+                    }))
                 }).collect()
-            }),
+            }) {
+                Some(Ok(v)) => Some(v),
+                Some(Err(e)) => return Err(e),
+                None => None,
+            },
             replace: None,
             patch: None,
             workspace: None,
             badges: self.badges.clone(),
             cargo_features: self.cargo_features.clone(),
-        };
+        });
 
-        fn map_deps(deps: Option<&BTreeMap<String, TomlDependency>>)
-                        -> Option<BTreeMap<String, TomlDependency>>
+        fn map_deps(config: &Config, deps: Option<&BTreeMap<String, TomlDependency>>)
+                        -> CargoResult<Option<BTreeMap<String, TomlDependency>>>
         {
             let deps = match deps {
                 Some(deps) => deps,
-                None => return None
+                None => return Ok(None),
             };
-            Some(deps.iter().map(|(k, v)| (k.clone(), map_dependency(v))).collect())
+            let deps = deps.iter()
+                .map(|(k, v)| Ok((k.clone(), map_dependency(config, v)?)))
+                .collect::<CargoResult<BTreeMap<_, _>>>()?;
+            Ok(Some(deps))
         }
 
-        fn map_dependency(dep: &TomlDependency) -> TomlDependency {
+        fn map_dependency(config: &Config, dep: &TomlDependency) -> CargoResult<TomlDependency> {
             match *dep {
                 TomlDependency::Detailed(ref d) => {
                     let mut d = d.clone();
                     d.path.take(); // path dependencies become crates.io deps
-                    TomlDependency::Detailed(d)
+                    // registry specifications are elaborated to the index URL
+                    if let Some(registry) = d.registry.take() {
+                        let src = SourceId::alt_registry(config, &registry)?;
+                        d.registry_index = Some(src.url().to_string());
+                    }
+                    Ok(TomlDependency::Detailed(d))
                 }
                 TomlDependency::Simple(ref s) => {
-                    TomlDependency::Detailed(DetailedTomlDependency {
+                    Ok(TomlDependency::Detailed(DetailedTomlDependency {
                         version: Some(s.clone()),
                         ..Default::default()
-                    })
+                    }))
                 }
             }
         }
@@ -933,10 +946,18 @@ impl TomlDependency {
             None => SourceId::crates_io(cx.config)?
         };
 
-        let new_source_id = match (details.git.as_ref(), details.path.as_ref(), details.registry.as_ref()) {
-            (Some(_), _, Some(_)) => bail!("dependency ({}) specification is ambiguous. \
+        let new_source_id = match (
+            details.git.as_ref(),
+            details.path.as_ref(),
+            details.registry.as_ref(),
+            details.registry_index.as_ref(),
+        ) {
+            (Some(_), _, Some(_), _) |
+            (Some(_), _, _, Some(_))=> bail!("dependency ({}) specification is ambiguous. \
                                             Only one of `git` or `registry` is allowed.", name),
-            (Some(git), maybe_path, _) => {
+            (_, _, Some(_), Some(_)) => bail!("dependency ({}) specification is ambiguous. \
+                                            Only one of `registry` or `registry-index` is allowed.", name),
+            (Some(git), maybe_path, _, _) => {
                 if maybe_path.is_some() {
                     let msg = format!("dependency ({}) specification is ambiguous. \
                                        Only one of `git` or `path` is allowed. \
@@ -963,7 +984,7 @@ impl TomlDependency {
                 let loc = git.to_url()?;
                 SourceId::for_git(&loc, reference)?
             },
-            (None, Some(path), _) => {
+            (None, Some(path), _, _) => {
                 cx.nested_paths.push(PathBuf::from(path));
                 // If the source id for the package we're parsing is a path
                 // source, then we normalize the path here to get rid of
@@ -981,8 +1002,12 @@ impl TomlDependency {
                     cx.source_id.clone()
                 }
             },
-            (None, None, Some(registry)) => SourceId::alt_registry(cx.config, registry)?,
-            (None, None, None) => SourceId::crates_io(cx.config)?,
+            (None, None, Some(registry), None) => SourceId::alt_registry(cx.config, registry)?,
+            (None, None, None, Some(registry_index)) => {
+                let url = registry_index.to_url()?;
+                SourceId::for_registry(&url)?
+            }
+            (None, None, None, None) => SourceId::crates_io(cx.config)?,
         };
 
         let version = details.version.as_ref().map(|v| &v[..]);
index 6ba37e98402bbe0b75ac3b698f66a1db82d566dd..31cade91b057c4d8612b43972b30d6a101d2d24d 100644 (file)
@@ -9,8 +9,8 @@ use std::fs::File;
 use std::io::prelude::*;
 use std::path::{Path, PathBuf};
 
-use cargotest::{cargo_process, process};
-use cargotest::support::{project, execs, paths, git, path2url, cargo_exe};
+use cargotest::{cargo_process, process, ChannelChanger};
+use cargotest::support::{project, execs, paths, git, path2url, cargo_exe, registry};
 use cargotest::support::registry::Package;
 use flate2::read::GzDecoder;
 use hamcrest::{assert_that, existing_file, contains, equal_to};
@@ -715,6 +715,8 @@ fn generated_manifest() {
     Package::new("ghi", "1.0.0").publish();
     let p = project("foo")
         .file("Cargo.toml", r#"
+            cargo-features = ["alternative-registries"]
+
             [project]
             name = "foo"
             version = "0.0.1"
@@ -730,7 +732,7 @@ fn generated_manifest() {
 
             [dependencies]
             bar = { path = "bar", version = "0.1" }
-            def = "1.0"
+            def = { version = "1.0", registry = "alternative" }
             ghi = "1.0"
             abc = "1.0"
         "#)
@@ -744,7 +746,9 @@ fn generated_manifest() {
         .file("bar/src/lib.rs", "")
         .build();
 
-    assert_that(p.cargo("package").arg("--no-verify"),
+    assert_that(p.cargo("package")
+                    .masquerade_as_nightly_cargo()
+                    .arg("--no-verify"),
                 execs().with_status(0));
 
     let f = File::open(&p.root().join("target/package/foo-0.0.1.crate")).unwrap();
@@ -761,6 +765,7 @@ fn generated_manifest() {
     // BTreeMap makes the order of dependencies in the generated file deterministic
     // by sorting alphabetically
     assert_that(&contents[..], equal_to(
+&*format!(
 r#"# THIS FILE IS AUTOMATICALLY GENERATED BY CARGO
 #
 # When uploading crates to the registry Cargo will automatically
@@ -773,6 +778,8 @@ r#"# THIS FILE IS AUTOMATICALLY GENERATED BY CARGO
 # editing this file be aware that the upstream Cargo.toml
 # will likely look very different (and much more reasonable)
 
+cargo-features = ["alternative-registries"]
+
 [package]
 name = "foo"
 version = "0.0.1"
@@ -791,10 +798,12 @@ version = "0.1"
 
 [dependencies.def]
 version = "1.0"
+registry_index = "{}"
 
 [dependencies.ghi]
 version = "1.0"
-"#));
+"#,
+    registry::alt_registry())));
 }
 
 #[test]