Skip to content
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

Add dependency registry to cargo metadata. #6500

Merged
merged 2 commits into from
Jan 2, 2019
Merged
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
2 changes: 2 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ path = "src/cargo/lib.rs"

[dependencies]
atty = "0.2"
byteorder = "1.2"
bytesize = "1.0"
crates-io = { path = "src/crates-io", version = "0.22" }
crossbeam-utils = "0.6"
Expand Down Expand Up @@ -57,6 +58,7 @@ tempfile = "3.0"
termcolor = "1.0"
toml = "0.4.2"
url = "1.1"
url_serde = "0.2.0"
clap = "2.31.2"
unicode-width = "0.1.5"
openssl = { version = '0.10.11', optional = true }
Expand Down
12 changes: 12 additions & 0 deletions src/cargo/core/dependency.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use semver::ReqParseError;
use semver::VersionReq;
use serde::ser;
use serde::Serialize;
use url::Url;

use crate::core::interning::InternedString;
use crate::core::{PackageId, SourceId, Summary};
Expand All @@ -25,6 +26,12 @@ pub struct Dependency {
struct Inner {
name: InternedString,
source_id: SourceId,
/// Source ID for the registry as specified in the manifest.
///
/// This will be None if it is not specified (crates.io dependency).
/// This is different from `source_id` for example when both a `path` and
/// `registry` is specified. Or in the case of a crates.io dependency,
/// `source_id` will be crates.io and this will be None.
registry_id: Option<SourceId>,
req: VersionReq,
specified_req: bool,
Expand Down Expand Up @@ -59,6 +66,10 @@ struct SerializedDependency<'a> {
uses_default_features: bool,
features: &'a [InternedString],
target: Option<&'a Platform>,
/// The registry URL this dependency is from.
/// If None, then it comes from the default registry (crates.io).
#[serde(with = "url_serde")]
registry: Option<Url>,
}

impl ser::Serialize for Dependency {
Expand All @@ -76,6 +87,7 @@ impl ser::Serialize for Dependency {
features: self.features(),
target: self.platform(),
rename: self.explicit_name_in_toml().map(|s| s.as_str()),
registry: self.registry_id().map(|sid| sid.url().clone()),
}
.serialize(s)
}
Expand Down
4 changes: 4 additions & 0 deletions src/cargo/core/source/source_id.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ struct SourceIdInner {
// e.g. the exact git revision of the specified branch for a Git Source
precise: Option<String>,
/// Name of the registry source for alternative registries
/// WARNING: This is not always set for alt-registries when the name is
/// not known.
name: Option<String>,
}

Expand Down Expand Up @@ -247,6 +249,8 @@ impl SourceId {
}

/// Is this source from an alternative registry
/// DEPRECATED: This is not correct if the registry name is not known
/// (for example when loaded from an index).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To confirm, is this left in for clients of the cargo crate? Or does Cargo still use this a lot internally?

(if it's the former I think we can just delete this)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is one usage in cargo. I believe it is currently correct due to where it is used, but I put it on my todo list to remove it in the future since it is easy to get wrong. Its removal isn't trivial, and was unrelated to this PR.

pub fn is_alt_registry(self) -> bool {
self.is_registry() && self.inner.name.is_some()
}
Expand Down
4 changes: 3 additions & 1 deletion src/cargo/ops/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -159,8 +159,10 @@ fn transmit(
// registry in the dependency.
let dep_registry_id = match dep.registry_id() {
Some(id) => id,
None => failure::bail!("dependency missing registry ID"),
None => SourceId::crates_io(config)?,
};
// In the index and Web API, None means "from the same registry"
// whereas in Cargo.toml, it means "from crates.io".
let dep_registry = if dep_registry_id != registry_id {
Some(dep_registry_id.url().to_string())
} else {
Expand Down
25 changes: 8 additions & 17 deletions src/cargo/sources/registry/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,7 @@ impl<'a> RegistryDependency<'a> {
package,
} = self;

let id = if let Some(registry) = registry {
let id = if let Some(registry) = &registry {
SourceId::for_registry(&registry.to_url()?)?
} else {
default
Expand Down Expand Up @@ -328,6 +328,12 @@ impl<'a> RegistryDependency<'a> {
// out here.
features.retain(|s| !s.is_empty());

// In index, "registry" is null if it is from the same index.
// In Cargo.toml, "registry" is None if it is from the default
if !id.is_default_registry() {
dep.set_registry_id(id);
}

dep.set_optional(optional)
.set_default_features(default_features)
.set_features(features)
Expand Down Expand Up @@ -486,22 +492,7 @@ impl<'cfg> RegistrySource<'cfg> {
MaybePackage::Ready(pkg) => pkg,
MaybePackage::Download { .. } => unreachable!(),
};

// Unfortunately the index and the actual Cargo.toml in the index can
// differ due to historical Cargo bugs. To paper over these we trash the
// *summary* loaded from the Cargo.toml we just downloaded with the one
// we loaded from the index.
let summaries = self
.index
.summaries(package.name().as_str(), &mut *self.ops)?;
let summary = summaries
.iter()
.map(|s| &s.0)
.find(|s| s.package_id() == package)
.expect("summary not found");
let mut manifest = pkg.manifest().clone();
manifest.set_summary(summary.clone());
Ok(Package::new(manifest, pkg.manifest_path()))
Ok(pkg)
}
}

Expand Down
29 changes: 19 additions & 10 deletions src/cargo/util/toml/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,12 @@ impl<'de> de::Deserialize<'de> for TomlDependency {
pub struct DetailedTomlDependency {
version: Option<String>,
registry: Option<String>,
/// The URL of the `registry` field.
/// This is an internal implementation detail. When Cargo creates a
/// package, it replaces `registry` with `registry-index` so that the
/// manifest contains the correct URL. All users won't have the same
/// registry names configured, so Cargo can't rely on just the name for
/// crates published by other users.
registry_index: Option<String>,
path: Option<String>,
git: Option<String>,
Expand Down Expand Up @@ -1307,14 +1313,6 @@ impl DetailedTomlDependency {
}
}

let registry_id = match self.registry {
Some(ref registry) => {
cx.features.require(Feature::alternative_registries())?;
SourceId::alt_registry(cx.config, registry)?
}
None => SourceId::crates_io(cx.config)?,
};

let new_source_id = match (
self.git.as_ref(),
self.path.as_ref(),
Expand Down Expand Up @@ -1410,8 +1408,19 @@ impl DetailedTomlDependency {
.unwrap_or(true),
)
.set_optional(self.optional.unwrap_or(false))
.set_platform(cx.platform.clone())
.set_registry_id(registry_id);
.set_platform(cx.platform.clone());
if let Some(registry) = &self.registry {
cx.features.require(Feature::alternative_registries())?;
let registry_id = SourceId::alt_registry(cx.config, registry)?;
dep.set_registry_id(registry_id);
}
if let Some(registry_index) = &self.registry_index {
cx.features.require(Feature::alternative_registries())?;
let url = registry_index.to_url()?;
let registry_id = SourceId::for_registry(&url)?;
dep.set_registry_id(registry_id);
}

if let Some(kind) = kind {
dep.set_kind(kind);
}
Expand Down
11 changes: 9 additions & 2 deletions src/doc/man/cargo-metadata.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,9 @@ The output has the following format:
{
/* The name of the dependency. */
"name": "bitflags",
/* The source ID of the dependency. */
/* The source ID of the dependency. May be null, see
description for the package source.
*/
"source": "registry+https://github.com/rust-lang/crates.io-index",
/* The version requirement for the dependency.
Dependencies without a version requirement have a value of "*".
Expand All @@ -84,7 +86,12 @@ The output has the following format:
/* The target platform for the dependency.
null if not a target dependency.
*/
"target": "cfg(windows)"
"target": "cfg(windows)",
/* A string of the URL of the registry this dependency is from.
If not specified or null, the dependency is from the default
registry (crates.io).
*/
"registry": null
}
],
/* Array of Cargo targets. */
Expand Down
Loading