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

Add support for extras in editable requirements #1531

Merged
merged 2 commits into from
Feb 16, 2024
Merged

Add support for extras in editable requirements #1531

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bd998d8
Add support for extras in editable requirements
charliermarsh Feb 16, 2024
173fd21
Reverts
charliermarsh Feb 16, 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
54 changes: 35 additions & 19 deletions crates/uv-resolver/src/resolution.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,10 +65,13 @@ impl ResolutionGraph {
match package {
PubGrubPackage::Package(package_name, None, None) => {
// Create the distribution.
let pinned_package = pins
.get(package_name, version)
.expect("Every package should be pinned")
.clone();
let pinned_package = if let Some((editable, _)) = editables.get(package_name) {
Dist::from_editable(package_name.clone(), editable.clone())?
} else {
pins.get(package_name, version)
.expect("Every package should be pinned")
.clone()
};

// Add its hashes to the index.
if let Some(versions_response) = packages.get(package_name) {
Expand All @@ -87,9 +90,7 @@ impl ResolutionGraph {
}
PubGrubPackage::Package(package_name, None, Some(url)) => {
// Create the distribution.
let pinned_package = if let Some((editable, _)) = editables.get(package_name) {
Dist::from_editable(package_name.clone(), editable.clone())?
} else {
let pinned_package = {
let url = redirects.get(url).map_or_else(
|| url.clone(),
|url| VerbatimUrl::unknown(url.value().clone()),
Expand All @@ -115,20 +116,35 @@ impl ResolutionGraph {
PubGrubPackage::Package(package_name, Some(extra), None) => {
// Validate that the `extra` exists.
let dist = PubGrubDistribution::from_registry(package_name, version);
let metadata = distributions
.get(&dist.package_id())
.expect("Every package should have metadata");

if !metadata.provides_extras.contains(extra) {
let pinned_package = pins
.get(package_name, version)
.expect("Every package should be pinned")
.clone();
if let Some((_, metadata)) = editables.get(package_name) {
if !metadata.provides_extras.contains(extra) {
let pinned_package = pins
.get(package_name, version)
.expect("Every package should be pinned")
.clone();

diagnostics.push(Diagnostic::MissingExtra {
dist: pinned_package,
extra: extra.clone(),
});
diagnostics.push(Diagnostic::MissingExtra {
dist: pinned_package,
extra: extra.clone(),
});
}
} else {
let metadata = distributions
.get(&dist.package_id())
.expect("Every package should have metadata");

if !metadata.provides_extras.contains(extra) {
let pinned_package = pins
.get(package_name, version)
.expect("Every package should be pinned")
.clone();

diagnostics.push(Diagnostic::MissingExtra {
dist: pinned_package,
extra: extra.clone(),
});
}
}
}
PubGrubPackage::Package(package_name, Some(extra), Some(url)) => {
Expand Down
65 changes: 49 additions & 16 deletions crates/uv-resolver/src/resolver/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,16 +160,8 @@ impl<'a, Provider: ResolverProvider> Resolver<'a, Provider> {
// Determine all the editable requirements.
let mut editables = FxHashMap::default();
for (editable_requirement, metadata) in &manifest.editables {
// Convert the editable requirement into a distribution.
let dist = Dist::from_editable(metadata.name.clone(), editable_requirement.clone())
.expect("This is a valid distribution");

// Mock editable responses.
let package_id = dist.package_id();
index.distributions.register(package_id.clone());
index.distributions.done(package_id, metadata.clone());
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think removing this is good.

editables.insert(
dist.name().clone(),
metadata.name.clone(),
(editable_requirement.clone(), metadata.clone()),
);
}
Expand Down Expand Up @@ -633,6 +625,16 @@ impl<'a, Provider: ResolverProvider> Resolver<'a, Provider> {
}

PubGrubPackage::Package(package_name, extra, None) => {
// If the dist is an editable, return the version from the editable metadata.
if let Some((_local, metadata)) = self.editables.get(package_name) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only downside to all of this is that we now pay the cost of doing these lookups for all packages.

let version = metadata.version.clone();
return if range.contains(&version) {
Ok(Some(ResolverVersion::Available(version)))
} else {
Ok(None)
};
}

// Wait for the metadata to be available.
let versions_response = self
.index
Expand Down Expand Up @@ -798,19 +800,15 @@ impl<'a, Provider: ResolverProvider> Resolver<'a, Provider> {
// Add a dependency on each editable.
for (editable, metadata) in self.editables.values() {
constraints.insert(
PubGrubPackage::Package(
metadata.name.clone(),
None,
Some(editable.url().clone()),
),
PubGrubPackage::Package(metadata.name.clone(), None, None),
Range::singleton(metadata.version.clone()),
);
for extra in &editable.extras {
constraints.insert(
PubGrubPackage::Package(
metadata.name.clone(),
Some(extra.clone()),
Some(editable.url().clone()),
None,
),
Range::singleton(metadata.version.clone()),
);
Expand All @@ -830,7 +828,37 @@ impl<'a, Provider: ResolverProvider> Resolver<'a, Provider> {
return Ok(Dependencies::Available(DependencyConstraints::default()));
}

// Determine the distribution to lookup
// Determine if the distribution is editable.
if let Some((_local, metadata)) = self.editables.get(package_name) {
let mut constraints = PubGrubDependencies::from_requirements(
&metadata.requires_dist,
&self.constraints,
&self.overrides,
Some(package_name),
extra.as_ref(),
self.markers,
)?;

for (package, version) in constraints.iter() {
debug!("Adding transitive dependency: {package}{version}");

// Emit a request to fetch the metadata for this package.
self.visit_package(package, priorities, request_sink)
.await?;
}

// If a package has an extra, insert a constraint on the base package.
if extra.is_some() {
constraints.insert(
PubGrubPackage::Package(package_name.clone(), None, None),
Range::singleton(version.clone()),
);
}

return Ok(Dependencies::Available(constraints.into()));
}

// Determine the distribution to lookup.
let dist = match url {
Some(url) => PubGrubDistribution::from_url(package_name, url),
None => PubGrubDistribution::from_registry(package_name, version),
Expand Down Expand Up @@ -984,6 +1012,11 @@ impl<'a, Provider: ResolverProvider> Resolver<'a, Provider> {

// Pre-fetch the package and distribution metadata.
Request::Prefetch(package_name, range) => {
// Ignore editables.
if self.editables.contains_key(&package_name) {
return Ok(None);
}

// Wait for the package metadata to become available.
let versions_response = self
.index
Expand Down
6 changes: 4 additions & 2 deletions crates/uv/tests/pip_compile.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2075,7 +2075,7 @@ fn compile_editable() -> Result<()> {
requirements_in.write_str(indoc! {r"
-e ../../scripts/editable-installs/poetry_editable
-e ${PROJECT_ROOT}/../../scripts/editable-installs/maturin_editable
-e file://../../scripts/editable-installs/black_editable[d]
-e file://../../scripts/editable-installs/black_editable[dev]
boltons # normal dependency for comparison
"
})?;
Expand Down Expand Up @@ -2122,12 +2122,14 @@ fn compile_editable() -> Result<()> {
# yarl
numpy==1.26.2
# via poetry-editable
uvloop==0.19.0
# via black
yarl==1.9.2
# via aiohttp

----- stderr -----
Built 3 editables in [TIME]
Resolved 12 packages in [TIME]
Resolved 13 packages in [TIME]
"###);

Ok(())
Expand Down
4 changes: 4 additions & 0 deletions scripts/editable-installs/black_editable/pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ jupyter = [
"ipython>=7.8.0",
"tokenize-rt>=3.2.0",
]
dev = [
"black[d]",
"black[uvloop]",
]

[build-system]
requires = ["flit_core>=3.4,<4"]
Expand Down