Merge #2826

2826: ci: run checks for all platforms on PR r=adamreichold a=davidhewitt

I've been struggling a little to merge PRs with the new bors workflow; overall I think the lighter PR workflow is better but the number of combinations of older pythons / platforms not covered makes it easy for the bors step to fail.

This PR tries to improve the situation by merging the `clippy` and `check-target` job and running it for all supported platforms on PR. Hopefully if these are green, then there's high likelihood that tests will build and should pass unless there's logic errors.

While creating this I found a build error on PyPy which made me notice we can support `PyList::get_item_unchecked` for PyPy, so I added it.

Co-authored-by: David Hewitt <1939362+davidhewitt@users.noreply.github.com>

.github/workflows/ci.yml

@@ -35,50 +35,82 @@ jobs:
         run: nox -s fmt-rust
 
   clippy:
-    if: github.ref != 'refs/heads/main'
-    runs-on: ubuntu-latest
-    steps:
-      - uses: actions/checkout@v3
-      - uses: actions/setup-python@v4
-      - run: pip install nox
-      - uses: dtolnay/rust-toolchain@stable
-        with:
-          components: clippy
-      - run: nox -s clippy
-
-  check-target:
     needs: [fmt]
-    runs-on: ubuntu-latest
-    if: ${{ github.event_name != 'pull_request' && github.ref != 'refs/heads/main' }}
+    runs-on: ${{ matrix.platform.os }}
+    if: github.ref != 'refs/heads/main'
     strategy:
       # If one platform fails, allow the rest to keep testing if `CI-no-fail-fast` label is present
       fail-fast: ${{ !contains(github.event.pull_request.labels.*.name, 'CI-no-fail-fast') }}
       matrix:
-        target: [powerpc64le-unknown-linux-gnu, s390x-unknown-linux-gnu, wasm32-wasi]
-    name: check-${{ matrix.target }}
+        rust: [stable]
+        platform: [
+          {
+            os: "macos-latest",
+            python-architecture: "x64",
+            rust-target: "x86_64-apple-darwin",
+          },
+          {
+            os: "ubuntu-latest",
+            python-architecture: "x64",
+            rust-target: "x86_64-unknown-linux-gnu",
+          },
+          {
+            os: "ubuntu-latest",
+            python-architecture: "x64",
+            rust-target: "powerpc64le-unknown-linux-gnu",
+          },
+          {
+            os: "ubuntu-latest",
+            python-architecture: "x64",
+            rust-target: "s390x-unknown-linux-gnu",
+          },
+          {
+            os: "ubuntu-latest",
+            python-architecture: "x64",
+            rust-target: "wasm32-wasi",
+          },
+          {
+            os: "windows-latest",
+            python-architecture: "x64",
+            rust-target: "x86_64-pc-windows-msvc",
+          },
+          {
+            os: "windows-latest",
+            python-architecture: "x86",
+            rust-target: "i686-pc-windows-msvc",
+          },
+        ]
+        include:
+          - rust: 1.48.0
+            python-version: "3.11"
+            platform:
+              {
+                os: "ubuntu-latest",
+                python-architecture: "x64",
+                rust-target: "x86_64-unknown-linux-gnu",
+              }
+            msrv: "MSRV"
+    name: clippy/${{ matrix.platform.rust-target }}/${{ matrix.rust }}
     steps:
       - uses: actions/checkout@v3
-      - name: Install Rust toolchain
-        uses: dtolnay/rust-toolchain@stable
+      - uses: dtolnay/rust-toolchain@master
         with:
-          targets: ${{ matrix.target }}
-      - name: Run cargo checks
-        run: |
-          set -x
-          VERSIONS=("3.7" "3.8" "3.9" "3.10" "3.11")
-          for VERSION in ${VERSIONS[@]}; do
-            echo "version=$VERSION" > config.txt
-            echo "suppress_build_script_link_lines=true" >> config.txt
-            PYO3_BUILD_CONFIG=$(pwd)/config.txt cargo check --all-targets --no-default-features
-            PYO3_BUILD_CONFIG=$(pwd)/config.txt cargo check --all-targets --no-default-features --features "abi3"
-            PYO3_BUILD_CONFIG=$(pwd)/config.txt cargo check --all-targets --features "full multiple-pymethods"
-            PYO3_BUILD_CONFIG=$(pwd)/config.txt cargo check --all-targets --features "abi3 full multiple-pymethods"
-          done
+          toolchain: ${{ matrix.rust }}
+          targets: ${{ matrix.rust-target }}
+          components: clippy
+      - uses: actions/setup-python@v4
+        with:
+          architecture: ${{ matrix.platform.python-architecture }}
+      - run: python -m pip install nox
+      - if: matrix.msrv == 'MSRV'
+        name: Prepare minimal package versions (MSRV only)
+        run: nox -s set-minimal-package-versions
+      - run: nox -s clippy-all
 
   build-pr:
     if: github.event_name == 'pull_request'
     name: python${{ matrix.python-version }}-${{ matrix.platform.python-architecture }} ${{ matrix.platform.os }} rust-${{ matrix.rust }}
-    needs: [fmt] # don't wait for clippy as fails rarely and takes longer
+    needs: [fmt]
     uses: ./.github/workflows/build.yml
     with:
       os: ${{ matrix.platform.os }}
@@ -114,12 +146,16 @@ jobs:
               python-architecture: "x64",
               rust-target: "x86_64-pc-windows-msvc",
             },
+            {
+              os: "windows-latest",
+              python-architecture: "x86",
+              rust-target: "i686-pc-windows-msvc",
+            }
           ]
-
   build-full:
     if: ${{ github.event_name != 'pull_request' && github.ref != 'refs/heads/main' }}
     name: python${{ matrix.python-version }}-${{ matrix.platform.python-architecture }} ${{ matrix.platform.os }} rust-${{ matrix.rust }}
-    needs: [fmt] # don't wait for clippy as fails rarely and takes longer
+    needs: [fmt]
     uses: ./.github/workflows/build.yml
     with:
       os: ${{ matrix.platform.os }}
@@ -317,7 +353,6 @@ jobs:
     needs:
       - fmt
       - clippy
-      - check-target
       - build-pr
       - build-full
       - valgrind

benches/bench_intern.rs

@@ -2,11 +2,9 @@ use criterion::{criterion_group, criterion_main, Bencher, Criterion};
 
 use pyo3::prelude::*;
 
-use pyo3::{intern, prepare_freethreaded_python};
+use pyo3::intern;
 
 fn getattr_direct(b: &mut Bencher<'_>) {
-    prepare_freethreaded_python();
-
     Python::with_gil(|py| {
         let sys = py.import("sys").unwrap();
 
@@ -15,8 +13,6 @@ fn getattr_direct(b: &mut Bencher<'_>) {
 }
 
 fn getattr_intern(b: &mut Bencher<'_>) {
-    prepare_freethreaded_python();
-
     Python::with_gil(|py| {
         let sys = py.import("sys").unwrap();
 

benches/bench_tuple.rs

@@ -37,7 +37,7 @@ fn tuple_get_item(b: &mut Bencher<'_>) {
     });
 }
 
-#[cfg(not(Py_LIMITED_API))]
+#[cfg(not(any(Py_LIMITED_API, PyPy)))]
 fn tuple_get_item_unchecked(b: &mut Bencher<'_>) {
     Python::with_gil(|py| {
         const LEN: usize = 50_000;
@@ -57,7 +57,7 @@ fn criterion_benchmark(c: &mut Criterion) {
     c.bench_function("iter_tuple", iter_tuple);
     c.bench_function("tuple_new", tuple_new);
     c.bench_function("tuple_get_item", tuple_get_item);
-    #[cfg(not(Py_LIMITED_API))]
+    #[cfg(not(any(Py_LIMITED_API, PyPy)))]
     c.bench_function("tuple_get_item_unchecked", tuple_get_item_unchecked);
 }
 

newsfragments/2826.added.md

@@ -0,0 +1 @@
+Add `PyList::get_item_unchecked` for PyPy.

noxfile.py

@@ -5,9 +5,10 @@ import subprocess
 import sys
 import tempfile
 import time
+from functools import lru_cache
 from glob import glob
 from pathlib import Path
-from typing import Any, Dict, List, Optional
+from typing import Any, Dict, List, Optional, Tuple
 
 import nox
 
@@ -15,6 +16,8 @@ nox.options.sessions = ["test", "clippy", "fmt"]
 
 
 PYO3_DIR = Path(__file__).parent
+PY_VERSIONS = ("3.7", "3.8", "3.9", "3.10", "3.11")
+PYPY_VERSIONS = ("3.7", "3.8", "3.9")
 
 
 @nox.session(venv_backend="none")
@@ -83,20 +86,71 @@ def fmt_py(session: nox.Session):
     _run(session, "black", ".", "--check")
 
 
-@nox.session(venv_backend="none")
-def clippy(session: nox.Session) -> None:
-    for feature_set in ["full", "abi3 full"]:
-        _run(
-            session,
-            "cargo",
-            "clippy",
-            f"--features={feature_set}",
-            "--all-targets",
-            "--workspace",
-            "--",
-            "--deny=warnings",
-            external=True,
-        )
+@nox.session(name="clippy", venv_backend="none")
+def clippy(session: nox.Session) -> bool:
+    if not _clippy(session):
+        session.error("one or more jobs failed")
+
+
+def _clippy(session: nox.Session, *, env: Dict[str, str] = None) -> bool:
+    success = True
+    env = env or os.environ
+    for feature_set in _get_feature_sets():
+        command = "clippy"
+        extra = ("--", "--deny=warnings")
+        if _get_rust_version()[:2] == (1, 48):
+            # 1.48 crashes during clippy because of lints requested
+            # in .cargo/config
+            command = "check"
+            extra = ()
+        try:
+            _run(
+                session,
+                "cargo",
+                command,
+                *feature_set,
+                "--all-targets",
+                "--workspace",
+                *extra,
+                external=True,
+                env=env,
+            )
+        except Exception:
+            success = False
+    return success
+
+
+@nox.session(name="clippy-all", venv_backend="none")
+def clippy_all(session: nox.Session) -> None:
+    success = True
+    with tempfile.NamedTemporaryFile("r+") as config:
+        env = os.environ.copy()
+        env["PYO3_CONFIG_FILE"] = config.name
+        env["PYO3_CI"] = "1"
+
+        def _clippy_with_config(implementation, version) -> bool:
+            config.seek(0)
+            config.truncate(0)
+            config.write(
+                f"""\
+implementation={implementation}
+version={version}
+suppress_build_script_link_lines=true
+"""
+            )
+            config.flush()
+
+            session.log(f"{implementation} {version}")
+            return _clippy(session, env=env)
+
+        for version in PY_VERSIONS:
+            success &= _clippy_with_config("CPython", version)
+
+        for version in PYPY_VERSIONS:
+            success &= _clippy_with_config("PyPy", version)
+
+    if not success:
+        session.error("one or more jobs failed")
 
 
 @nox.session(venv_backend="none")
@@ -396,14 +450,56 @@ def set_minimal_package_versions(session: nox.Session):
     _run(session, "cargo", "metadata", silent=True, external=True)
 
 
-def _get_rust_target() -> str:
+@lru_cache()
+def _get_rust_info() -> Tuple[str, ...]:
     output = _get_output("rustc", "-vV")
 
-    for line in output.splitlines():
+    return tuple(output.splitlines())
+
+
+def _get_rust_version() -> Tuple[int, int, int, List[str]]:
+    for line in _get_rust_info():
+        if line.startswith(_RELEASE_LINE_START):
+            version = line[len(_RELEASE_LINE_START) :].strip()
+            # e.g. 1.67.0-beta.2
+            (version_number, *extra) = version.split("-", maxsplit=1)
+            return (*map(int, version_number.split(".")), extra)
+
+
+def _get_rust_target() -> str:
+    for line in _get_rust_info():
         if line.startswith(_HOST_LINE_START):
             return line[len(_HOST_LINE_START) :].strip()
 
 
+@lru_cache()
+def _get_feature_sets() -> Tuple[Tuple[str, ...], ...]:
+    """Returns feature sets to use for clippy job"""
+    rust_version = _get_rust_version()
+    if rust_version[:2] >= (1, 62):
+        # multiple-pymethods feature not supported before 1.62
+        return (
+            ("--no-default-features",),
+            (
+                "--no-default-features",
+                "--features=abi3",
+            ),
+            ("--features=full multiple-pymethods",),
+            ("--features=abi3 full multiple-pymethods",),
+        )
+    else:
+        return (
+            ("--no-default-features",),
+            (
+                "--no-default-features",
+                "--features=abi3",
+            ),
+            ("--features=full",),
+            ("--features=abi3 full",),
+        )
+
+
+_RELEASE_LINE_START = "release: "
 _HOST_LINE_START = "host: "