From 7301a350f9696e0b1acb6b29325e19d2b34b9cc9 Mon Sep 17 00:00:00 2001 From: Zanie Date: Mon, 6 Nov 2023 14:12:04 -0600 Subject: [PATCH] Improve error messages for incompatible, direct requirements --- crates/puffin-cli/tests/pip_compile.rs | 146 +++++++++++++++++- ..._multiple_direct_versions_for_package.snap | 28 ++++ ...ble_requirements_direct_mixed_ranges.snap} | 9 +- ...unsolvable_requirements_direct_pinned.snap | 19 +++ ...unsolvable_requirements_direct_ranges.snap | 19 +++ crates/puffin-resolver/src/error.rs | 3 + .../src/pubgrub/dependencies.rs | 19 ++- vendor/pubgrub/src/range.rs | 4 + 8 files changed, 231 insertions(+), 16 deletions(-) create mode 100644 crates/puffin-cli/tests/snapshots/pip_compile__compile_solvable_requirements_multiple_direct_versions_for_package.snap rename crates/puffin-cli/tests/snapshots/{pip_compile__compile_unsolvable_requirements.snap => pip_compile__compile_unsolvable_requirements_direct_mixed_ranges.snap} (58%) create mode 100644 crates/puffin-cli/tests/snapshots/pip_compile__compile_unsolvable_requirements_direct_pinned.snap create mode 100644 crates/puffin-cli/tests/snapshots/pip_compile__compile_unsolvable_requirements_direct_ranges.snap diff --git a/crates/puffin-cli/tests/pip_compile.rs b/crates/puffin-cli/tests/pip_compile.rs index e2ee827892e0..2eace24a9a39 100644 --- a/crates/puffin-cli/tests/pip_compile.rs +++ b/crates/puffin-cli/tests/pip_compile.rs @@ -1432,9 +1432,10 @@ optional-dependencies.bar = [ Ok(()) } -/// Compile requirements that cannot be solved due to conflict in a `pyproject.toml` fil;e. +/// Compile requirements that cannot be solved due to conflict in a `pyproject.toml` file +/// where two direct requirements of a package have incompatible pinned versions. #[test] -fn compile_unsolvable_requirements() -> Result<()> { +fn compile_unsolvable_requirements_direct_pinned() -> Result<()> { let temp_dir = assert_fs::TempDir::new()?; let cache_dir = assert_fs::TempDir::new()?; let venv = temp_dir.child(".venv"); @@ -1462,11 +1463,142 @@ dependencies = ["django==5.0b1", "django==5.0a1"] )?; insta::with_settings!({ - filters => vec![ - (r"\d(ms|s)", "[TIME]"), - (r"# .* pip-compile", "# [BIN_PATH] pip-compile"), - (r"--cache-dir .*", "--cache-dir [CACHE_DIR]"), - ] + filters => INSTA_FILTERS.to_vec() + }, { + assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME)) + .arg("pip-compile") + .arg("pyproject.toml") + .arg("--cache-dir") + .arg(cache_dir.path()) + .env("VIRTUAL_ENV", venv.as_os_str()) + .current_dir(&temp_dir)); + }); + + Ok(()) +} + +/// Compile requirements that cannot be solved due to conflict in a `pyproject.toml` file +/// where two direct requirements of a package have incompatible version ranges. +#[test] +fn compile_unsolvable_requirements_direct_ranges() -> Result<()> { + let temp_dir = assert_fs::TempDir::new()?; + let cache_dir = assert_fs::TempDir::new()?; + let venv = temp_dir.child(".venv"); + + Command::new(get_cargo_bin(BIN_NAME)) + .arg("venv") + .arg(venv.as_os_str()) + .arg("--cache-dir") + .arg(cache_dir.path()) + .current_dir(&temp_dir) + .assert() + .success(); + venv.assert(predicates::path::is_dir()); + + let pyproject_toml = temp_dir.child("pyproject.toml"); + pyproject_toml.touch()?; + pyproject_toml.write_str( + r#"[build-system] +requires = ["setuptools", "wheel"] + +[project] +name = "my-project" +dependencies = ["django>=4.0", "django<=3.0"] +"#, + )?; + + insta::with_settings!({ + filters => INSTA_FILTERS.to_vec() + }, { + assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME)) + .arg("pip-compile") + .arg("pyproject.toml") + .arg("--cache-dir") + .arg(cache_dir.path()) + .env("VIRTUAL_ENV", venv.as_os_str()) + .current_dir(&temp_dir)); + }); + + Ok(()) +} + +/// Compile requirements that cannot be solved due to conflict in a `pyproject.toml` file +/// where direct requirements of a package have a mix of compatible and incompatible version ranges. +#[test] +fn compile_unsolvable_requirements_direct_mixed_ranges() -> Result<()> { + let temp_dir = assert_fs::TempDir::new()?; + let cache_dir = assert_fs::TempDir::new()?; + let venv = temp_dir.child(".venv"); + + Command::new(get_cargo_bin(BIN_NAME)) + .arg("venv") + .arg(venv.as_os_str()) + .arg("--cache-dir") + .arg(cache_dir.path()) + .current_dir(&temp_dir) + .assert() + .success(); + venv.assert(predicates::path::is_dir()); + + let pyproject_toml = temp_dir.child("pyproject.toml"); + pyproject_toml.touch()?; + pyproject_toml.write_str( + r#"[build-system] +requires = ["setuptools", "wheel"] + +[project] +name = "my-project" +dependencies = ["django>=3.0", "django<=3.0", "django>5.0"] +"#, + )?; + + insta::with_settings!({ + filters => INSTA_FILTERS.to_vec() + }, { + assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME)) + .arg("pip-compile") + .arg("pyproject.toml") + .arg("--cache-dir") + .arg(cache_dir.path()) + .env("VIRTUAL_ENV", venv.as_os_str()) + .current_dir(&temp_dir)); + }); + + Ok(()) +} + +/// Compile requirements in a `pyproject.toml` file where there are multiple direct requirements of a package +/// but the versions are compatible. +#[test] +fn compile_solvable_requirements_multiple_direct_versions_for_package() -> Result<()> { + let temp_dir = assert_fs::TempDir::new()?; + let cache_dir = assert_fs::TempDir::new()?; + let venv = temp_dir.child(".venv"); + + Command::new(get_cargo_bin(BIN_NAME)) + .arg("venv") + .arg(venv.as_os_str()) + .arg("--cache-dir") + .arg(cache_dir.path()) + .current_dir(&temp_dir) + .assert() + .success(); + venv.assert(predicates::path::is_dir()); + + let pyproject_toml = temp_dir.child("pyproject.toml"); + pyproject_toml.touch()?; + pyproject_toml.write_str( + r#"[build-system] +requires = ["setuptools", "wheel"] + +[project] +name = "my-project" +dependencies = ["django>=3.0", "django<=3.0", "django==3.0"] +"#, + )?; + + insta::with_settings!({ + filters => INSTA_FILTERS.to_vec() }, { assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME)) .arg("pip-compile") diff --git a/crates/puffin-cli/tests/snapshots/pip_compile__compile_solvable_requirements_multiple_direct_versions_for_package.snap b/crates/puffin-cli/tests/snapshots/pip_compile__compile_solvable_requirements_multiple_direct_versions_for_package.snap new file mode 100644 index 000000000000..b397e5e8dc40 --- /dev/null +++ b/crates/puffin-cli/tests/snapshots/pip_compile__compile_solvable_requirements_multiple_direct_versions_for_package.snap @@ -0,0 +1,28 @@ +--- +source: crates/puffin-cli/tests/pip_compile.rs +info: + program: puffin + args: + - pip-compile + - pyproject.toml + - "--cache-dir" + - /var/folders/bc/qlsk3t6x7c9fhhbvvcg68k9c0000gp/T/.tmpcfI6sl + env: + VIRTUAL_ENV: /var/folders/bc/qlsk3t6x7c9fhhbvvcg68k9c0000gp/T/.tmpv4PQoW/.venv +--- +success: true +exit_code: 0 +----- stdout ----- +# This file was autogenerated by Puffin v0.0.1 via the following command: +# [BIN_PATH] pip-compile pyproject.toml --cache-dir [CACHE_DIR] +asgiref==3.7.2 + # via django +django==3.0 +pytz==2023.3.post1 + # via django +sqlparse==0.4.4 + # via django + +----- stderr ----- +Resolved 4 packages in [TIME] + diff --git a/crates/puffin-cli/tests/snapshots/pip_compile__compile_unsolvable_requirements.snap b/crates/puffin-cli/tests/snapshots/pip_compile__compile_unsolvable_requirements_direct_mixed_ranges.snap similarity index 58% rename from crates/puffin-cli/tests/snapshots/pip_compile__compile_unsolvable_requirements.snap rename to crates/puffin-cli/tests/snapshots/pip_compile__compile_unsolvable_requirements_direct_mixed_ranges.snap index 61a6e7f1e855..4e6ec7bad2d0 100644 --- a/crates/puffin-cli/tests/snapshots/pip_compile__compile_unsolvable_requirements.snap +++ b/crates/puffin-cli/tests/snapshots/pip_compile__compile_unsolvable_requirements_direct_mixed_ranges.snap @@ -6,15 +6,14 @@ info: - pip-compile - pyproject.toml - "--cache-dir" - - /var/folders/bc/qlsk3t6x7c9fhhbvvcg68k9c0000gp/T/.tmpN531dN + - /var/folders/bc/qlsk3t6x7c9fhhbvvcg68k9c0000gp/T/.tmpHT5k4n env: - VIRTUAL_ENV: /var/folders/bc/qlsk3t6x7c9fhhbvvcg68k9c0000gp/T/.tmp99w9dK/.venv + VIRTUAL_ENV: /var/folders/bc/qlsk3t6x7c9fhhbvvcg68k9c0000gp/T/.tmpS0cCaw/.venv --- success: false -exit_code: 1 +exit_code: 2 ----- stdout ----- ----- stderr ----- - × No solution found when resolving dependencies: - ╰─▶ my-project depends on django∅ +error: Conflicting versions for package `django`: ==3.0 is incompatible with >5.0 diff --git a/crates/puffin-cli/tests/snapshots/pip_compile__compile_unsolvable_requirements_direct_pinned.snap b/crates/puffin-cli/tests/snapshots/pip_compile__compile_unsolvable_requirements_direct_pinned.snap new file mode 100644 index 000000000000..6adb6aa47a9d --- /dev/null +++ b/crates/puffin-cli/tests/snapshots/pip_compile__compile_unsolvable_requirements_direct_pinned.snap @@ -0,0 +1,19 @@ +--- +source: crates/puffin-cli/tests/pip_compile.rs +info: + program: puffin + args: + - pip-compile + - pyproject.toml + - "--cache-dir" + - /var/folders/bc/qlsk3t6x7c9fhhbvvcg68k9c0000gp/T/.tmpgsozkO + env: + VIRTUAL_ENV: /var/folders/bc/qlsk3t6x7c9fhhbvvcg68k9c0000gp/T/.tmpwjg05J/.venv +--- +success: false +exit_code: 2 +----- stdout ----- + +----- stderr ----- +error: Conflicting versions for package `django`: ==5.0b1 is incompatible with ==5.0a1 + diff --git a/crates/puffin-cli/tests/snapshots/pip_compile__compile_unsolvable_requirements_direct_ranges.snap b/crates/puffin-cli/tests/snapshots/pip_compile__compile_unsolvable_requirements_direct_ranges.snap new file mode 100644 index 000000000000..70907d135370 --- /dev/null +++ b/crates/puffin-cli/tests/snapshots/pip_compile__compile_unsolvable_requirements_direct_ranges.snap @@ -0,0 +1,19 @@ +--- +source: crates/puffin-cli/tests/pip_compile.rs +info: + program: puffin + args: + - pip-compile + - pyproject.toml + - "--cache-dir" + - /var/folders/bc/qlsk3t6x7c9fhhbvvcg68k9c0000gp/T/.tmpQZyk8c + env: + VIRTUAL_ENV: /var/folders/bc/qlsk3t6x7c9fhhbvvcg68k9c0000gp/T/.tmpOV0S3e/.venv +--- +success: false +exit_code: 2 +----- stdout ----- + +----- stderr ----- +error: Conflicting versions for package `django`: >=4.0 is incompatible with <=3.0 + diff --git a/crates/puffin-resolver/src/error.rs b/crates/puffin-resolver/src/error.rs index 85eae54c5299..887283d7f445 100644 --- a/crates/puffin-resolver/src/error.rs +++ b/crates/puffin-resolver/src/error.rs @@ -39,6 +39,9 @@ pub enum ResolveError { #[error("Conflicting URLs for package `{0}`: {1} and {2}")] ConflictingUrls(PackageName, String, String), + #[error("Conflicting versions for package `{0}`: {1} is incompatible with {2}")] + ConflictingPackageVersions(PackageName, Range, Range), + #[error("Package `{0}` attempted to resolve via URL: {1}. URL dependencies must be expressed as direct requirements or constraints. Consider adding `{0} @ {1}` to your dependencies or constraints file.")] DisallowedUrl(PackageName, Url), diff --git a/crates/puffin-resolver/src/pubgrub/dependencies.rs b/crates/puffin-resolver/src/pubgrub/dependencies.rs index f59247384958..8f1818b4d7e5 100644 --- a/crates/puffin-resolver/src/pubgrub/dependencies.rs +++ b/crates/puffin-resolver/src/pubgrub/dependencies.rs @@ -59,7 +59,7 @@ impl PubGrubDependencies { if let Some(entry) = dependencies.get_key_value(&package) { // Merge the versions. - let version = merge_versions(entry.1, &version); + let version = merge_versions(&requirement.name, entry.1, &version)?; // Merge the package. if let Some(package) = merge_package(entry.0, &package)? { @@ -107,7 +107,7 @@ impl PubGrubDependencies { if let Some(entry) = dependencies.get_key_value(&package) { // Merge the versions. - let version = merge_versions(entry.1, &version); + let version = merge_versions(&constraint.name, entry.1, &version)?; // Merge the package. if let Some(package) = merge_package(entry.0, &package)? { @@ -178,10 +178,21 @@ fn to_pubgrub( /// Merge two [`PubGrubVersion`] ranges. fn merge_versions( + package_name: &PackageName, left: &Range, right: &Range, -) -> Range { - left.intersection(right) +) -> Result, ResolveError> { + let version = left.intersection(right); + + if version.is_empty() { + Err(ResolveError::ConflictingPackageVersions( + package_name.clone(), + left.clone(), + right.clone(), + )) + } else { + Ok(version) + } } /// Merge two [`PubGrubPackage`] instances. diff --git a/vendor/pubgrub/src/range.rs b/vendor/pubgrub/src/range.rs index 47e76e44f52b..ef8d8533403a 100644 --- a/vendor/pubgrub/src/range.rs +++ b/vendor/pubgrub/src/range.rs @@ -117,6 +117,10 @@ impl Range { segments: SmallVec::one((Included(v1.into()), Excluded(v2.into()))), } } + + pub fn is_empty(&self) -> bool { + return self.segments.is_empty(); + } } impl Range {