Skip to content

Commit

Permalink
Auto merge of #11387 - arlosi:sparse-url, r=weihanglo
Browse files Browse the repository at this point in the history
Store the sparse+ prefix in the SourceId for sparse registries

#11209 added a new `SourceKind::SparseRegistry` and removed the `sparse+` prefix from the URLs stored in the `SourceId`.

The removal of the `sparse+` prefix from the URLs in the `SourceId` has led to several bugs, since registry URLs no longer round-trip through a `SourceId`. The most recent one I found was that the example configuration generated by `cargo vendor` did not include the `sparse+` prefix. Any place that calls the `.url()` method on a `SourceId` potentially has this bug.

This change puts the `sparse+` prefix back in the URL stored in the `SourceId`, but keeps the new `SourceKind::SparseRegistry`.

A test is added for doing `cargo vendor` on an alternative registry using the sparse protocol.
  • Loading branch information
bors committed Nov 27, 2022
2 parents 1382b44 + 37d9b5c commit a004f94
Show file tree
Hide file tree
Showing 4 changed files with 55 additions and 23 deletions.
29 changes: 10 additions & 19 deletions src/cargo/core/source/source_id.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,17 +102,11 @@ impl SourceId {
SourceId { inner }
}

fn remote_source_kind(url: &Url) -> (SourceKind, Url) {
fn remote_source_kind(url: &Url) -> SourceKind {
if url.as_str().starts_with("sparse+") {
let url = url
.to_string()
.strip_prefix("sparse+")
.expect("we just found that prefix")
.into_url()
.expect("a valid url without a protocol specifier should still be valid");
(SourceKind::SparseRegistry, url)
SourceKind::SparseRegistry
} else {
(SourceKind::Registry, url.to_owned())
SourceKind::Registry
}
}

Expand Down Expand Up @@ -196,14 +190,14 @@ impl SourceId {
/// Use [`SourceId::for_alt_registry`] if a name can provided, which
/// generates better messages for cargo.
pub fn for_registry(url: &Url) -> CargoResult<SourceId> {
let (kind, url) = Self::remote_source_kind(url);
SourceId::new(kind, url, None)
let kind = Self::remote_source_kind(url);
SourceId::new(kind, url.to_owned(), None)
}

/// Creates a `SourceId` from a remote registry URL with given name.
pub fn for_alt_registry(url: &Url, name: &str) -> CargoResult<SourceId> {
let (kind, url) = Self::remote_source_kind(url);
SourceId::new(kind, url, Some(name))
let kind = Self::remote_source_kind(url);
SourceId::new(kind, url.to_owned(), Some(name))
}

/// Creates a SourceId from a local registry path.
Expand Down Expand Up @@ -263,7 +257,7 @@ impl SourceId {
return Self::crates_io(config);
}
let url = config.get_registry_index(key)?;
let (kind, url) = Self::remote_source_kind(&url);
let kind = Self::remote_source_kind(&url);
Ok(SourceId::wrap(SourceIdInner {
kind,
canonical_url: CanonicalUrl::new(&url)?,
Expand Down Expand Up @@ -428,10 +422,7 @@ impl SourceId {
let url = self.inner.url.as_str();
url == CRATES_IO_INDEX
|| url == CRATES_IO_HTTP_INDEX
|| std::env::var("__CARGO_TEST_CRATES_IO_URL_DO_NOT_USE_THIS")
.as_deref()
.map(|u| u.trim_start_matches("sparse+"))
== Ok(url)
|| std::env::var("__CARGO_TEST_CRATES_IO_URL_DO_NOT_USE_THIS").as_deref() == Ok(url)
}

/// Hashes `self`.
Expand Down Expand Up @@ -755,7 +746,7 @@ impl<'a> fmt::Display for SourceIdAsUrl<'a> {
ref url,
..
} => {
write!(f, "sparse+{url}")
write!(f, "{url}")
}
SourceIdInner {
kind: SourceKind::LocalRegistry,
Expand Down
10 changes: 8 additions & 2 deletions src/cargo/sources/registry/http_remote.rs
Original file line number Diff line number Diff line change
Expand Up @@ -163,15 +163,21 @@ impl<'cfg> HttpRegistry<'cfg> {
let url = source_id.url().as_str();
// Ensure the url ends with a slash so we can concatenate paths.
if !url.ends_with('/') {
anyhow::bail!("sparse registry url must end in a slash `/`: sparse+{url}")
anyhow::bail!("sparse registry url must end in a slash `/`: {url}")
}
assert!(source_id.is_sparse());
let url = url
.strip_prefix("sparse+")
.expect("sparse registry needs sparse+ prefix")
.into_url()
.expect("a url with the sparse+ stripped should still be valid");

Ok(HttpRegistry {
index_path: config.registry_index_path().join(name),
cache_path: config.registry_cache_path().join(name),
source_id,
config,
url: source_id.url().to_owned(),
url,
multi: Multi::new(),
multiplexing: false,
downloads: Downloads {
Expand Down
2 changes: 1 addition & 1 deletion src/cargo/sources/registry/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ use crate::util::{

const PACKAGE_SOURCE_LOCK: &str = ".cargo-ok";
pub const CRATES_IO_INDEX: &str = "https://github.com/rust-lang/crates.io-index";
pub const CRATES_IO_HTTP_INDEX: &str = "https://index.crates.io/";
pub const CRATES_IO_HTTP_INDEX: &str = "sparse+https://index.crates.io/";
pub const CRATES_IO_REGISTRY: &str = "crates-io";
pub const CRATES_IO_DOMAIN: &str = "crates.io";
const CRATE_TEMPLATE: &str = "{crate}";
Expand Down
37 changes: 36 additions & 1 deletion tests/testsuite/vendor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
use std::fs;

use cargo_test_support::git;
use cargo_test_support::registry::{self, Package};
use cargo_test_support::registry::{self, Package, RegistryBuilder};
use cargo_test_support::{basic_lib_manifest, paths, project, Project};

#[cargo_test]
Expand Down Expand Up @@ -68,6 +68,41 @@ directory = "vendor"
.run();
}

#[cargo_test]
fn vendor_sample_config_alt_registry() {
let registry = RegistryBuilder::new().alternative().http_index().build();
let p = project()
.file(
"Cargo.toml",
r#"
[package]
name = "foo"
version = "0.1.0"
[dependencies]
log = { version = "0.3.5", registry = "alternative" }
"#,
)
.file("src/lib.rs", "")
.build();

Package::new("log", "0.3.5").alternative(true).publish();

p.cargo("vendor --respect-source-config -Z sparse-registry")
.masquerade_as_nightly_cargo(&["sparse-registry"])
.with_stdout(format!(
r#"[source."{0}"]
registry = "{0}"
replace-with = "vendored-sources"
[source.vendored-sources]
directory = "vendor"
"#,
registry.index_url()
))
.run();
}

#[cargo_test]
fn vendor_path_specified() {
let p = project()
Expand Down

0 comments on commit a004f94

Please sign in to comment.