Multiple entries in PUFFIN_PYTHON_PATH for windows tests (astral-sh#1254

)

There are no binary installers for the latests patch versions of cpython
for windows, and building them is hard. As an alternative, we download
python-build-standanlone cpythons and put them into `<project
root>/bin`. On unix, we can symlink `pythonx.y.z` into this directory
and point `PUFFIN_PYTHON_PATH` to it. On windows, all pythons are called
`python.exe` and they don't like being linked. Instead, we add the path
to each directory containing a `python.exe` to `PUFFIN_PYTHON_PATH`,
similar to the regular `PATH`. The python discovery on windows was
extended to respect `PUFFIN_PYTHON_PATH` where needed.

These changes mean that we don't need to (sym)link pythons anymore and
could drop that part to the script.

435 tests run: 389 passed (21 slow), 46 failed, 1 skipped

crates/puffin-interpreter/src/python_query.rs

@@ -1,5 +1,6 @@
 //! Find a user requested python version/interpreter.
 
+use std::env;
 use std::path::PathBuf;
 use std::process::Command;
 
@@ -37,6 +38,19 @@ pub fn find_requested_python(request: &str) -> Result<PathBuf, Error> {
             let formatted = PathBuf::from(format!("python{request}"));
             Interpreter::find_executable(&formatted)
         } else if cfg!(windows) {
+            if let Some(python_overwrite) = env::var_os("PUFFIN_PYTHON_PATH") {
+                for path in env::split_paths(&python_overwrite) {
+                    if path
+                        .as_os_str()
+                        .to_str()
+                        // Good enough since we control the bootstrap directory
+                        .is_some_and(|path| path.contains(&format!("@{request}")))
+                    {
+                        return Ok(path);
+                    }
+                }
+            }
+
             if let [major, minor] = versions.as_slice() {
                 find_python_windows(*major, *minor)?.ok_or(Error::NoSuchPython {
                     major: *major,
@@ -58,16 +72,22 @@ pub fn find_requested_python(request: &str) -> Result<PathBuf, Error> {
 }
 
 /// Pick a sensible default for the python a user wants when they didn't specify a version.
+///
+/// We prefer the test overwrite `PUFFIN_PYTHON_PATH` if it is set, otherwise `python3`/`python` or
+/// `python.exe` respectively.
 #[instrument]
 pub fn find_default_python() -> Result<PathBuf, Error> {
+    let current_dir = env::current_dir()?;
     let python = if cfg!(unix) {
-        which::which("python3")
+        which::which_in("python3", env::var_os("PUFFIN_PYTHON_PATH"), current_dir)
             .or_else(|_| which::which("python"))
             .map_err(|_| Error::NoPythonInstalledUnix)?
     } else if cfg!(windows) {
         // TODO(konstin): Is that the right order, or should we look for `py --list-paths` first? With the current way
         // it works even if the python launcher is not installed.
-        if let Ok(python) = which::which("python.exe") {
+        if let Ok(python) =
+            which::which_in("python.exe", env::var_os("PUFFIN_PYTHON_PATH"), current_dir)
+        {
             python
         } else {
             installed_pythons_windows()?
@@ -87,6 +107,8 @@ pub fn find_default_python() -> Result<PathBuf, Error> {
 /// The command takes 8ms on my machine. TODO(konstin): Implement <https://peps.python.org/pep-0514/> to read python
 /// installations from the registry instead.
 fn installed_pythons_windows() -> Result<Vec<(u8, u8, PathBuf)>, Error> {
+    // TODO(konstin): We're not checking PUFFIN_PYTHON_PATH here, no test currently depends on it.
+
     // TODO(konstin): Special case the not found error
     let output = info_span!("py_list_paths")
         .in_scope(|| Command::new("py").arg("--list-paths").output())

crates/puffin/tests/common/mod.rs

@@ -1,7 +1,6 @@
 // The `unreachable_pub` is to silence false positives in RustRover.
 #![allow(dead_code, unreachable_pub)]
 
-use std::env;
 use std::path::{Path, PathBuf};
 use std::process::Output;
 
@@ -92,9 +91,65 @@ pub fn venv_to_interpreter(venv: &Path) -> PathBuf {
     }
 }
 
+/// If bootstrapped python build standalone pythons exists in `<project root>/bin`,
+/// return the paths to the directories containing the python binaries (i.e. as paths that
+/// `which::which_in` can use).
+///
+/// Use `scripts/bootstrap/install.py` to bootstrap.
+///
+/// Python versions are sorted from newest to oldest.
+pub fn bootstrapped_pythons() -> Option<Vec<PathBuf>> {
+    // Current dir is `<project root>/crates/puffin`.
+    let bootstrapped_pythons = std::env::current_dir()
+        .unwrap()
+        .parent()
+        .unwrap()
+        .parent()
+        .unwrap()
+        .join("bin")
+        .join("versions");
+    let Ok(bootstrapped_pythons) = fs_err::read_dir(bootstrapped_pythons) else {
+        return None;
+    };
+
+    let mut bootstrapped_pythons: Vec<PathBuf> = bootstrapped_pythons
+        .map(Result::unwrap)
+        .filter(|entry| entry.metadata().unwrap().is_dir())
+        .map(|entry| {
+            if cfg!(unix) {
+                entry.path().join("install").join("bin")
+            } else if cfg!(windows) {
+                entry.path().join("install")
+            } else {
+                unimplemented!("Only Windows and Unix are supported")
+            }
+        })
+        .collect();
+    bootstrapped_pythons.sort();
+    // Prefer the most recent patch version.
+    bootstrapped_pythons.reverse();
+    Some(bootstrapped_pythons)
+}
+
 /// Create a virtual environment named `.venv` in a temporary directory with the given
 /// Python version. Expected format for `python` is "python<version>".
 pub fn create_venv(temp_dir: &TempDir, cache_dir: &TempDir, python: &str) -> PathBuf {
+    let python = if let Some(bootstrapped_pythons) = bootstrapped_pythons() {
+        bootstrapped_pythons
+            .into_iter()
+            // Good enough since we control the directory
+            .find(|path| path.to_str().unwrap().contains(&format!("@{python}")))
+            .expect("Missing python bootstrap version")
+            .join(if cfg!(unix) {
+                "python3"
+            } else if cfg!(windows) {
+                "python.exe"
+            } else {
+                unimplemented!("Only Windows and Unix are supported")
+            })
+    } else {
+        PathBuf::from(python)
+    };
     let venv = temp_dir.child(".venv");
     Command::new(get_bin())
         .arg("venv")

crates/puffin/tests/pip_compile_scenarios.rs

@@ -5,6 +5,7 @@
 //!
 #![cfg(all(feature = "python", feature = "pypi"))]
 
+use std::env;
 use std::path::PathBuf;
 use std::process::Command;
 
@@ -17,7 +18,7 @@ use fs_err::os::unix::fs::symlink as symlink_file;
 use fs_err::os::windows::fs::symlink_file;
 use predicates::prelude::predicate;
 
-use common::{get_bin, puffin_snapshot, TestContext, INSTA_FILTERS};
+use common::{bootstrapped_pythons, get_bin, puffin_snapshot, TestContext, INSTA_FILTERS};
 use puffin_interpreter::find_requested_python;
 
 mod common;
@@ -27,6 +28,18 @@ pub(crate) fn create_bin_with_executables(
     temp_dir: &assert_fs::TempDir,
     python_versions: &[&str],
 ) -> Result<PathBuf> {
+    if let Some(bootstrapped_pythons) = bootstrapped_pythons() {
+        let selected_pythons = bootstrapped_pythons.into_iter().filter(|path| {
+            python_versions.iter().any(|python_version| {
+                // Good enough since we control the directory
+                path.to_str()
+                    .unwrap()
+                    .contains(&format!("@{python_version}"))
+            })
+        });
+        return Ok(env::join_paths(selected_pythons)?.into());
+    }
+
     let bin = temp_dir.child("bin");
     fs_err::create_dir(&bin)?;
     for request in python_versions {

scripts/scenarios/templates/compile.mustache

@@ -5,6 +5,7 @@
 //!
 #![cfg(all(feature = "python", feature = "pypi"))]
 
+use std::env;
 use std::path::PathBuf;
 use std::process::Command;
 
@@ -17,7 +18,7 @@ use fs_err::os::unix::fs::symlink as symlink_file;
 use fs_err::os::windows::fs::symlink_file;
 use predicates::prelude::predicate;
 
-use common::{get_bin, puffin_snapshot, TestContext, INSTA_FILTERS};
+use common::{bootstrapped_pythons, get_bin, puffin_snapshot, TestContext, INSTA_FILTERS};
 use puffin_interpreter::find_requested_python;
 
 mod common;
@@ -27,6 +28,18 @@ pub(crate) fn create_bin_with_executables(
     temp_dir: &assert_fs::TempDir,
     python_versions: &[&str],
 ) -> Result<PathBuf> {
+    if let Some(bootstrapped_pythons) = bootstrapped_pythons() {
+        let selected_pythons = bootstrapped_pythons.into_iter().filter(|path| {
+            python_versions.iter().any(|python_version| {
+                // Good enough since we control the directory
+                path.to_str()
+                    .unwrap()
+                    .contains(&format!("@{python_version}"))
+            })
+        });
+        return Ok(env::join_paths(selected_pythons)?.into());
+    }
+
     let bin = temp_dir.child("bin");
     fs_err::create_dir(&bin)?;
     for request in python_versions {