Skip to content

Commit

Permalink
fix: editable non-satisfiable (#1251)
Browse files Browse the repository at this point in the history
Fixes an issue where multiple requirements of editable/non-editable
would conflict and cause the lock-file to be non satisfiable. However,
the logic should be that if just one requirement specifies an editable
install then it should be editable regardless of what other requirements
specify.

You also get a nice error message:

```
expected bar, and foo to be editable but in the lock-file they are not, whereas baz is NOT expected to be editable which in the lock-file it is
```

Note that I also included several tests that test several cases.
  • Loading branch information
baszalmstra authored Apr 22, 2024
1 parent b283011 commit d3d251b
Show file tree
Hide file tree
Showing 15 changed files with 1,092 additions and 18 deletions.
127 changes: 109 additions & 18 deletions src/lock_file/satisfiability.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ use rattler_conda_types::{
};
use rattler_lock::{ConversionError, Package, PypiPackageData, PypiSourceTreeHashable, UrlOrPath};
use requirements_txt::EditableRequirement;
use std::fmt::Display;
use std::ops::Sub;
use std::path::{Path, PathBuf};
use std::str::FromStr;
use std::{
Expand All @@ -29,6 +31,12 @@ pub enum EnvironmentUnsat {
ChannelsMismatch,
}

#[derive(Debug, Error)]
pub struct EditablePackagesMismatch {
pub expected_editable: Vec<PackageName>,
pub unexpected_editable: Vec<PackageName>,
}

#[derive(Debug, Error, Diagnostic)]
pub enum PlatformUnsat {
#[error("the requirement '{0}' could not be satisfied (required by '{1}')")]
Expand Down Expand Up @@ -78,11 +86,8 @@ pub enum PlatformUnsat {
#[error("direct pypi url dependency to a conda installed package '{0}' is not supported")]
DirectUrlDependencyOnCondaInstalledPackage(PackageName),

#[error("locked package {0} should be editable")]
ExpectedEditablePackage(PackageName),

#[error("locked package {0} should not be editable")]
UnexpectedEditablePackage(PackageName),
#[error(transparent)]
EditablePackageMismatch(EditablePackagesMismatch),

#[error("failed to determine pypi source tree hash for {0}")]
FailedToDetermineSourceTreeHash(PackageName, std::io::Error),
Expand All @@ -105,7 +110,7 @@ impl PlatformUnsat {
| PlatformUnsat::AsPep508Error(_, _)
| PlatformUnsat::FailedToDetermineSourceTreeHash(_, _)
| PlatformUnsat::PythonVersionMismatch(_, _, _)
| PlatformUnsat::UnexpectedEditablePackage(_)
| PlatformUnsat::EditablePackageMismatch(_)
| PlatformUnsat::SourceTreeHashMismatch(_),
)
}
Expand Down Expand Up @@ -399,6 +404,7 @@ pub fn verify_package_platform_satisfiability(
// requirements. We want to ensure we always check the conda packages first.
let mut conda_queue = conda_specs;
let mut pypi_queue = pypi_requirements;
let mut expected_editable_pypi_packages = HashSet::new();
while let Some(package) = conda_queue.pop().or_else(|| pypi_queue.pop()) {
enum FoundPackage {
Conda(usize),
Expand Down Expand Up @@ -502,28 +508,21 @@ pub fn verify_package_platform_satisfiability(
let record = &locked_pypi_environment.records[idx];
match requirement {
RequirementOrEditable::Editable(package_name, requirement) => {
if !record.0.editable {
return Err(PlatformUnsat::ExpectedEditablePackage(
record.0.name.clone(),
));
}

if !pypi_satifisfies_editable(&record.0, &requirement) {
return Err(PlatformUnsat::UnsatisfiableRequirement(
RequirementOrEditable::Editable(package_name, requirement),
source.into_owned(),
));
}

// Record that we want this package to be editable. This is used to
// check at the end if packages that should be editable are actually
// editable and vice versa.
expected_editable_pypi_packages.insert(package_name.clone());

FoundPackage::PyPi(idx, requirement.extras)
}
RequirementOrEditable::Pep508Requirement(requirement) => {
if record.0.editable {
return Err(PlatformUnsat::UnexpectedEditablePackage(
record.0.name.clone(),
));
}

if !pypi_satifisfies_requirement(&record.0, &requirement) {
return Err(PlatformUnsat::UnsatisfiableRequirement(
RequirementOrEditable::Pep508Requirement(requirement),
Expand Down Expand Up @@ -652,6 +651,24 @@ pub fn verify_package_platform_satisfiability(
));
}

// Check if all packages that should be editable are actually editable and vice versa.
let locked_editable_packages = locked_pypi_environment
.records
.iter()
.filter(|record| record.0.editable)
.map(|record| record.0.name.clone())
.collect::<HashSet<_>>();
let expected_editable = expected_editable_pypi_packages.sub(&locked_editable_packages);
let unexpected_editable = locked_editable_packages.sub(&expected_editable_pypi_packages);
if !expected_editable.is_empty() || !unexpected_editable.is_empty() {
return Err(PlatformUnsat::EditablePackageMismatch(
EditablePackagesMismatch {
expected_editable: expected_editable.into_iter().sorted().collect(),
unexpected_editable: unexpected_editable.into_iter().sorted().collect(),
},
));
}

Ok(())
}

Expand Down Expand Up @@ -701,6 +718,80 @@ impl MatchesMatchspec for GenericVirtualPackage {
}
}

impl Display for EditablePackagesMismatch {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
if !self.expected_editable.is_empty() && self.unexpected_editable.is_empty() {
write!(f, "expected ")?;
format_package_list(f, &self.expected_editable)?;
write!(
f,
" to be editable but in the lock-file {they} {are} not",
they = it_they(self.expected_editable.len()),
are = is_are(self.expected_editable.len())
)?
} else if self.expected_editable.is_empty() && !self.unexpected_editable.is_empty() {
write!(f, "expected ")?;
format_package_list(f, &self.unexpected_editable)?;
write!(
f,
"NOT to be editable but in the lock-file {they} {are}",
they = it_they(self.unexpected_editable.len()),
are = is_are(self.unexpected_editable.len())
)?
} else {
write!(f, "expected ")?;
format_package_list(f, &self.expected_editable)?;
write!(
f,
" to be editable but in the lock-file but {they} {are} not, whereas ",
they = it_they(self.expected_editable.len()),
are = is_are(self.expected_editable.len())
)?;
format_package_list(f, &self.unexpected_editable)?;
write!(
f,
" {are} NOT expected to be editable which in the lock-file {they} {are}",
they = it_they(self.unexpected_editable.len()),
are = is_are(self.unexpected_editable.len())
)?
}

return Ok(());

fn format_package_list(
f: &mut std::fmt::Formatter<'_>,
packages: &[PackageName],
) -> std::fmt::Result {
for (idx, package) in packages.iter().enumerate() {
if idx == packages.len() - 1 && idx > 0 {
write!(f, " and ")?;
} else if idx > 0 {
write!(f, ", ")?;
}
write!(f, "{}", package)?;
}

Ok(())
}

fn is_are(count: usize) -> &'static str {
if count == 1 {
"is"
} else {
"are"
}
}

fn it_they(count: usize) -> &'static str {
if count == 1 {
"it"
} else {
"they"
}
}
}
}

#[cfg(test)]
mod tests {
use super::*;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
source: src/lock_file/satisfiability.rs
assertion_line: 874
expression: s
input_file: tests/non-satisfiability/expected-editable-multiple/pixi.toml
---
environment 'default' does not satisfy the requirements of the project for platform 'win-64
Diagnostic severity: error
Caused by: expected bar and foo to be editable but in the lock-file but they are not, whereas baz is NOT expected to be editable which in the lock-file it is
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
source: src/lock_file/satisfiability.rs
assertion_line: 874
expression: s
input_file: tests/non-satisfiability/expected-editable/pixi.toml
---
environment 'default' does not satisfy the requirements of the project for platform 'win-64
Diagnostic severity: error
Caused by: expected foo to be editable but in the lock-file it is not
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
[project]
name = "bar"
version = "0.1.0"
dependencies = []

[build-system]
requires = ["setuptools"]
build-backend = "setuptools.build_meta"

[tool.pixi.project]
channels = ["conda-forge"]
platforms = ["win-64"]

[tool.pixi.pypi-dependencies]
bar = { path = ".", editable = true }
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
[project]
name = "baz"
version = "0.1.0"
dependencies = []

[build-system]
requires = ["setuptools"]
build-backend = "setuptools.build_meta"

[tool.pixi.project]
channels = ["conda-forge"]
platforms = ["win-64"]

[tool.pixi.pypi-dependencies]
bar = { path = ".", editable = true }
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
[project]
name = "foo"
version = "0.1.0"
dependencies = []

[build-system]
requires = ["setuptools"]
build-backend = "setuptools.build_meta"

[tool.pixi.project]
channels = ["conda-forge"]
platforms = ["win-64"]

[tool.pixi.pypi-dependencies]
foo = { path = ".", editable = true }
Loading

0 comments on commit d3d251b

Please sign in to comment.