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
111 stars 34 forks source link

`3.29.1` is broken on Python 3.7 due to `importlib_metadata` backport #489

Closed stephen-hansen closed 4 months ago

stephen-hansen commented 5 months ago

Hi, I see that there have been recent changes made to drop Python versions <3.7, but my understanding from reading here is that Python 3.7 should still be supported by this cmake package.

https://github.com/scikit-build/cmake-python-distributions/blob/557c82af7e68009b7555d7b816dd45ef2617c6e7/pyproject.toml#L28

This morning I saw a number of 3.7 pipelines fail due to ModuleNotFoundError: No module named 'importlib_metadata'. I believe the same issue was reported in https://github.com/scikit-build/cmake-python-distributions/issues/472, but the patch only fixed the error for Python versions 3.8-3.9.

On my end we're fine pinning the cmake package to an earlier version and/or adding the missing importlib-metadata dependency in our projects, but it would be appreciated if importlib-metadata could be added as a project dependency for Python 3.7 as suggested in the original issue above.

stephen-hansen commented 4 months ago

Ah, it looks like the importlib-metadata dependency was there in pyproject.toml at one point, but was removed in https://github.com/scikit-build/cmake-python-distributions/commit/215cc1c45139ad8b7063c64aec51ad93cd5b22d3. But it looks like the __init__.py file still references this package.

https://github.com/scikit-build/cmake-python-distributions/blob/557c82af7e68009b7555d7b816dd45ef2617c6e7/src/cmake/__init__.py#L8-L11

stephen-hansen commented 4 months ago

Also seeing some 3.7 pipelines pass but those appear to be pulling importlib-metadata from pytest not from cmake.

YunchuWang commented 4 months ago

+1, it starts breaking our pipeline since yesterday https://github.com/Azure/azure-functions-python-worker/actions/runs/8637840975/job/23680955694?pr=1455 for all 3.7 tests.

JeanChristopheMorinPerso commented 4 months ago

I created a PR to add the missing dependency: https://github.com/scikit-build/cmake-python-distributions/pull/490

jcfr commented 4 months ago

Thanks for the detailed report and fix :pray: , once the CI associated with #490 completes, changes will be integrated and a new patch release will be created :rocket:

Thanks for the patience :hourglass_flowing_sand:

jcfr commented 4 months ago

3.29.1.1^1 has been tagged, once the associated GitHub workflow^2 completes, the packages will be available for download.

Update: Release of 3.29.2 has also been initiated:


@henryiii I am then thinking to yank 3.29.1 from PyPI and update the published releases so that 3.29.1 effectively reference 3.29.1.1 tag. How does this sound ?

henryiii commented 4 months ago

You need to update the version in pyproject.toml too if you do a patch release. I'd have just yanked .1 and pushed .2 when it's ready.

jcfr commented 4 months ago

I cancelled 3.29.1.1 workflow, will remove the corresponding tag and proceed as you suggested by yanking 3.29.1

jcfr commented 4 months ago
henryiii commented 4 months ago

I've tagged 3.29.2 with the fix. I'll make a release when it's done building (unless I forget). It'll show up on PyPI either way.

stephen-hansen commented 4 months ago

Thanks for the fast turnaround!

jcfr commented 4 months ago

Thanks @henryiii for finalizing the release :pray: