Skip to content

Commit 1650bce

Browse files
charliermarshcharliermarsh
committed
Add a test for workspace members in dependency groups
1 parent 35ff802 commit 1650bce

6 files changed

Lines changed: 401 additions & 15 deletions

File tree

‎crates/uv-distribution-types/src/resolution.rs‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,16 @@ pub enum Node {
188188
},
189189
}
190190

191+
impl Node {
192+
/// Returns `true` if the node should be installed.
193+
pub fn install(&self) -> bool {
194+
match self {
195+
Self::Root => false,
196+
Self::Dist { install, .. } => *install,
197+
}
198+
}
199+
}
200+
191201
/// An edge in the resolution graph, along with the marker that must be satisfied to traverse it.
192202
#[derive(Debug, Clone)]
193203
pub enum Edge {

‎crates/uv-resolver/src/lock/target.rs‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -240,7 +240,19 @@ impl<'env> InstallTarget<'env> {
240240
index
241241
}
242242
Entry::Occupied(entry) => {
243+
// Critically, if the package is already in the graph, then it's a workspace
244+
// member. If it was omitted due to, e.g., `--only-dev`, but is itself
245+
// referenced as a development dependency, then we need to re-enable it.
243246
let index = *entry.get();
247+
let node = &mut petgraph[index];
248+
if !dev.prod() {
249+
*node = self.package_to_node(
250+
dep_dist,
251+
tags,
252+
build_options,
253+
install_options,
254+
)?;
255+
}
244256
index
245257
}
246258
};
@@ -314,7 +326,14 @@ impl<'env> InstallTarget<'env> {
314326
index
315327
}
316328
Entry::Occupied(entry) => {
329+
// Critically, if the package is already in the graph, then it's a workspace
330+
// member. If it was omitted due to, e.g., `--only-dev`, but is itself
331+
// referenced as a development dependency, then we need to re-enable it.
317332
let index = *entry.get();
333+
let node = &mut petgraph[index];
334+
if !dev.prod() {
335+
*node = self.package_to_node(dist, tags, build_options, install_options)?;
336+
}
318337
index
319338
}
320339
};

‎crates/uv-resolver/src/pubgrub/dependencies.rs‎

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ use std::iter;
33
use pubgrub::Ranges;
44
use tracing::warn;
55

6-
use uv_normalize::{ExtraName, PackageName};
6+
use uv_normalize::{ExtraName, GroupName, PackageName};
77
use uv_pep440::{Version, VersionSpecifiers};
88
use uv_pypi_types::{
99
ParsedArchiveUrl, ParsedDirectoryUrl, ParsedGitUrl, ParsedPathUrl, ParsedUrl, Requirement,
@@ -29,6 +29,7 @@ pub(crate) struct PubGrubDependency {
2929
impl PubGrubDependency {
3030
pub(crate) fn from_requirement<'a>(
3131
requirement: &'a Requirement,
32+
dev: Option<&'a GroupName>,
3233
source_name: Option<&'a PackageName>,
3334
) -> impl Iterator<Item = Self> + 'a {
3435
// Add the package, plus any extra variants.
@@ -45,9 +46,11 @@ impl PubGrubDependency {
4546
match &*package {
4647
PubGrubPackageInner::Package { name, .. } => {
4748
// Detect self-dependencies.
48-
if source_name.is_some_and(|source_name| source_name == name) {
49-
warn!("{name} has a dependency on itself");
50-
return None;
49+
if dev.is_none() {
50+
if source_name.is_some_and(|source_name| source_name == name) {
51+
warn!("{name} has a dependency on itself");
52+
return None;
53+
}
5154
}
5255

5356
Some(PubGrubDependency {
@@ -64,10 +67,13 @@ impl PubGrubDependency {
6467
url,
6568
}),
6669
PubGrubPackageInner::Extra { name, .. } => {
67-
debug_assert!(
68-
!source_name.is_some_and(|source_name| source_name == name),
69-
"extras not flattened for {name}"
70-
);
70+
// Detect self-dependencies.
71+
if dev.is_none() {
72+
debug_assert!(
73+
!source_name.is_some_and(|source_name| source_name == name),
74+
"extras not flattened for {name}"
75+
);
76+
}
7177
Some(PubGrubDependency {
7278
package: package.clone(),
7379
version: version.clone(),

‎crates/uv-resolver/src/resolution/output.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ impl Display for ResolutionGraphNode {
8383
}
8484
}
8585

86-
#[derive(Eq, PartialEq, Hash)]
86+
#[derive(Debug, Eq, PartialEq, Hash)]
8787
struct PackageRef<'a> {
8888
package_name: &'a PackageName,
8989
version: &'a Version,

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

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1259,7 +1259,9 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
12591259

12601260
requirements
12611261
.iter()
1262-
.flat_map(|requirement| PubGrubDependency::from_requirement(requirement, None))
1262+
.flat_map(|requirement| {
1263+
PubGrubDependency::from_requirement(requirement, None, None)
1264+
})
12631265
.collect()
12641266
}
12651267
PubGrubPackageInner::Package {
@@ -1445,7 +1447,7 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
14451447
let mut dependencies: Vec<_> = requirements
14461448
.iter()
14471449
.flat_map(|requirement| {
1448-
PubGrubDependency::from_requirement(requirement, Some(name))
1450+
PubGrubDependency::from_requirement(requirement, dev.as_ref(), Some(name))
14491451
})
14501452
.collect();
14511453

@@ -1577,6 +1579,12 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
15771579
.requirements_for_extra(regular_and_dev_dependencies, extra, env, python_requirement)
15781580
.collect::<Vec<_>>();
15791581

1582+
// Dependency groups can include the project itself, so no need to flatten recursive
1583+
// dependencies.
1584+
if dev.is_some() {
1585+
return requirements;
1586+
}
1587+
15801588
// Check if there are recursive self inclusions and we need to go into the expensive branch.
15811589
if !requirements
15821590
.iter()
@@ -2416,7 +2424,9 @@ impl ForkState {
24162424
dev: ref dependency_dev,
24172425
..
24182426
} => {
2419-
if self_name.is_some_and(|self_name| self_name == dependency_name) {
2427+
if self_dev.is_none()
2428+
&& self_name.is_some_and(|self_name| self_name == dependency_name)
2429+
{
24202430
continue;
24212431
}
24222432
let to_url = self.fork_urls.get(dependency_name);
@@ -2444,7 +2454,9 @@ impl ForkState {
24442454
marker: ref dependency_marker,
24452455
..
24462456
} => {
2447-
if self_name.is_some_and(|self_name| self_name == dependency_name) {
2457+
if self_dev.is_none()
2458+
&& self_name.is_some_and(|self_name| self_name == dependency_name)
2459+
{
24482460
continue;
24492461
}
24502462
let to_url = self.fork_urls.get(dependency_name);
@@ -2473,7 +2485,9 @@ impl ForkState {
24732485
marker: ref dependency_marker,
24742486
..
24752487
} => {
2476-
if self_name.is_some_and(|self_name| self_name == dependency_name) {
2488+
if self_dev.is_none()
2489+
&& self_name.is_some_and(|self_name| self_name == dependency_name)
2490+
{
24772491
continue;
24782492
}
24792493
let to_url = self.fork_urls.get(dependency_name);
@@ -2502,7 +2516,9 @@ impl ForkState {
25022516
marker: ref dependency_marker,
25032517
..
25042518
} => {
2505-
if self_name.is_some_and(|self_name| self_name == dependency_name) {
2519+
if self_dev.is_none()
2520+
&& self_name.is_some_and(|self_name| self_name == dependency_name)
2521+
{
25062522
continue;
25072523
}
25082524
let to_url = self.fork_urls.get(dependency_name);

0 commit comments

Comments
 (0)