ci: minor cleanups following 1.63 MSRV (#4239)

* ci: minor cleanups following 1.63 MSRV * correct `invalid_pymethods_duplicates` UI test * fix `nightly` feature
PyO3 · Jun 21, 2024 · ca82681 · ca82681
1 parent a983b2f
commit ca82681
Show file tree

Hide file tree

Showing 11 changed files with 31 additions and 126 deletions.
diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml
@@ -16,9 +16,6 @@ on:
       rust-target:
         required: true
         type: string
-      extra-features:
-        required: true
-        type: string
 
 jobs:
   build:
@@ -62,6 +59,10 @@ jobs:
         name: Ignore changed error messages when using trybuild
         run: echo "TRYBUILD=overwrite" >> "$GITHUB_ENV"
 
+      - if: inputs.rust == 'nightly'
+        name: Prepare to test on nightly rust
+        run: echo "MAYBE_NIGHTLY=nightly" >> "$GITHUB_ENV"
+
       - name: Build docs
         run: nox -s docs
 
@@ -88,26 +89,31 @@ jobs:
 
       - name: Build (all additive features)
         if: ${{ !startsWith(inputs.python-version, 'graalpy') }}
-        run: cargo build --lib --tests --no-default-features --features "full ${{ inputs.extra-features }}"
+        run: cargo build --lib --tests --no-default-features --features "multiple-pymethods full $MAYBE_NIGHTLY"
 
       - if: ${{ startsWith(inputs.python-version, 'pypy') }}
         name: Build PyPy (abi3-py37)
-        run: cargo build --lib --tests --no-default-features --features "abi3-py37 full ${{ inputs.extra-features }}"
+        run: cargo build --lib --tests --no-default-features --features "multiple-pymethods abi3-py37 full $MAYBE_NIGHTLY"
 
       # Run tests (except on PyPy, because no embedding API).
       - if: ${{ !startsWith(inputs.python-version, 'pypy') && !startsWith(inputs.python-version, 'graalpy') }}
         name: Test
-        run: cargo test --no-default-features --features "full ${{ inputs.extra-features }}"
+        run: cargo test --no-default-features --features "full $MAYBE_NIGHTLY"
+
+      # Repeat, with multiple-pymethods feature enabled (it's not entirely additive)
+      - if: ${{ !startsWith(inputs.python-version, 'pypy') && !startsWith(inputs.python-version, 'graalpy') }}
+        name: Test
+        run: cargo test --no-default-features --features "multiple-pymethods full $MAYBE_NIGHTLY"
 
       # Run tests again, but in abi3 mode
       - if: ${{ !startsWith(inputs.python-version, 'pypy') && !startsWith(inputs.python-version, 'graalpy') }}
         name: Test (abi3)
-        run: cargo test --no-default-features --features "abi3 full ${{ inputs.extra-features }}"
+        run: cargo test --no-default-features --features "multiple-pymethods abi3 full $MAYBE_NIGHTLY"
 
       # Run tests again, for abi3-py37 (the minimal Python version)
       - if: ${{ (!startsWith(inputs.python-version, 'pypy') && !startsWith(inputs.python-version, 'graalpy')) && (inputs.python-version != '3.7') }}
         name: Test (abi3-py37)
-        run: cargo test --no-default-features --features "abi3-py37 full ${{ inputs.extra-features }}"
+        run: cargo test --no-default-features --features "multiple-pymethods abi3-py37 full $MAYBE_NIGHTLY"
 
       - name: Test proc-macro code
         run: cargo test --manifest-path=pyo3-macros-backend/Cargo.toml

diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -149,13 +149,11 @@ jobs:
       python-architecture: ${{ matrix.platform.python-architecture }}
       rust: ${{ matrix.rust }}
       rust-target: ${{ matrix.platform.rust-target }}
-      extra-features: ${{ matrix.platform.extra-features }}
     secrets: inherit
     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:
-        extra-features: ["multiple-pymethods"]
         rust: [stable]
         python-version: ["3.12"]
         platform:
@@ -197,7 +195,6 @@ jobs:
                 python-architecture: "x64",
                 rust-target: "x86_64-unknown-linux-gnu",
               }
-            extra-features: "nightly multiple-pymethods"
   build-full:
     if: ${{ contains(github.event.pull_request.labels.*.name, 'CI-build-full') || github.event_name != 'pull_request' }}
     name: python${{ matrix.python-version }}-${{ matrix.platform.python-architecture }} ${{ matrix.platform.os }} rust-${{ matrix.rust }}
@@ -209,13 +206,11 @@ jobs:
       python-architecture: ${{ matrix.platform.python-architecture }}
       rust: ${{ matrix.rust }}
       rust-target: ${{ matrix.platform.rust-target }}
-      extra-features: ${{ matrix.platform.extra-features }}
     secrets: inherit
     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:
-        extra-features: ["multiple-pymethods"]  # Because MSRV doesn't support this
         rust: [stable]
         python-version: [
           "3.7",
@@ -264,7 +259,6 @@ jobs:
                 python-architecture: "x64",
                 rust-target: "x86_64-unknown-linux-gnu",
               }
-            extra-features: ""
 
           # Test the `nightly` feature
           - rust: nightly
@@ -275,7 +269,6 @@ jobs:
                 python-architecture: "x64",
                 rust-target: "x86_64-unknown-linux-gnu",
               }
-            extra-features: "nightly multiple-pymethods"
 
           # Run rust beta to help catch toolchain regressions
           - rust: beta
@@ -286,7 +279,6 @@ jobs:
                 python-architecture: "x64",
                 rust-target: "x86_64-unknown-linux-gnu",
               }
-            extra-features: "multiple-pymethods"
 
           # Test 32-bit Windows only with the latest Python version
           - rust: stable
@@ -297,7 +289,6 @@ jobs:
                 python-architecture: "x86",
                 rust-target: "i686-pc-windows-msvc",
               }
-            extra-features: "multiple-pymethods"
 
           # test arm macos runner with the latest Python version
           # NB: if the full matrix switchess to arm, switch to x86_64 here
@@ -309,7 +300,6 @@ jobs:
                 python-architecture: "arm64",
                 rust-target: "aarch64-apple-darwin",
               }
-            extra-features: "multiple-pymethods"
 
   valgrind:
     if: ${{ contains(github.event.pull_request.labels.*.name, 'CI-build-full') || github.event_name != 'pull_request' }}

diff --git a/Cargo.toml b/Cargo.toml
@@ -116,7 +116,7 @@ nightly = []
 # This is mostly intended for testing purposes - activating *all* of these isn't particularly useful.
 full = [
     "macros",
-    # "multiple-pymethods", # TODO re-add this when MSRV is greater than 1.62
+    # "multiple-pymethods", # Not supported by wasm
     "anyhow",
     "chrono",
     "chrono-tz",

diff --git a/guide/src/features.md b/guide/src/features.md
@@ -95,9 +95,9 @@ These macros require a number of dependencies which may not be needed by users w
 
 ### `multiple-pymethods`
 
-This feature enables a dependency on `inventory`, which enables each `#[pyclass]` to have more than one `#[pymethods]` block. This feature also requires a minimum Rust version of 1.62 due to limitations in the `inventory` crate.
+This feature enables each `#[pyclass]` to have more than one `#[pymethods]` block.
 
-Most users should only need a single `#[pymethods]` per `#[pyclass]`. In addition, not all platforms (e.g. Wasm) are supported by `inventory`. For this reason this feature is not enabled by default, meaning fewer dependencies and faster compilation for the majority of users.
+Most users should only need a single `#[pymethods]` per `#[pyclass]`. In addition, not all platforms (e.g. Wasm) are supported by `inventory`, which is used in the implementation of the feature. For this reason this feature is not enabled by default, meaning fewer dependencies and faster compilation for the majority of users.
 
 See [the `#[pyclass]` implementation details](class.md#implementation-details) for more information.
 

diff --git a/noxfile.py b/noxfile.py
@@ -663,7 +663,7 @@ def check_feature_powerset(session: nox.Session):
         "default",
         "auto-initialize",
         "generate-import-lib",
-        "multiple-pymethods",  # TODO add this after MSRV 1.62
+        "multiple-pymethods",  # Because it's not supported on wasm
     }
 
     features = cargo_toml["features"]
@@ -764,10 +764,9 @@ def _get_rust_default_target() -> str:
 @lru_cache()
 def _get_feature_sets() -> Tuple[Tuple[str, ...], ...]:
     """Returns feature sets to use for clippy job"""
-    rust_version = _get_rust_version()
     cargo_target = os.getenv("CARGO_BUILD_TARGET", "")
-    if rust_version[:2] >= (1, 62) and "wasm32-wasi" not in cargo_target:
-        # multiple-pymethods feature not supported before 1.62 or on WASI
+    if "wasm32-wasi" not in cargo_target:
+        # multiple-pymethods not supported on wasm
         return (
             ("--no-default-features",),
             (

diff --git a/pyo3-ffi/src/methodobject.rs b/pyo3-ffi/src/methodobject.rs
@@ -186,9 +186,8 @@ impl std::fmt::Pointer for PyMethodDefPointer {
     }
 }
 
-// TODO: This can be a const assert on Rust 1.57
 const _: () =
-    [()][mem::size_of::<PyMethodDefPointer>() - mem::size_of::<Option<extern "C" fn()>>()];
+    assert!(mem::size_of::<PyMethodDefPointer>() == mem::size_of::<Option<extern "C" fn()>>());
 
 #[cfg(not(Py_3_9))]
 extern "C" {

diff --git a/src/conversions/chrono.rs b/src/conversions/chrono.rs
@@ -19,9 +19,6 @@
 //! # Example: Convert a `datetime.datetime` to chrono's `DateTime<Utc>`
 //!
 //! ```rust
-//! # // `chrono::Duration` has been renamed to `chrono::TimeDelta` and its constructors changed
-//! # // TODO: upgrade to Chrono 0.4.35+ after upgrading our MSRV to 1.61+
-//! # #![allow(deprecated)]
 //! use chrono::{DateTime, Duration, TimeZone, Utc};
 //! use pyo3::{Python, ToPyObject};
 //!
@@ -43,10 +40,6 @@
 //! }
 //! ```
 
-// `chrono::Duration` has been renamed to `chrono::TimeDelta` and its constructors changed
-// TODO: upgrade to Chrono 0.4.35+ after upgrading our MSRV to 1.61+
-#![allow(deprecated)]
-
 use crate::exceptions::{PyTypeError, PyUserWarning, PyValueError};
 #[cfg(Py_LIMITED_API)]
 use crate::sync::GILOnceCell;

diff --git a/src/conversions/std/array.rs b/src/conversions/std/array.rs
@@ -75,7 +75,7 @@ where
 }
 
 // TODO use std::array::try_from_fn, if that stabilises:
-// (https://github.com/rust-lang/rust/pull/75644)
+// (https://github.com/rust-lang/rust/issues/89379)
 fn array_try_from_fn<E, F, T, const N: usize>(mut cb: F) -> Result<[T; N], E>
 where
     F: FnMut(usize) -> Result<T, E>,

diff --git a/src/marker.rs b/src/marker.rs
@@ -281,6 +281,7 @@ mod nightly {
 
     // All the borrowing wrappers
     #[allow(deprecated)]
+    #[cfg(feature = "gil-refs")]
     impl<T> !Ungil for crate::PyCell<T> {}
     impl<T> !Ungil for crate::PyRef<'_, T> {}
     impl<T> !Ungil for crate::PyRefMut<'_, T> {}

diff --git a/tests/ui/invalid_pymethods_duplicates.rs b/tests/ui/invalid_pymethods_duplicates.rs
@@ -3,6 +3,7 @@
 
 use pyo3::prelude::*;
 
+#[pyclass]
 struct TwoNew {}
 
 #[pymethods]
@@ -18,6 +19,7 @@ impl TwoNew {
     }
 }
 
+#[pyclass]
 struct DuplicateMethod {}
 
 #[pymethods]

diff --git a/tests/ui/invalid_pymethods_duplicates.stderr b/tests/ui/invalid_pymethods_duplicates.stderr
@@ -1,35 +1,18 @@
 error[E0119]: conflicting implementations of trait `pyo3::impl_::pyclass::PyClassNewTextSignature<TwoNew>` for type `pyo3::impl_::pyclass::PyClassImplCollector<TwoNew>`
- --> tests/ui/invalid_pymethods_duplicates.rs:8:1
+ --> tests/ui/invalid_pymethods_duplicates.rs:9:1
   |
-8 | #[pymethods]
+9 | #[pymethods]
   | ^^^^^^^^^^^^
   | |
   | first implementation here
   | conflicting implementation for `pyo3::impl_::pyclass::PyClassImplCollector<TwoNew>`
   |
   = note: this error originates in the attribute macro `pymethods` (in Nightly builds, run with -Z macro-backtrace for more info)
 
-error[E0277]: the trait bound `TwoNew: PyTypeInfo` is not satisfied
- --> tests/ui/invalid_pymethods_duplicates.rs:9:6
-  |
-9 | impl TwoNew {
-  |      ^^^^^^ the trait `PyTypeInfo` is not implemented for `TwoNew`
-  |
-  = help: the following other types implement trait `PyTypeInfo`:
-            CancelledError
-            IncompleteReadError
-            InvalidStateError
-            LimitOverrunError
-            PanicException
-            PyAny
-            PyArithmeticError
-            PyAssertionError
-          and $N others
-
 error[E0592]: duplicate definitions with name `__pymethod___new____`
- --> tests/ui/invalid_pymethods_duplicates.rs:8:1
+ --> tests/ui/invalid_pymethods_duplicates.rs:9:1
   |
-8 | #[pymethods]
+9 | #[pymethods]
   | ^^^^^^^^^^^^
   | |
   | duplicate definitions for `__pymethod___new____`
@@ -38,80 +21,12 @@ error[E0592]: duplicate definitions with name `__pymethod___new____`
   = note: this error originates in the attribute macro `pymethods` (in Nightly builds, run with -Z macro-backtrace for more info)
 
 error[E0592]: duplicate definitions with name `__pymethod_func__`
-  --> tests/ui/invalid_pymethods_duplicates.rs:23:1
+  --> tests/ui/invalid_pymethods_duplicates.rs:25:1
    |
-23 | #[pymethods]
+25 | #[pymethods]
    | ^^^^^^^^^^^^
    | |
    | duplicate definitions for `__pymethod_func__`
    | other definition for `__pymethod_func__`
    |
    = note: this error originates in the attribute macro `pymethods` (in Nightly builds, run with -Z macro-backtrace for more info)
-
-error[E0277]: the trait bound `TwoNew: PyClass` is not satisfied
- --> tests/ui/invalid_pymethods_duplicates.rs:8:1
-  |
-8 | #[pymethods]
-  | ^^^^^^^^^^^^ the trait `PyClass` is not implemented for `TwoNew`
-  |
-  = help: the trait `PyClass` is implemented for `pyo3::coroutine::Coroutine`
-note: required by a bound in `PyClassInitializer`
- --> src/pyclass_init.rs
-  |
-  | pub struct PyClassInitializer<T: PyClass>(PyClassInitializerImpl<T>);
-  |                                  ^^^^^^^ required by this bound in `PyClassInitializer`
-  = note: this error originates in the attribute macro `pymethods` (in Nightly builds, run with -Z macro-backtrace for more info)
-
-error[E0599]: no method named `convert` found for struct `TwoNew` in the current scope
- --> tests/ui/invalid_pymethods_duplicates.rs:8:1
-  |
-6 | struct TwoNew {}
-  | ------------- method `convert` not found for this struct
-7 |
-8 | #[pymethods]
-  | ^^^^^^^^^^^^ method not found in `TwoNew`
-  |
-  = help: items from traits can only be used if the trait is implemented and in scope
-  = note: the following trait defines an item `convert`, perhaps you need to implement it:
-          candidate #1: `IntoPyCallbackOutput`
-  = note: this error originates in the attribute macro `pymethods` (in Nightly builds, run with -Z macro-backtrace for more info)
-
-error[E0277]: the trait bound `DuplicateMethod: PyClass` is not satisfied
-  --> tests/ui/invalid_pymethods_duplicates.rs:26:15
-   |
-26 |     fn func_a(&self) {}
-   |               ^ the trait `PyClass` is not implemented for `DuplicateMethod`
-   |
-   = help: the trait `PyClass` is implemented for `pyo3::coroutine::Coroutine`
-note: required by a bound in `extract_pyclass_ref`
-  --> src/impl_/extract_argument.rs
-   |
-   | pub fn extract_pyclass_ref<'a, 'py: 'a, T: PyClass>(
-   |                                            ^^^^^^^ required by this bound in `extract_pyclass_ref`
-
-error[E0277]: the trait bound `DuplicateMethod: PyClass` is not satisfied
-  --> tests/ui/invalid_pymethods_duplicates.rs:23:1
-   |
-23 | #[pymethods]
-   | ^^^^^^^^^^^^ the trait `PyClass` is not implemented for `DuplicateMethod`
-   |
-   = help: the trait `PyClass` is implemented for `pyo3::coroutine::Coroutine`
-note: required by a bound in `PyRef`
-  --> src/pycell.rs
-   |
-   | pub struct PyRef<'p, T: PyClass> {
-   |                         ^^^^^^^ required by this bound in `PyRef`
-   = note: this error originates in the attribute macro `pymethods` (in Nightly builds, run with -Z macro-backtrace for more info)
-
-error[E0277]: the trait bound `DuplicateMethod: PyClass` is not satisfied
-  --> tests/ui/invalid_pymethods_duplicates.rs:29:15
-   |
-29 |     fn func_b(&self) {}
-   |               ^ the trait `PyClass` is not implemented for `DuplicateMethod`
-   |
-   = help: the trait `PyClass` is implemented for `pyo3::coroutine::Coroutine`
-note: required by a bound in `extract_pyclass_ref`
-  --> src/impl_/extract_argument.rs
-   |
-   | pub fn extract_pyclass_ref<'a, 'py: 'a, T: PyClass>(
-   |                                            ^^^^^^^ required by this bound in `extract_pyclass_ref`