Skip to content

Commit 0f8b797

Browse files
dzbarskyillicitonion
dzbarsky
and
illicitonion
authored
[cargo-bazel] Avoid repeatedly hitting the filesystem during CrateIndexLookup (#3589)
Only use the crate index if we don't have the checksums already from the Cargo.lock. Also we can compute the crate_index just once. --------- Co-authored-by: Daniel Wagner-Hall <[email protected]>
1 parent 3585384 commit 0f8b797

File tree

2 files changed

+109
-79
lines changed

2 files changed

+109
-79
lines changed

‎crate_universe/src/splicing.rs‎

Lines changed: 49 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -263,7 +263,7 @@ impl WorkspaceMetadata {
263263
) -> Result<()> {
264264
let mut manifest = read_manifest(input_manifest_path)?;
265265

266-
let mut workspace_metaata = WorkspaceMetadata::try_from(
266+
let mut workspace_metadata = WorkspaceMetadata::try_from(
267267
manifest
268268
.workspace
269269
.as_ref()
@@ -326,43 +326,61 @@ impl WorkspaceMetadata {
326326
let index = if cargo.use_sparse_registries_for_crates_io()?
327327
&& index_url == utils::CRATES_IO_INDEX_URL
328328
{
329-
CrateIndexLookup::Http(crates_index::SparseIndex::from_url_with_hash_kind(
329+
let index = crates_index::SparseIndex::from_url_with_hash_kind(
330330
"sparse+https://index.crates.io/",
331331
&crate_index_hash_kind,
332-
)?)
332+
)?;
333+
let index_config = index
334+
.index_config()
335+
.context("Failed to get crate index config")?;
336+
CrateIndexLookup::Http {
337+
index,
338+
index_config,
339+
}
333340
} else if index_url.starts_with("sparse+") {
334-
CrateIndexLookup::Http(crates_index::SparseIndex::from_url_with_hash_kind(
341+
let index = crates_index::SparseIndex::from_url_with_hash_kind(
335342
index_url,
336343
&crate_index_hash_kind,
337-
)?)
344+
)?;
345+
let index_config = index
346+
.index_config()
347+
.context("Failed to get crate index config")?;
348+
CrateIndexLookup::Http {
349+
index,
350+
index_config,
351+
}
338352
} else {
339353
match source_kind {
340354
SourceKind::Registry => {
341-
let index = {
342-
// Load the index for the current url
343-
let index = crates_index::GitIndex::from_url_with_hash_kind(
344-
index_url,
345-
&crate_index_hash_kind,
346-
)
347-
.with_context(|| {
348-
format!("Failed to load index for url: {index_url}")
349-
})?;
350-
351-
// Ensure each index has a valid index config
352-
index.index_config().with_context(|| {
353-
format!("`config.json` not found in index: {index_url}")
354-
})?;
355-
356-
index
357-
};
358-
CrateIndexLookup::Git(index)
355+
let index = crates_index::GitIndex::from_url_with_hash_kind(
356+
index_url,
357+
&crate_index_hash_kind,
358+
)
359+
.with_context(|| {
360+
format!("Failed to load index for url: {index_url}")
361+
})?;
362+
363+
let index_config = index.index_config().with_context(|| {
364+
format!("`config.json` not found in index: {index_url}")
365+
})?;
366+
CrateIndexLookup::Git {
367+
index,
368+
index_config,
369+
}
359370
}
360-
SourceKind::SparseRegistry => CrateIndexLookup::Http(
361-
crates_index::SparseIndex::from_url_with_hash_kind(
371+
SourceKind::SparseRegistry => {
372+
let index = crates_index::SparseIndex::from_url_with_hash_kind(
362373
format!("sparse+{}", index_url).as_str(),
363374
&crate_index_hash_kind,
364-
)?,
365-
),
375+
)?;
376+
let index_config = index
377+
.index_config()
378+
.context("Failed to get crate index config")?;
379+
CrateIndexLookup::Http {
380+
index,
381+
index_config,
382+
}
383+
}
366384
unknown => {
367385
return Err(anyhow!(
368386
"'{:?}' crate index type is not supported (caused by '{}')",
@@ -398,17 +416,11 @@ impl WorkspaceMetadata {
398416
})
399417
.collect::<Result<Vec<_>>>()?;
400418

401-
workspace_metaata
419+
workspace_metadata
402420
.sources
403-
.extend(
404-
additional_sources
405-
.into_iter()
406-
.filter_map(|(crate_id, source_info)| {
407-
source_info.map(|source_info| (crate_id, source_info))
408-
}),
409-
);
410-
workspace_metaata.tree_metadata = resolver_data;
411-
workspace_metaata.inject_into(&mut manifest)?;
421+
.extend(additional_sources.into_iter());
422+
workspace_metadata.tree_metadata = resolver_data;
423+
workspace_metadata.inject_into(&mut manifest)?;
412424

413425
write_root_manifest(output_manifest_path.as_std_path(), manifest)?;
414426

‎crate_universe/src/splicing/crate_index_lookup.rs‎

Lines changed: 60 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -4,49 +4,59 @@ use crates_index::IndexConfig;
44
use hex::ToHex;
55

66
pub(crate) enum CrateIndexLookup {
7-
Git(crates_index::GitIndex),
8-
Http(crates_index::SparseIndex),
7+
Git {
8+
index: crates_index::GitIndex,
9+
index_config: IndexConfig,
10+
},
11+
Http {
12+
index: crates_index::SparseIndex,
13+
index_config: IndexConfig,
14+
},
915
}
1016

1117
impl CrateIndexLookup {
12-
pub(crate) fn get_source_info(&self, pkg: &cargo_lock::Package) -> Result<Option<SourceInfo>> {
13-
let index_config = self
18+
pub(crate) fn get_source_info(&self, pkg: &cargo_lock::Package) -> Result<SourceInfo> {
19+
let url = self
1420
.index_config()
15-
.context("Failed to get crate index config")?;
16-
let crate_ = match self {
17-
// The crates we care about should all be in the cache already,
18-
// because `cargo metadata` ran which should have fetched them.
19-
Self::Http(index) => {
20-
Some(index.crate_from_cache(pkg.name.as_str()).with_context(|| {
21-
format!("Failed to get crate from cache: {:?}\n{:?}", index, pkg)
22-
})?)
23-
}
24-
Self::Git(index) => index.crate_(pkg.name.as_str()),
25-
};
26-
let source_info = crate_.and_then(|crate_idx| {
27-
crate_idx
28-
.versions()
29-
.iter()
30-
.find(|v| v.version() == pkg.version.to_string())
31-
.and_then(|v| {
32-
v.download_url(&index_config).map(|url| {
33-
let sha256 = pkg
34-
.checksum
35-
.as_ref()
36-
.and_then(|sum| sum.as_sha256().map(|sum| sum.encode_hex::<String>()))
37-
.unwrap_or_else(|| v.checksum().encode_hex::<String>());
38-
SourceInfo { url, sha256 }
21+
.download_url(pkg.name.as_str(), &pkg.version.to_string())
22+
.context("no url for crate")?;
23+
let sha256 = pkg
24+
.checksum
25+
.as_ref()
26+
.and_then(|sum| sum.as_sha256().map(|sum| sum.encode_hex::<String>()))
27+
.unwrap_or_else(|| {
28+
let crate_ = match self {
29+
// The crates we care about should all be in the cache already,
30+
// because `cargo metadata` ran which should have fetched them.
31+
Self::Http { index, .. } => Some(
32+
index
33+
.crate_from_cache(pkg.name.as_str())
34+
.with_context(|| {
35+
format!("Failed to get crate from cache: {:?}\n{:?}", index, pkg)
36+
})
37+
.unwrap(),
38+
),
39+
Self::Git { index, .. } => index.crate_(pkg.name.as_str()),
40+
};
41+
crate_
42+
.and_then(|crate_idx| {
43+
crate_idx
44+
.versions()
45+
.iter()
46+
.find(|v| v.version() == pkg.version.to_string())
47+
.map(|v| v.checksum().encode_hex::<String>())
3948
})
40-
})
41-
});
42-
Ok(source_info)
49+
.unwrap()
50+
});
51+
52+
Ok(SourceInfo { url, sha256 })
4353
}
4454

4555
#[allow(clippy::result_large_err)]
46-
fn index_config(&self) -> Result<IndexConfig, crates_index::Error> {
56+
fn index_config(&self) -> &IndexConfig {
4757
match self {
48-
Self::Git(index) => index.index_config(),
49-
Self::Http(index) => index.index_config(),
58+
Self::Git { index_config, .. } => index_config,
59+
Self::Http { index_config, .. } => index_config,
5060
}
5161
}
5262
}
@@ -73,9 +83,14 @@ mod test {
7383
.unwrap(),
7484
);
7585

76-
let index = CrateIndexLookup::Http(
77-
crates_index::SparseIndex::from_url("sparse+https://index.crates.io/").unwrap(),
78-
);
86+
let index =
87+
crates_index::SparseIndex::from_url("sparse+https://index.crates.io/").unwrap();
88+
let index_config = index.index_config().unwrap();
89+
let index = CrateIndexLookup::Http {
90+
index,
91+
index_config,
92+
};
93+
7994
let source_info = index
8095
.get_source_info(&cargo_lock::Package {
8196
name: "lazy_static".parse().unwrap(),
@@ -85,7 +100,6 @@ mod test {
85100
dependencies: Vec::new(),
86101
replace: None,
87102
})
88-
.unwrap()
89103
.unwrap();
90104
assert_eq!(
91105
source_info.url,
@@ -100,9 +114,14 @@ mod test {
100114
let _e = EnvVarResetter::set("CARGO_HOME",
101115
runfiles::rlocation!(runfiles, "rules_rust/crate_universe/test_data/crate_indexes/rewritten_lazy_static/cargo_home").unwrap());
102116

103-
let index = CrateIndexLookup::Http(
104-
crates_index::SparseIndex::from_url("sparse+https://index.crates.io/").unwrap(),
105-
);
117+
let index =
118+
crates_index::SparseIndex::from_url("sparse+https://index.crates.io/").unwrap();
119+
let index_config = index.index_config().unwrap();
120+
let index = CrateIndexLookup::Http {
121+
index,
122+
index_config,
123+
};
124+
106125
let source_info = index
107126
.get_source_info(&cargo_lock::Package {
108127
name: "lazy_static".parse().unwrap(),
@@ -112,7 +131,6 @@ mod test {
112131
dependencies: Vec::new(),
113132
replace: None,
114133
})
115-
.unwrap()
116134
.unwrap();
117135
assert_eq!(
118136
source_info.url,

0 commit comments

Comments
 (0)