Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ jobs:

- name: Install min requirements
run: |
uv pip install -e. --group=test --resolution=lowest-direct --system
uv pip install -e. --group=test-pybind11 --resolution=lowest-direct --system

- name: Setup CMake 3.15
uses: jwlawson/actions-setup-cmake@v2.0
Expand Down Expand Up @@ -223,7 +223,7 @@ jobs:
run: python3.13t -m venv /venv

- name: Install deps
run: /venv/bin/pip install -e . --group=test ninja
run: /venv/bin/pip install -e . --group=test-pybind11 ninja

- name: Test package
run: /venv/bin/pytest
Expand All @@ -246,7 +246,7 @@ jobs:
cmake ninja git make gcc-g++ python39 python39-devel python39-pip

- name: Install
run: python3.9 -m pip install . --group=test
run: python3.9 -m pip install . --group=test-pybind11

- name: Test package
run:
Expand Down Expand Up @@ -312,7 +312,7 @@ jobs:
persist-credentials: false

- name: Install
run: python -m pip install . --group=test
run: python -m pip install . --group=test-pybind11

- name: Test package
run: >-
Expand Down
10 changes: 4 additions & 6 deletions pyproject.toml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
[build-system]
requires = ["hatchling", "hatch-vcs"]
requires = ["hatchling >=1.24", "hatch-vcs >=0.4"]
build-backend = "hatchling.build"

[project]
Expand Down Expand Up @@ -77,6 +77,9 @@ hatch.scikit-build = "scikit_build_core.hatch.hooks"
test = [
"build >=0.8",
"cattrs >=22.2.0",
"filelock >=3.8.0",
"hatchling >=1.24",
"hatch-vcs >=0.4",
"pip>=23; python_version<'3.13'",
"pip>=24.1; python_version>='3.13'",
"pytest >=7.2",
Expand All @@ -89,10 +92,6 @@ test = [
"virtualenv >=20.20",
"wheel >=0.40",
]
test-hatchling = [
{ include-group = "test" },
"hatchling >=1.24.0",
]
test-meta = [
{ include-group = "test" },
"hatch-fancy-pypi-readme>=22.3",
Expand Down Expand Up @@ -120,7 +119,6 @@ cov = [
]
dev = [
{ include-group = "cov" },
{ include-group = "test-hatchling" },
{ include-group = "test-meta" },
{ include-group = "test-numpy" },
{ include-group = "test-pybind11" },
Expand Down
73 changes: 27 additions & 46 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
from typing import Any, Literal, overload

import virtualenv as _virtualenv
from filelock import FileLock

if sys.version_info < (3, 11):
import tomli as tomllib
Expand All @@ -26,6 +27,7 @@
from typing import TypeGuard


import download_wheels
import pytest
from packaging.requirements import Requirement
from packaging.version import Version
Expand All @@ -37,53 +39,32 @@


@pytest.fixture(scope="session")
def pep518_wheelhouse(tmp_path_factory: pytest.TempPathFactory) -> Path:
wheelhouse = tmp_path_factory.mktemp("wheelhouse")

subprocess.run(
[
sys.executable,
"-m",
"pip",
"wheel",
"--wheel-dir",
str(wheelhouse),
f"{BASE}",
],
check=True,
)
packages = [
"build",
"cython",
"hatchling",
"pip",
"setuptools",
"virtualenv",
"wheel",
]
def pep518_wheelhouse(pytestconfig: pytest.Config) -> Path:
wheelhouse = pytestconfig.cache.mkdir("wheelhouse")

main_lock = FileLock(wheelhouse / "main.lock")
with main_lock:
Copy link
Collaborator

@LecrisUT LecrisUT Dec 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you need to gate this by presence of scikit-build-core wheel.

Also don't think we need to 2 different locks?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not an editable wheel, so it needs to be rebuilt every time the tests run - here it's being rebuilt by every process, but that's hopefully pretty cheap. Probably cheaper if we use build isolation.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With two locks we can have one process start downloading wheels while the others are rebuilding the scikit-build-core wheel.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not an editable wheel, so it needs to be rebuilt every time the tests run

Wait I don't get this part. Isn't it building the wheel so that every subsequent pip imstall is taking that wheel? I am thinking about the guarding for across the workers, but maybe you are considering it across different commits? It should be checked across the hatch-vcs version, which I think you were trying to do previously?

With two locks we can have one process start downloading wheels while the others are rebuilding the scikit-build-core wheel.

Good point, but that would only happen if it skips the locked functions gated by main.lock right? But speaking of which is it re-using the pip/uv cache? Using a uv.lock or equivalent could make the downloads be effectively no-ops

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's testing the current source of scikit-build-core, so it has to be fresh. Yes, I'd like to only build it on one worker, but it's not that easy unless we make sure we always clean it up after a run (regardless of if the run failed, cancelled, etc).

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's testing the current source of scikit-build-core, so it has to be fresh. Yes, I'd like to only build it on one worker, but it's not that easy unless we make sure we always clean it up after a run (regardless of if the run failed, cancelled, etc).

I did some local debugging hoping that hatch vcs could help with the dirty files, but it seems like that would not work since it doesn't check the actual contents of the files when it is dirty, only the current date and if there are any dirty files.

The only alternative I can think of is to explicitly check against the worker_id and only run it on master, but are we able to do that without hard-coding pytest-xdist dependency?

subprocess.run(
[
sys.executable,
"-m",
"pip",
"wheel",
"--wheel-dir",
str(wheelhouse),
"--no-build-isolation",
f"{BASE}",
],
check=True,
capture_output=True,
text=True,
)

wheels_lock = FileLock(wheelhouse / "wheels.lock")
with wheels_lock:
if not all(list(wheelhouse.glob(f"{p}*.whl")) for p in download_wheels.WHEELS):
download_wheels.prepare(wheelhouse)

if importlib.util.find_spec("cmake") is not None:
packages.append("cmake")

if importlib.util.find_spec("ninja") is not None:
packages.append("ninja")

if importlib.util.find_spec("pybind11") is not None:
packages.append("pybind11")

subprocess.run(
[
sys.executable,
"-m",
"pip",
"download",
"-q",
"-d",
str(wheelhouse),
*packages,
],
check=True,
)
return wheelhouse


Expand Down
56 changes: 56 additions & 0 deletions tests/utils/download_wheels.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
# /// script
# dependencies = ["pip"]
# ///

"""
Download wheels into the pytest cache. Must be run from the pytest directory
(project root, usually). If run in an environment, requires pip. Only downloads
pybind11, ninja, or cmake if those are in the environment already.
"""

import importlib.util
import subprocess
import sys
from pathlib import Path

WHEELS = [
"build",
"cython",
"hatchling",
"pip",
"setuptools",
"virtualenv",
"wheel",
]

if importlib.util.find_spec("cmake") is not None:
WHEELS.append("cmake")

if importlib.util.find_spec("ninja") is not None:
WHEELS.append("ninja")

if importlib.util.find_spec("pybind11") is not None:
WHEELS.append("pybind11")


def prepare(wheelhouse: Path) -> None:
subprocess.run(
[
sys.executable,
"-m",
"pip",
"download",
"-q",
"-d",
str(wheelhouse),
*WHEELS,
],
check=True,
)
print(f"Downloaded wheels to {wheelhouse}")


if __name__ == "__main__":
wheelhouse = Path(".pytest_cache/d/wheelhouse")
wheelhouse.mkdir(parents=True, exist_ok=True)
prepare(wheelhouse)
Loading