Auto merge of #13924 - weihanglo:registry, r=epage

refactor: more comments and variable rename
This commit is contained in:
bors 2024-05-17 15:05:37 +00:00
commit 2b8804475d

View File

@ -113,11 +113,23 @@ pub struct PackageRegistry<'gctx> {
yanked_whitelist: HashSet<PackageId>, yanked_whitelist: HashSet<PackageId>,
source_config: SourceConfigMap<'gctx>, source_config: SourceConfigMap<'gctx>,
/// Patches registered during calls to [`PackageRegistry::patch`].
///
/// These are available for `query` after calling [`PackageRegistry::lock_patches`],
/// which `lock`s them all to specific versions.
patches: HashMap<CanonicalUrl, Vec<Summary>>, patches: HashMap<CanonicalUrl, Vec<Summary>>,
/// Whether patches are locked. That is, they are available to resolution. /// Whether patches are locked. That is, they are available to resolution.
/// ///
/// See [`PackageRegistry::lock_patches`] and [`PackageRegistry::patch`] for more. /// See [`PackageRegistry::lock_patches`] and [`PackageRegistry::patch`] for more.
patches_locked: bool, patches_locked: bool,
/// Patches available for each source.
///
/// This is for determining whether a dependency entry from a lockfile
/// happened through `[patch]`, during calls to [`lock`] to rewrite
/// summaries to point directly at these patched entries.
///
/// This is constructed during calls to [`PackageRegistry::patch`],
/// along with the `patches` field, thoough these entries never get locked.
patches_available: HashMap<CanonicalUrl, Vec<PackageId>>, patches_available: HashMap<CanonicalUrl, Vec<PackageId>>,
} }
@ -157,6 +169,15 @@ enum Kind {
Normal, Normal,
} }
/// This tuple is an argument to [`PackageRegistry::patch`].
///
/// * The first element is the patch definition straight from the manifest.
/// * The second element is an optional variant where the patch has been locked.
/// It is the patch locked to a specific version found in Cargo.lock.
/// This will be `None` if `Cargo.lock` doesn't exist,
/// or the patch did not match any existing entries in `Cargo.lock`.
pub type PatchDependency<'a> = (&'a Dependency, Option<LockedPatchDependency>);
/// Argument to [`PackageRegistry::patch`] which is information about a `[patch]` /// Argument to [`PackageRegistry::patch`] which is information about a `[patch]`
/// directive that we found in a lockfile, if present. /// directive that we found in a lockfile, if present.
pub struct LockedPatchDependency { pub struct LockedPatchDependency {
@ -303,14 +324,8 @@ impl<'gctx> PackageRegistry<'gctx> {
/// The `deps` is an array of all the entries in the `[patch]` section of /// The `deps` is an array of all the entries in the `[patch]` section of
/// the manifest. /// the manifest.
/// ///
/// Here the `deps` will be resolved to a precise version and stored /// Here the `patch_deps` will be resolved to a precise version and stored
/// internally for future calls to `query` below. `deps` should be a tuple /// internally for future calls to `query` below.
/// where the first element is the patch definition straight from the
/// manifest, and the second element is an optional variant where the
/// patch has been locked. This locked patch is the patch locked to
/// a specific version found in Cargo.lock. This will be `None` if
/// `Cargo.lock` doesn't exist, or the patch did not match any existing
/// entries in `Cargo.lock`.
/// ///
/// Note that the patch list specified here *will not* be available to /// Note that the patch list specified here *will not* be available to
/// [`Registry::query`] until [`PackageRegistry::lock_patches`] is called /// [`Registry::query`] until [`PackageRegistry::lock_patches`] is called
@ -322,7 +337,7 @@ impl<'gctx> PackageRegistry<'gctx> {
pub fn patch( pub fn patch(
&mut self, &mut self,
url: &Url, url: &Url,
deps: &[(&Dependency, Option<LockedPatchDependency>)], patch_deps: &[PatchDependency<'_>],
) -> CargoResult<Vec<(Dependency, PackageId)>> { ) -> CargoResult<Vec<(Dependency, PackageId)>> {
// NOTE: None of this code is aware of required features. If a patch // NOTE: None of this code is aware of required features. If a patch
// is missing a required feature, you end up with an "unused patch" // is missing a required feature, you end up with an "unused patch"
@ -333,9 +348,9 @@ impl<'gctx> PackageRegistry<'gctx> {
// Return value of patches that shouldn't be locked. // Return value of patches that shouldn't be locked.
let mut unlock_patches = Vec::new(); let mut unlock_patches = Vec::new();
// First up we need to actually resolve each `deps` specification to // First up we need to actually resolve each `patch_deps` specification
// precisely one summary. We're not using the `query` method below as it // to precisely one summary. We're not using the `query` method below
// internally uses maps we're building up as part of this method // as it internally uses maps we're building up as part of this method
// (`patches_available` and `patches`). Instead we're going straight to // (`patches_available` and `patches`). Instead we're going straight to
// the source to load information from it. // the source to load information from it.
// //
@ -343,12 +358,12 @@ impl<'gctx> PackageRegistry<'gctx> {
// precisely one package, so that's why we're just creating a flat list // precisely one package, so that's why we're just creating a flat list
// of summaries which should be the same length as `deps` above. // of summaries which should be the same length as `deps` above.
let mut deps_remaining: Vec<_> = deps.iter().collect(); let mut patch_deps_remaining: Vec<_> = patch_deps.iter().collect();
let mut unlocked_summaries = Vec::new(); let mut unlocked_summaries = Vec::new();
while !deps_remaining.is_empty() { while !patch_deps_remaining.is_empty() {
let mut deps_pending = Vec::new(); let mut patch_deps_pending = Vec::new();
for dep_remaining in deps_remaining { for patch_dep_remaining in patch_deps_remaining {
let (orig_patch, locked) = dep_remaining; let (orig_patch, locked) = patch_dep_remaining;
// Use the locked patch if it exists, otherwise use the original. // Use the locked patch if it exists, otherwise use the original.
let dep = match locked { let dep = match locked {
@ -388,7 +403,7 @@ impl<'gctx> PackageRegistry<'gctx> {
let summaries = match source.query_vec(dep, QueryKind::Exact)? { let summaries = match source.query_vec(dep, QueryKind::Exact)? {
Poll::Ready(deps) => deps, Poll::Ready(deps) => deps,
Poll::Pending => { Poll::Pending => {
deps_pending.push(dep_remaining); patch_deps_pending.push(patch_dep_remaining);
continue; continue;
} }
}; };
@ -399,7 +414,7 @@ impl<'gctx> PackageRegistry<'gctx> {
match summary_for_patch(orig_patch, &locked, summaries, source) { match summary_for_patch(orig_patch, &locked, summaries, source) {
Poll::Ready(x) => x, Poll::Ready(x) => x,
Poll::Pending => { Poll::Pending => {
deps_pending.push(dep_remaining); patch_deps_pending.push(patch_dep_remaining);
continue; continue;
} }
} }
@ -432,7 +447,7 @@ impl<'gctx> PackageRegistry<'gctx> {
unlocked_summaries.push(summary); unlocked_summaries.push(summary);
} }
deps_remaining = deps_pending; patch_deps_remaining = patch_deps_pending;
self.block_until_ready()?; self.block_until_ready()?;
} }
@ -450,25 +465,18 @@ impl<'gctx> PackageRegistry<'gctx> {
} }
} }
// Calculate a list of all patches available for this source which is // Calculate a list of all patches available for this source.
// then used later during calls to `lock` to rewrite summaries to point
// directly at these patched entries.
//
// Note that this is somewhat subtle where the list of `ids` for a
// canonical URL is extend with possibly two ids per summary. This is done
// to handle the transition from the v2->v3 lock file format where in
// v2 DefaultBranch was either DefaultBranch or Branch("master") for
// git dependencies. In this case if `summary.package_id()` is
// Branch("master") then alt_package_id will be DefaultBranch. This
// signifies that there's a patch available for either of those
// dependency directives if we see them in the dependency graph.
//
// This is a bit complicated and hopefully an edge case we can remove
// in the future, but for now it hopefully doesn't cause too much
// harm...
let mut ids = Vec::new(); let mut ids = Vec::new();
for (summary, (_, lock)) in unlocked_summaries.iter().zip(deps) { for (summary, (_, lock)) in unlocked_summaries.iter().zip(patch_deps) {
ids.push(summary.package_id()); ids.push(summary.package_id());
// This is subtle where the list of `ids` for a canonical URL is
// extend with possibly two ids per summary. This is done to handle
// the transition from the v2->v3 lock file format where in v2
// DefaultBranch was either DefaultBranch or Branch("master") for
// git dependencies. In this case if `summary.package_id()` is
// Branch("master") then alt_package_id will be DefaultBranch. This
// signifies that there's a patch available for either of those
// dependency directives if we see them in the dependency graph.
if let Some(lock) = lock { if let Some(lock) = lock {
ids.extend(lock.alt_package_id); ids.extend(lock.alt_package_id);
} }
@ -636,139 +644,126 @@ impl<'gctx> Registry for PackageRegistry<'gctx> {
f: &mut dyn FnMut(IndexSummary), f: &mut dyn FnMut(IndexSummary),
) -> Poll<CargoResult<()>> { ) -> Poll<CargoResult<()>> {
assert!(self.patches_locked); assert!(self.patches_locked);
let (override_summary, n, to_warn) = { // Look for an override and get ready to query the real source.
// Look for an override and get ready to query the real source. let override_summary = ready!(self.query_overrides(dep))?;
let override_summary = ready!(self.query_overrides(dep))?;
// Next up on our list of candidates is to check the `[patch]` // Next up on our list of candidates is to check the `[patch]` section
// section of the manifest. Here we look through all patches // of the manifest. Here we look through all patches relevant to the
// relevant to the source that `dep` points to, and then we match // source that `dep` points to, and then we match name/version. Note
// name/version. Note that we don't use `dep.matches(..)` because // that we don't use `dep.matches(..)` because the patches, by definition,
// the patches, by definition, come from a different source. // come from a different source. This means that `dep.matches(..)` will
// This means that `dep.matches(..)` will always return false, when // always return false, when what we really care about is the name/version match.
// what we really care about is the name/version match. let mut patches = Vec::<Summary>::new();
let mut patches = Vec::<Summary>::new(); if let Some(extra) = self.patches.get(dep.source_id().canonical_url()) {
if let Some(extra) = self.patches.get(dep.source_id().canonical_url()) { patches.extend(
patches.extend( extra
extra .iter()
.iter() .filter(|s| dep.matches_ignoring_source(s.package_id()))
.filter(|s| dep.matches_ignoring_source(s.package_id())) .cloned(),
.cloned(), );
); }
}
// A crucial feature of the `[patch]` feature is that we don't query the
// A crucial feature of the `[patch]` feature is that we *don't* // actual registry if we have a "locked" dependency. A locked dep basically
// query the actual registry if we have a "locked" dependency. A // just means a version constraint of `=a.b.c`, and because patches take
// locked dep basically just means a version constraint of `=a.b.c`, // priority over the actual source then if we have a candidate we're done.
// and because patches take priority over the actual source then if if patches.len() == 1 && dep.is_locked() {
// we have a candidate we're done. let patch = patches.remove(0);
if patches.len() == 1 && dep.is_locked() { match override_summary {
let patch = patches.remove(0); Some(override_summary) => {
match override_summary { let override_summary = override_summary.into_summary();
Some(summary) => (summary, 1, Some(IndexSummary::Candidate(patch))), self.warn_bad_override(&override_summary, &patch)?;
None => { f(IndexSummary::Candidate(self.lock(override_summary)));
f(IndexSummary::Candidate(patch)); }
return Poll::Ready(Ok(())); None => f(IndexSummary::Candidate(patch)),
} }
}
} else { return Poll::Ready(Ok(()));
if !patches.is_empty() { }
debug!(
"found {} patches with an unlocked dep on `{}` at {} \ if !patches.is_empty() {
with `{}`, \ debug!(
looking at sources", "found {} patches with an unlocked dep on `{}` at {} \
patches.len(), with `{}`, \
dep.package_name(), looking at sources",
dep.source_id(), patches.len(),
dep.version_req() dep.package_name(),
); dep.source_id(),
} dep.version_req()
);
// Ensure the requested source_id is loaded }
self.ensure_loaded(dep.source_id(), Kind::Normal)
.with_context(|| { // Ensure the requested source_id is loaded
format!( self.ensure_loaded(dep.source_id(), Kind::Normal)
"failed to load source for dependency `{}`", .with_context(|| {
dep.package_name() format!(
) "failed to load source for dependency `{}`",
})?; dep.package_name()
)
let source = self.sources.get_mut(dep.source_id()); })?;
match (override_summary, source) {
(Some(_), None) => { let source = self.sources.get_mut(dep.source_id());
return Poll::Ready(Err(anyhow::anyhow!("override found but no real ones"))) match (override_summary, source) {
} (Some(_), None) => {
(None, None) => return Poll::Ready(Ok(())), return Poll::Ready(Err(anyhow::anyhow!("override found but no real ones")))
}
// If we don't have an override then we just ship (None, None) => return Poll::Ready(Ok(())),
// everything upstairs after locking the summary
(None, Some(source)) => { // If we don't have an override then we just ship everything upstairs after locking the summary
for patch in patches.iter() { (None, Some(source)) => {
f(IndexSummary::Candidate(patch.clone())); for patch in patches.iter() {
} f(IndexSummary::Candidate(patch.clone()));
}
// Our sources shouldn't ever come back to us with two
// summaries that have the same version. We could, // Our sources shouldn't ever come back to us with two summaries
// however, have an `[patch]` section which is in use // that have the same version. We could, however, have an `[patch]`
// to override a version in the registry. This means // section which is in use to override a version in the registry.
// that if our `summary` in this loop has the same // This means that if our `summary` in this loop has the same
// version as something in `patches` that we've // version as something in `patches` that we've already selected,
// already selected, then we skip this `summary`. // then we skip this `summary`.
let locked = &self.locked; let locked = &self.locked;
let all_patches = &self.patches_available; let all_patches = &self.patches_available;
let callback = &mut |summary: IndexSummary| { let callback = &mut |summary: IndexSummary| {
for patch in patches.iter() { for patch in patches.iter() {
let patch = patch.package_id().version(); let patch = patch.package_id().version();
if summary.package_id().version() == patch { if summary.package_id().version() == patch {
return; return;
} }
} }
f(IndexSummary::Candidate(lock( let summary = summary.into_summary();
locked, f(IndexSummary::Candidate(lock(locked, all_patches, summary)))
all_patches, };
summary.into_summary(), return source.query(dep, kind, callback);
))) }
};
return source.query(dep, kind, callback); // If we have an override summary then we query the source to sanity check its results.
} // We don't actually use any of the summaries it gives us though.
(Some(override_summary), Some(source)) => {
// If we have an override summary then we query the source if !patches.is_empty() {
// to sanity check its results. We don't actually use any of return Poll::Ready(Err(anyhow::anyhow!("found patches and a path override")));
// the summaries it gives us though. }
(Some(override_summary), Some(source)) => { let mut n = 0;
if !patches.is_empty() { let mut to_warn = None;
return Poll::Ready(Err(anyhow::anyhow!( let callback = &mut |summary| {
"found patches and a path override" n += 1;
))); to_warn = Some(summary);
} };
let mut n = 0; let pend = source.query(dep, kind, callback);
let mut to_warn = None; if pend.is_pending() {
{ return Poll::Pending;
let callback = &mut |summary| { }
n += 1; if n > 1 {
to_warn = Some(summary); return Poll::Ready(Err(anyhow::anyhow!(
}; "found an override with a non-locked list"
let pend = source.query(dep, kind, callback); )));
if pend.is_pending() { }
return Poll::Pending; let override_summary = override_summary.into_summary();
} if let Some(to_warn) = to_warn {
} self.warn_bad_override(&override_summary, to_warn.as_summary())?;
(override_summary, n, to_warn) }
} f(IndexSummary::Candidate(self.lock(override_summary)));
} }
}
};
if n > 1 {
return Poll::Ready(Err(anyhow::anyhow!(
"found an override with a non-locked list"
)));
} else if let Some(summary) = to_warn {
self.warn_bad_override(override_summary.as_summary(), summary.as_summary())?;
} }
f(IndexSummary::Candidate(
self.lock(override_summary.into_summary()),
));
Poll::Ready(Ok(())) Poll::Ready(Ok(()))
} }