Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: make resolve_with_previous clearer #13727

Merged
merged 4 commits into from
Apr 10, 2024
Merged

refactor: make resolve_with_previous clearer #13727

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e55c205
refactor: move to_avoid closure to `cargo update`
weihanglo Apr 9, 2024
57c814e
refactor: extract patch registration to a function
weihanglo Apr 9, 2024
f978671
refactor: unify naming of `keep` closure
weihanglo Apr 9, 2024
80d0d94
doc: add commment to `summary_for_patch`
weihanglo Apr 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
refactor: extract patch registration to a function
  • Loading branch information
@weihanglo
weihanglo committed Apr 9, 2024
commit 57c814eeda756bad67350642f27beee504eb64f2
232 changes: 123 additions & 109 deletions src/cargo/ops/resolve.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -307,115 +307,11 @@ pub fn resolve_with_previous<'gctx>(
version_prefs.max_rust_version(ws.rust_version().cloned().map(RustVersion::into_partial));
}

// This is a set of PackageIds of `[patch]` entries, and some related locked PackageIds, for
// which locking should be avoided (but which will be preferred when searching dependencies,
// via prefer_patch_deps below)
let mut avoid_patch_ids = HashSet::new();

if register_patches {
for (url, patches) in ws.root_patch()?.iter() {
for patch in patches {
version_prefs.prefer_dependency(patch.clone());
}
let Some(previous) = previous else {
let patches: Vec<_> = patches.iter().map(|p| (p, None)).collect();
let unlock_ids = registry.patch(url, &patches)?;
// Since nothing is locked, this shouldn't possibly return anything.
assert!(unlock_ids.is_empty());
continue;
};

// This is a list of pairs where the first element of the pair is
// the raw `Dependency` which matches what's listed in `Cargo.toml`.
// The second element is, if present, the "locked" version of
// the `Dependency` as well as the `PackageId` that it previously
// resolved to. This second element is calculated by looking at the
// previous resolve graph, which is primarily what's done here to
// build the `registrations` list.
let mut registrations = Vec::new();
for dep in patches {
let candidates = || {
previous
.iter()
.chain(previous.unused_patches().iter().cloned())
.filter(&pre_patch_keep)
};

let lock = match candidates().find(|id| dep.matches_id(*id)) {
// If we found an exactly matching candidate in our list of
// candidates, then that's the one to use.
Some(package_id) => {
let mut locked_dep = dep.clone();
locked_dep.lock_to(package_id);
Some(LockedPatchDependency {
dependency: locked_dep,
package_id,
alt_package_id: None,
})
}
None => {
// If the candidate does not have a matching source id
// then we may still have a lock candidate. If we're
// loading a v2-encoded resolve graph and `dep` is a
// git dep with `branch = 'master'`, then this should
// also match candidates without `branch = 'master'`
// (which is now treated separately in Cargo).
//
// In this scenario we try to convert candidates located
// in the resolve graph to explicitly having the
// `master` branch (if they otherwise point to
// `DefaultBranch`). If this works and our `dep`
// matches that then this is something we'll lock to.
match candidates().find(|&id| {
match master_branch_git_source(id, previous) {
Some(id) => dep.matches_id(id),
None => false,
}
}) {
Some(id_using_default) => {
let id_using_master = id_using_default.with_source_id(
dep.source_id()
.with_precise_from(id_using_default.source_id()),
);

let mut locked_dep = dep.clone();
locked_dep.lock_to(id_using_master);
Some(LockedPatchDependency {
dependency: locked_dep,
package_id: id_using_master,
// Note that this is where the magic
// happens, where the resolve graph
// probably has locks pointing to
// DefaultBranch sources, and by including
// this here those will get transparently
// rewritten to Branch("master") which we
// have a lock entry for.
alt_package_id: Some(id_using_default),
})
}

// No locked candidate was found
None => None,
}
}
};

registrations.push((dep, lock));
}

let canonical = CanonicalUrl::new(url)?;
for (orig_patch, unlock_id) in registry.patch(url, &registrations)? {
// Avoid the locked patch ID.
avoid_patch_ids.insert(unlock_id);
// Also avoid the thing it is patching.
avoid_patch_ids.extend(previous.iter().filter(|id| {
orig_patch.matches_ignoring_source(*id)
&& *id.source_id().canonical_url() == canonical
}));
}
}
}
debug!("avoid_patch_ids={:?}", avoid_patch_ids);
let avoid_patch_ids = if register_patches {
register_patch_entries(registry, ws, previous, &mut version_prefs, pre_patch_keep)?
} else {
HashSet::new()
};

let keep = |p: &PackageId| pre_patch_keep(p) && !avoid_patch_ids.contains(p);

Expand Down Expand Up @@ -860,3 +756,121 @@ fn emit_warnings_of_unused_patches(

return Ok(());
}

/// Informs `registry` and `version_pref` that `[patch]` entries are available
/// and preferable for the dependency resolution.
///
/// This returns a set of PackageIds of `[patch]` entries, and some related
/// locked PackageIds, for which locking should be avoided (but which will be
/// preferred when searching dependencies, via [`VersionPreferences::prefer_patch_deps`]).
#[tracing::instrument(level = "debug", skip_all, ret)]
fn register_patch_entries(
registry: &mut PackageRegistry<'_>,
ws: &Workspace<'_>,
previous: Option<&Resolve>,
version_prefs: &mut VersionPreferences,
pre_patch_keep: &dyn Fn(&PackageId) -> bool,
) -> CargoResult<HashSet<PackageId>> {
let mut avoid_patch_ids = HashSet::new();
for (url, patches) in ws.root_patch()?.iter() {
for patch in patches {
version_prefs.prefer_dependency(patch.clone());
}
let Some(previous) = previous else {
let patches: Vec<_> = patches.iter().map(|p| (p, None)).collect();
let unlock_ids = registry.patch(url, &patches)?;
// Since nothing is locked, this shouldn't possibly return anything.
assert!(unlock_ids.is_empty());
continue;
};

// This is a list of pairs where the first element of the pair is
// the raw `Dependency` which matches what's listed in `Cargo.toml`.
// The second element is, if present, the "locked" version of
// the `Dependency` as well as the `PackageId` that it previously
// resolved to. This second element is calculated by looking at the
// previous resolve graph, which is primarily what's done here to
// build the `registrations` list.
let mut registrations = Vec::new();
for dep in patches {
let candidates = || {
previous
.iter()
.chain(previous.unused_patches().iter().cloned())
.filter(&pre_patch_keep)
};

let lock = match candidates().find(|id| dep.matches_id(*id)) {
// If we found an exactly matching candidate in our list of
// candidates, then that's the one to use.
Some(package_id) => {
let mut locked_dep = dep.clone();
locked_dep.lock_to(package_id);
Some(LockedPatchDependency {
dependency: locked_dep,
package_id,
alt_package_id: None,
})
}
None => {
// If the candidate does not have a matching source id
// then we may still have a lock candidate. If we're
// loading a v2-encoded resolve graph and `dep` is a
// git dep with `branch = 'master'`, then this should
// also match candidates without `branch = 'master'`
// (which is now treated separately in Cargo).
//
// In this scenario we try to convert candidates located
// in the resolve graph to explicitly having the
// `master` branch (if they otherwise point to
// `DefaultBranch`). If this works and our `dep`
// matches that then this is something we'll lock to.
match candidates().find(|&id| match master_branch_git_source(id, previous) {
Some(id) => dep.matches_id(id),
None => false,
}) {
Some(id_using_default) => {
let id_using_master = id_using_default.with_source_id(
dep.source_id()
.with_precise_from(id_using_default.source_id()),
);

let mut locked_dep = dep.clone();
locked_dep.lock_to(id_using_master);
Some(LockedPatchDependency {
dependency: locked_dep,
package_id: id_using_master,
// Note that this is where the magic
// happens, where the resolve graph
// probably has locks pointing to
// DefaultBranch sources, and by including
// this here those will get transparently
// rewritten to Branch("master") which we
// have a lock entry for.
alt_package_id: Some(id_using_default),
})
}

// No locked candidate was found
None => None,
}
}
};

registrations.push((dep, lock));
}

let canonical = CanonicalUrl::new(url)?;
for (orig_patch, unlock_id) in registry.patch(url, &registrations)? {
// Avoid the locked patch ID.
avoid_patch_ids.insert(unlock_id);
// Also avoid the thing it is patching.
avoid_patch_ids.extend(previous.iter().filter(|id| {
orig_patch.matches_ignoring_source(*id)
&& *id.source_id().canonical_url() == canonical
}));
}
}

Ok(avoid_patch_ids)
}