feat: Validate environment after injecting packages (#57)

pavelzw · delsner · web-flow · commit daf227ef11be · 2024-10-18T17:40:21.000+02:00
* feat: Validate environment after injecting packages

* Fix clippy

* Add tests

* Review

* bump version

---------

Co-authored-by: Daniel Elsner &lt;daniel.elsner@quantco.com&gt;
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -1,7 +1,7 @@
 [package]
 name = "pixi-pack"
 description = "A command line tool to pack and unpack conda environments for easy sharing"
-version = "0.1.8"
+version = "0.2.0"
 edition = "2021"
 
 [features]
diff --git a/README.md b/README.md
@@ -131,6 +131,7 @@ pixi-pack pack --inject local-package-1.0.0-hbefa133_0.conda --manifest-pack pix
 ```
 
 This can be particularly useful if you build the project itself and want to include the built package in the environment but still want to use `pixi.lock` from the project.
+Before creating the pack, `pixi-pack` will ensure that the injected packages' dependencies and constraints are compatible with the packages in the environment.
 
 ### Unpacking without `pixi-pack`
 
diff --git a/examples/webserver/README.md b/examples/webserver/README.md
@@ -12,3 +12,8 @@ pixi-pack pack -e prod --platform win-64 --inject output/noarch/my-webserver-*.c
 > [!NOTE]
 > The files `my-webserver-0.1.0-pyh4616a5c_0.conda` and `my-webserver-0.1.0-pyh4616a5c_0.tar.bz2` are only for testing, in a real scenario it would be in the `output/noarch` directory generated by `rattler-build`.
 > They were generated using `rattler-build build -r recipe/recipe.yaml --package-format conda` and `rattler-build build -r recipe/recipe.yaml --package-format tar-bz2`.
+
+## Failure for invalid dependencies/constraints
+
+In case the recipe of the package is incompatible with the packed pixi environment, `pixi-pack` should raise a validation error.
+To generate the incompatible package, run `rattler-build build -r recipe/recipe-broken.yaml --package-format conda`.
diff --git a/examples/webserver/my-webserver-0.1.0-pyh4616a5c_0.conda b/examples/webserver/my-webserver-0.1.0-pyh4616a5c_0.conda
diff --git a/examples/webserver/my-webserver-0.1.0-pyh4616a5c_0.tar.bz2 b/examples/webserver/my-webserver-0.1.0-pyh4616a5c_0.tar.bz2
diff --git a/examples/webserver/my-webserver-broken-0.1.0-pyh4616a5c_0.conda b/examples/webserver/my-webserver-broken-0.1.0-pyh4616a5c_0.conda
diff --git a/examples/webserver/pixi.toml b/examples/webserver/pixi.toml
@@ -8,7 +8,7 @@ dev = "uvicorn my_project:app --reload"
 start = "uvicorn my_project:app --host 0.0.0.0"
 
 [dependencies]
-fastapi = "*"
+fastapi = ">=0.111,<0.112"
 uvicorn = "*"
 
 [feature.dev.dependencies]
diff --git a/examples/webserver/recipe/recipe-broken.yaml b/examples/webserver/recipe/recipe-broken.yaml
@@ -0,0 +1,33 @@
+# yaml-language-server: $schema=https://raw.githubusercontent.com/prefix-dev/recipe-format/main/schema.json
+
+package:
+  name: my-webserver-broken
+  version: "0.1.0"
+
+source:
+  path: ../
+
+build:
+  number: 0
+  noarch: python
+  script:
+    - python -m pip install . --no-deps --ignore-installed -vv --no-build-isolation --disable-pip-version-check
+
+requirements:
+  host:
+    - python >=3.12
+    - pip
+    - hatchling
+  run:
+    - python >=3.12
+    - fastapi >=0.112
+  run_constraints:
+    - pydantic >=2,<3
+
+tests:
+  - python:
+      imports:
+        - my_webserver
+  - package_contents:
+      site_packages:
+        - my_webserver/__init__.py
diff --git a/examples/webserver/recipe/recipe.yaml b/examples/webserver/recipe/recipe.yaml
@@ -21,6 +21,8 @@ requirements:
   run:
     - python >=3.12
     - fastapi
+  run_constraints:
+    - pydantic >=2,<3
 
 tests:
   - python:
diff --git a/src/pack.rs b/src/pack.rs
@@ -15,7 +15,10 @@ use tokio::{
 
 use anyhow::Result;
 use futures::{stream, StreamExt, TryFutureExt, TryStreamExt};
-use rattler_conda_types::{package::ArchiveType, ChannelInfo, PackageRecord, Platform, RepoData};
+use rattler_conda_types::{
+    package::ArchiveType, ChannelInfo, MatchSpec, Matches, PackageRecord, ParseStrictness,
+    Platform, RepoData,
+};
 use rattler_lock::{CondaPackage, LockFile, Package};
 use rattler_networking::{AuthenticationMiddleware, AuthenticationStorage};
 use reqwest_middleware::ClientWithMiddleware;
@@ -133,14 +136,14 @@ pub async fn pack(options: PackOptions) -> Result<()> {
         .collect();
 
     tracing::info!("Injecting {} packages", injected_packages.len());
-    for (path, archive_type) in injected_packages {
+    for (path, archive_type) in injected_packages.iter() {
         // step 1: Derive PackageRecord from index.json inside the package
         let package_record = match archive_type {
-            ArchiveType::TarBz2 => package_record_from_tar_bz2(&path),
-            ArchiveType::Conda => package_record_from_conda(&path),
+            ArchiveType::TarBz2 => package_record_from_tar_bz2(path),
+            ArchiveType::Conda => package_record_from_conda(path),
         }?;
 
-        // step 2: copy file into channel dir
+        // step 2: Copy file into channel dir
         let subdir = &package_record.subdir;
         let filename = path
             .file_name()
@@ -156,6 +159,12 @@ pub async fn pack(options: PackOptions) -> Result<()> {
         conda_packages.push((filename, package_record));
     }
 
+    // In case we injected packages, we need to validate that these packages are solvable with the
+    // environment (i.e., that each packages dependencies and run constraints are still satisfied).
+    if !injected_packages.is_empty() {
+        validate_package_records(conda_packages.iter().map(|(_, p)| p.clone()).collect())?;
+    }
+
     // Create `repodata.json` files.
     tracing::info!("Creating repodata.json files");
     create_repodata_files(conda_packages.iter(), &channel_dir).await?;
@@ -394,3 +403,42 @@ async fn create_repodata_files(
 
     Ok(())
 }
+
+/// Validate that the given package records are valid w.r.t. 'depends' and 'constrains'.
+/// This might eventually be part of rattler, xref: https://github.com/conda/rattler/issues/906
+fn validate_package_records(package_records: Vec<PackageRecord>) -> Result<()> {
+    for package in package_records.iter() {
+        // First we check if all dependencies are in the environment.
+        for dep in package.depends.iter() {
+            // We ignore virtual packages, e.g. `__unix`.
+            if dep.starts_with("__") {
+                continue;
+            }
+            let dep_spec = MatchSpec::from_str(dep, ParseStrictness::Lenient)?;
+            if !package_records.iter().any(|p| dep_spec.matches(p)) {
+                return Err(anyhow!(
+                    "package {} has dependency '{}', which is not in the environment",
+                    package.name.as_normalized(),
+                    dep
+                ));
+            }
+        }
+
+        // Then we check if all constraints are satisfied.
+        for constraint in package.constrains.iter() {
+            let constraint_spec = MatchSpec::from_str(constraint, ParseStrictness::Lenient)?;
+            let matching_package = package_records
+                .iter()
+                .find(|record| Some(record.name.clone()) == constraint_spec.name);
+            if matching_package.is_some_and(|p| !constraint_spec.matches(p)) {
+                return Err(anyhow!(
+                    "package {} has constraint '{}', which is not satisfied by {} in the environment",
+                    package.name.as_normalized(),
+                    constraint,
+                    matching_package.unwrap().name.as_normalized()
+                ));
+            }
+        }
+    }
+    Ok(())
+}
diff --git a/tests/integration_test.rs b/tests/integration_test.rs
@@ -152,6 +152,25 @@ async fn test_inject(
         });
 }
 
+#[rstest]
+#[tokio::test]
+async fn test_inject_failure(options: Options) {
+    let mut pack_options = options.pack_options;
+    pack_options.injected_packages.push(PathBuf::from(
+        "examples/webserver/my-webserver-broken-0.1.0-pyh4616a5c_0.conda",
+    ));
+    pack_options.manifest_path = PathBuf::from("examples/webserver/pixi.toml");
+
+    let pack_result = pixi_pack::pack(pack_options).await;
+
+    assert!(pack_result.is_err());
+    assert!(pack_result
+        .err()
+        .unwrap()
+        .to_string()
+        .contains("package my-webserver-broken has dependency 'fastapi >=0.112'"));
+}
+
 #[rstest]
 #[tokio::test]
 async fn test_includes_repodata_patches(options: Options) {
