Skip to content

Commit

Permalink
Only respect preferences across the same indexes
Browse files Browse the repository at this point in the history
  • Loading branch information
charliermarsh committed Nov 21, 2024
1 parent c6482dd commit a265770
Show file tree
Hide file tree
Showing 10 changed files with 561 additions and 15 deletions.
20 changes: 15 additions & 5 deletions crates/uv-resolver/src/candidate_selector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use std::fmt::{Display, Formatter};
use tracing::{debug, trace};

use uv_configuration::IndexStrategy;
use uv_distribution_types::{CompatibleDist, IncompatibleDist, IncompatibleSource};
use uv_distribution_types::{CompatibleDist, IncompatibleDist, IncompatibleSource, IndexUrl};
use uv_distribution_types::{DistributionMetadata, IncompatibleWheel, Name, PrioritizedDist};
use uv_normalize::PackageName;
use uv_pep440::Version;
Expand Down Expand Up @@ -80,11 +80,12 @@ impl CandidateSelector {
preferences: &'a Preferences,
installed_packages: &'a InstalledPackages,
exclusions: &'a Exclusions,
index: Option<&'a IndexUrl>,
env: &ResolverEnvironment,
) -> Option<Candidate<'a>> {
let is_excluded = exclusions.contains(package_name);

// Check for a preference from a lockfile or a previous fork that satisfies the range and
// Check for a preference from a lockfile or a previous fork that satisfies the range and
// is allowed.
if let Some(preferred) = self.get_preferred(
package_name,
Expand All @@ -93,6 +94,7 @@ impl CandidateSelector {
preferences,
installed_packages,
is_excluded,
index,
env,
) {
trace!("Using preference {} {}", preferred.name, preferred.version);
Expand Down Expand Up @@ -131,23 +133,31 @@ impl CandidateSelector {
preferences: &'a Preferences,
installed_packages: &'a InstalledPackages,
is_excluded: bool,
index: Option<&'a IndexUrl>,
env: &ResolverEnvironment,
) -> Option<Candidate> {
// In the branches, we "sort" the preferences by marker-matching through an iterator that
// first has the matching half and then the mismatching half.
let preferences_match = preferences.get(package_name).filter(|(marker, _version)| {
let preferences_match = preferences.get(package_name).filter(|(marker, _index, _version)| {
// `.unwrap_or(true)` because the universal marker is considered matching.
marker
.map(|marker| env.included_by_marker(marker))
.unwrap_or(true)
});
let preferences_mismatch = preferences.get(package_name).filter(|(marker, _version)| {
let preferences_mismatch = preferences.get(package_name).filter(|(marker,_index, _version)| {
marker
.map(|marker| !env.included_by_marker(marker))
.unwrap_or(false)
});
let preferences = preferences_match.chain(preferences_mismatch).filter_map(|(marker, source, version)| {
if source == index {
Some((marker, version))
} else {
None
}
});
self.get_preferred_from_iter(
preferences_match.chain(preferences_mismatch),
preferences,
package_name,
range,
version_maps,
Expand Down
19 changes: 11 additions & 8 deletions crates/uv-resolver/src/preferences.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use std::str::FromStr;
use rustc_hash::FxHashMap;
use tracing::trace;

use uv_distribution_types::{InstalledDist, InstalledMetadata, InstalledVersion, Name};
use uv_distribution_types::{IndexUrl, InstalledDist, InstalledMetadata, InstalledVersion, Name};
use uv_normalize::PackageName;
use uv_pep440::{Operator, Version};
use uv_pep508::{MarkerTree, VersionOrUrl};
Expand Down Expand Up @@ -114,7 +114,7 @@ impl Preference {
/// Preferences should be prioritized first by whether their marker matches and then by the order
/// they are stored, so that a lockfile has higher precedence than sibling forks.
#[derive(Debug, Clone, Default)]
pub struct Preferences(FxHashMap<PackageName, Vec<(Option<MarkerTree>, Pin)>>);
pub struct Preferences(FxHashMap<PackageName, Vec<(Option<MarkerTree>, Option<IndexUrl>, Pin)>>);

impl Preferences {
/// Create a map of pinned packages from an iterator of [`Preference`] entries.
Expand Down Expand Up @@ -153,6 +153,7 @@ impl Preferences {
slf.insert(
preference.name,
None,
None,
Pin {
version: preference.version,
hashes: preference.hashes,
Expand All @@ -162,6 +163,7 @@ impl Preferences {
for fork_marker in preference.fork_markers {
slf.insert(
preference.name.clone(),
None,
Some(fork_marker),
Pin {
version: preference.version.clone(),
Expand All @@ -179,13 +181,14 @@ impl Preferences {
pub(crate) fn insert(
&mut self,
package_name: PackageName,
index: Option<IndexUrl>,
markers: Option<MarkerTree>,
pin: impl Into<Pin>,
) {
self.0
.entry(package_name)
.or_default()
.push((markers, pin.into()));
.push((markers, index, pin.into()));
}

/// Returns an iterator over the preferences.
Expand All @@ -202,7 +205,7 @@ impl Preferences {
name,
preferences
.iter()
.map(|(markers, pin)| (markers.as_ref(), pin.version())),
.map(|(markers, _index, pin)| (markers.as_ref(), pin.version())),
)
})
}
Expand All @@ -211,12 +214,12 @@ impl Preferences {
pub(crate) fn get(
&self,
package_name: &PackageName,
) -> impl Iterator<Item = (Option<&MarkerTree>, &Version)> {
) -> impl Iterator<Item = (Option<&MarkerTree>, Option<&IndexUrl>, &Version)> {
self.0
.get(package_name)
.into_iter()
.flatten()
.map(|(markers, pin)| (markers.as_ref(), pin.version()))
.map(|(markers, index, pin)| (markers.as_ref(), index.as_ref(), pin.version()))
}

/// Return the hashes for a package, if the version matches that of the pin.
Expand All @@ -229,8 +232,8 @@ impl Preferences {
.get(package_name)
.into_iter()
.flatten()
.find(|(_markers, pin)| pin.version() == version)
.map(|(_markers, pin)| pin.hashes())
.find(|(_markers, _index, pin)| pin.version() == version)
.map(|(_markers, _index, pin)| pin.hashes())
}
}

Expand Down
8 changes: 8 additions & 0 deletions crates/uv-resolver/src/resolver/environment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -333,6 +333,14 @@ impl ResolverEnvironment {
}
}

/// Returns `true` if this resolver environment has conflicts.
pub(crate) fn has_conflicts(&self) -> bool {
match self.kind {
Kind::Specific { .. } => false,
Kind::Universal { ref exclude, .. } => !exclude.is_empty(),
}
}

/// Returns a requires-python version range derived from the marker
/// expression describing this resolver environment.
///
Expand Down
6 changes: 6 additions & 0 deletions crates/uv-resolver/src/resolver/fork_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,4 +64,10 @@ impl<T> ForkMap<T> {
.map(|entry| &entry.value)
.collect()
}
//
// /// Returns the single value associated with a package, if the value is compatible with all
// /// forks.
// pub fn entries(&self, package_name: &PackageName) -> &[Entry<T>] {
// self.0.get(package_name).map(Vec::as_slice).unwrap_or_default()
// }
}
16 changes: 16 additions & 0 deletions crates/uv-resolver/src/resolver/indexes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,22 @@ impl Indexes {
self.0.contains_key(name)
}

// /// Returns the explicit index used for a package, if it's known to be used for all forks.
// pub(crate) fn contains_many(&self, name: &PackageName) -> bool {
// match self.0.entries(name) {
// [] => false,
// [index] => index. index.conflict.is_none(),
// _ => false,
//
// }
// let entry = self.0.get(name)?;
// if entry.conflict.is_none() {
// Some(&entry.index)
// } else {
// None
// }
// }

/// Return the explicit index used for a package in the given fork.
pub(crate) fn get(&self, name: &PackageName, env: &ResolverEnvironment) -> Vec<&IndexUrl> {
let entries = self.0.get(name, env);
Expand Down
8 changes: 6 additions & 2 deletions crates/uv-resolver/src/resolver/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -381,6 +381,7 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
for (package, version) in &resolution.nodes {
preferences.insert(
package.name.clone(),
package.index.clone(),
resolution.env.try_markers().cloned(),
version.clone(),
);
Expand Down Expand Up @@ -669,14 +670,15 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
diverging_packages: &'a [PackageName],
) -> impl Iterator<Item = Result<ForkState, ResolveError>> + 'a {
debug!(
"Splitting resolution on {}=={} over {} into {} resolution with separate markers",
"Splitting resolution on {}=={} over {} into {} resolution{} with separate markers",
current_state.next,
version,
diverging_packages
.iter()
.map(ToString::to_string)
.join(", "),
forks.len()
forks.len(),
if forks.len() == 1 { "" } else { "s" }
);
assert!(forks.len() >= 2);
// This is a somewhat tortured technique to ensure
Expand Down Expand Up @@ -1075,6 +1077,7 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
preferences,
&self.installed_packages,
&self.exclusions,
index,
env,
) else {
// Short circuit: we couldn't find _any_ versions for a package.
Expand Down Expand Up @@ -1934,6 +1937,7 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
&self.preferences,
&self.installed_packages,
&self.exclusions,
None,
&env,
) else {
return Ok(None);
Expand Down
Empty file added foo/README.md
Empty file.
6 changes: 6 additions & 0 deletions foo/hello.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
def main():
print("Hello from foo!")


if __name__ == "__main__":
main()
36 changes: 36 additions & 0 deletions foo/pyproject.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
[project]
name = "project"
version = "0.1.0"
requires-python = ">=3.12.0"
dependencies = []

[project.optional-dependencies]
cpu = [
"torch>=2.5.1",
"torchvision>=0.20.1",
]
cu124 = [
"torch>=2.5.1",
"torchvision>=0.20.1",
]

[tool.uv]
conflicts = [
[
{ extra = "cpu" },
{ extra = "cu124" },
],
]

[tool.uv.sources]
torch = [
{ index = "pytorch-cpu", extra = "cpu", marker = "platform_system != 'Darwin'" },
]
torchvision = [
{ index = "pytorch-cpu", extra = "cpu", marker = "platform_system != 'Darwin'" },
]

[[tool.uv.index]]
name = "pytorch-cpu"
url = "https://download.pytorch.org/whl/cpu"
explicit = true
Loading

0 comments on commit a265770

Please sign in to comment.