Filter out incompatible dists (#398)

Filter out source dists and wheels whose `requires-python` from the
simple api is incompatible with the current python version.

This change showed an important problem: When we use a fake python
version for resolving, building source distributions breaks down because
we can only build with versions we actually have.

This change became surprisingly big. The tests now require python 3.7 to
be installed, but changing that would mean an even bigger change.

Fixes #388

.github/workflows/ci.yaml

@@ -48,10 +48,13 @@ jobs:
     name: "cargo test | ${{ matrix.os }}"
     steps:
       - uses: actions/checkout@v4
+      # TODO(konstin): Mock the venv in the installer test so we don't need 3.8 anymore
       - name: "Install Python"
         uses: actions/setup-python@v4
         with:
-          python-version: ${{ env.PYTHON_VERSION }}
+          python-version: | 
+            3.8
+            ${{ env.PYTHON_VERSION }}
       - name: "Install Rust toolchain"
         run: rustup show
       - name: "Install cargo insta"

.gitignore

@@ -13,3 +13,6 @@ target/
 
 # Use e.g. `--cache-dir cache-docker` to keep a cache across container invocations
 cache-*
+
+# python tmp files
+__pycache__

crates/pep440-rs/src/version_specifier.rs

@@ -540,6 +540,9 @@ impl Display for VersionSpecifier {
 /// ```
 pub fn parse_version_specifiers(spec: &str) -> Result<Vec<VersionSpecifier>, Pep440Error> {
     let mut version_ranges = Vec::new();
+    if spec.is_empty() {
+        return Ok(version_ranges);
+    }
     let mut start: usize = 0;
     let separator = ",";
     for version_range_spec in spec.split(separator) {
@@ -1301,4 +1304,11 @@ mod test {
             ">=3.7, <4.0, !=3.9.0"
         );
     }
+
+    /// These occur in the simple api, e.g.
+    /// <https://pypi.org/simple/geopandas/?format=application/vnd.pypi.simple.v1+json>
+    #[test]
+    fn test_version_specifiers_empty() {
+        assert_eq!(VersionSpecifiers::from_str("").unwrap().to_string(), "");
+    }
 }

crates/puffin-cli/src/commands/pip_compile.rs

@@ -125,6 +125,11 @@ pub(crate) async fn pip_compile(
         || Cow::Borrowed(venv.interpreter_info().markers()),
         |python_version| Cow::Owned(python_version.markers(venv.interpreter_info().markers())),
     );
+    // Inject the fake python version if necessary
+    let interpreter_info = venv
+        .interpreter_info()
+        .clone()
+        .patch_markers(markers.clone().into_owned());
 
     // Instantiate a client.
     let client = {
@@ -143,7 +148,7 @@ pub(crate) async fn pip_compile(
     let build_dispatch = BuildDispatch::new(
         client.clone(),
         cache.to_path_buf(),
-        venv.interpreter_info().clone(),
+        interpreter_info,
         fs::canonicalize(venv.python_executable())?,
         no_build,
     );

crates/puffin-cli/src/commands/pip_sync.rs

@@ -128,8 +128,9 @@ pub(crate) async fn sync_requirements(
     } else {
         let start = std::time::Instant::now();
 
-        let wheel_finder = puffin_resolver::DistFinder::new(&tags, &client)
-            .with_reporter(FinderReporter::from(printer).with_length(remote.len() as u64));
+        let wheel_finder =
+            puffin_resolver::DistFinder::new(&tags, &client, venv.interpreter_info())
+                .with_reporter(FinderReporter::from(printer).with_length(remote.len() as u64));
         let resolution = wheel_finder.resolve(&remote).await?;
 
         let s = if resolution.len() == 1 { "" } else { "s" };

crates/puffin-cli/tests/pip_compile.rs

@@ -630,6 +630,55 @@ fn compile_python_37() -> Result<()> {
     Ok(())
 }
 
+/// Test that we select the last 3.8 compatible numpy version instead of trying to compile an
+/// incompatible sdist <https://github.com/astral-sh/puffin/issues/388>
+#[test]
+fn compile_numpy_py38() -> 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 requirements_in = temp_dir.child("requirements.in");
+    requirements_in.touch()?;
+    requirements_in.write_str("numpy")?;
+
+    insta::with_settings!({
+            filters => INSTA_FILTERS.to_vec()
+        }, {
+            assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME))
+                .arg("pip-compile")
+                .arg("requirements.in")
+                .arg("--python-version")
+                .arg("py38")
+                .arg("--cache-dir")
+                .arg(cache_dir.path())
+    // Check that we select the wheel and not the sdist
+                .arg("--no-build")
+                .env("VIRTUAL_ENV", venv.as_os_str())
+                .current_dir(&temp_dir), @r###"
+            success: false
+            exit_code: 2
+            ----- stdout -----
+
+            ----- stderr -----
+            error: Failed to build distribution: numpy-1.24.4.tar.gz
+              Caused by: Building source distributions is disabled
+            "###);
+        });
+
+    Ok(())
+}
+
 /// Resolve a specific Flask wheel via a URL dependency.
 #[test]
 fn compile_wheel_url_dependency() -> Result<()> {

crates/puffin-cli/tests/pip_sync.rs

@@ -2,7 +2,7 @@
 
 use std::process::Command;
 
-use anyhow::Result;
+use anyhow::{Context, Result};
 use assert_cmd::prelude::*;
 use assert_fs::prelude::*;
 use insta_cmd::_macro_support::insta;
@@ -928,3 +928,50 @@ fn install_version_then_install_url() -> Result<()> {
 
     Ok(())
 }
+
+/// Test that we select the last 3.8 compatible numpy version instead of trying to compile an
+/// incompatible sdist <https://github.com/astral-sh/puffin/issues/388>
+#[test]
+fn install_numpy_py38() -> 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("--python")
+        // TODO(konstin): Mock the venv in the installer test so we don't need this anymore
+        .arg(which::which("python3.8").context("python3.8 must be installed")?)
+        .arg("--cache-dir")
+        .arg(cache_dir.path())
+        .current_dir(&temp_dir)
+        .assert()
+        .success();
+    venv.assert(predicates::path::is_dir());
+
+    let requirements_txt = temp_dir.child("requirements.txt");
+    requirements_txt.touch()?;
+    requirements_txt.write_str("numpy")?;
+
+    insta::with_settings!({
+        filters => INSTA_FILTERS.to_vec()
+    }, {
+        assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME))
+            .arg("pip-sync")
+            .arg("requirements.txt")
+            .arg("--cache-dir")
+            .arg(cache_dir.path())
+            .env("VIRTUAL_ENV", venv.as_os_str())
+            .current_dir(&temp_dir));
+    });
+
+    Command::new(venv.join("bin").join("python"))
+        .arg("-c")
+        .arg("import numpy")
+        .current_dir(&temp_dir)
+        .assert()
+        .success();
+
+    Ok(())
+}

crates/puffin-cli/tests/snapshots/pip_compile__compile_constraints_markers.snap

@@ -8,9 +8,9 @@ info:
     - "--constraint"
     - constraints.txt
     - "--cache-dir"
-    - /var/folders/bc/qlsk3t6x7c9fhhbvvcg68k9c0000gp/T/.tmp1Ts53o
+    - /tmp/.tmp81zzAa
   env:
-    VIRTUAL_ENV: /var/folders/bc/qlsk3t6x7c9fhhbvvcg68k9c0000gp/T/.tmp1vzBQa/.venv
+    VIRTUAL_ENV: /tmp/.tmpFXMFLd/.venv
 ---
 success: true
 exit_code: 0

crates/puffin-cli/tests/snapshots/pip_compile__compile_constraints_txt.snap

@@ -8,9 +8,9 @@ info:
     - "--constraint"
     - constraints.txt
     - "--cache-dir"
-    - /var/folders/nt/6gf2v7_s3k13zq_t3944rwz40000gn/T/.tmpvjdHOb
+    - /tmp/.tmpuS4Jn4
   env:
-    VIRTUAL_ENV: /var/folders/nt/6gf2v7_s3k13zq_t3944rwz40000gn/T/.tmpSAaWi3/.venv
+    VIRTUAL_ENV: /tmp/.tmpnFovuD/.venv
 ---
 success: true
 exit_code: 0

crates/puffin-cli/tests/snapshots/pip_compile__compile_pyproject_toml.snap

@@ -6,9 +6,9 @@ info:
     - pip-compile
     - pyproject.toml
     - "--cache-dir"
-    - /var/folders/nt/6gf2v7_s3k13zq_t3944rwz40000gn/T/.tmpOOTFwj
+    - /tmp/.tmpWzFzu7
   env:
-    VIRTUAL_ENV: /var/folders/nt/6gf2v7_s3k13zq_t3944rwz40000gn/T/.tmpU0VXyY/.venv
+    VIRTUAL_ENV: /tmp/.tmpSiRk6d/.venv
 ---
 success: true
 exit_code: 0

crates/puffin-cli/tests/snapshots/pip_compile__compile_pyproject_toml_all_extras.snap

@@ -7,9 +7,9 @@ info:
     - pyproject.toml
     - "--all-extras"
     - "--cache-dir"
-    - /var/folders/nt/6gf2v7_s3k13zq_t3944rwz40000gn/T/.tmpFzJKRe
+    - /tmp/.tmpQHVFqr
   env:
-    VIRTUAL_ENV: /var/folders/nt/6gf2v7_s3k13zq_t3944rwz40000gn/T/.tmpdadcmu/.venv
+    VIRTUAL_ENV: /tmp/.tmpmKXqaF/.venv
 ---
 success: true
 exit_code: 0

crates/puffin-cli/tests/snapshots/pip_compile__compile_pyproject_toml_extra.snap

@@ -8,9 +8,9 @@ info:
     - "--extra"
     - foo
     - "--cache-dir"
-    - /var/folders/bc/qlsk3t6x7c9fhhbvvcg68k9c0000gp/T/.tmpAYEAdM
+    - /tmp/.tmpLjFVr0
   env:
-    VIRTUAL_ENV: /var/folders/bc/qlsk3t6x7c9fhhbvvcg68k9c0000gp/T/.tmp1xuOcV/.venv
+    VIRTUAL_ENV: /tmp/.tmpDT0oRC/.venv
 ---
 success: true
 exit_code: 0

crates/puffin-cli/tests/snapshots/pip_compile__compile_pyproject_toml_extra_name_normalization.snap

@@ -8,9 +8,9 @@ info:
     - "--extra"
     - FRiENDlY-...-_-BARd
     - "--cache-dir"
-    - /var/folders/bc/qlsk3t6x7c9fhhbvvcg68k9c0000gp/T/.tmpFyLXQn
+    - /tmp/.tmpga6JO1
   env:
-    VIRTUAL_ENV: /var/folders/bc/qlsk3t6x7c9fhhbvvcg68k9c0000gp/T/.tmpxfZatv/.venv
+    VIRTUAL_ENV: /tmp/.tmpwmJIym/.venv
 ---
 success: true
 exit_code: 0

crates/puffin-cli/tests/snapshots/pip_compile__compile_python_37.snap

@@ -8,38 +8,16 @@ info:
     - "--python-version"
     - py37
     - "--cache-dir"
-    - /var/folders/nt/6gf2v7_s3k13zq_t3944rwz40000gn/T/.tmpzwzUVe
+    - /tmp/.tmpHz7Dc2
   env:
-    VIRTUAL_ENV: /var/folders/nt/6gf2v7_s3k13zq_t3944rwz40000gn/T/.tmpqFv4YL/.venv
+    VIRTUAL_ENV: /tmp/.tmpoMjDJT/.venv
 ---
-success: true
-exit_code: 0
+success: false
+exit_code: 1
 ----- stdout -----
-# This file was autogenerated by Puffin v0.0.1 via the following command:
-#    [BIN_PATH] pip-compile requirements.in --python-version py37 --cache-dir [CACHE_DIR]
-black==23.10.1
-click==8.1.7
-    # via black
-importlib-metadata==6.8.0
-    # via click
-mypy-extensions==1.0.0
-    # via black
-packaging==23.2
-    # via black
-pathspec==0.11.2
-    # via black
-platformdirs==4.0.0
-    # via black
-tomli==2.0.1
-    # via black
-typing-extensions==4.8.0
-    # via
-    #   black
-    #   importlib-metadata
-    #   platformdirs
-zipp==3.17.0
-    # via importlib-metadata
 
 ----- stderr -----
-Resolved 10 packages in [TIME]
+  × No solution found when resolving dependencies:
+  ╰─▶ Because there is no version of black available matching ==23.10.1 and
+      root depends on black==23.10.1, version solving failed.
 

crates/puffin-cli/tests/snapshots/pip_compile__compile_requirements_in.snap

@@ -6,9 +6,9 @@ info:
     - pip-compile
     - requirements.in
     - "--cache-dir"
-    - /var/folders/nt/6gf2v7_s3k13zq_t3944rwz40000gn/T/.tmpxF1zFY
+    - /tmp/.tmpQB4Ze4
   env:
-    VIRTUAL_ENV: /var/folders/nt/6gf2v7_s3k13zq_t3944rwz40000gn/T/.tmp33QZdv/.venv
+    VIRTUAL_ENV: /tmp/.tmpSYcMgG/.venv
 ---
 success: true
 exit_code: 0

crates/puffin-cli/tests/snapshots/pip_sync__install_numpy_py38.snap

@@ -0,0 +1,23 @@
+---
+source: crates/puffin-cli/tests/pip_sync.rs
+info:
+  program: puffin
+  args:
+    - pip-sync
+    - requirements.txt
+    - "--cache-dir"
+    - /tmp/.tmpL9E9fl
+  env:
+    VIRTUAL_ENV: /tmp/.tmp0yDHj5/.venv
+---
+success: true
+exit_code: 0
+----- stdout -----
+
+----- stderr -----
+Resolved 1 package in [TIME]
+Downloaded 1 package in [TIME]
+Unzipped 1 package in [TIME]
+Installed 1 package in [TIME]
+ + numpy==1.24.4
+

crates/puffin-dispatch/src/lib.rs

@@ -135,7 +135,7 @@ impl BuildContext for BuildDispatch {
                     if remote.len() == 1 { "" } else { "s" },
                     remote.iter().map(ToString::to_string).join(", ")
                 );
-                let resolution = DistFinder::new(&tags, &self.client)
+                let resolution = DistFinder::new(&tags, &self.client, self.interpreter_info())
                     .resolve(&remote)
                     .await
                     .context("Failed to resolve build dependencies")?;

crates/puffin-distribution/src/traits.rs

@@ -12,8 +12,8 @@ pub trait Metadata {
     /// Return the normalized [`PackageName`] of the distribution.
     fn name(&self) -> &PackageName;
 
-    /// Return a [`Version`], for registry-based distributions, or a [`Url`], for URL-based
-    /// distributions.
+    /// Return a [`pep440_rs::Version`], for registry-based distributions, or a [`url::Url`],
+    /// for URL-based distributions.
     fn version_or_url(&self) -> VersionOrUrl;
 
     /// Returns a unique identifier for the package.