Run `cargo test` on windows in CI, pulling the switch on tier 1 windows
support.
These changes make the bootstrap script virtually required for running
the tests. This gives us consistency between and CI, but it also locks
our tests to python-build-standalone and an articificial `PATH`.
I've deleted the shell bootstrap script in favor of only the python one,
which also runs on windows. I've left the (sym)link creation of the
bootstrap in place, even though it is not used by the tests anymore.
I've reactivated the three tests that would previously stack overflow by
doubling their stack sizes. The stack overflows only happen in debug
mode, so this is neither a user facing problem nor an actual problem
with our code and this workaround seems better than optimizing our code
for case that the (release) compiler can optimize much better for.
The handling of patch versions will be fixed in a follow-up PR.
Closes#1160Closes#1161
---------
Co-authored-by: Charlie Marsh <charlie.r.marsh@gmail.com>
## Summary
Enables tests for macOS in CI, using the M1 runners (which are free in
public, but count against our quota in private
repos). For now, I'm just running them on `main` to save quota.
I did the math, and the M1 runners are the best value:
Closes#1053.
## Summary
We have some flags in Puffin that enable us to opt-in to certain tests.
To date, they've been opt-in, so we've run our tests with
`--all-features`. This PR makes them opt-out, and we now run tests with
default features.
The main motivation here is that I want to get tests working for macOS
on CI, but for unknown reasons, macOS can't compile the PyO3 features at
the same time as everything else due to strange linker issues. By
avoiding `--all-features` for tests, we thus avoid unnecessarily
including features that we don't actually use in Puffin.
I verified that the exact same number of tests (439) are run before and
after this change. For users, the primary difference is that you now
need to specify `--no-default-features --features pypi --features
python` to avoid (e.g.) including the Git tests.
A 1:1 port of the Bash script to Python for use on Windows.
Pulls some parts of #1068 but much more minimal. Avoids an additional
dependency on `requests`. Because we require `zstandard` to unzip the
distributions we unfortunately cannot be dependency free and cannot have
`bootstrap.sh` download the Python version needed to run this script
without it doing a non-trivial amount of work.
Retains the Bash script for now so you can bootstrap without Python
available. I may drop it in the future?
Replaces https://github.com/astral-sh/puffin/pull/1068 and #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.
## Background
In virtual environments, we want to install python programs as console
commands, e.g. `black .` over `python -m black .`. They may be called
[entrypoints](https://packaging.python.org/en/latest/specifications/entry-points/)
or scripts. For entrypoints, we're given a module name and function to
call in that module.
On Unix, we generate a minimal python script launcher. Text files are
runnable on unix by adding a shebang at their top, e.g.
```python
#!/usr/bin/env python
```
will make the operating system run the file with the current python
interpreter. A venv launcher for black in `/home/ferris/colorize/.venv`
(module name: `black`, function to call: `patched_main`) would look like
this:
```python
#!/home/ferris/colorize/.venv/bin/python
# -*- coding: utf-8 -*-
import re
import sys
from black import patched_main
if __name__ == "__main__":
sys.argv[0] = re.sub(r"(-script\.pyw|\.exe)?$", "", sys.argv[0])
sys.exit(patched_main())
```
On windows, this doesn't work, we can only rely on launching `.exe`
files.
## Summary
We use posy's rust implementation of a trampoline, which is based on
distlib's c++ implementation. We pre-build a minimal exe and append the
launcher script as stored zip archive behind it. The exe will look for
the venv python interpreter next to it and use it to execute the
appended script.
The changes in this PR make the `black` entrypoint work:
```powershell
cargo run -- venv .venv
cargo run -q -- pip install black
.\.venv\Scripts\black --version
```
Integration with our existing tests will be done in follow-up PRs.
## Implementation and Details
I've vendored the posy trampoline crate. It is a formatted, renamed and
slightly changed for embedding version of
https://github.com/njsmith/posy/pull/28.
The posy launchers are smaller than the distlib launchers, 16K vs 106K
for black. Currently only `x86_64-pc-windows-msvc` is supported. The
crate requires a nightly compiler for its no-std binary size tricks.
On windows, an application can be launched with a console or without (to
create windows instead), which needs two different launchers. The gui
launcher will subsequently use `pythonw.exe` while the console launcher
uses `python.exe`.
Missing piece for the release.
## Test Plan
Built the image locally:
```shell
❯ docker run 99956098e1f8f04e209dcfc4a0afcee67df1fe8a726c164884e67f035b1a0f42
Usage: puffin [OPTIONS] <COMMAND>
Commands:
pip Resolve and install Python packages
venv Create a virtual environment
clean Clear the cache
help Print this message or the help of the given subcommand(s)
Options:
-q, --quiet Do not print any output
-v, --verbose Use verbose output
-n, --no-cache Avoid reading from or writing to the cache
--cache-dir <CACHE_DIR> Path to the cache directory [env: PUFFIN_CACHE_DIR=]
-h, --help Print help
-V, --version Print version
```
## Summary
This PR adds a release workflow powered by `cargo-dist`. It's similar to
the version that's PR'd in Ruff
(https://github.com/astral-sh/ruff/pull/9559), with the exception that
it doesn't include the Docker build or the "update dependents" step for
pre-commit.
Alternative to #875. Instead of partitioning tests across multiple
runners via nextest, we use a larger GitHub Actions runner.
Additionally, we explore using nextest to take advantage of the
increased number of cores.
On the 8-core machine, nextest is 22% faster than insta. In combination
with the vastly more readable output, I think this means we should
switch over. As noted in #875 we lose the ability to detect unreferenced
snapshot files but since we inline all of our snapshots this shouldn't
matter.
### Benchmarks
The following are the runtime of _just_ the test portion of the test job
in GitHub Actions except the partitioned case from #875 which requires a
separate build step making runner overhead relevant.
The compile times are noted as a reference as a possible lower bound of
test times. The compile time **is included** in all of the test times
shown.
Where the nextest thread count is not noted, it is inferred from the CPU
count.
```
test time diff
------------------------------------------------------
2-core (main) 4m 53s
2-core-nextest-partioned (#875) 3m 56s -19%
4-core-compilation 32s
4-core-insta 1m 47s -63%
4-core-nextest 1m 40s -66%
8-core-compilation 18s
8-core-insta 1m 9s -76%
8-core-nextest 1m 5s -78%
8-core-nextest-12-threads 54s -82%
8-core-nextest-16-threads 55s -82%
```
### Cost
We must pay per-minute costs for these runners:
> Larger runners are not eligible for the use of included minutes on
private repositories. For both private and public repositories, when
larger runners are in use, they will always be billed at the per-minute
rate.
>
> Compared to standard GitHub-hosted runners, larger runners are billed
differently. Larger runners are only billed at the per-minute rate for
the amount of time workflows are executed on them.
[[source]](https://docs.github.com/en/actions/using-github-hosted-runners/about-larger-runners/about-larger-runners#understanding-billing)
The per-minute rates are as follows:
> Linux 2 $0.008 (main)
> Linux 4 $0.016
> Linux 8 $0.032 (pull request)
[[source]](https://docs.github.com/en/billing/managing-billing-for-github-actions/about-billing-for-github-actions#per-minute-rates)
The per-minute cost increases by 4x but the workflow is 5.2x faster
since we are making use of the extra compute. We will not get any free
minutes executing these runners once the repository is public.
Additionally, we will not make use of our [3,000 minutes /
month](https://docs.github.com/en/billing/managing-billing-for-github-actions/about-billing-for-github-actions#included-storage-and-minutes)
of included minutes. Using the 8-core machines, the included 3,000
minutes should account for approximately ~$100.
Here's a brief analysis of costs from the last few
```
Minutes used
------------
November 1090 + 3000 = 4090
December 1357 + 3000 = 4357
January 2655 in 7 days
~3x more expected
= 11000 estimated
Costs
-----
November 1090 * 0.008 = $ 8.72
December 1357 * 0.008 = $10.86
January 8000 * 0.008 = $64 projected using 3000 included minutes and 2-core machines
(11000 - (0.82 * 11000)) * 0.032
= $63 projected without included minutes and 4-core machines with perf improvement
(11000 - (0.70 * 11000)) * 0.032
= $100 projected with a more conservative 70% reduction in total runtime
```
We can reduce costs (once public) by disabling larger runners for
non-organization users e.g.
https://github.com/PrefectHQ/prefect/pull/9519
Derived from https://github.com/astral-sh/puffin/pull/875
This gets us a significant speedup.
I would not read the commits individually. I can squash them but they
were used for testing various scenarios.
### Test compile times
Ranges are the lowest and highest I've seen. Huge variability in GitHub
Actions runners.
**Before:**
7m 21s - 8m 22s (cold cache)
110s - 120s (warm cache)
**After:**
6m 15s - 7m 05s (cold cache)
57s - 70s (warm cache)
**Improvement:**
4% - 25% (cold cache)
36% - 52% (warm cache)
Each cache entry is ~1 GB of our allotted 10 GB for the repository which
is quite a bit. We're probably losing cache entries all the time since
we add an entry per commit per pull request.
Saving the cache takes ~3 minutes
([example](https://github.com/astral-sh/puffin/actions/runs/7479909295/job/20358124969)),
it's probably just slowing down CI. It's ~25% of our test runtime and
~50% of our clippy runtime.
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
Zanie
konsti
Charlie Marsh