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 <[email protected]>

Author: davidhewitt

.github/workflows/ci.yml

@@ -480,6 +480,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
@@ -494,6 +510,7 @@ jobs:
       - emscripten
       - test-debug
       - test-version-limits
+      - check-feature-powerset
     if: always()
     runs-on: ubuntu-latest
     steps:

Cargo.toml

@@ -107,20 +107,21 @@ nightly = []
 # This is mostly intended for testing purposes - activating *all* of these isn't particularly useful.
 full = [
     "macros",
+    "gil-refs",
     # "multiple-pymethods", # TODO re-add this when MSRV is greater than 1.62
+    "anyhow",
     "chrono",
     "chrono-tz",
-    "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]

newsfragments/3834.fixed.md

@@ -0,0 +1 @@
+Fix compilation failure with `either` feature enabled without `experimental-inspect` enabled.

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"]
 
 
@@ -479,10 +487,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,
@@ -593,6 +599,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")

pytests/src/comparisons.rs

@@ -1,5 +1,4 @@
 use pyo3::prelude::*;
-use pyo3::{types::PyModule, Python};
 
 #[pyclass]
 struct Eq(i64);

src/conversions/either.rs

@@ -94,7 +94,13 @@ where
         } else if let Ok(r) = obj.extract::<R>() {
             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::<L>(),
+                std::any::type_name::<R>()
+            );
             Err(PyTypeError::new_err(err_msg))
         }
     }
@@ -134,7 +140,7 @@ mod tests {
             assert!(err.is_instance_of::<PyTypeError>(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);