From 3fc27b2db7bb460edd09995526f238feedfb9a95 Mon Sep 17 00:00:00 2001 From: Jordan Doyle Date: Fri, 18 Mar 2022 01:28:19 +0000 Subject: [PATCH] Introduce `dl` key to allow per-crate download links This change allows the registry to override its own global `dl` key defined in the root config.json for each package version. --- crates/cargo-test-support/src/registry.rs | 11 ++++ crates/resolver-tests/src/lib.rs | 4 ++ src/cargo/core/resolver/version_prefs.rs | 10 ++- src/cargo/core/summary.rs | 6 ++ src/cargo/sources/registry/download.rs | 55 +++++++++-------- src/cargo/sources/registry/http_remote.rs | 8 ++- src/cargo/sources/registry/index.rs | 19 +++--- src/cargo/sources/registry/local.rs | 7 ++- src/cargo/sources/registry/mod.rs | 34 ++++++++--- src/cargo/sources/registry/remote.rs | 8 ++- src/cargo/util/toml/mod.rs | 1 + src/doc/src/reference/registries.md | 5 ++ tests/testsuite/alt_registry.rs | 74 ++++++++++++++++++++++- 13 files changed, 198 insertions(+), 44 deletions(-) diff --git a/crates/cargo-test-support/src/registry.rs b/crates/cargo-test-support/src/registry.rs index d3f3e71642a..1b25410f9bd 100644 --- a/crates/cargo-test-support/src/registry.rs +++ b/crates/cargo-test-support/src/registry.rs @@ -324,6 +324,7 @@ pub struct Package { rust_version: Option, cargo_features: Vec, v: Option, + dl: Option, } type FeatureMap = BTreeMap>; @@ -561,6 +562,7 @@ impl Package { rust_version: None, cargo_features: Vec::new(), v: None, + dl: None, } } @@ -709,6 +711,12 @@ impl Package { self } + /// Specifies a different download URI for the package. + pub fn dl(&mut self, dl: &str) -> &mut Package { + self.dl = Some(dl.to_string()); + self + } + pub fn cargo_feature(&mut self, feature: &str) -> &mut Package { self.cargo_features.push(feature.to_owned()); self @@ -788,6 +796,9 @@ impl Package { if let Some(v) = self.v { json["v"] = serde_json::json!(v); } + if let Some(dl) = &self.dl { + json["dl"] = serde_json::json!(dl); + } let line = json.to_string(); let file = make_dep_path(&self.name, false); diff --git a/crates/resolver-tests/src/lib.rs b/crates/resolver-tests/src/lib.rs index eae6469545d..8f5c1576e10 100644 --- a/crates/resolver-tests/src/lib.rs +++ b/crates/resolver-tests/src/lib.rs @@ -180,6 +180,7 @@ pub fn resolve_with_config_raw( deps, &BTreeMap::new(), None::<&String>, + None, ) .unwrap(); let opts = ResolveOpts::everything(); @@ -581,6 +582,7 @@ pub fn pkg_dep(name: T, dep: Vec) -> Summary { dep, &BTreeMap::new(), link, + None, ) .unwrap() } @@ -609,6 +611,7 @@ pub fn pkg_loc(name: &str, loc: &str) -> Summary { Vec::new(), &BTreeMap::new(), link, + None, ) .unwrap() } @@ -623,6 +626,7 @@ pub fn remove_dep(sum: &Summary, ind: usize) -> Summary { deps, &BTreeMap::new(), sum.links().map(|a| a.as_str()), + None, ) .unwrap() } diff --git a/src/cargo/core/resolver/version_prefs.rs b/src/cargo/core/resolver/version_prefs.rs index 8eb800c4058..5326efa1b99 100644 --- a/src/cargo/core/resolver/version_prefs.rs +++ b/src/cargo/core/resolver/version_prefs.rs @@ -92,7 +92,15 @@ mod test { let pkg_id = pkgid(name, version); let config = Config::default().unwrap(); let features = BTreeMap::new(); - Summary::new(&config, pkg_id, Vec::new(), &features, None::<&String>).unwrap() + Summary::new( + &config, + pkg_id, + Vec::new(), + &features, + None::<&String>, + None, + ) + .unwrap() } fn describe(summaries: &Vec) -> String { diff --git a/src/cargo/core/summary.rs b/src/cargo/core/summary.rs index 9c5396eecf5..00437be6644 100644 --- a/src/cargo/core/summary.rs +++ b/src/cargo/core/summary.rs @@ -25,6 +25,7 @@ struct Inner { features: Rc, checksum: Option, links: Option, + dl: Option, } impl Summary { @@ -34,6 +35,7 @@ impl Summary { dependencies: Vec, features: &BTreeMap>, links: Option>, + dl: Option, ) -> CargoResult { // ****CAUTION**** If you change anything here that may raise a new // error, be sure to coordinate that change with either the index @@ -55,6 +57,7 @@ impl Summary { features: Rc::new(feature_map), checksum: None, links: links.map(|l| l.into()), + dl, }), }) } @@ -84,6 +87,9 @@ impl Summary { pub fn links(&self) -> Option { self.inner.links } + pub fn dl(&self) -> Option { + self.inner.dl + } pub fn override_id(mut self, id: PackageId) -> Summary { Rc::make_mut(&mut self.inner).package_id = id; diff --git a/src/cargo/sources/registry/download.rs b/src/cargo/sources/registry/download.rs index cc39d7c1113..2e294035ce3 100644 --- a/src/cargo/sources/registry/download.rs +++ b/src/cargo/sources/registry/download.rs @@ -24,6 +24,7 @@ pub(super) fn download( cache_path: &Filesystem, config: &Config, pkg: PackageId, + override_dl: Option<&str>, checksum: &str, registry_config: RegistryConfig, ) -> CargoResult { @@ -44,30 +45,36 @@ pub(super) fn download( } } - let mut url = registry_config.dl; - if !url.contains(CRATE_TEMPLATE) - && !url.contains(VERSION_TEMPLATE) - && !url.contains(PREFIX_TEMPLATE) - && !url.contains(LOWER_PREFIX_TEMPLATE) - && !url.contains(CHECKSUM_TEMPLATE) - { - // Original format before customizing the download URL was supported. - write!( - url, - "/{}/{}/download", - pkg.name(), - pkg.version().to_string() - ) - .unwrap(); - } else { - let prefix = make_dep_prefix(&*pkg.name()); - url = url - .replace(CRATE_TEMPLATE, &*pkg.name()) - .replace(VERSION_TEMPLATE, &pkg.version().to_string()) - .replace(PREFIX_TEMPLATE, &prefix) - .replace(LOWER_PREFIX_TEMPLATE, &prefix.to_lowercase()) - .replace(CHECKSUM_TEMPLATE, checksum); - } + let url = override_dl.map_or_else( + || { + let mut url = registry_config.dl; + if !url.contains(CRATE_TEMPLATE) + && !url.contains(VERSION_TEMPLATE) + && !url.contains(PREFIX_TEMPLATE) + && !url.contains(LOWER_PREFIX_TEMPLATE) + && !url.contains(CHECKSUM_TEMPLATE) + { + // Original format before customizing the download URL was supported. + write!( + url, + "/{}/{}/download", + pkg.name(), + pkg.version().to_string() + ) + .unwrap(); + } else { + let prefix = make_dep_prefix(&*pkg.name()); + url = url + .replace(CRATE_TEMPLATE, &*pkg.name()) + .replace(VERSION_TEMPLATE, &pkg.version().to_string()) + .replace(PREFIX_TEMPLATE, &prefix) + .replace(LOWER_PREFIX_TEMPLATE, &prefix.to_lowercase()) + .replace(CHECKSUM_TEMPLATE, checksum); + } + url + }, + ToString::to_string, + ); Ok(MaybeLock::Download { url, diff --git a/src/cargo/sources/registry/http_remote.rs b/src/cargo/sources/registry/http_remote.rs index 326c3f34855..f4a82e30c4f 100644 --- a/src/cargo/sources/registry/http_remote.rs +++ b/src/cargo/sources/registry/http_remote.rs @@ -548,7 +548,12 @@ impl<'cfg> RegistryData for HttpRegistry<'cfg> { self.requested_update = true; } - fn download(&mut self, pkg: PackageId, checksum: &str) -> CargoResult { + fn download( + &mut self, + pkg: PackageId, + override_dl: Option<&str>, + checksum: &str, + ) -> CargoResult { let registry_config = loop { match self.config()? { Poll::Pending => self.block_until_ready()?, @@ -559,6 +564,7 @@ impl<'cfg> RegistryData for HttpRegistry<'cfg> { &self.cache_path, &self.config, pkg, + override_dl, checksum, registry_config, ) diff --git a/src/cargo/sources/registry/index.rs b/src/cargo/sources/registry/index.rs index e7c8d220947..8a31d78a475 100644 --- a/src/cargo/sources/registry/index.rs +++ b/src/cargo/sources/registry/index.rs @@ -265,19 +265,21 @@ impl<'cfg> RegistryIndex<'cfg> { } } - /// Returns the hash listed for a specified `PackageId`. - pub fn hash(&mut self, pkg: PackageId, load: &mut dyn RegistryData) -> Poll> { + /// Returns the package summary for a specified `PackageId`. + pub fn summary( + &mut self, + pkg: PackageId, + load: &mut dyn RegistryData, + ) -> Poll> { let req = OptVersionReq::exact(pkg.version()); let summary = self.summaries(pkg.name(), &req, load)?; let summary = match summary { Poll::Ready(mut summary) => summary.next(), Poll::Pending => return Poll::Pending, }; - Poll::Ready(Ok(summary - .ok_or_else(|| internal(format!("no hash listed for {}", pkg)))? - .summary - .checksum() - .ok_or_else(|| internal(format!("no hash listed for {}", pkg)))?)) + Poll::Ready(Ok( + summary.ok_or_else(|| internal(format!("no summary for {}", pkg)))? + )) } /// Load a list of summaries for `name` package in this registry which @@ -854,6 +856,7 @@ impl IndexSummary { yanked, links, v, + dl, } = serde_json::from_slice(line)?; let v = v.unwrap_or(1); log::trace!("json parsed registry {}/{}", name, vers); @@ -867,7 +870,7 @@ impl IndexSummary { features.entry(name).or_default().extend(values); } } - let mut summary = Summary::new(config, pkgid, deps, &features, links)?; + let mut summary = Summary::new(config, pkgid, deps, &features, links, dl)?; summary.set_checksum(cksum); Ok(IndexSummary { summary, diff --git a/src/cargo/sources/registry/local.rs b/src/cargo/sources/registry/local.rs index 474c5f03b32..f781cca1e07 100644 --- a/src/cargo/sources/registry/local.rs +++ b/src/cargo/sources/registry/local.rs @@ -99,7 +99,12 @@ impl<'cfg> RegistryData for LocalRegistry<'cfg> { self.updated } - fn download(&mut self, pkg: PackageId, checksum: &str) -> CargoResult { + fn download( + &mut self, + pkg: PackageId, + _override_dl: Option<&str>, + checksum: &str, + ) -> CargoResult { let crate_file = format!("{}-{}.crate", pkg.name(), pkg.version()); // Note that the usage of `into_path_unlocked` here is because the local diff --git a/src/cargo/sources/registry/mod.rs b/src/cargo/sources/registry/mod.rs index f0a770c4c51..2d0b0bf94cd 100644 --- a/src/cargo/sources/registry/mod.rs +++ b/src/cargo/sources/registry/mod.rs @@ -176,8 +176,10 @@ use tar::Archive; use crate::core::dependency::{DepKind, Dependency}; use crate::core::source::MaybePackage; use crate::core::{Package, PackageId, Source, SourceId, Summary}; +use crate::sources::registry::index::IndexSummary; use crate::sources::PathSource; use crate::util::hex; +use crate::util::internal; use crate::util::interning::InternedString; use crate::util::into_url::IntoUrl; use crate::util::network::PollExt; @@ -277,6 +279,8 @@ pub struct RegistryPackage<'a> { /// Added early 2018 (see ), /// can be `None` if published before then. links: Option, + /// Allows overriding the registry's global `dl` link for this package only. + dl: Option, /// The schema version for this entry. /// /// If this is None, it defaults to version 1. Entries with unknown @@ -481,7 +485,12 @@ pub trait RegistryData { /// `finish_download`. For already downloaded `.crate` files, it does not /// validate the checksum, assuming the filesystem does not suffer from /// corruption or manipulation. - fn download(&mut self, pkg: PackageId, checksum: &str) -> CargoResult; + fn download( + &mut self, + pkg: PackageId, + override_dl: Option<&str>, + checksum: &str, + ) -> CargoResult; /// Finish a download by saving a `.crate` file to disk. /// @@ -764,13 +773,20 @@ impl<'cfg> Source for RegistrySource<'cfg> { } fn download(&mut self, package: PackageId) -> CargoResult { - let hash = loop { - match self.index.hash(package, &mut *self.ops)? { + let (hash, override_dl) = loop { + match self.index.summary(package, &mut *self.ops)? { Poll::Pending => self.block_until_ready()?, - Poll::Ready(hash) => break hash, + Poll::Ready(IndexSummary { ref summary, .. }) => { + break ( + summary + .checksum() + .ok_or_else(|| internal(format!("no hash listed for {}", package)))?, + summary.dl(), + ) + } } }; - match self.ops.download(package, hash)? { + match self.ops.download(package, override_dl.as_deref(), hash)? { MaybeLock::Ready(file) => self.get_pkg(package, &file).map(MaybePackage::Ready), MaybeLock::Download { url, descriptor } => { Ok(MaybePackage::Download { url, descriptor }) @@ -780,9 +796,13 @@ impl<'cfg> Source for RegistrySource<'cfg> { fn finish_download(&mut self, package: PackageId, data: Vec) -> CargoResult { let hash = loop { - match self.index.hash(package, &mut *self.ops)? { + match self.index.summary(package, &mut *self.ops)? { Poll::Pending => self.block_until_ready()?, - Poll::Ready(hash) => break hash, + Poll::Ready(IndexSummary { ref summary, .. }) => { + break summary + .checksum() + .ok_or_else(|| internal(format!("no hash listed for {}", package)))? + } } }; let file = self.ops.finish_download(package, hash, &data)?; diff --git a/src/cargo/sources/registry/remote.rs b/src/cargo/sources/registry/remote.rs index d3ce8864d70..cbd288b26f1 100644 --- a/src/cargo/sources/registry/remote.rs +++ b/src/cargo/sources/registry/remote.rs @@ -296,7 +296,12 @@ impl<'cfg> RegistryData for RemoteRegistry<'cfg> { self.updated } - fn download(&mut self, pkg: PackageId, checksum: &str) -> CargoResult { + fn download( + &mut self, + pkg: PackageId, + override_dl: Option<&str>, + checksum: &str, + ) -> CargoResult { let registry_config = loop { match self.config()? { Poll::Pending => self.block_until_ready()?, @@ -308,6 +313,7 @@ impl<'cfg> RegistryData for RemoteRegistry<'cfg> { &self.cache_path, &self.config, pkg, + override_dl, checksum, registry_config, ) diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index b49bdce5139..ac7590f0f61 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -1627,6 +1627,7 @@ impl TomlManifest { deps, me.features.as_ref().unwrap_or(&empty_features), project.links.as_deref(), + None, )?; let metadata = ManifestMetadata { diff --git a/src/doc/src/reference/registries.md b/src/doc/src/reference/registries.md index 45af03236f4..0a12ee9bede 100644 --- a/src/doc/src/reference/registries.md +++ b/src/doc/src/reference/registries.md @@ -264,6 +264,11 @@ explaining the format of the entry. // The `links` string value from the package's manifest, or null if not // specified. This field is optional and defaults to null. "links": null, + // This optional field specifies the download link for this package + // only, overriding the global `dl` link declared in the registry's + // `config.toml`. The `{crate}` markers et al. are not interpolated + // in this context. + "dl": "https://crates.io/api/v1/crates/foo/0.1.0/download" // An unsigned 32-bit integer value indicating the schema version of this // entry. // diff --git a/tests/testsuite/alt_registry.rs b/tests/testsuite/alt_registry.rs index b0580d57eae..1fd20eb9112 100644 --- a/tests/testsuite/alt_registry.rs +++ b/tests/testsuite/alt_registry.rs @@ -2,7 +2,7 @@ use cargo::util::IntoUrl; use cargo_test_support::publish::validate_alt_upload; -use cargo_test_support::registry::{self, Package}; +use cargo_test_support::registry::{self, generate_alt_dl_url, Package}; use cargo_test_support::{basic_manifest, git, paths, project}; use std::fs; @@ -737,6 +737,78 @@ fn no_api() { .run(); } +#[cargo_test] +fn dl_override() { + registry::alt_init(); + Package::new("bar", "0.0.1") + .alternative(true) + .dl(®istry::alt_dl_url() + .replace("{crate}", "bar") + .replace("{version}", "0.0.1")) + .publish(); + Package::new("qux", "0.0.1").alternative(true).publish(); + let repo = git2::Repository::open(registry::alt_registry_path()).unwrap(); + let cfg_path = registry::alt_registry_path().join("config.json"); + fs::write( + cfg_path, + format!(r#"{{"dl": "{}"}}"#, generate_alt_dl_url("fake")), + ) + .unwrap(); + git::add(&repo); + git::commit(&repo); + + // First check that a dependency works. + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.0.1" + + [dependencies.bar] + version = "0.0.1" + registry = "alternative" + "#, + ) + .file("src/lib.rs", "") + .build(); + + p.cargo("build") + .with_stderr( + "\ +[UPDATING] `alternative` index +[DOWNLOADING] crates ... +[DOWNLOADED] bar v0.0.1 (registry `alternative`) +[COMPILING] bar v0.0.1 (registry `alternative`) +[COMPILING] foo v0.0.1 ([CWD]) +[FINISHED] dev [unoptimized + debuginfo] target(s) in [..]s +", + ) + .run(); + + // Then check our qux dependency fails because it uses the fake path + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.0.1" + + [dependencies.qux] + version = "0.0.1" + registry = "alternative" + "#, + ) + .file("src/lib.rs", "") + .build(); + + p.cargo("build") + .with_stderr_contains("[..]error: failed to download from[..]") + .run_expect_error(); +} + #[cargo_test] fn alt_reg_metadata() { // Check for "registry" entries in `cargo metadata` with alternative registries.