Auto merge of #5993 - alexcrichton:publish-renames, r=ehuss

Fix publishing renamed dependencies to crates.io

This commit fixes publishing crates which contain locally renamed dependencies
to crates.io. Previously this lack of information meant that although we could
resolve the crate graph correctly it wouldn't work well with respect to optional
features and optional dependencies. The fix here is to persist this information
into the registry about the crate being renamed in `Cargo.toml`, allowing Cargo
to correctly deduce feature names as it does when it has `Cargo.toml` locally.

A dual side of this commit is to publish this information to crates.io. We'll
want to merge the associated PR (link to come soon) on crates.io first and make
sure that's deployed as well before we stabilize the crate renaming feature.

The index format is updated as well as part of this change. The `name` key for
dependencies is now unconditionally what was written in `Cargo.toml` as the
left-hand-side of the dependency specification. In other words this is the raw
crate name, but only for the local crate. A new key, `package`, is added to
dependencies (and it can be `None`). This key indicates the crates.io package is
being linked against, an represents the `package` key in `Cargo.toml`.

It's important to consider the interaction with older Cargo implementations
which don't support the `package` key in the index. In these situations older
Cargo binaries will likely fail to resolve entirely as the renamed name is
unlikely to exist on crates.io. For example the `futures` crate now has an
optional dependency with the name `futures01` which depends on an older version
of `futures` on crates.io. The string `futures01` will be listed in the index
under the `"name"` key, but no `futures01` crate exists on crates.io so older
Cargo will generate an error. If the crate does exist on crates.io, then even
weirder error messages will likely result.

Closes #5962

src/cargo/core/dependency.rs

@@ -28,7 +28,7 @@ struct Inner {
     specified_req: bool,
     kind: Kind,
     only_match_name: bool,
-    rename: Option<InternedString>,
+    explicit_name_in_toml: Option<InternedString>,
 
     optional: bool,
     default_features: bool,
@@ -73,7 +73,7 @@ impl ser::Serialize for Dependency {
             uses_default_features: self.uses_default_features(),
             features: self.features(),
             target: self.platform(),
-            rename: self.rename().map(|s| s.as_str()),
+            rename: self.explicit_name_in_toml().map(|s| s.as_str()),
         }.serialize(s)
     }
 }
@@ -199,7 +199,7 @@ impl Dependency {
                 default_features: true,
                 specified_req: false,
                 platform: None,
-                rename: None,
+                explicit_name_in_toml: None,
             }),
         }
     }
@@ -229,7 +229,7 @@ impl Dependency {
     /// foo = { version = "0.1", package = 'bar' }
     /// ```
     pub fn name_in_toml(&self) -> InternedString {
-        self.rename().unwrap_or(self.inner.name)
+        self.explicit_name_in_toml().unwrap_or(self.inner.name)
     }
 
     /// The name of the package that this `Dependency` depends on.
@@ -285,8 +285,8 @@ impl Dependency {
     ///
     /// If the `package` key is used in `Cargo.toml` then this returns the same
     /// value as `name_in_toml`.
-    pub fn rename(&self) -> Option<InternedString> {
-        self.inner.rename
+    pub fn explicit_name_in_toml(&self) -> Option<InternedString> {
+        self.inner.explicit_name_in_toml
     }
 
     pub fn set_kind(&mut self, kind: Kind) -> &mut Dependency {
@@ -330,8 +330,8 @@ impl Dependency {
         self
     }
 
-    pub fn set_rename(&mut self, rename: &str) -> &mut Dependency {
-        Rc::make_mut(&mut self.inner).rename = Some(InternedString::new(rename));
+    pub fn set_explicit_name_in_toml(&mut self, name: &str) -> &mut Dependency {
+        Rc::make_mut(&mut self.inner).explicit_name_in_toml = Some(InternedString::new(name));
         self
     }
 

src/cargo/core/resolver/resolve.rs

@@ -227,7 +227,7 @@ unable to verify that `{0}` is the same as when the lockfile was generated
 
         let crate_name = to_target.crate_name();
         let mut names = deps.iter()
-            .map(|d| d.rename().map(|s| s.as_str()).unwrap_or(&crate_name));
+            .map(|d| d.explicit_name_in_toml().map(|s| s.as_str()).unwrap_or(&crate_name));
         let name = names.next().unwrap_or(&crate_name);
         for n in names {
             if n == name {

src/cargo/ops/registry.rs

@@ -177,6 +177,7 @@ fn transmit(
                     Kind::Development => "dev",
                 }.to_string(),
                 registry: dep_registry,
+                explicit_name_in_toml: dep.explicit_name_in_toml().map(|s| s.to_string()),
             })
         })
         .collect::<CargoResult<Vec<NewCrateDependency>>>()?;

src/cargo/sources/registry/mod.rs

@@ -275,6 +275,7 @@ struct RegistryDependency<'a> {
     target: Option<Cow<'a, str>>,
     kind: Option<Cow<'a, str>>,
     registry: Option<Cow<'a, str>>,
+    package: Option<Cow<'a, str>>,
 }
 
 impl<'a> RegistryDependency<'a> {
@@ -289,6 +290,7 @@ impl<'a> RegistryDependency<'a> {
             target,
             kind,
             registry,
+            package,
         } = self;
 
         let id = if let Some(registry) = registry {
@@ -297,7 +299,15 @@ impl<'a> RegistryDependency<'a> {
             default.clone()
         };
 
-        let mut dep = Dependency::parse_no_deprecated(&name, Some(&req), &id)?;
+
+        let mut dep = Dependency::parse_no_deprecated(
+            package.as_ref().unwrap_or(&name),
+            Some(&req),
+            &id,
+        )?;
+        if package.is_some() {
+            dep.set_explicit_name_in_toml(&name);
+        }
         let kind = match kind.as_ref().map(|s| &s[..]).unwrap_or("") {
             "dev" => Kind::Development,
             "build" => Kind::Build,

src/cargo/util/toml/mod.rs

@@ -1249,7 +1249,7 @@ impl TomlDependency {
 impl DetailedTomlDependency {
     fn to_dependency(
         &self,
-        name: &str,
+        name_in_toml: &str,
         cx: &mut Context,
         kind: Option<Kind>,
     ) -> CargoResult<Dependency> {
@@ -1259,7 +1259,7 @@ impl DetailedTomlDependency {
                  providing a local path, Git repository, or \
                  version to use. This will be considered an \
                  error in future versions",
-                name
+                name_in_toml
             );
             cx.warnings.push(msg);
         }
@@ -1276,7 +1276,7 @@ impl DetailedTomlDependency {
                     let msg = format!(
                         "key `{}` is ignored for dependency ({}). \
                          This will be considered an error in future versions",
-                        key_name, name
+                        key_name, name_in_toml
                     );
                     cx.warnings.push(msg)
                 }
@@ -1300,20 +1300,20 @@ impl DetailedTomlDependency {
             (Some(_), _, Some(_), _) | (Some(_), _, _, Some(_)) => bail!(
                 "dependency ({}) specification is ambiguous. \
                  Only one of `git` or `registry` is allowed.",
-                name
+                name_in_toml
             ),
             (_, _, Some(_), Some(_)) => bail!(
                 "dependency ({}) specification is ambiguous. \
                  Only one of `registry` or `registry-index` is allowed.",
-                name
+                name_in_toml
             ),
             (Some(git), maybe_path, _, _) => {
                 if maybe_path.is_some() {
                     let msg = format!(
                         "dependency ({}) specification is ambiguous. \
                          Only one of `git` or `path` is allowed. \
                          This will be considered an error in future versions",
-                        name
+                        name_in_toml
                     );
                     cx.warnings.push(msg)
                 }
@@ -1328,7 +1328,7 @@ impl DetailedTomlDependency {
                         "dependency ({}) specification is ambiguous. \
                          Only one of `branch`, `tag` or `rev` is allowed. \
                          This will be considered an error in future versions",
-                        name
+                        name_in_toml
                     );
                     cx.warnings.push(msg)
                 }
@@ -1369,15 +1369,15 @@ impl DetailedTomlDependency {
             (None, None, None, None) => SourceId::crates_io(cx.config)?,
         };
 
-        let (pkg_name, rename) = match self.package {
-            Some(ref s) => (&s[..], Some(name)),
-            None => (name, None),
+        let (pkg_name, explicit_name_in_toml) = match self.package {
+            Some(ref s) => (&s[..], Some(name_in_toml)),
+            None => (name_in_toml, None),
         };
 
         let version = self.version.as_ref().map(|v| &v[..]);
         let mut dep = match cx.pkgid {
             Some(id) => Dependency::parse(pkg_name, version, &new_source_id, id, cx.config)?,
-            None => Dependency::parse_no_deprecated(name, version, &new_source_id)?,
+            None => Dependency::parse_no_deprecated(pkg_name, version, &new_source_id)?,
         };
         dep.set_features(self.features.iter().flat_map(|x| x))
             .set_default_features(
@@ -1391,9 +1391,9 @@ impl DetailedTomlDependency {
         if let Some(kind) = kind {
             dep.set_kind(kind);
         }
-        if let Some(rename) = rename {
+        if let Some(name_in_toml) = explicit_name_in_toml {
             cx.features.require(Feature::rename_dependency())?;
-            dep.set_rename(rename);
+            dep.set_explicit_name_in_toml(name_in_toml);
         }
         Ok(dep)
     }

src/crates-io/lib.rs

@@ -68,7 +68,10 @@ pub struct NewCrateDependency {
     pub version_req: String,
     pub target: Option<String>,
     pub kind: String,
-    #[serde(skip_serializing_if = "Option::is_none")] pub registry: Option<String>,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub registry: Option<String>,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub explicit_name_in_toml: Option<String>,
 }
 
 #[derive(Deserialize)]

tests/testsuite/registry.rs

@@ -6,7 +6,7 @@ use cargo::util::paths::remove_dir_all;
 use support::cargo_process;
 use support::git;
 use support::paths::{self, CargoPathExt};
-use support::registry::{self, Package};
+use support::registry::{self, Package, Dependency};
 use support::{basic_manifest, project};
 use url::Url;
 
@@ -1711,3 +1711,57 @@ fn git_init_templatedir_missing() {
     p.cargo("build").run();
     p.cargo("build").run();
 }
+
+#[test]
+fn rename_deps_and_features() {
+    Package::new("foo", "0.1.0")
+        .file("src/lib.rs", "pub fn f1() {}")
+        .publish();
+    Package::new("foo", "0.2.0")
+        .file("src/lib.rs", "pub fn f2() {}")
+        .publish();
+    Package::new("bar", "0.2.0")
+        .add_dep(Dependency::new("foo01", "0.1.0").package("foo").optional(true))
+        .add_dep(Dependency::new("foo02", "0.2.0").package("foo"))
+        .feature("another", &["foo01"])
+        .file(
+            "src/lib.rs",
+            r#"
+                extern crate foo02;
+                #[cfg(feature = "foo01")]
+                extern crate foo01;
+
+                pub fn foo() {
+                    foo02::f2();
+                    #[cfg(feature = "foo01")]
+                    foo01::f1();
+                }
+            "#,
+        )
+        .publish();
+
+    let p = project()
+        .file(
+            "Cargo.toml",
+            r#"
+                [project]
+                name = "a"
+                version = "0.5.0"
+                authors = []
+
+                [dependencies]
+                bar = "0.2"
+            "#,
+        ).file(
+            "src/main.rs",
+            "
+                extern crate bar;
+                fn main() { bar::foo(); }
+            ",
+        )
+        .build();
+
+    p.cargo("build").run();
+    p.cargo("build --features bar/foo01").run();
+    p.cargo("build --features bar/another").run();
+}