Skip to content

Commit 5e48819

Browse files
Only respect preferences across the same indexes (#9302)
## Summary The issue here is fairly complex. Consider the following: ```toml [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 ``` When solving this project, we first pick a PyTorch version from PyPI, to solve the `cu124` extra, selecting `2.5.1`. Later, we try to solve the `cpu` extra. In solving that extra, we look at the PyTorch CPU index. Ideally, we'd select `2.5.1+cpu`... But `2.5.1` is already a preference. So we choose that. Now, we only respect preferences for explicit indexes if they came from the same index. Closes #9295.
1 parent c6482dd commit 5e48819

File tree

9 files changed

+129
-64
lines changed

9 files changed

+129
-64
lines changed

crates/uv-requirements/src/upgrade.rs

+8-4
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use uv_configuration::Upgrade;
77
use uv_fs::CWD;
88
use uv_git::ResolvedRepositoryReference;
99
use uv_requirements_txt::RequirementsTxt;
10-
use uv_resolver::{Lock, Preference, PreferenceError};
10+
use uv_resolver::{Lock, LockError, Preference, PreferenceError};
1111

1212
#[derive(Debug, Default)]
1313
pub struct LockedRequirements {
@@ -63,7 +63,11 @@ pub async fn read_requirements_txt(
6363
}
6464

6565
/// Load the preferred requirements from an existing lockfile, applying the upgrade strategy.
66-
pub fn read_lock_requirements(lock: &Lock, upgrade: &Upgrade) -> LockedRequirements {
66+
pub fn read_lock_requirements(
67+
lock: &Lock,
68+
install_path: &Path,
69+
upgrade: &Upgrade,
70+
) -> Result<LockedRequirements, LockError> {
6771
let mut preferences = Vec::new();
6872
let mut git = Vec::new();
6973

@@ -74,13 +78,13 @@ pub fn read_lock_requirements(lock: &Lock, upgrade: &Upgrade) -> LockedRequireme
7478
}
7579

7680
// Map each entry in the lockfile to a preference.
77-
preferences.push(Preference::from_lock(package));
81+
preferences.push(Preference::from_lock(package, install_path)?);
7882

7983
// Map each entry in the lockfile to a Git SHA.
8084
if let Some(git_ref) = package.as_git_ref() {
8185
git.push(git_ref);
8286
}
8387
}
8488

85-
LockedRequirements { preferences, git }
89+
Ok(LockedRequirements { preferences, git })
8690
}

crates/uv-resolver/src/candidate_selector.rs

+32-14
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ use std::fmt::{Display, Formatter};
44
use tracing::{debug, trace};
55

66
use uv_configuration::IndexStrategy;
7-
use uv_distribution_types::{CompatibleDist, IncompatibleDist, IncompatibleSource};
7+
use uv_distribution_types::{CompatibleDist, IncompatibleDist, IncompatibleSource, IndexUrl};
88
use uv_distribution_types::{DistributionMetadata, IncompatibleWheel, Name, PrioritizedDist};
99
use uv_normalize::PackageName;
1010
use uv_pep440::Version;
@@ -80,11 +80,12 @@ impl CandidateSelector {
8080
preferences: &'a Preferences,
8181
installed_packages: &'a InstalledPackages,
8282
exclusions: &'a Exclusions,
83+
index: Option<&'a IndexUrl>,
8384
env: &ResolverEnvironment,
8485
) -> Option<Candidate<'a>> {
8586
let is_excluded = exclusions.contains(package_name);
8687

87-
// Check for a preference from a lockfile or a previous fork that satisfies the range and
88+
// Check for a preference from a lockfile or a previous fork that satisfies the range and
8889
// is allowed.
8990
if let Some(preferred) = self.get_preferred(
9091
package_name,
@@ -93,6 +94,7 @@ impl CandidateSelector {
9394
preferences,
9495
installed_packages,
9596
is_excluded,
97+
index,
9698
env,
9799
) {
98100
trace!("Using preference {} {}", preferred.name, preferred.version);
@@ -131,23 +133,39 @@ impl CandidateSelector {
131133
preferences: &'a Preferences,
132134
installed_packages: &'a InstalledPackages,
133135
is_excluded: bool,
136+
index: Option<&'a IndexUrl>,
134137
env: &ResolverEnvironment,
135138
) -> Option<Candidate> {
136139
// In the branches, we "sort" the preferences by marker-matching through an iterator that
137140
// first has the matching half and then the mismatching half.
138-
let preferences_match = preferences.get(package_name).filter(|(marker, _version)| {
139-
// `.unwrap_or(true)` because the universal marker is considered matching.
140-
marker
141-
.map(|marker| env.included_by_marker(marker))
142-
.unwrap_or(true)
143-
});
144-
let preferences_mismatch = preferences.get(package_name).filter(|(marker, _version)| {
145-
marker
146-
.map(|marker| !env.included_by_marker(marker))
147-
.unwrap_or(false)
148-
});
141+
let preferences_match =
142+
preferences
143+
.get(package_name)
144+
.filter(|(marker, _index, _version)| {
145+
// `.unwrap_or(true)` because the universal marker is considered matching.
146+
marker
147+
.map(|marker| env.included_by_marker(marker))
148+
.unwrap_or(true)
149+
});
150+
let preferences_mismatch =
151+
preferences
152+
.get(package_name)
153+
.filter(|(marker, _index, _version)| {
154+
marker
155+
.map(|marker| !env.included_by_marker(marker))
156+
.unwrap_or(false)
157+
});
158+
let preferences = preferences_match.chain(preferences_mismatch).filter_map(
159+
|(marker, source, version)| {
160+
// If the package is mapped to an explicit index, only consider preferences that
161+
// match the index.
162+
index
163+
.map_or(true, |index| source == Some(index))
164+
.then_some((marker, version))
165+
},
166+
);
149167
self.get_preferred_from_iter(
150-
preferences_match.chain(preferences_mismatch),
168+
preferences,
151169
package_name,
152170
range,
153171
version_maps,

crates/uv-resolver/src/preferences.rs

+50-24
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,17 @@
1+
use std::path::Path;
12
use std::str::FromStr;
23

34
use rustc_hash::FxHashMap;
45
use tracing::trace;
56

6-
use uv_distribution_types::{InstalledDist, InstalledMetadata, InstalledVersion, Name};
7+
use uv_distribution_types::{IndexUrl, InstalledDist, InstalledMetadata, InstalledVersion, Name};
78
use uv_normalize::PackageName;
89
use uv_pep440::{Operator, Version};
910
use uv_pep508::{MarkerTree, VersionOrUrl};
1011
use uv_pypi_types::{HashDigest, HashError};
1112
use uv_requirements_txt::{RequirementEntry, RequirementsTxtRequirement};
1213

13-
use crate::ResolverEnvironment;
14+
use crate::{LockError, ResolverEnvironment};
1415

1516
#[derive(thiserror::Error, Debug)]
1617
pub enum PreferenceError {
@@ -25,6 +26,8 @@ pub struct Preference {
2526
version: Version,
2627
/// The markers on the requirement itself (those after the semicolon).
2728
marker: MarkerTree,
29+
/// The index URL of the package, if any.
30+
index: Option<IndexUrl>,
2831
/// If coming from a package with diverging versions, the markers of the forks this preference
2932
/// is part of, otherwise `None`.
3033
fork_markers: Vec<MarkerTree>,
@@ -60,6 +63,7 @@ impl Preference {
6063
marker: requirement.marker,
6164
// requirements.txt doesn't have fork annotations.
6265
fork_markers: vec![],
66+
index: None,
6367
hashes: entry
6468
.hashes
6569
.iter()
@@ -79,21 +83,26 @@ impl Preference {
7983
name: dist.name().clone(),
8084
version: version.clone(),
8185
marker: MarkerTree::TRUE,
86+
index: None,
8287
// Installed distributions don't have fork annotations.
8388
fork_markers: vec![],
8489
hashes: Vec::new(),
8590
}
8691
}
8792

8893
/// Create a [`Preference`] from a locked distribution.
89-
pub fn from_lock(package: &crate::lock::Package) -> Self {
90-
Self {
94+
pub fn from_lock(
95+
package: &crate::lock::Package,
96+
install_path: &Path,
97+
) -> Result<Self, LockError> {
98+
Ok(Self {
9199
name: package.id.name.clone(),
92100
version: package.id.version.clone(),
93101
marker: MarkerTree::TRUE,
102+
index: package.index(install_path)?,
94103
fork_markers: package.fork_markers().to_vec(),
95104
hashes: Vec::new(),
96-
}
105+
})
97106
}
98107

99108
/// Return the [`PackageName`] of the package for this [`Preference`].
@@ -107,22 +116,29 @@ impl Preference {
107116
}
108117
}
109118

119+
#[derive(Debug, Clone)]
120+
struct Entry {
121+
marker: Option<MarkerTree>,
122+
index: Option<IndexUrl>,
123+
pin: Pin,
124+
}
125+
110126
/// A set of pinned packages that should be preserved during resolution, if possible.
111127
///
112128
/// The marker is the marker of the fork that resolved to the pin, if any.
113129
///
114130
/// Preferences should be prioritized first by whether their marker matches and then by the order
115131
/// they are stored, so that a lockfile has higher precedence than sibling forks.
116132
#[derive(Debug, Clone, Default)]
117-
pub struct Preferences(FxHashMap<PackageName, Vec<(Option<MarkerTree>, Pin)>>);
133+
pub struct Preferences(FxHashMap<PackageName, Vec<Entry>>);
118134

119135
impl Preferences {
120136
/// Create a map of pinned packages from an iterator of [`Preference`] entries.
121137
///
122138
/// The provided [`ResolverEnvironment`] will be used to filter the preferences
123139
/// to an applicable subset.
124-
pub fn from_iter<PreferenceIterator: IntoIterator<Item = Preference>>(
125-
preferences: PreferenceIterator,
140+
pub fn from_iter(
141+
preferences: impl IntoIterator<Item = Preference>,
126142
env: &ResolverEnvironment,
127143
) -> Self {
128144
let mut slf = Self::default();
@@ -152,6 +168,7 @@ impl Preferences {
152168
if preference.fork_markers.is_empty() {
153169
slf.insert(
154170
preference.name,
171+
preference.index,
155172
None,
156173
Pin {
157174
version: preference.version,
@@ -162,6 +179,7 @@ impl Preferences {
162179
for fork_marker in preference.fork_markers {
163180
slf.insert(
164181
preference.name.clone(),
182+
preference.index.clone(),
165183
Some(fork_marker),
166184
Pin {
167185
version: preference.version.clone(),
@@ -179,13 +197,15 @@ impl Preferences {
179197
pub(crate) fn insert(
180198
&mut self,
181199
package_name: PackageName,
200+
index: Option<IndexUrl>,
182201
markers: Option<MarkerTree>,
183202
pin: impl Into<Pin>,
184203
) {
185-
self.0
186-
.entry(package_name)
187-
.or_default()
188-
.push((markers, pin.into()));
204+
self.0.entry(package_name).or_default().push(Entry {
205+
marker: markers,
206+
index,
207+
pin: pin.into(),
208+
});
189209
}
190210

191211
/// Returns an iterator over the preferences.
@@ -194,15 +214,19 @@ impl Preferences {
194214
) -> impl Iterator<
195215
Item = (
196216
&PackageName,
197-
impl Iterator<Item = (Option<&MarkerTree>, &Version)>,
217+
impl Iterator<Item = (Option<&MarkerTree>, Option<&IndexUrl>, &Version)>,
198218
),
199219
> {
200220
self.0.iter().map(|(name, preferences)| {
201221
(
202222
name,
203-
preferences
204-
.iter()
205-
.map(|(markers, pin)| (markers.as_ref(), pin.version())),
223+
preferences.iter().map(|entry| {
224+
(
225+
entry.marker.as_ref(),
226+
entry.index.as_ref(),
227+
entry.pin.version(),
228+
)
229+
}),
206230
)
207231
})
208232
}
@@ -211,12 +235,14 @@ impl Preferences {
211235
pub(crate) fn get(
212236
&self,
213237
package_name: &PackageName,
214-
) -> impl Iterator<Item = (Option<&MarkerTree>, &Version)> {
215-
self.0
216-
.get(package_name)
217-
.into_iter()
218-
.flatten()
219-
.map(|(markers, pin)| (markers.as_ref(), pin.version()))
238+
) -> impl Iterator<Item = (Option<&MarkerTree>, Option<&IndexUrl>, &Version)> {
239+
self.0.get(package_name).into_iter().flatten().map(|entry| {
240+
(
241+
entry.marker.as_ref(),
242+
entry.index.as_ref(),
243+
entry.pin.version(),
244+
)
245+
})
220246
}
221247

222248
/// Return the hashes for a package, if the version matches that of the pin.
@@ -229,8 +255,8 @@ impl Preferences {
229255
.get(package_name)
230256
.into_iter()
231257
.flatten()
232-
.find(|(_markers, pin)| pin.version() == version)
233-
.map(|(_markers, pin)| pin.hashes())
258+
.find(|entry| entry.pin.version() == version)
259+
.map(|entry| entry.pin.hashes())
234260
}
235261
}
236262

crates/uv-resolver/src/resolver/mod.rs

+6-2
Original file line numberDiff line numberDiff line change
@@ -381,6 +381,7 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
381381
for (package, version) in &resolution.nodes {
382382
preferences.insert(
383383
package.name.clone(),
384+
package.index.clone(),
384385
resolution.env.try_markers().cloned(),
385386
version.clone(),
386387
);
@@ -669,14 +670,15 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
669670
diverging_packages: &'a [PackageName],
670671
) -> impl Iterator<Item = Result<ForkState, ResolveError>> + 'a {
671672
debug!(
672-
"Splitting resolution on {}=={} over {} into {} resolution with separate markers",
673+
"Splitting resolution on {}=={} over {} into {} resolution{} with separate markers",
673674
current_state.next,
674675
version,
675676
diverging_packages
676677
.iter()
677678
.map(ToString::to_string)
678679
.join(", "),
679-
forks.len()
680+
forks.len(),
681+
if forks.len() == 1 { "" } else { "s" }
680682
);
681683
assert!(forks.len() >= 2);
682684
// This is a somewhat tortured technique to ensure
@@ -1075,6 +1077,7 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
10751077
preferences,
10761078
&self.installed_packages,
10771079
&self.exclusions,
1080+
index,
10781081
env,
10791082
) else {
10801083
// Short circuit: we couldn't find _any_ versions for a package.
@@ -1934,6 +1937,7 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
19341937
&self.preferences,
19351938
&self.installed_packages,
19361939
&self.exclusions,
1940+
None,
19371941
&env,
19381942
) else {
19391943
return Ok(None);

crates/uv-resolver/src/yanks.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ impl AllowedYanks {
4545
allowed_yanks
4646
.entry(name.clone())
4747
.or_default()
48-
.extend(preferences.map(|(_markers, version)| version.clone()));
48+
.extend(preferences.map(|(.., version)| version.clone()));
4949
}
5050

5151
Self(Arc::new(allowed_yanks))

crates/uv/src/commands/project/lock.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -563,7 +563,8 @@ async fn do_lock(
563563

564564
// If an existing lockfile exists, build up a set of preferences.
565565
let LockedRequirements { preferences, git } = versions_lock
566-
.map(|lock| read_lock_requirements(lock, upgrade))
566+
.map(|lock| read_lock_requirements(lock, workspace.install_path(), upgrade))
567+
.transpose()?
567568
.unwrap_or_default();
568569

569570
// Populate the Git resolver.

0 commit comments

Comments
 (0)