Skip to content

Commit

Permalink
Add bootstrapping and isolation of development Python versions (astra…
Browse files Browse the repository at this point in the history
…l-sh#1105)

Replaces astral-sh#1068 and astral-sh#1070 which
were more complicated than I wanted.

- Introduces a `.python-versions` file which defines the Python versions
needed for development
- Adds a Bash script at `scripts/bootstrap/install` which installs the
required Python versions from `python-build-standalone` to `./bin`
- Checks in a `versions.json` file with metadata about available
versions on each platform and a `fetch-version` Python script derived
from `rye` for updating the versions
- Updates CI to use these Python builds instead of the `setup-python`
action
- Updates to the latest packse scenarios which require Python 3.8+
instead of 3.7+ since we cannot use 3.7 anymore and includes new test
coverage of patch Python version requests
- Adds a `PUFFIN_PYTHON_PATH` variable to prevent lookup of system
Python versions for isolation during development

Tested on Linux (via CI) and macOS (locally) — presumably it will be a
bit more complicated to do proper Windows support.
  • Loading branch information
zanieb committed Jan 26, 2024
1 parent cc0e211 commit 21577ad
Show file tree
Hide file tree
Showing 16 changed files with 5,789 additions and 444 deletions.
3 changes: 3 additions & 0 deletions .envrc
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
PATH=$PWD/bin:$PATH
export PUFFIN_PYTHON_PATH=$PWD/bin

18 changes: 7 additions & 11 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -54,16 +54,11 @@ jobs:
name: "cargo test | ${{ matrix.os }}"
steps:
- uses: actions/checkout@v4
- name: "Install Pythons"
uses: actions/setup-python@v4
with:
python-version: |
3.7
3.8
3.9
3.10
3.11
3.12
- name: "Install required Python versions"
run: |
sudo apt install direnv
scripts/bootstrap/install.sh
direnv allow .envrc
- name: "Install Rust toolchain"
run: rustup show
- uses: rui314/setup-mold@v1
Expand All @@ -75,7 +70,8 @@ jobs:
with:
save-if: ${{ github.ref == 'refs/heads/main' }}
- name: "Tests"
run: cargo nextest run --workspace --all-features --status-level skip --failure-output immediate-final --no-fail-fast -j 12
run: |
direnv exec . cargo nextest run --all --all-features --status-level skip --failure-output immediate-final --no-fail-fast -j 12
# TODO(konstin): Merge with the cargo-test job once the tests pass
windows:
Expand Down
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@
debug/
target/

# Bootstrapped Python versions
bin/

# These are backup files generated by rustfmt
**/*.rs.bk

Expand Down
6 changes: 6 additions & 0 deletions .python-versions
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
3.8.12
3.8.18
3.9.18
3.10.13
3.11.7
3.12.1
49 changes: 46 additions & 3 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,24 +2,66 @@

## Setup

You need [Rust](https://rustup.rs/), a C compiler, and CMake to build Puffin. To run the tests, you need Python 3.8, 3.9 and 3.12.
[Rust](https://rustup.rs/), a C compiler, and CMake are required to build Puffin.

To run the tests we recommend [nextest](https://nexte.st/). Make sure to run the tests with `--all-features`, otherwise you'll miss most of our integration tests.
Testing Puffin requires multiple specific Python versions. We provide a script to bootstrap development by downloading the required versions.

### Linux

We recommend [pyenv](https://github.com/pyenv/pyenv) to manage multiple Python versions.

On Ubuntu and other Debian-based distributions, you can install the C compiler and CMake with

```shell
sudo apt install build-essential cmake
```

### macOS

CMake may be installed with Homebrew:

```
brew install cmake
```

The Python bootstrapping script requires `coreutils` and `zstd`; we recommend installing them with Homebrew:

```
brew install coreutils zstd
```

See the [Python](#python) section for instructions on installing the Python versions.

### Windows

You can install CMake from the [installers](https://cmake.org/download/) or with `pipx install cmake` (make sure that the pipx install path is in `PATH`, pipx complains if it isn't).

### Python

Install required Python versions with the bootstrapping script:

```
scripts/bootstrap/install.sh
```

Then add the Python binaries to your path:

```
export PATH=$PWD/bin:$PATH
```

We also strongly recommend setting the `PUFFIN_PYTHON_PATH` variable during development; this will prevent your
system Python versions from being found during tests:

```
export PUFFIN_PYTHON_PATH=$PWD/bin
```

If you use [direnv](https://direnv.net/), these variables will be exported automatically after you run `direnv allow`.

## Testing

To run the tests we recommend [nextest](https://nexte.st/). Make sure to run the tests with `--all-features`, otherwise you'll miss most of our integration tests.

## Running inside a docker container

Source distributions can run arbitrary code on build and can make unwanted modifications to your system (https://moyix.blogspot.com/2022/09/someones-been-messing-with-my-subnormals.html, https://pypi.org/project/nvidia-pyindex/), which can even occur when just resolving requirements. To prevent this, there's a Docker container you can run commands in:
Expand All @@ -32,3 +74,4 @@ docker run --rm -it -v $(pwd):/app puffin-builder /app/target/x86_64-unknown-lin
```

We recommend using this container if you don't trust the dependency tree of the package(s) you are trying to resolve or install.

29 changes: 26 additions & 3 deletions crates/puffin-interpreter/src/interpreter.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use std::ffi::{OsStr, OsString};
use std::path::{Path, PathBuf};
use std::process::Command;

Expand Down Expand Up @@ -136,6 +137,10 @@ impl Interpreter {
/// - If a python version is given: `pythonx.y`
/// - `python3` (unix) or `python.exe` (windows)
///
/// If `PUFFIN_PYTHON_PATH` is set, we will not check for Python versions in the
/// global PATH, instead we will search using the provided path. Virtual environments
/// will still be respected.
///
/// If a version is provided and an interpreter cannot be found with the given version,
/// we will return [`None`].
pub fn find_version(
Expand Down Expand Up @@ -170,7 +175,8 @@ impl Interpreter {
python_version.major(),
python_version.minor()
);
if let Ok(executable) = which::which(&requested) {

if let Ok(executable) = Interpreter::find_executable(&requested) {
debug!("Resolved {requested} to {}", executable.display());
let interpreter = Interpreter::query(&executable, &platform.0, cache)?;
if version_matches(&interpreter) {
Expand All @@ -179,7 +185,7 @@ impl Interpreter {
}
}

if let Ok(executable) = which::which("python3") {
if let Ok(executable) = Interpreter::find_executable("python3") {
debug!("Resolved python3 to {}", executable.display());
let interpreter = Interpreter::query(&executable, &platform.0, cache)?;
if version_matches(&interpreter) {
Expand All @@ -198,7 +204,7 @@ impl Interpreter {
}
}

if let Ok(executable) = which::which("python.exe") {
if let Ok(executable) = Interpreter::find_executable("python.exe") {
let interpreter = Interpreter::query(&executable, &platform.0, cache)?;
if version_matches(&interpreter) {
return Ok(Some(interpreter));
Expand All @@ -211,6 +217,23 @@ impl Interpreter {
Ok(None)
}

pub fn find_executable<R: AsRef<OsStr> + Into<OsString> + Copy>(
requested: R,
) -> Result<PathBuf, Error> {
if let Some(isolated) = std::env::var_os("PUFFIN_PYTHON_PATH") {
if let Ok(cwd) = std::env::current_dir() {
which::which_in(requested, Some(isolated), cwd)
.map_err(|err| Error::Which(requested.into(), err))
} else {
which::which_in_global(requested, Some(isolated))
.map_err(|err| Error::Which(requested.into(), err))
.and_then(|mut paths| paths.next().ok_or(Error::PythonNotFound))
}
} else {
which::which(requested).map_err(|err| Error::Which(requested.into(), err))
}
}

/// Returns the path to the Python virtual environment.
#[inline]
pub fn platform(&self) -> &Platform {
Expand Down
7 changes: 5 additions & 2 deletions crates/puffin-interpreter/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use std::ffi::OsString;
use std::io;
use std::path::PathBuf;
use std::time::SystemTimeError;
Expand Down Expand Up @@ -49,6 +50,8 @@ pub enum Error {
NoPythonInstalledUnix,
#[error("Could not find `python.exe` in PATH and `py --list-paths` did not list any Python versions. Do you need to install Python?")]
NoPythonInstalledWindows,
#[error("Patch versions cannot be requested on Windows")]
PatchVersionRequestedWindows,
#[error("{message}:\n--- stdout:\n{stdout}\n--- stderr:\n{stderr}\n---")]
PythonSubcommandOutput {
message: String,
Expand All @@ -63,6 +66,6 @@ pub enum Error {
Encode(#[from] rmp_serde::encode::Error),
#[error("Failed to parse pyvenv.cfg")]
Cfg(#[from] cfg::Error),
#[error("Couldn't find `{0}` in PATH")]
Which(PathBuf, #[source] which::Error),
#[error("Couldn't find `{}` in PATH", _0.to_string_lossy())]
Which(OsString, #[source] which::Error),
}
29 changes: 18 additions & 11 deletions crates/puffin-interpreter/src/python_query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use once_cell::sync::Lazy;
use regex::Regex;
use tracing::{info_span, instrument};

use crate::Error;
use crate::{Error, Interpreter};

/// ```text
/// -V:3.12 C:\Users\Ferris\AppData\Local\Programs\Python\Python312\python.exe
Expand All @@ -27,23 +27,30 @@ static PY_LIST_PATHS: Lazy<Regex> = Lazy::new(|| {
/// * `-p /home/ferris/.local/bin/python3.10` uses this exact Python.
#[instrument]
pub fn find_requested_python(request: &str) -> Result<PathBuf, Error> {
let major_minor = request
.split_once('.')
.and_then(|(major, minor)| Some((major.parse::<u8>().ok()?, minor.parse::<u8>().ok()?)));
if let Some((major, minor)) = major_minor {
// `-p 3.10`
let versions = request
.splitn(3, '.')
.map(str::parse::<u8>)
.collect::<Result<Vec<_>, _>>();
if let Ok(versions) = versions {
// `-p 3.10` or `-p 3.10.1`
if cfg!(unix) {
let formatted = PathBuf::from(format!("python{major}.{minor}"));
which::which(&formatted).map_err(|err| Error::Which(formatted, err))
let formatted = PathBuf::from(format!("python{request}"));
Interpreter::find_executable(&formatted)
} else if cfg!(windows) {
find_python_windows(major, minor)?.ok_or(Error::NoSuchPython { major, minor })
if let [major, minor] = versions.as_slice() {
find_python_windows(*major, *minor)?.ok_or(Error::NoSuchPython {
major: *major,
minor: *minor,
})
} else {
Err(Error::PatchVersionRequestedWindows)
}
} else {
unimplemented!("Only Windows and Unix are supported")
}
} else if !request.contains(std::path::MAIN_SEPARATOR) {
// `-p python3.10`; Generally not used on windows because all Python are `python.exe`.
let request = PathBuf::from(request);
which::which(&request).map_err(|err| Error::Which(request, err))
Interpreter::find_executable(request)
} else {
// `-p /home/ferris/.local/bin/python3.10`
Ok(fs_err::canonicalize(request)?)
Expand Down
1 change: 1 addition & 0 deletions crates/puffin/tests/pip_compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -696,6 +696,7 @@ fn compile_python_37() -> Result<()> {
----- stdout -----
----- stderr -----
warning: The requested Python version 3.7 is not available; 3.12.1 will be used to build dependencies instead.
× No solution found when resolving dependencies:
╰─▶ Because the requested Python version (3.7) does not satisfy Python>=3.8
and black==23.10.1 depends on Python>=3.8, we can conclude that
Expand Down
Loading

0 comments on commit 21577ad

Please sign in to comment.