diff --git a/crates/uv-resolver/src/resolution/display.rs b/crates/uv-resolver/src/resolution/display.rs index 45c88df99..04532ba84 100644 --- a/crates/uv-resolver/src/resolution/display.rs +++ b/crates/uv-resolver/src/resolution/display.rs @@ -1,6 +1,7 @@ use std::collections::BTreeSet; use owo_colors::OwoColorize; +use petgraph::algo::greedy_feedback_arc_set; use petgraph::visit::{EdgeRef, Topo}; use petgraph::Direction; use rustc_hash::{FxBuildHasher, FxHashMap}; @@ -348,6 +349,28 @@ fn to_requirements_txt_graph(graph: &ResolutionPetGraph) -> IntermediatePetGraph /// The graph is directed, so if any edge contains a marker, we need to propagate it to all /// downstream nodes. fn propagate_markers(mut graph: IntermediatePetGraph) -> IntermediatePetGraph { + // Remove any cycles. By absorption, it should be fine to ignore cycles. + // + // Imagine a graph: `A -> B -> C -> A`. Assume that `A` has weight `1`, `B` has weight `2`, + // and `C` has weight `3`. The weights are the marker trees. + // + // When propagating, we'd return to `A` when we hit the cycle, to create `1 or (1 and 2 and 3)`, + // which resolves to `1`. + // + // TODO(charlie): The above reasoning could be incorrect. Consider using a graph algorithm that + // can handle weight propagation with cycles. + let edges = { + let fas = greedy_feedback_arc_set(&graph) + .map(|edge| edge.id()) + .collect::>(); + let mut edges = Vec::with_capacity(fas.len()); + for edge_id in fas { + edges.push(graph.edge_endpoints(edge_id).unwrap()); + graph.remove_edge(edge_id); + } + edges + }; + let mut topo = Topo::new(&graph); while let Some(index) = topo.next(&graph) { let marker_tree: Option = { @@ -386,6 +409,12 @@ fn propagate_markers(mut graph: IntermediatePetGraph) -> IntermediatePetGraph { }; } + // Re-add the removed edges. We no longer care about the edge _weights_, but we do want the + // edges to be present, to power the `# via` annotations. + for (source, target) in edges { + graph.add_edge(source, target, None); + } + graph } diff --git a/crates/uv/tests/pip_compile.rs b/crates/uv/tests/pip_compile.rs index af0ed8e93..5ab9aec38 100644 --- a/crates/uv/tests/pip_compile.rs +++ b/crates/uv/tests/pip_compile.rs @@ -6300,7 +6300,7 @@ fn universal() -> Result<()> { trio ; sys_platform == 'win32' "})?; - uv_snapshot!(context.pip_compile() + uv_snapshot!(context.filters(), windows_filters=false, context.pip_compile() .arg("requirements.in") .arg("--universal"), @r###" success: true @@ -6345,7 +6345,7 @@ fn universal_conflicting() -> Result<()> { trio==0.10.0 ; sys_platform == 'win32' "})?; - uv_snapshot!(context.pip_compile() + uv_snapshot!(context.filters(), windows_filters=false, context.pip_compile() .arg("requirements.in") .arg("--universal"), @r###" success: true @@ -6384,6 +6384,137 @@ fn universal_conflicting() -> Result<()> { Ok(()) } +/// Perform a universal resolution with a package that contains cycles in its dependency graph. +#[test] +fn universal_cycles() -> Result<()> { + let context = TestContext::new("3.12"); + let requirements_in = context.temp_dir.child("requirements.in"); + requirements_in.write_str(indoc::indoc! {r" + poetry + "})?; + + uv_snapshot!(context.filters(), windows_filters=false, context.pip_compile() + .arg("requirements.in") + .arg("--universal"), @r###" + success: true + exit_code: 0 + ----- stdout ----- + # This file was autogenerated by uv via the following command: + # uv pip compile --cache-dir [CACHE_DIR] requirements.in --universal + build==1.1.1 + # via poetry + cachecontrol==0.14.0 + # via poetry + certifi==2024.2.2 + # via requests + cffi==1.16.0 ; sys_platform == 'darwin' or (platform_python_implementation != 'PyPy' and sys_platform == 'linux') + # via + # cryptography + # xattr + charset-normalizer==3.3.2 + # via requests + cleo==2.1.0 + # via poetry + colorama==0.4.6 ; os_name == 'nt' + # via build + crashtest==0.4.1 + # via + # cleo + # poetry + cryptography==42.0.5 ; sys_platform == 'linux' + # via secretstorage + distlib==0.3.8 + # via virtualenv + dulwich==0.21.7 + # via poetry + fastjsonschema==2.19.1 + # via poetry + filelock==3.13.1 + # via + # cachecontrol + # virtualenv + idna==3.6 + # via requests + installer==0.7.0 + # via poetry + jaraco-classes==3.3.1 + # via keyring + jeepney==0.8.0 ; sys_platform == 'linux' + # via + # keyring + # secretstorage + keyring==24.3.1 + # via poetry + more-itertools==10.2.0 + # via jaraco-classes + msgpack==1.0.8 + # via cachecontrol + packaging==24.0 + # via + # build + # poetry + pexpect==4.9.0 + # via poetry + pkginfo==1.10.0 + # via poetry + platformdirs==4.2.0 + # via + # poetry + # virtualenv + poetry==1.8.2 + # via + # -r requirements.in + # poetry-plugin-export + poetry-core==1.9.0 + # via + # poetry + # poetry-plugin-export + poetry-plugin-export==1.7.1 + # via poetry + ptyprocess==0.7.0 + # via pexpect + pycparser==2.21 ; sys_platform == 'darwin' or (platform_python_implementation != 'PyPy' and sys_platform == 'linux') + # via cffi + pyproject-hooks==1.0.0 + # via + # build + # poetry + pywin32-ctypes==0.2.2 ; sys_platform == 'win32' + # via keyring + rapidfuzz==3.7.0 + # via cleo + requests==2.31.0 + # via + # cachecontrol + # poetry + # requests-toolbelt + requests-toolbelt==1.0.0 + # via poetry + secretstorage==3.3.3 ; sys_platform == 'linux' + # via keyring + shellingham==1.5.4 + # via poetry + tomlkit==0.12.4 + # via poetry + trove-classifiers==2024.3.3 + # via poetry + urllib3==2.2.1 + # via + # dulwich + # requests + virtualenv==20.25.1 + # via poetry + xattr==1.1.0 ; sys_platform == 'darwin' + # via poetry + + ----- stderr ----- + Resolved 41 packages in [TIME] + "### + ); + + Ok(()) +} + /// Resolve a package from a `requirements.in` file, with a `constraints.txt` file pinning one of /// its transitive dependencies to a specific version. #[test]