Skip to content

Commit 5788d76

Browse files
committedOct 12, 2022
Auto merge of #11209 - arlosi:sparse-kind, r=ehuss
Add new SourceKind::SparseRegistry to differentiate sparse registries Refactor sparse registry to have its own `SourceKind`. Follow up from #11177 (comment) r? `@ehuss`
2 parents 642a0e6 + 56f6816 commit 5788d76

File tree

4 files changed

+60
-33
lines changed

4 files changed

+60
-33
lines changed
 

‎src/cargo/core/source/source_id.rs

+54-23
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,8 @@ enum SourceKind {
5555
Path,
5656
/// A remote registry.
5757
Registry,
58+
/// A sparse registry.
59+
SparseRegistry,
5860
/// A local filesystem-based registry.
5961
LocalRegistry,
6062
/// A directory-based registry.
@@ -100,6 +102,20 @@ impl SourceId {
100102
SourceId { inner }
101103
}
102104

105+
fn remote_source_kind(url: &Url) -> (SourceKind, Url) {
106+
if url.as_str().starts_with("sparse+") {
107+
let url = url
108+
.to_string()
109+
.strip_prefix("sparse+")
110+
.expect("we just found that prefix")
111+
.into_url()
112+
.expect("a valid url without a protocol specifier should still be valid");
113+
(SourceKind::SparseRegistry, url)
114+
} else {
115+
(SourceKind::Registry, url.to_owned())
116+
}
117+
}
118+
103119
/// Parses a source URL and returns the corresponding ID.
104120
///
105121
/// ## Example
@@ -142,8 +158,8 @@ impl SourceId {
142158
.with_precise(Some("locked".to_string())))
143159
}
144160
"sparse" => {
145-
let url = string.into_url()?;
146-
Ok(SourceId::new(SourceKind::Registry, url, None)?
161+
let url = url.into_url()?;
162+
Ok(SourceId::new(SourceKind::SparseRegistry, url, None)?
147163
.with_precise(Some("locked".to_string())))
148164
}
149165
"path" => {
@@ -180,12 +196,14 @@ impl SourceId {
180196
/// Use [`SourceId::for_alt_registry`] if a name can provided, which
181197
/// generates better messages for cargo.
182198
pub fn for_registry(url: &Url) -> CargoResult<SourceId> {
183-
SourceId::new(SourceKind::Registry, url.clone(), None)
199+
let (kind, url) = Self::remote_source_kind(url);
200+
SourceId::new(kind, url, None)
184201
}
185202

186203
/// Creates a `SourceId` from a remote registry URL with given name.
187204
pub fn for_alt_registry(url: &Url, name: &str) -> CargoResult<SourceId> {
188-
SourceId::new(SourceKind::Registry, url.clone(), Some(name))
205+
let (kind, url) = Self::remote_source_kind(url);
206+
SourceId::new(kind, url, Some(name))
189207
}
190208

191209
/// Creates a SourceId from a local registry path.
@@ -218,7 +236,7 @@ impl SourceId {
218236
if Self::crates_io_is_sparse(config)? {
219237
config.check_registry_index_not_set()?;
220238
let url = CRATES_IO_HTTP_INDEX.into_url().unwrap();
221-
SourceId::new(SourceKind::Registry, url, Some(CRATES_IO_REGISTRY))
239+
SourceId::new(SourceKind::SparseRegistry, url, Some(CRATES_IO_REGISTRY))
222240
} else {
223241
Self::crates_io(config)
224242
}
@@ -245,8 +263,9 @@ impl SourceId {
245263
return Self::crates_io(config);
246264
}
247265
let url = config.get_registry_index(key)?;
266+
let (kind, url) = Self::remote_source_kind(&url);
248267
Ok(SourceId::wrap(SourceIdInner {
249-
kind: SourceKind::Registry,
268+
kind,
250269
canonical_url: CanonicalUrl::new(&url)?,
251270
url,
252271
precise: None,
@@ -313,16 +332,24 @@ impl SourceId {
313332
pub fn is_registry(self) -> bool {
314333
matches!(
315334
self.inner.kind,
316-
SourceKind::Registry | SourceKind::LocalRegistry
335+
SourceKind::Registry | SourceKind::SparseRegistry | SourceKind::LocalRegistry
317336
)
318337
}
319338

339+
/// Returns `true` if this source is from a sparse registry.
340+
pub fn is_sparse(self) -> bool {
341+
matches!(self.inner.kind, SourceKind::SparseRegistry)
342+
}
343+
320344
/// Returns `true` if this source is a "remote" registry.
321345
///
322346
/// "remote" may also mean a file URL to a git index, so it is not
323347
/// necessarily "remote". This just means it is not `local-registry`.
324348
pub fn is_remote_registry(self) -> bool {
325-
matches!(self.inner.kind, SourceKind::Registry)
349+
matches!(
350+
self.inner.kind,
351+
SourceKind::Registry | SourceKind::SparseRegistry
352+
)
326353
}
327354

328355
/// Returns `true` if this source from a Git repository.
@@ -346,11 +373,9 @@ impl SourceId {
346373
};
347374
Ok(Box::new(PathSource::new(&path, self, config)))
348375
}
349-
SourceKind::Registry => Ok(Box::new(RegistrySource::remote(
350-
self,
351-
yanked_whitelist,
352-
config,
353-
)?)),
376+
SourceKind::Registry | SourceKind::SparseRegistry => Ok(Box::new(
377+
RegistrySource::remote(self, yanked_whitelist, config)?,
378+
)),
354379
SourceKind::LocalRegistry => {
355380
let path = match self.inner.url.to_file_path() {
356381
Ok(p) => p,
@@ -397,7 +422,7 @@ impl SourceId {
397422
/// Returns `true` if the remote registry is the standard <https://crates.io>.
398423
pub fn is_crates_io(self) -> bool {
399424
match self.inner.kind {
400-
SourceKind::Registry => {}
425+
SourceKind::Registry | SourceKind::SparseRegistry => {}
401426
_ => return false,
402427
}
403428
let url = self.inner.url.as_str();
@@ -529,7 +554,9 @@ impl fmt::Display for SourceId {
529554
Ok(())
530555
}
531556
SourceKind::Path => write!(f, "{}", url_display(&self.inner.url)),
532-
SourceKind::Registry => write!(f, "registry `{}`", self.display_registry_name()),
557+
SourceKind::Registry | SourceKind::SparseRegistry => {
558+
write!(f, "registry `{}`", self.display_registry_name())
559+
}
533560
SourceKind::LocalRegistry => write!(f, "registry `{}`", url_display(&self.inner.url)),
534561
SourceKind::Directory => write!(f, "dir {}", url_display(&self.inner.url)),
535562
}
@@ -643,6 +670,10 @@ impl Ord for SourceKind {
643670
(SourceKind::Registry, _) => Ordering::Less,
644671
(_, SourceKind::Registry) => Ordering::Greater,
645672

673+
(SourceKind::SparseRegistry, SourceKind::SparseRegistry) => Ordering::Equal,
674+
(SourceKind::SparseRegistry, _) => Ordering::Less,
675+
(_, SourceKind::SparseRegistry) => Ordering::Greater,
676+
646677
(SourceKind::LocalRegistry, SourceKind::LocalRegistry) => Ordering::Equal,
647678
(SourceKind::LocalRegistry, _) => Ordering::Less,
648679
(_, SourceKind::LocalRegistry) => Ordering::Greater,
@@ -714,14 +745,14 @@ impl<'a> fmt::Display for SourceIdAsUrl<'a> {
714745
ref url,
715746
..
716747
} => {
717-
// For sparse http registry the URL already contains the prefix.
718-
if url.scheme().starts_with("sparse+") {
719-
// e.g. sparse+http://example.com
720-
write!(f, "{url}")
721-
} else {
722-
// e.g. registry+http://example.com
723-
write!(f, "registry+{url}")
724-
}
748+
write!(f, "registry+{url}")
749+
}
750+
SourceIdInner {
751+
kind: SourceKind::SparseRegistry,
752+
ref url,
753+
..
754+
} => {
755+
write!(f, "sparse+{url}")
725756
}
726757
SourceIdInner {
727758
kind: SourceKind::LocalRegistry,

‎src/cargo/sources/registry/http_remote.rs

+3-7
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ use crate::sources::registry::MaybeLock;
99
use crate::sources::registry::{LoadResponse, RegistryConfig, RegistryData};
1010
use crate::util::errors::{CargoResult, HttpNotSuccessful};
1111
use crate::util::network::Retry;
12-
use crate::util::{internal, Config, Filesystem, IntoUrl, Progress, ProgressStyle};
12+
use crate::util::{internal, Config, Filesystem, Progress, ProgressStyle};
1313
use anyhow::Context;
1414
use cargo_util::paths;
1515
use curl::easy::{HttpVersion, List};
@@ -137,19 +137,15 @@ impl<'cfg> HttpRegistry<'cfg> {
137137
let url = source_id.url().as_str();
138138
// Ensure the url ends with a slash so we can concatenate paths.
139139
if !url.ends_with('/') {
140-
anyhow::bail!("registry url must end in a slash `/`: {url}")
140+
anyhow::bail!("sparse registry url must end in a slash `/`: sparse+{url}")
141141
}
142-
let url = url
143-
.trim_start_matches("sparse+")
144-
.into_url()
145-
.expect("a url with the protocol stripped should still be valid");
146142

147143
Ok(HttpRegistry {
148144
index_path: config.registry_index_path().join(name),
149145
cache_path: config.registry_cache_path().join(name),
150146
source_id,
151147
config,
152-
url,
148+
url: source_id.url().to_owned(),
153149
multi: Multi::new(),
154150
multiplexing: false,
155151
downloads: Downloads {

‎src/cargo/sources/registry/mod.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -549,7 +549,7 @@ impl<'cfg> RegistrySource<'cfg> {
549549
config: &'cfg Config,
550550
) -> CargoResult<RegistrySource<'cfg>> {
551551
let name = short_name(source_id);
552-
let ops = if source_id.url().scheme().starts_with("sparse+") {
552+
let ops = if source_id.is_sparse() {
553553
Box::new(http_remote::HttpRegistry::new(source_id, config, &name)?) as Box<_>
554554
} else {
555555
Box::new(remote::RemoteRegistry::new(source_id, config, &name)) as Box<_>

‎tests/testsuite/registry.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -2723,10 +2723,10 @@ fn protocol() {
27232723

27242724
#[cargo_test]
27252725
fn http_requires_trailing_slash() {
2726-
cargo_process("-Z sparse-registry install bar --index sparse+https://index.crates.io")
2726+
cargo_process("-Z sparse-registry install bar --index sparse+https://invalid.crates.io/test")
27272727
.masquerade_as_nightly_cargo(&["sparse-registry"])
27282728
.with_status(101)
2729-
.with_stderr("[ERROR] registry url must end in a slash `/`: sparse+https://index.crates.io")
2729+
.with_stderr("[ERROR] sparse registry url must end in a slash `/`: sparse+https://invalid.crates.io/test")
27302730
.run()
27312731
}
27322732

0 commit comments

Comments
 (0)
Please sign in to comment.