-
Notifications
You must be signed in to change notification settings - Fork 628
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.
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
Allow multiple pinned indexes in tool.uv.sources
#7769
base: charlie/index-api
Are you sure you want to change the base?
Allow multiple pinned indexes in tool.uv.sources
#7769
Conversation
f2a6d3d
to
ca30bc3
Compare
051f118
to
31a5e8b
Compare
| ^ | ||
Sources can only include a single index source | ||
Resolved 4 packages in [TIME] | ||
error: found duplicate package `jinja2==3.1.3 @ registry+https://download.pytorch.org/whl/cu118` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one actually fails:
[project]
name = "project"
version = "0.1.0"
requires-python = ">=3.12"
dependencies = ["jinja2>=3"]
[tool.uv.sources]
jinja2 = [
{ index = "torch-cu118", marker = "sys_platform == 'win32'"},
]
[[tool.uv.index]]
name = "torch-cu118"
url = "https://download.pytorch.org/whl/cu118"
(It succeeds if you use explicit = true
on the index.)
The issue is that the sys_platform == 'win32'
branch uses the torch-cu118
index explicitly, but then the implied sys_platform != 'win32'
branch also resolves to that same index.
31a5e8b
to
495b958
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
YES
Mark this fat in the release notes
@@ -846,8 +874,10 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag | |||
if let Some(url) = package.name().and_then(|name| fork_urls.get(name)) { | |||
self.choose_version_url(name, range, url, python_requirement) | |||
} else { | |||
// STOPSHIP: pass in the URL |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
?
Entry::Vacant(vacant) => { | ||
vacant.insert(index.clone()); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit:
} | |
if let Some(previous) = self.0.insert(package_name.clone(), index.clone()) { | |
if &previous != index { | |
let mut conflicts = vec![previous.to_string(), index.to_string()]; | |
conflicts.sort(); | |
return match fork_markers { | |
ResolverMarkers::Universal { .. } | ResolverMarkers::SpecificEnvironment(_) => { | |
Err(ResolveError::ConflictingIndexesUniversal( | |
package_name.clone(), | |
conflicts, | |
)) | |
} | |
ResolverMarkers::Fork(fork_markers) => { | |
Err(ResolveError::ConflictingIndexesFork { | |
package_name: package_name.clone(), | |
indexes: conflicts, | |
fork_markers: fork_markers.clone(), | |
}) | |
} | |
}; | |
} | |
} |
Summary
This PR lifts the restriction that a package must come from a single index. For example, you can now do:
The construction is very similar to the way we handle URLs today: you can have multiple URLs for a given package, but they must appear in disjoint forks. So most of the code is just adding that abstraction to the resolver, following our handling of URLs.
Closes #7761.