From 3447f0e77da744679e28036d8e98d35550ad6eca Mon Sep 17 00:00:00 2001 From: David Hewitt Date: Thu, 22 Feb 2024 08:05:37 +0000 Subject: [PATCH] fix `either` feature conditional compilation, again (#3834) * fix `either` feature conditional compilation, again * test feature powerset in CI * install `rust-src` for feature powerset tests * review: adamreichold feedback * Fix one more case of redundant imports. * just check feature powerset for now --------- Co-authored-by: Adam Reichold --- .github/workflows/ci.yml | 17 ++++++++ Cargo.toml | 16 ++++---- newsfragments/3834.fixed.md | 1 + noxfile.py | 82 +++++++++++++++++++++++++++++++++++-- pytests/src/comparisons.rs | 1 - src/conversions/either.rs | 10 ++++- 6 files changed, 112 insertions(+), 15 deletions(-) create mode 100644 newsfragments/3834.fixed.md diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 849c63a459d..19363fd8968 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -459,6 +459,22 @@ jobs: - run: python3 -m pip install --upgrade pip && pip install nox - run: python3 -m nox -s test-version-limits + check-feature-powerset: + needs: [fmt] + if: ${{ contains(github.event.pull_request.labels.*.name, 'CI-build-full') || (github.event_name != 'pull_request' && github.ref != 'refs/heads/main') }} + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-python@v5 + - uses: Swatinem/rust-cache@v2 + continue-on-error: true + - uses: dtolnay/rust-toolchain@stable + with: + components: rust-src + - uses: taiki-e/install-action@cargo-hack + - run: python3 -m pip install --upgrade pip && pip install nox + - run: python3 -m nox -s check-feature-powerset + conclusion: needs: - fmt @@ -473,6 +489,7 @@ jobs: - emscripten - test-debug - test-version-limits + - check-feature-powerset if: always() runs-on: ubuntu-latest steps: diff --git a/Cargo.toml b/Cargo.toml index b9a75791669..930a003bd8e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -103,18 +103,18 @@ nightly = [] full = [ "macros", # "multiple-pymethods", # TODO re-add this when MSRV is greater than 1.62 + "anyhow", "chrono", - "num-bigint", - "num-complex", - "hashbrown", - "smallvec", - "serde", - "indexmap", "either", - "eyre", - "anyhow", "experimental-inspect", + "eyre", + "hashbrown", + "indexmap", + "num-bigint", + "num-complex", "rust_decimal", + "serde", + "smallvec", ] [workspace] diff --git a/newsfragments/3834.fixed.md b/newsfragments/3834.fixed.md new file mode 100644 index 00000000000..fa77e84f36e --- /dev/null +++ b/newsfragments/3834.fixed.md @@ -0,0 +1 @@ +Fix compilation failure with `either` feature enabled without `experimental-inspect` enabled. diff --git a/noxfile.py b/noxfile.py index 73f2bc85068..87a5011e7f6 100644 --- a/noxfile.py +++ b/noxfile.py @@ -13,6 +13,14 @@ import nox import nox.command +try: + import tomllib as toml +except ImportError: + try: + import toml + except ImportError: + toml = None + nox.options.sessions = ["test", "clippy", "rustfmt", "ruff", "docs"] @@ -467,10 +475,8 @@ def check_changelog(session: nox.Session): def set_minimal_package_versions(session: nox.Session): from collections import defaultdict - try: - import tomllib as toml - except ImportError: - import toml + if toml is None: + session.error("requires Python 3.11 or `toml` to be installed") projects = ( None, @@ -581,6 +587,74 @@ def test_version_limits(session: nox.Session): _run_cargo(session, "check", env=env, expect_error=True) +@nox.session(name="check-feature-powerset", venv_backend="none") +def check_feature_powerset(session: nox.Session): + if toml is None: + session.error("requires Python 3.11 or `toml` to be installed") + + with (PYO3_DIR / "Cargo.toml").open("rb") as cargo_toml_file: + cargo_toml = toml.load(cargo_toml_file) + + EXCLUDED_FROM_FULL = { + "nightly", + "extension-module", + "full", + "default", + "auto-initialize", + "generate-import-lib", + "multiple-pymethods", # TODO add this after MSRV 1.62 + } + + features = cargo_toml["features"] + + full_feature = set(features["full"]) + abi3_features = {feature for feature in features if feature.startswith("abi3")} + abi3_version_features = abi3_features - {"abi3"} + + expected_full_feature = features.keys() - EXCLUDED_FROM_FULL - abi3_features + + uncovered_features = expected_full_feature - full_feature + if uncovered_features: + session.error( + f"some features missing from `full` meta feature: {uncovered_features}" + ) + + experimental_features = { + feature for feature in features if feature.startswith("experimental-") + } + full_without_experimental = full_feature - experimental_features + + if len(experimental_features) >= 2: + # justification: we always assume that feature within these groups are + # mutually exclusive to simplify CI + features_to_group = [ + full_without_experimental, + experimental_features, + ] + elif len(experimental_features) == 1: + # no need to make an experimental features group + features_to_group = [full_without_experimental] + else: + session.error("no experimental features exist; please simplify the noxfile") + + features_to_skip = [ + *EXCLUDED_FROM_FULL, + *abi3_version_features, + ] + + comma_join = ",".join + _run_cargo( + session, + "hack", + "--feature-powerset", + '--optional-deps=""', + f'--skip="{comma_join(features_to_skip)}"', + *(f"--group-features={comma_join(group)}" for group in features_to_group), + "check", + "--all-targets", + ) + + def _build_docs_for_ffi_check(session: nox.Session) -> None: # pyo3-ffi-check needs to scrape docs of pyo3-ffi _run_cargo(session, "doc", _FFI_CHECK, "-p", "pyo3-ffi", "--no-deps") diff --git a/pytests/src/comparisons.rs b/pytests/src/comparisons.rs index d8c2f5a6a52..b3ba293186a 100644 --- a/pytests/src/comparisons.rs +++ b/pytests/src/comparisons.rs @@ -1,5 +1,4 @@ use pyo3::prelude::*; -use pyo3::{types::PyModule, Python}; #[pyclass] struct Eq(i64); diff --git a/src/conversions/either.rs b/src/conversions/either.rs index 759b282e416..2fb1f23168d 100644 --- a/src/conversions/either.rs +++ b/src/conversions/either.rs @@ -93,7 +93,13 @@ where } else if let Ok(r) = obj.extract::() { Ok(Either::Right(r)) } else { - let err_msg = format!("failed to convert the value to '{}'", Self::type_input()); + // TODO: it might be nice to use the `type_input()` name here once `type_input` + // is not experimental, rather than the Rust type names. + let err_msg = format!( + "failed to convert the value to 'Union[{}, {}]'", + std::any::type_name::(), + std::any::type_name::() + ); Err(PyTypeError::new_err(err_msg)) } } @@ -133,7 +139,7 @@ mod tests { assert!(err.is_instance_of::(py)); assert_eq!( err.to_string(), - "TypeError: failed to convert the value to 'Union[int, float]'" + "TypeError: failed to convert the value to 'Union[i32, f32]'" ); let obj_i = 42.to_object(py);