search

scikit-build / cmake-python-distributions

This project provides the infrastructure to build CMake Python wheels.
https://cmake-python-distributions.readthedocs.io
Apache License 2.0
114 stars 34 forks source link

`3.29.0` release seems to be broken on Python 3.9 and earlier due to `importlib_metadata` backport #472

Closed agriyakhetarpal closed 8 months ago

agriyakhetarpal commented 8 months ago

CMake seems to use the importlib_metadata backport dependency for Python 3.8 and 3.9, but maybe it's not declaring that?

I noticed the failure in the CI logs, and then triggered a re-run just in case this was a one-off failure:

  1. https://github.com/pybamm-team/PyBaMM/actions/runs/8436810245/job/23105126965
  2. https://github.com/pybamm-team/PyBaMM/actions/runs/8436810245/job/23106957328

Edit: okay so it is declaring that, just in the optional dependencies table: https://github.com/scikit-build/cmake-python-distributions/blob/85a457bf3e0de6efa65cc0f47f017810936772f0/pyproject.toml#L36-L42

and I suppose it should be a required dependency too. It's being used here: https://github.com/scikit-build/cmake-python-distributions/blob/85a457bf3e0de6efa65cc0f47f017810936772f0/src/cmake/__init__.py#L5-L9

agriyakhetarpal commented 8 months ago

I'm happy to fix this and add maybe add a test for this too, as it is deemed necessary.

mloubout commented 8 months ago

The issue is that importlib change is for python >= 3.8 not > 3.10 so it should be 

if sys.version_info < (3, 8): 
     from importlib_metadata import distribution 
 else: 
     from importlib.metadata import distribution

FYI ran into that issue in my CI as well.

mloubout commented 8 months ago

I filed a patch for it in case people agree it is correct. Can a release be made if the patch gets merged?

braindevices commented 8 months ago

this affect us too

awni commented 8 months ago

+1 would be great to have a patch for this.

henryiii commented 8 months ago

3.29.0 is yanked and we'll produce a 3.29.0.1 later today.

henryiii commented 8 months ago

New version out!

mloubout commented 8 months ago

Fixed the issue on my side thanks!