Skip to content

Commit

Permalink
Improve error messages for incompatible, direct requirements
Browse files Browse the repository at this point in the history
  • Loading branch information
zanieb committed Nov 6, 2023
1 parent e952557 commit 7301a35
Show file tree
Hide file tree
Showing 8 changed files with 231 additions and 16 deletions.
146 changes: 139 additions & 7 deletions crates/puffin-cli/tests/pip_compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down Expand Up @@ -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")
Expand Down
Original file line number Diff line number Diff line change
@@ -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]

Original file line number Diff line number Diff line change
Expand Up @@ -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

Original file line number Diff line number Diff line change
@@ -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

Original file line number Diff line number Diff line change
@@ -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

3 changes: 3 additions & 0 deletions crates/puffin-resolver/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<PubGrubVersion>, Range<PubGrubVersion>),

#[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),

Expand Down
19 changes: 15 additions & 4 deletions crates/puffin-resolver/src/pubgrub/dependencies.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)? {
Expand Down Expand Up @@ -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)? {
Expand Down Expand Up @@ -178,10 +178,21 @@ fn to_pubgrub(

/// Merge two [`PubGrubVersion`] ranges.
fn merge_versions(
package_name: &PackageName,
left: &Range<PubGrubVersion>,
right: &Range<PubGrubVersion>,
) -> Range<PubGrubVersion> {
left.intersection(right)
) -> Result<Range<PubGrubVersion>, 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.
Expand Down
4 changes: 4 additions & 0 deletions vendor/pubgrub/src/range.rs
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,10 @@ impl<V> Range<V> {
segments: SmallVec::one((Included(v1.into()), Excluded(v2.into()))),
}
}

pub fn is_empty(&self) -> bool {
return self.segments.is_empty();
}
}

impl<V: Clone> Range<V> {
Expand Down

0 comments on commit 7301a35

Please sign in to comment.