Add support for development dependencies (#4036)

## Summary

Externally, development dependencies are currently structured as a flat
list of PEP 580-compatible requirements:

```toml
[tool.uv]
dev-dependencies = ["werkzeug"]
```

When locking, we lock all development dependencies; when syncing, users
can provide `--dev`.

Internally, though, we model them as dependency groups, similar to
Poetry, PDM, and [PEP 735](https://peps.python.org/pep-0735). This
enables us to change out the user-facing frontend without changing the
internal implementation, once we've decided how these should be exposed
to users.

A few important decisions encoded in the implementation (which we can
change later):

1. Groups are enabled globally, for all dependencies. This differs from
extras, which are enabled on a per-requirement basis. Note, however,
that we'll only discover groups for uv-enabled packages anyway.
2. Installing a group requires installing the base package. We rely on
this in PubGrub to ensure that we resolve to the same version (even
though we only expect groups to come from workspace dependencies anyway,
which are unique). But anyway, that's encoded in the resolver right now,
just as it is for extras.
This commit is contained in:
Charlie Marsh 2024-06-05 21:40:17 -04:00 committed by GitHub
parent a81fb92ee6
commit 0acae9bd9c
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
37 changed files with 642 additions and 79 deletions

View file

@ -1,6 +1,7 @@
//! Given a set of requirements, find a set of compatible packages.
use std::borrow::Cow;
use std::collections::BTreeMap;
use std::fmt::{Display, Formatter};
use std::sync::Arc;
use std::thread;
@ -30,7 +31,7 @@ pub(crate) use urls::Urls;
use uv_configuration::{Constraints, Overrides};
use uv_distribution::{ArchiveMetadata, DistributionDatabase};
use uv_git::GitResolver;
use uv_normalize::{ExtraName, PackageName};
use uv_normalize::{ExtraName, GroupName, PackageName};
use uv_types::{BuildContext, HashStrategy, InstalledPackagesProvider};
use crate::candidate_selector::{CandidateDist, CandidateSelector};
@ -80,6 +81,7 @@ struct ResolverState<InstalledPackages: InstalledPackagesProvider> {
requirements: Vec<Requirement>,
constraints: Constraints,
overrides: Overrides,
dev: Vec<GroupName>,
preferences: Preferences,
git: GitResolver,
exclusions: Exclusions,
@ -190,6 +192,7 @@ impl<Provider: ResolverProvider, InstalledPackages: InstalledPackagesProvider>
requirements: manifest.requirements,
constraints: manifest.constraints,
overrides: manifest.overrides,
dev: manifest.dev,
preferences: Preferences::from_iter(manifest.preferences, markers),
exclusions: manifest.exclusions,
hasher: hasher.clone(),
@ -577,6 +580,7 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
PubGrubPackageInner::Root(_) => {}
PubGrubPackageInner::Python(_) => {}
PubGrubPackageInner::Extra { .. } => {}
PubGrubPackageInner::Dev { .. } => {}
PubGrubPackageInner::Package {
name, url: None, ..
} => {
@ -622,6 +626,7 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
let PubGrubPackageInner::Package {
name,
extra: None,
dev: None,
marker: _marker,
url: None,
} = &**package
@ -662,6 +667,11 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
url: Some(url),
..
}
| PubGrubPackageInner::Dev {
name,
url: Some(url),
..
}
| PubGrubPackageInner::Package {
name,
url: Some(url),
@ -751,6 +761,9 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
PubGrubPackageInner::Extra {
name, url: None, ..
}
| PubGrubPackageInner::Dev {
name, url: None, ..
}
| PubGrubPackageInner::Package {
name, url: None, ..
} => {
@ -870,9 +883,13 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
let name = match &**pkg {
// A root can never be a dependency of another package, and a `Python` pubgrub
// package is never returned by `get_dependencies`. So these cases never occur.
// TODO(charlie): This might be overly conservative for `Extra` and `Group`. If
// multiple groups are enabled, we shouldn't need to fork. Similarly, if multiple
// extras are enabled, we shouldn't need to fork.
PubGrubPackageInner::Root(_) | PubGrubPackageInner::Python(_) => unreachable!(),
PubGrubPackageInner::Package { ref name, .. }
| PubGrubPackageInner::Extra { ref name, .. } => name,
| PubGrubPackageInner::Extra { ref name, .. }
| PubGrubPackageInner::Dev { ref name, .. } => name,
};
by_grouping
.entry(name)
@ -918,10 +935,12 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
// Add the root requirements.
let dependencies = PubGrubDependencies::from_requirements(
&self.requirements,
&BTreeMap::default(),
&self.constraints,
&self.overrides,
None,
None,
None,
&self.urls,
&self.locals,
&self.git,
@ -955,6 +974,7 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
PubGrubPackageInner::Package {
name,
extra,
dev,
marker,
url,
} => {
@ -967,6 +987,7 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
PubGrubPackage::from(PubGrubPackageInner::Package {
name: name.clone(),
extra: extra.clone(),
dev: dev.clone(),
marker: None,
url: url.clone(),
}),
@ -1070,10 +1091,12 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
let mut dependencies = PubGrubDependencies::from_requirements(
&metadata.requires_dist,
&metadata.dev_dependencies,
&self.constraints,
&self.overrides,
Some(name),
extra.as_ref(),
dev.as_ref(),
&self.urls,
&self.locals,
&self.git,
@ -1090,6 +1113,25 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
self.visit_package(dep_package, request_sink)?;
}
// If a package has metadata for an enabled dependency group,
// add a dependency from it to the same package with the group
// enabled.
if extra.is_none() && dev.is_none() {
for dev in &self.dev {
if metadata.dev_dependencies.contains_key(dev) {
dependencies.push(
PubGrubPackage::from(PubGrubPackageInner::Dev {
name: name.clone(),
dev: dev.clone(),
marker: marker.clone(),
url: url.clone(),
}),
Range::singleton(version.clone()),
);
}
}
}
// If a package has a marker, add a dependency from it to the
// same package without markers.
//
@ -1106,6 +1148,7 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
PubGrubPackage::from(PubGrubPackageInner::Package {
name: name.clone(),
extra: extra.clone(),
dev: dev.clone(),
marker: None,
url: url.clone(),
}),
@ -1127,6 +1170,7 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
PubGrubPackage::from(PubGrubPackageInner::Package {
name: name.clone(),
extra: None,
dev: None,
marker: marker.clone(),
url: url.clone(),
}),
@ -1136,6 +1180,36 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
PubGrubPackage::from(PubGrubPackageInner::Package {
name: name.clone(),
extra: Some(extra.clone()),
dev: None,
marker: marker.clone(),
url: url.clone(),
}),
Range::singleton(version.clone()),
),
])),
// Add a dependency on both the development dependency group and base package.
PubGrubPackageInner::Dev {
name,
dev,
marker,
url,
} => Ok(Dependencies::Available(vec![
(
PubGrubPackage::from(PubGrubPackageInner::Package {
name: name.clone(),
extra: None,
dev: None,
marker: marker.clone(),
url: url.clone(),
}),
Range::singleton(version.clone()),
),
(
PubGrubPackage::from(PubGrubPackageInner::Package {
name: name.clone(),
extra: None,
dev: Some(dev.clone()),
marker: marker.clone(),
url: url.clone(),
}),
@ -1371,6 +1445,7 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
PubGrubPackageInner::Root(_) => {}
PubGrubPackageInner::Python(_) => {}
PubGrubPackageInner::Extra { .. } => {}
PubGrubPackageInner::Dev { .. } => {}
PubGrubPackageInner::Package {
name,
url: Some(url),
@ -1462,6 +1537,7 @@ impl SolveState {
let PubGrubPackageInner::Package {
name: ref self_name,
extra: ref self_extra,
dev: ref self_dev,
..
} = &**self_package
else {
@ -1472,6 +1548,7 @@ impl SolveState {
PubGrubPackageInner::Package {
name: ref dependency_name,
extra: ref dependency_extra,
dev: ref dependency_dev,
..
} => {
if self_name == dependency_name {
@ -1484,8 +1561,10 @@ impl SolveState {
let versions = ResolutionDependencyVersions {
from_version: self_version.clone(),
from_extra: self_extra.clone(),
from_dev: self_dev.clone(),
to_version: dependency_version.clone(),
to_extra: dependency_extra.clone(),
to_dev: dependency_dev.clone(),
};
dependencies.entry(names).or_default().insert(versions);
}
@ -1505,8 +1584,33 @@ impl SolveState {
let versions = ResolutionDependencyVersions {
from_version: self_version.clone(),
from_extra: self_extra.clone(),
from_dev: self_dev.clone(),
to_version: dependency_version.clone(),
to_extra: Some(dependency_extra.clone()),
to_dev: None,
};
dependencies.entry(names).or_default().insert(versions);
}
PubGrubPackageInner::Dev {
name: ref dependency_name,
dev: ref dependency_dev,
..
} => {
if self_name == dependency_name {
continue;
}
let names = ResolutionDependencyNames {
from: self_name.clone(),
to: dependency_name.clone(),
};
let versions = ResolutionDependencyVersions {
from_version: self_version.clone(),
from_extra: self_extra.clone(),
from_dev: self_dev.clone(),
to_version: dependency_version.clone(),
to_extra: None,
to_dev: Some(dependency_dev.clone()),
};
dependencies.entry(names).or_default().insert(versions);
}
@ -1545,8 +1649,10 @@ pub(crate) struct ResolutionDependencyNames {
pub(crate) struct ResolutionDependencyVersions {
pub(crate) from_version: Version,
pub(crate) from_extra: Option<ExtraName>,
pub(crate) from_dev: Option<GroupName>,
pub(crate) to_version: Version,
pub(crate) to_extra: Option<ExtraName>,
pub(crate) to_dev: Option<GroupName>,
}
impl Resolution {