From 93d3093a2a79fd8e0c61091d65b2e102ef33e9d2 Mon Sep 17 00:00:00 2001 From: Zanie Blue Date: Wed, 10 Jan 2024 14:16:23 -0600 Subject: [PATCH] Improve formatting of package ranges in error messages (#864) Closes #810 Closes https://github.com/astral-sh/puffin/issues/812 Requires https://github.com/zanieb/pubgrub/pull/19 and https://github.com/zanieb/pubgrub/pull/18 - Always pair package ranges with names e.g. `... of a matching a<1.0` instead of `... of a matching <1.0` - Split range segments onto multiple lines when not a singleton as suggested in [#850](https://github.com/astral-sh/puffin/pull/850#discussion_r1446419610) - Improve formatting when ranges are split across multiple lines e.g. by avoiding extra spaces and improving wording Note review will require expanding the hidden files as there are significant changes to the report formatter and snapshots. Bear with me here as these are definitely not perfect still. The following changes build on top of this independently for further improvements: - #868 - #867 - #866 - #871 --- Cargo.lock | 2 +- Cargo.toml | 2 +- crates/puffin-cli/tests/pip_compile.rs | 29 +- crates/puffin-cli/tests/pip_install.rs | 7 +- .../puffin-cli/tests/pip_install_scenarios.rs | 211 ++++++++--- crates/puffin-resolver/src/pubgrub/report.rs | 338 ++++++++++++++++-- crates/puffin-resolver/tests/resolver.rs | 10 +- 7 files changed, 499 insertions(+), 100 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index aa48f9677..4a37c8d94 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2267,7 +2267,7 @@ dependencies = [ [[package]] name = "pubgrub" version = "0.2.1" -source = "git+https://github.com/zanieb/pubgrub?rev=866c0f2a87fee1e8abe804d40a2ee934de0973d7#866c0f2a87fee1e8abe804d40a2ee934de0973d7" +source = "git+https://github.com/zanieb/pubgrub?rev=0e02ea9fc8d021fb6a6b9e77b09ade4332068f42#0e02ea9fc8d021fb6a6b9e77b09ade4332068f42" dependencies = [ "indexmap 2.1.0", "log", diff --git a/Cargo.toml b/Cargo.toml index f74838c44..25fa6038a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -55,7 +55,7 @@ owo-colors = { version = "3.5.0" } petgraph = { version = "0.6.4" } platform-info = { version = "2.0.2" } plist = { version = "1.6.0" } -pubgrub = { git = "https://github.com/zanieb/pubgrub", rev = "866c0f2a87fee1e8abe804d40a2ee934de0973d7" } +pubgrub = { git = "https://github.com/zanieb/pubgrub", rev = "0e02ea9fc8d021fb6a6b9e77b09ade4332068f42" } pyo3 = { version = "0.20.2" } pyo3-log = { version = "0.9.0"} pyproject-toml = { version = "0.8.1" } diff --git a/crates/puffin-cli/tests/pip_compile.rs b/crates/puffin-cli/tests/pip_compile.rs index 9554dc7fb..ae762047a 100644 --- a/crates/puffin-cli/tests/pip_compile.rs +++ b/crates/puffin-cli/tests/pip_compile.rs @@ -670,9 +670,11 @@ fn compile_python_37() -> Result<()> { ----- stderr ----- × No solution found when resolving dependencies: - ╰─▶ Because there are no versions of Python>=3.8 and black==23.10.1 depends - on Python>=3.8, black==23.10.1 is forbidden. - And because root depends on black==23.10.1, version solving failed. + ╰─▶ Because there are no versions of Python that satisfy Python>=3.8 + and black==23.10.1 depends on Python>=3.8, we can conclude that + black==23.10.1 is forbidden. + And because root depends on black==23.10.1 we can conclude that the + requirements are unsatisfiable. "###); }); @@ -1407,8 +1409,9 @@ fn conflicting_direct_url_dependency() -> Result<()> { ----- stderr ----- × No solution found when resolving dependencies: - ╰─▶ Because there is no version of werkzeug==3.0.0 and root depends on - werkzeug==3.0.0, version solving failed. + ╰─▶ Because there is no version of werkzeug==3.0.0 and root depends + on werkzeug==3.0.0, we can conclude that the requirements are + unsatisfiable. "###); }); @@ -1558,8 +1561,10 @@ fn conflicting_transitive_url_dependency() -> Result<()> { ----- stderr ----- × No solution found when resolving dependencies: ╰─▶ Because flask==3.0.0 depends on werkzeug>=3.0.0 and there are no - versions of werkzeug>=3.0.0, flask==3.0.0 is forbidden. - And because root depends on flask==3.0.0, version solving failed. + versions of werkzeug that satisfy werkzeug>=3.0.0, we can conclude that + flask==3.0.0 is forbidden. + And because root depends on flask==3.0.0 we can conclude that the + requirements are unsatisfiable. "###); }); @@ -1901,8 +1906,9 @@ dependencies = ["django==300.1.4"] ----- stderr ----- × No solution found when resolving dependencies: - ╰─▶ Because there is no version of django==300.1.4 and my-project depends on - django==300.1.4, version solving failed. + ╰─▶ Because there is no version of django==300.1.4 and my-project + depends on django==300.1.4, we can conclude that the requirements are + unsatisfiable. "###); }); @@ -2227,8 +2233,9 @@ fn compile_yanked_version_indirect() -> Result<()> { ----- stderr ----- × No solution found when resolving dependencies: - ╰─▶ Because there are no versions of attrs>20.3.0, <21.2.0 and root depends - on attrs>20.3.0, <21.2.0, version solving failed. + ╰─▶ Because there are no versions of attrs that satisfy attrs>20.3.0,<21.2.0 + and root depends on attrs>20.3.0,<21.2.0, we can conclude that the + requirements are unsatisfiable. "###); }); diff --git a/crates/puffin-cli/tests/pip_install.rs b/crates/puffin-cli/tests/pip_install.rs index 5255bc0e4..a89a1bc8a 100644 --- a/crates/puffin-cli/tests/pip_install.rs +++ b/crates/puffin-cli/tests/pip_install.rs @@ -77,10 +77,11 @@ fn no_solution() -> Result<()> { ----- stderr ----- × No solution found when resolving dependencies: - ╰─▶ Because there are no versions of flask>3.0.0 and flask==3.0.0 depends on - werkzeug>=3.0.0, flask>=3.0.0 depends on werkzeug>=3.0.0. + ╰─▶ Because there are no versions of flask that satisfy flask>3.0.0 + and flask==3.0.0 depends on werkzeug>=3.0.0, we can conclude that + flask>=3.0.0 depends on werkzeug>=3.0.0. And because root depends on flask>=3.0.0 and root depends on - werkzeug<1.0.0, version solving failed. + werkzeug<1.0.0, we can conclude that the requirements are unsatisfiable. "###); Ok(()) diff --git a/crates/puffin-cli/tests/pip_install_scenarios.rs b/crates/puffin-cli/tests/pip_install_scenarios.rs index b5ddae2ef..2bd24f467 100644 --- a/crates/puffin-cli/tests/pip_install_scenarios.rs +++ b/crates/puffin-cli/tests/pip_install_scenarios.rs @@ -82,7 +82,13 @@ fn excluded_only_version() -> Result<()> { ----- stderr ----- × No solution found when resolving dependencies: - ╰─▶ Because there are no versions of a<1.0.0 | >1.0.0 and root depends on a<1.0.0 | >1.0.0, version solving failed. + ╰─▶ Because there are no versions of a that satisfy any of: + a<1.0.0 + a>1.0.0 + and root depends on one of: + a<1.0.0 + a>1.0.0 + we can conclude that the requirements are unsatisfiable. "###); }); @@ -150,9 +156,23 @@ fn excluded_only_compatible_version() -> Result<()> { ----- stderr ----- × No solution found when resolving dependencies: - ╰─▶ Because there are no versions of a<1.0.0 | >1.0.0, <2.0.0 | >2.0.0, <3.0.0 | >3.0.0 and a==1.0.0 depends on b==1.0.0, a<2.0.0 depends on b==1.0.0. - And because a==3.0.0 depends on b==3.0.0, a<2.0.0 | >2.0.0 depends on b<=1.0.0 | >=3.0.0. - And because root depends on b>=2.0.0, <3.0.0 and root depends on a<2.0.0 | >2.0.0, version solving failed. + ╰─▶ Because there are no versions of a that satisfy any of: + a<1.0.0 + a>1.0.0,<2.0.0 + a>2.0.0,<3.0.0 + a>3.0.0 + and a==1.0.0 depends on b==1.0.0, we can conclude that a<2.0.0 depends on b==1.0.0. + And because a==3.0.0 depends on b==3.0.0 we can conclude that any of: + a<2.0.0 + a>2.0.0 + depends on one of: + b<=1.0.0 + b>=3.0.0 + + And because root depends on b>=2.0.0,<3.0.0 and root depends on one of: + a<2.0.0 + a>2.0.0 + we can conclude that the requirements are unsatisfiable. "###); }); @@ -258,13 +278,27 @@ fn dependency_excludes_range_of_compatible_versions() -> Result<()> { ----- stderr ----- × No solution found when resolving dependencies: - ╰─▶ Because there are no versions of a<1.0.0 | >1.0.0, <2.0.0 | >3.0.0 and a==1.0.0 depends on b==1.0.0, a<2.0.0 depends on b==1.0.0. (1) + ╰─▶ Because there are no versions of a that satisfy any of: + a<1.0.0 + a>1.0.0,<2.0.0 + a>3.0.0 + and a==1.0.0 depends on b==1.0.0, we can conclude that a<2.0.0 depends on b==1.0.0. (1) - Because there are no versions of c<1.0.0 | >1.0.0, <2.0.0 | >2.0.0 and c==1.0.0 depends on a<2.0.0, c<2.0.0 depends on a<2.0.0. - And because c==2.0.0 depends on a>=3.0.0, c depends on a<2.0.0 | >=3.0.0. - And because a<2.0.0 depends on b==1.0.0 (1), a!=3.0.0, c*, b!=1.0.0 are incompatible. - And because a==3.0.0 depends on b==3.0.0, c depends on b<=1.0.0 | >=3.0.0. - And because root depends on c and root depends on b>=2.0.0, <3.0.0, version solving failed. + Because there are no versions of c that satisfy any of: + c<1.0.0 + c>1.0.0,<2.0.0 + c>2.0.0 + and c==1.0.0 depends on a<2.0.0, we can conclude that c<2.0.0 depends on a<2.0.0. + And because c==2.0.0 depends on a>=3.0.0 we can conclude that c depends on one of: + a<2.0.0 + a>=3.0.0 + + And because we know from (1) that a<2.0.0 depends on b==1.0.0, we can conclude that a!=3.0.0, c*, b!=1.0.0 are incompatible. + And because a==3.0.0 depends on b==3.0.0 we can conclude that c depends on one of: + b<=1.0.0 + b>=3.0.0 + + And because root depends on c and root depends on b>=2.0.0,<3.0.0, we can conclude that the requirements are unsatisfiable. "###); }); @@ -386,13 +420,39 @@ fn dependency_excludes_non_contiguous_range_of_compatible_versions() -> Result<( ----- stderr ----- × No solution found when resolving dependencies: - ╰─▶ Because a==1.0.0 depends on b==1.0.0 and there are no versions of a<1.0.0 | >1.0.0, <2.0.0 | >3.0.0, a<2.0.0 depends on b==1.0.0. - And because a==3.0.0 depends on b==3.0.0, a<2.0.0 | >=3.0.0 depends on b<=1.0.0 | >=3.0.0. (1) + ╰─▶ Because a==1.0.0 depends on b==1.0.0 and there are no versions of a that satisfy any of: + a<1.0.0 + a>1.0.0,<2.0.0 + a>3.0.0 + we can conclude that a<2.0.0 depends on b==1.0.0. + And because a==3.0.0 depends on b==3.0.0 we can conclude that any of: + a<2.0.0 + a>=3.0.0 + depends on one of: + b<=1.0.0 + b>=3.0.0 + (1) - Because there are no versions of c<1.0.0 | >1.0.0, <2.0.0 | >2.0.0 and c==1.0.0 depends on a<2.0.0, c<2.0.0 depends on a<2.0.0. - And because c==2.0.0 depends on a>=3.0.0, c depends on a<2.0.0 | >=3.0.0. - And because a<2.0.0 | >=3.0.0 depends on b<=1.0.0 | >=3.0.0 (1), c depends on b<=1.0.0 | >=3.0.0. - And because root depends on b>=2.0.0, <3.0.0 and root depends on c, version solving failed. + Because there are no versions of c that satisfy any of: + c<1.0.0 + c>1.0.0,<2.0.0 + c>2.0.0 + and c==1.0.0 depends on a<2.0.0, we can conclude that c<2.0.0 depends on a<2.0.0. + And because c==2.0.0 depends on a>=3.0.0 we can conclude that c depends on one of: + a<2.0.0 + a>=3.0.0 + + And because we know from (1) that any of: + a<2.0.0 + a>=3.0.0 + depends on one of: + b<=1.0.0 + b>=3.0.0 + we can conclude that c depends on one of: + b<=1.0.0 + b>=3.0.0 + + And because root depends on b>=2.0.0,<3.0.0 and root depends on c, we can conclude that the requirements are unsatisfiable. "###); }); @@ -521,7 +581,7 @@ fn requires_package_only_prereleases_in_range() -> Result<()> { ----- stderr ----- × No solution found when resolving dependencies: - ╰─▶ Because there are no versions of a>0.1.0 and root depends on a>0.1.0, version solving failed. + ╰─▶ Because there are no versions of a that satisfy a>0.1.0 and root depends on a>0.1.0, we can conclude that the requirements are unsatisfiable. hint: Pre-releases are available for a in the requested range (e.g., 1.0.0a1), but pre-releases weren't enabled (try: `--prerelease=allow`) "###); @@ -1116,8 +1176,11 @@ fn requires_transitive_package_only_prereleases_in_range() -> Result<()> { ----- stderr ----- × No solution found when resolving dependencies: - ╰─▶ Because there are no versions of b>0.1 and a==0.1.0 depends on b>0.1, a==0.1.0 is forbidden. - And because there are no versions of a<0.1.0 | >0.1.0 and root depends on a, version solving failed. + ╰─▶ Because there are no versions of b that satisfy b>0.1 and a==0.1.0 depends on b>0.1, we can conclude that a==0.1.0 is forbidden. + And because there are no versions of a that satisfy any of: + a<0.1.0 + a>0.1.0 + and root depends on a, we can conclude that the requirements are unsatisfiable. hint: Pre-releases are available for b in the requested range (e.g., 1.0.0a1), but pre-releases weren't enabled (try: `--prerelease=allow`) "###); @@ -1271,10 +1334,13 @@ fn requires_transitive_prerelease_and_stable_dependency() -> Result<()> { ----- stderr ----- × No solution found when resolving dependencies: - ╰─▶ Because there is no version of c==2.0.0b1 and a==1.0.0 depends on c==2.0.0b1, a==1.0.0 is forbidden. - And because there are no versions of a<1.0.0 | >1.0.0 and root depends on a, version solving failed. + ╰─▶ Because there is no version of c==2.0.0b1 and a==1.0.0 depends on c==2.0.0b1, we can conclude that a==1.0.0 is forbidden. + And because there are no versions of a that satisfy any of: + a<1.0.0 + a>1.0.0 + and root depends on a, we can conclude that the requirements are unsatisfiable. - hint: c was requested with a pre-release marker (e.g., ==2.0.0b1), but pre-releases weren't enabled (try: `--prerelease=allow`) + hint: c was requested with a pre-release marker (e.g., c==2.0.0b1), but pre-releases weren't enabled (try: `--prerelease=allow`) "###); }); @@ -1468,12 +1534,18 @@ fn requires_transitive_prerelease_and_stable_dependency_many_versions() -> Resul ----- stderr ----- × No solution found when resolving dependencies: - ╰─▶ Because there are no versions of b<1.0.0 | >1.0.0 and b==1.0.0 depends on c, b depends on c. - And because there are no versions of c>=2.0.0b1, b depends on c<2.0.0b1. - And because a==1.0.0 depends on c>=2.0.0b1 and there are no versions of a<1.0.0 | >1.0.0, b*, a* are incompatible. - And because root depends on b and root depends on a, version solving failed. + ╰─▶ Because there are no versions of b that satisfy any of: + b<1.0.0 + b>1.0.0 + and b==1.0.0 depends on c, we can conclude that b depends on c. + And because there are no versions of c that satisfy c>=2.0.0b1 we can conclude that b depends on c<2.0.0b1. + And because a==1.0.0 depends on c>=2.0.0b1 and there are no versions of a that satisfy any of: + a<1.0.0 + a>1.0.0 + we can conclude that b*, a* are incompatible. + And because root depends on b and root depends on a, we can conclude that the requirements are unsatisfiable. - hint: c was requested with a pre-release marker (e.g., >=2.0.0b1), but pre-releases weren't enabled (try: `--prerelease=allow`) + hint: c was requested with a pre-release marker (e.g., c>=2.0.0b1), but pre-releases weren't enabled (try: `--prerelease=allow`) "###); }); @@ -1567,10 +1639,25 @@ fn requires_transitive_prerelease_and_stable_dependency_many_versions_holes() -> ----- stderr ----- × No solution found when resolving dependencies: - ╰─▶ Because there are no versions of c>1.0.0, <2.0.0a5 | >2.0.0a7, <2.0.0b1 | >2.0.0b1, <2.0.0b5 and a==1.0.0 depends on c>1.0.0, <2.0.0a5 | >2.0.0a7, <2.0.0b1 | >2.0.0b1, <2.0.0b5, a==1.0.0 is forbidden. - And because there are no versions of a<1.0.0 | >1.0.0 and root depends on a, version solving failed. + ╰─▶ Because there are no versions of c that satisfy any of: + c>1.0.0,<2.0.0a5 + c>2.0.0a7,<2.0.0b1 + c>2.0.0b1,<2.0.0b5 + and a==1.0.0 depends on one of: + c>1.0.0,<2.0.0a5 + c>2.0.0a7,<2.0.0b1 + c>2.0.0b1,<2.0.0b5 + we can conclude that a==1.0.0 is forbidden. + And because there are no versions of a that satisfy any of: + a<1.0.0 + a>1.0.0 + and root depends on a, we can conclude that the requirements are unsatisfiable. - hint: c was requested with a pre-release marker (e.g., >1.0.0, <2.0.0a5 | >2.0.0a7, <2.0.0b1 | >2.0.0b1, <2.0.0b5), but pre-releases weren't enabled (try: `--prerelease=allow`) + hint: c was requested with a pre-release marker (e.g., any of: + c>1.0.0,<2.0.0a5 + c>2.0.0a7,<2.0.0b1 + c>2.0.0b1,<2.0.0b5 + ), but pre-releases weren't enabled (try: `--prerelease=allow`) "###); }); @@ -1681,7 +1768,7 @@ fn requires_exact_version_does_not_exist() -> Result<()> { ----- stderr ----- × No solution found when resolving dependencies: - ╰─▶ Because there is no version of a==2.0.0 and root depends on a==2.0.0, version solving failed. + ╰─▶ Because there is no version of a==2.0.0 and root depends on a==2.0.0, we can conclude that the requirements are unsatisfiable. "###); }); @@ -1737,7 +1824,7 @@ fn requires_greater_version_does_not_exist() -> Result<()> { ----- stderr ----- × No solution found when resolving dependencies: - ╰─▶ Because there are no versions of a>1.0.0 and root depends on a>1.0.0, version solving failed. + ╰─▶ Because there are no versions of a that satisfy a>1.0.0 and root depends on a>1.0.0, we can conclude that the requirements are unsatisfiable. "###); }); @@ -1794,7 +1881,7 @@ fn requires_less_version_does_not_exist() -> Result<()> { ----- stderr ----- × No solution found when resolving dependencies: - ╰─▶ Because there are no versions of a<2.0.0 and root depends on a<2.0.0, version solving failed. + ╰─▶ Because there are no versions of a that satisfy a<2.0.0 and root depends on a<2.0.0, we can conclude that the requirements are unsatisfiable. "###); }); @@ -1978,8 +2065,11 @@ fn requires_transitive_incompatible_with_root_version() -> Result<()> { ----- stderr ----- × No solution found when resolving dependencies: - ╰─▶ Because there are no versions of a<1.0.0 | >1.0.0 and a==1.0.0 depends on b==2.0.0, a depends on b==2.0.0. - And because root depends on a and root depends on b==1.0.0, version solving failed. + ╰─▶ Because there are no versions of a that satisfy any of: + a<1.0.0 + a>1.0.0 + and a==1.0.0 depends on b==2.0.0, we can conclude that a depends on b==2.0.0. + And because root depends on a and root depends on b==1.0.0, we can conclude that the requirements are unsatisfiable. "###); }); @@ -2054,9 +2144,15 @@ fn requires_transitive_incompatible_with_transitive() -> Result<()> { ----- stderr ----- × No solution found when resolving dependencies: - ╰─▶ Because there are no versions of b<1.0.0 | >1.0.0 and b==1.0.0 depends on c==2.0.0, b depends on c==2.0.0. - And because a==1.0.0 depends on c==1.0.0 and there are no versions of a<1.0.0 | >1.0.0, a*, b* are incompatible. - And because root depends on b and root depends on a, version solving failed. + ╰─▶ Because there are no versions of b that satisfy any of: + b<1.0.0 + b>1.0.0 + and b==1.0.0 depends on c==2.0.0, we can conclude that b depends on c==2.0.0. + And because a==1.0.0 depends on c==1.0.0 and there are no versions of a that satisfy any of: + a<1.0.0 + a>1.0.0 + we can conclude that a*, b* are incompatible. + And because root depends on b and root depends on a, we can conclude that the requirements are unsatisfiable. "###); }); @@ -2116,8 +2212,8 @@ fn requires_python_version_does_not_exist() -> Result<()> { ----- stderr ----- × No solution found when resolving dependencies: - ╰─▶ Because there are no versions of Python>=4.0 and a==1.0.0 depends on Python>=4.0, a==1.0.0 is forbidden. - And because root depends on a==1.0.0, version solving failed. + ╰─▶ Because there are no versions of Python that satisfy Python>=4.0 and a==1.0.0 depends on Python>=4.0, we can conclude that a==1.0.0 is forbidden. + And because root depends on a==1.0.0 we can conclude that the requirements are unsatisfiable. "###); }); @@ -2173,8 +2269,8 @@ fn requires_python_version_less_than_current() -> Result<()> { ----- stderr ----- × No solution found when resolving dependencies: - ╰─▶ Because there are no versions of Python<=3.8 and a==1.0.0 depends on Python<=3.8, a==1.0.0 is forbidden. - And because root depends on a==1.0.0, version solving failed. + ╰─▶ Because there are no versions of Python that satisfy Python<=3.8 and a==1.0.0 depends on Python<=3.8, we can conclude that a==1.0.0 is forbidden. + And because root depends on a==1.0.0 we can conclude that the requirements are unsatisfiable. "###); }); @@ -2233,8 +2329,8 @@ fn requires_python_version_greater_than_current() -> Result<()> { ----- stderr ----- × No solution found when resolving dependencies: - ╰─▶ Because there are no versions of Python>=3.10 and a==1.0.0 depends on Python>=3.10, a==1.0.0 is forbidden. - And because root depends on a==1.0.0, version solving failed. + ╰─▶ Because there are no versions of Python that satisfy Python>=3.10 and a==1.0.0 depends on Python>=3.10, we can conclude that a==1.0.0 is forbidden. + And because root depends on a==1.0.0 we can conclude that the requirements are unsatisfiable. "###); }); @@ -2315,7 +2411,7 @@ fn requires_python_version_greater_than_current_many() -> Result<()> { ----- stderr ----- × No solution found when resolving dependencies: - ╰─▶ Because there is no version of a==1.0.0 and root depends on a==1.0.0, version solving failed. + ╰─▶ Because there is no version of a==1.0.0 and root depends on a==1.0.0, we can conclude that the requirements are unsatisfiable. "###); }); @@ -2451,17 +2547,24 @@ fn requires_python_version_greater_than_current_excluded() -> Result<()> { ----- stderr ----- × No solution found when resolving dependencies: - ╰─▶ Because there are no versions of Python>=3.10, <3.11 and there are no versions of Python>=3.12, Python>=3.10, <3.11 | >=3.12 are incompatible. - And because there are no versions of Python>=3.11, <3.12, Python>=3.10 are incompatible. - And because a==2.0.0 depends on Python>=3.10 and there are no versions of a>2.0.0, <3.0.0 | >3.0.0, <4.0.0 | >4.0.0, a>=2.0.0, <3.0.0 is forbidden. (1) + ╰─▶ Because there are no versions of Python that satisfy Python>=3.10,<3.11 and there are no versions of Python that satisfy Python>=3.12, we can conclude that any of: + Python>=3.10,<3.11 + Python>=3.12 + are incompatible. + And because there are no versions of Python that satisfy Python>=3.11,<3.12 we can conclude that Python>=3.10 are incompatible. + And because a==2.0.0 depends on Python>=3.10 and there are no versions of a that satisfy any of: + a>2.0.0,<3.0.0 + a>3.0.0,<4.0.0 + a>4.0.0 + we can conclude that a>=2.0.0,<3.0.0 is forbidden. (1) - Because there are no versions of Python>=3.11, <3.12 and there are no versions of Python>=3.12, Python>=3.11 are incompatible. - And because a==3.0.0 depends on Python>=3.11, a==3.0.0 is forbidden. - And because a>=2.0.0, <3.0.0 is forbidden (1), a>=2.0.0, <4.0.0 is forbidden. (2) + Because there are no versions of Python that satisfy Python>=3.11,<3.12 and there are no versions of Python that satisfy Python>=3.12, we can conclude that Python>=3.11 are incompatible. + And because a==3.0.0 depends on Python>=3.11 we can conclude that a==3.0.0 is forbidden. + And because we know from (1) that a>=2.0.0,<3.0.0 is forbidden, we can conclude that a>=2.0.0,<4.0.0 is forbidden. (2) - Because there are no versions of Python>=3.12 and a==4.0.0 depends on Python>=3.12, a==4.0.0 is forbidden. - And because a>=2.0.0, <4.0.0 is forbidden (2), a>=2.0.0 is forbidden. - And because root depends on a>=2.0.0, version solving failed. + Because there are no versions of Python that satisfy Python>=3.12 and a==4.0.0 depends on Python>=3.12, we can conclude that a==4.0.0 is forbidden. + And because we know from (2) that a>=2.0.0,<4.0.0 is forbidden, we can conclude that a>=2.0.0 is forbidden. + And because root depends on a>=2.0.0 we can conclude that the requirements are unsatisfiable. "###); }); diff --git a/crates/puffin-resolver/src/pubgrub/report.rs b/crates/puffin-resolver/src/pubgrub/report.rs index 2ae80e207..a708c099f 100644 --- a/crates/puffin-resolver/src/pubgrub/report.rs +++ b/crates/puffin-resolver/src/pubgrub/report.rs @@ -1,9 +1,10 @@ use std::borrow::Cow; +use std::ops::Bound; use derivative::Derivative; use owo_colors::OwoColorize; use pubgrub::range::Range; -use pubgrub::report::{DerivationTree, External, ReportFormatter}; +use pubgrub::report::{DerivationTree, Derived, External, ReportFormatter}; use pubgrub::term::Term; use pubgrub::type_aliases::Map; use rustc_hash::{FxHashMap, FxHashSet}; @@ -37,7 +38,11 @@ impl ReportFormatter> for PubGrubReportFor } else if set.as_singleton().is_some() { format!("there is no version of {package}{set}") } else { - format!("there are no versions of {package}{set}") + format!( + "there are no versions of {} that satisfy {}", + package, + PackageRange::requires(package, &set) + ) } } External::UnavailableDependencies(package, set) => { @@ -45,7 +50,10 @@ impl ReportFormatter> for PubGrubReportFor if set.as_ref() == &Range::full() { format!("dependencies of {package} are unavailable") } else { - format!("dependencies of {package}{set} are unavailable") + format!( + "dependencies of {}are unavailable", + Padded::new("", &PackageRange::requires(package, &set), " ") + ) } } External::UnusableDependencies(package, set, reason) => { @@ -57,7 +65,10 @@ impl ReportFormatter> for PubGrubReportFor if set.as_ref() == &Range::full() { format!("dependencies of {package} are unusable: {reason}") } else { - format!("dependencies of {package}{set} are unusable: {reason}",) + format!( + "dependencies of {}are unusable: {reason}", + Padded::new("", &PackageRange::requires(package, &set), " ") + ) } } } else { @@ -65,7 +76,10 @@ impl ReportFormatter> for PubGrubReportFor if set.as_ref() == &Range::full() { format!("dependencies of {package} are unusable") } else { - format!("dependencies of {package}{set} are unusable") + format!( + "dependencies of {}are unusable", + Padded::new("", &PackageRange::requires(package, &set), " ") + ) } } } @@ -77,20 +91,33 @@ impl ReportFormatter> for PubGrubReportFor { format!("{package} depends on {dependency}") } else if package_set.as_ref() == &Range::full() { - format!("{package} depends on {dependency}{dependency_set}") + format!( + "{package} depends on {}", + PackageRange::depends(dependency, &dependency_set) + ) } else if dependency_set.as_ref() == &Range::full() { if matches!(package, PubGrubPackage::Root(_)) { // Exclude the dummy version for root packages format!("{package} depends on {dependency}") } else { - format!("{package}{package_set} depends on {dependency}") + format!( + "{}depends on {dependency}", + Padded::new("", &PackageRange::requires(package, &package_set), " ") + ) } } else { if matches!(package, PubGrubPackage::Root(_)) { // Exclude the dummy version for root packages - format!("{package} depends on {dependency}{dependency_set}") + format!( + "{package} depends on {}", + PackageRange::depends(dependency, &dependency_set) + ) } else { - format!("{package}{package_set} depends on {dependency}{dependency_set}") + format!( + "{}depends on {}", + Padded::new("", &PackageRange::requires(package, &package_set), " "), + PackageRange::depends(dependency, &dependency_set) + ) } } } @@ -101,7 +128,7 @@ impl ReportFormatter> for PubGrubReportFor fn format_terms(&self, terms: &Map>>) -> String { let terms_vec: Vec<_> = terms.iter().collect(); match terms_vec.as_slice() { - [] | [(PubGrubPackage::Root(_), _)] => "version solving failed".into(), + [] | [(PubGrubPackage::Root(_), _)] => "the requirements are unsatisfiable".into(), [(package @ PubGrubPackage::Package(..), Term::Positive(range))] => { let range = range.simplify( self.available_versions @@ -109,7 +136,7 @@ impl ReportFormatter> for PubGrubReportFor .unwrap_or(&vec![]) .iter(), ); - format!("{package}{range} is forbidden") + format!("{} is forbidden", PackageRange::requires(package, &range)) } [(package @ PubGrubPackage::Package(..), Term::Negative(range))] => { let range = range.simplify( @@ -118,7 +145,7 @@ impl ReportFormatter> for PubGrubReportFor .unwrap_or(&vec![]) .iter(), ); - format!("{package}{range} is mandatory") + format!("{} is mandatory", PackageRange::requires(package, &range)) } [(p1, Term::Positive(r1)), (p2, Term::Negative(r2))] => self.format_external( &External::FromDependencyOf((*p1).clone(), r1.clone(), (*p2).clone(), r2.clone()), @@ -129,12 +156,134 @@ impl ReportFormatter> for PubGrubReportFor slice => { let str_terms: Vec<_> = slice .iter() - .map(|(p, t)| format!("{p}{}", PubGrubTerm::from_term((*t).clone()))) + .map(|(p, t)| format!("{}", PackageTerm::new(p, t))) .collect(); str_terms.join(", ") + " are incompatible" } } } + + /// Simplest case, we just combine two external incompatibilities. + fn explain_both_external( + &self, + external1: &External>, + external2: &External>, + current_terms: &Map>>, + ) -> String { + let external1 = self.format_external(external1); + let external2 = self.format_external(external2); + let terms = self.format_terms(current_terms); + + format!( + "Because {}and {}we can conclude that {}", + Padded::from_string("", &external1, " "), + Padded::from_string("", &external2, ", "), + Padded::from_string("", &terms, ".") + ) + } + + /// Both causes have already been explained so we use their refs. + fn explain_both_ref( + &self, + ref_id1: usize, + derived1: &Derived>, + ref_id2: usize, + derived2: &Derived>, + current_terms: &Map>>, + ) -> String { + // TODO: order should be chosen to make it more logical. + + let derived1_terms = self.format_terms(&derived1.terms); + let derived2_terms = self.format_terms(&derived2.terms); + let current_terms = self.format_terms(current_terms); + + format!( + "Because we know from ({}) that {}and we know from ({}) that {}{}", + ref_id1, + Padded::new("", &derived1_terms, " "), + ref_id2, + Padded::new("", &derived2_terms, ", "), + Padded::new("", ¤t_terms, "."), + ) + } + + /// One cause is derived (already explained so one-line), + /// the other is a one-line external cause, + /// and finally we conclude with the current incompatibility. + fn explain_ref_and_external( + &self, + ref_id: usize, + derived: &Derived>, + external: &External>, + current_terms: &Map>>, + ) -> String { + // TODO: order should be chosen to make it more logical. + + let derived_terms = self.format_terms(&derived.terms); + let external = self.format_external(external); + let current_terms = self.format_terms(current_terms); + + format!( + "Because we know from ({}) that {}and {}we can conclude that {}", + ref_id, + Padded::new("", &derived_terms, " "), + Padded::new("", &external, ", "), + Padded::new("", ¤t_terms, "."), + ) + } + + /// Add an external cause to the chain of explanations. + fn and_explain_external( + &self, + external: &External>, + current_terms: &Map>>, + ) -> String { + let external = self.format_external(external); + let terms = self.format_terms(current_terms); + + format!( + "And because {}we can conclude that {}", + Padded::from_string("", &external, " "), + Padded::from_string("", &terms, "."), + ) + } + + /// Add an already explained incompat to the chain of explanations. + fn and_explain_ref( + &self, + ref_id: usize, + derived: &Derived>, + current_terms: &Map>>, + ) -> String { + let derived = self.format_terms(&derived.terms); + let current = self.format_terms(current_terms); + + format!( + "And because we know from ({}) that {}we can conclude that {}", + ref_id, + Padded::from_string("", &derived, ", "), + Padded::from_string("", ¤t, "."), + ) + } + + /// Add an already explained incompat to the chain of explanations. + fn and_explain_prior_and_external( + &self, + prior_external: &External>, + external: &External>, + current_terms: &Map>>, + ) -> String { + let prior_external = self.format_external(prior_external); + let external = self.format_external(external); + let terms = self.format_terms(current_terms); + + format!( + "And because {}and {}we can conclude that {}", + Padded::from_string("", &prior_external, " "), + Padded::from_string("", &external, ", "), + Padded::from_string("", &terms, "."), + ) + } } impl PubGrubReportFormatter<'_> { @@ -267,35 +416,174 @@ impl std::fmt::Display for PubGrubHint { "hint".bold().cyan(), ":".bold(), package.bold(), - range.bold() + PackageRange::requires(package, range).bold() ) } } } } -/// A derivative of the [Term] type with custom formatting. -struct PubGrubTerm { - inner: Term>, +/// A [`Term`] and [`PubGrubPackage`] combination for display. +struct PackageTerm<'a> { + package: &'a PubGrubPackage, + term: &'a Term>, } -impl std::fmt::Display for PubGrubTerm { +impl std::fmt::Display for PackageTerm<'_> { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match &self.inner { - Term::Positive(set) => write!(f, "{set}"), + match &self.term { + Term::Positive(set) => write!(f, "{}", PackageRange::requires(self.package, set)), Term::Negative(set) => { if let Some(version) = set.as_singleton() { - write!(f, "!={version}") + let package = self.package; + write!(f, "{package}!={version}") } else { - write!(f, "!( {set} )") + write!(f, "!( {} )", PackageRange::requires(self.package, set)) } } } } } -impl PubGrubTerm { - fn from_term(term: Term>) -> PubGrubTerm { - PubGrubTerm { inner: term } +impl PackageTerm<'_> { + fn new<'a>( + package: &'a PubGrubPackage, + term: &'a Term>, + ) -> PackageTerm<'a> { + PackageTerm { package, term } + } +} + +/// The kind of version ranges being displayed in [`PackageRange`] +#[derive(Debug)] +enum PackageRangeKind { + Depends, + Requires, +} + +/// A [`Range`] and [`PubGrubPackage`] combination for display. +#[derive(Debug)] +struct PackageRange<'a> { + package: &'a PubGrubPackage, + range: &'a Range, + kind: PackageRangeKind, +} + +impl std::fmt::Display for PackageRange<'_> { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + if self.range.is_empty() { + write!(f, "∅")?; + } else { + let segments: Vec<_> = self.range.iter().collect(); + if segments.len() > 1 { + match self.kind { + PackageRangeKind::Depends => write!(f, "one of:")?, + PackageRangeKind::Requires => write!(f, "any of:")?, + } + } + for segment in &segments { + if segments.len() > 1 { + write!(f, "\n ")?; + } + write!(f, "{}", self.package)?; + match segment { + (Bound::Unbounded, Bound::Unbounded) => write!(f, "*")?, + (Bound::Unbounded, Bound::Included(v)) => write!(f, "<={v}")?, + (Bound::Unbounded, Bound::Excluded(v)) => write!(f, "<{v}")?, + (Bound::Included(v), Bound::Unbounded) => write!(f, ">={v}")?, + (Bound::Included(v), Bound::Included(b)) => { + if v == b { + write!(f, "=={v}")?; + } else { + write!(f, ">={v},<={b}")?; + } + } + (Bound::Included(v), Bound::Excluded(b)) => write!(f, ">={v},<{b}")?, + (Bound::Excluded(v), Bound::Unbounded) => write!(f, ">{v}")?, + (Bound::Excluded(v), Bound::Included(b)) => write!(f, ">{v},<={b}")?, + (Bound::Excluded(v), Bound::Excluded(b)) => write!(f, ">{v},<{b}")?, + }; + } + if segments.len() > 1 { + writeln!(f)?; + } + } + Ok(()) + } +} + +impl PackageRange<'_> { + fn requires<'a>( + package: &'a PubGrubPackage, + range: &'a Range, + ) -> PackageRange<'a> { + PackageRange { + package, + range, + kind: PackageRangeKind::Requires, + } + } + + fn depends<'a>( + package: &'a PubGrubPackage, + range: &'a Range, + ) -> PackageRange<'a> { + PackageRange { + package, + range, + kind: PackageRangeKind::Depends, + } + } +} + +/// Inserts the given padding on the left and right sides of the content if +/// the content does not start and end with whitespace respectively. +#[derive(Debug)] +struct Padded<'a, T: std::fmt::Display> { + left: &'a str, + content: &'a T, + right: &'a str, +} + +impl<'a, T: std::fmt::Display> Padded<'a, T> { + fn new(left: &'a str, content: &'a T, right: &'a str) -> Self { + Padded { + left, + content, + right, + } + } +} + +impl<'a> Padded<'a, String> { + fn from_string(left: &'a str, content: &'a String, right: &'a str) -> Self { + Padded { + left, + content, + right, + } + } +} + +impl std::fmt::Display for Padded<'_, T> { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + let mut result = String::new(); + let content = self.content.to_string(); + + if let Some(char) = content.chars().next() { + if !char.is_whitespace() { + result.push_str(self.left); + } + } + + result.push_str(&content); + + if let Some(char) = content.chars().last() { + if !char.is_whitespace() { + result.push_str(self.right); + } + } + + write!(f, "{result}") } } diff --git a/crates/puffin-resolver/tests/resolver.rs b/crates/puffin-resolver/tests/resolver.rs index 74ffdf450..52db13acf 100644 --- a/crates/puffin-resolver/tests/resolver.rs +++ b/crates/puffin-resolver/tests/resolver.rs @@ -502,7 +502,7 @@ async fn black_disallow_prerelease() -> Result<()> { .unwrap_err(); assert_snapshot!(err, @r###" - Because there are no versions of black<=20.0 and root depends on black<=20.0, version solving failed. + Because there are no versions of black that satisfy black<=20.0 and root depends on black<=20.0, we can conclude that the requirements are unsatisfiable. hint: Pre-releases are available for black in the requested range (e.g., 19.10b0), but pre-releases weren't enabled (try: `--prerelease=allow`) "###); @@ -524,7 +524,7 @@ async fn black_allow_prerelease_if_necessary() -> Result<()> { .unwrap_err(); assert_snapshot!(err, @r###" - Because there are no versions of black<=20.0 and root depends on black<=20.0, version solving failed. + Because there are no versions of black that satisfy black<=20.0 and root depends on black<=20.0, we can conclude that the requirements are unsatisfiable. hint: Pre-releases are available for black in the requested range (e.g., 19.10b0), but pre-releases weren't enabled (try: `--prerelease=allow`) "###); @@ -650,10 +650,10 @@ async fn msgraph_sdk() -> Result<()> { .unwrap_err(); assert_snapshot!(err, @r###" - Because there are no versions of msgraph-core>=1.0.0a2 and msgraph-sdk==1.0.0 depends on msgraph-core>=1.0.0a2, msgraph-sdk==1.0.0 is forbidden. - And because root depends on msgraph-sdk==1.0.0, version solving failed. + Because there are no versions of msgraph-core that satisfy msgraph-core>=1.0.0a2 and msgraph-sdk==1.0.0 depends on msgraph-core>=1.0.0a2, we can conclude that msgraph-sdk==1.0.0 is forbidden. + And because root depends on msgraph-sdk==1.0.0 we can conclude that the requirements are unsatisfiable. - hint: msgraph-core was requested with a pre-release marker (e.g., >=1.0.0a2), but pre-releases weren't enabled (try: `--prerelease=allow`) + hint: msgraph-core was requested with a pre-release marker (e.g., msgraph-core>=1.0.0a2), but pre-releases weren't enabled (try: `--prerelease=allow`) "###); Ok(())