rapidfuzz / RapidFuzz

Rapid fuzzy string matching in Python using various string metrics
https://rapidfuzz.github.io/RapidFuzz/
MIT License
2.64k stars 117 forks source link

Cmake>= 3.15 is missing dependency for wheel. #410

Open Hains opened 6 days ago

Hains commented 6 days ago

Building Rapidfuzz with Yocto project., Scarthgap branch. https://git.openembedded.org/openembedded-core/log/?h=scarthgap

I ported Rapidfuzz to the scikit-build-core build system in our build-environment https://github.com/OpenPLi/openpli-oe-core/commit/12b5e8690f9347cf2d64ccf8be36744d739aa9fa

I need to apply a patch to remove the cmake check in the pyproject.toml file as workaround. https://github.com/OpenPLi/openpli-oe-core/commit/12b5e8690f9347cf2d64ccf8be36744d739aa9fa#diff-938b3f5a03475ad51edd749573d6c87e01a6267ce468572141204948d80bc37fR12

To fix compile error:

`Log data follows: DEBUG: Executing shell function do_compile * Getting build dependencies for wheel...
ERROR Missing dependencies:
cmake>=3.15
WARNING: exit code 1 from a shell command.

`

Cmake version 3.28.3 is used. This is the only problem.

Do you know a proper solution to fix this?

maxbachmann commented 6 days ago

A couple small things I noticed: 1) both the rapidfuzz and scikit-build-core file list an incorrect license 2) the rapidfuzz version lists a version 3.9.7. I suppose this should be 3.10.0 3) is cmake always available? Just wondering because ninja is listed as a dependency but cmake is not. 4) in a package system you probably want to set the RAPIDFUZZ_BUILD_EXTENSION environment variable to ensure it's actually building the C++ extension. Otherwise it might fall back to the pure python version on compilation error.

It's completely unclear how removing the system-cmake line would fix the build though. From my understanding this line would either:

Without the line it might fall back to the python version since it detects a system cmake. This would mean that it previously detected the system cmake as well. @henryiii do you have any idea what might go wrong here?

Hains commented 6 days ago

Incorrect licenses and version fixed. Thanks for reporting.

cmake(-native) isn't added to DEPENDS because it doesn't fix the error. While adding Ninja and python3-cython native does fix the same error for Ninja and python3-cython.

maxbachmann commented 6 days ago

Did you validate that that your patched version properly builds the package and doesn't just fall back to the pure Python version?

Hains commented 6 days ago

ipk file extracted, and it's indeed Python only.

maxbachmann commented 6 days ago

Then the patch only prevented the correct overload from being chosen.

Was the error message you did send initially all that was printed? Is there a way for you to increase the verbosity? Maybe that helps tracking the issue down.

I would assume cmake-native to be needed unless this is somehow provided by the environment by default.

Hains commented 6 days ago

Regarding log. This is all unfortunately.

henryiii commented 6 days ago

Couple of thoughts:

I notice you are inheriting from the python_flit_core thing (not familiar with the system - recipe?). There's nothing related to flit here, so I'm guessing you might be picking up a custom PEP 517 build step (implemented in do_compile perhaps?) from here. If the build went away when you changed do_compile, that seems to indicate to me that it's the step that probably has the wheel build in it.

CMake needs to be on the path. It's hard to tell if that's inherits or depends - it's a build-time dependency only. Is the output of cmake -E capabilities or cmake --version fine?

Where do I find these recipes? Especially the "build-meta" ones. I found https://git.openembedded.org/openembedded-core/tree/meta/recipes-devtools/python/python3-flit-core_3.9.0.bb?h=scarthgap, though that inherits "pypi" and python-flit-core, so not sure where those are.

EDIT:

NumPy still uses the old setuptools build, sadly, that would have been an interesting recipe to compare to. The flit recipes use do_compile:class-native () { python_flit_core_do_manual_build }, which is an oddly flit-core specific name, I'd think you'd just do a PEP 517 build.

EDIT:

Ah, found the inherit bits. https://git.openembedded.org/openembedded-core/tree/meta/classes-recipe/python_pep517.bbclass?h=scarthgap looks like the one you want. The flit-core one is doing a flit-core bootstrap build, which technically was only supposed to be used for bootstrapping minimal dependencies.

Hains commented 6 days ago

When inherit python_pep517 instead of python_flit_core, results in an empty main package. Only dev, debug and source packages. So not even a Python only version of Rapidfuzz.

Hains commented 6 days ago

Rapidfuzz is the only recipe which using scikit-build-core. OE doesn't provide recipes and bbclasses for scikit-build-core. So I had to make it myself. Probably something missing or wrong? https://github.com/OpenPLi/openpli-oe-core/blob/e620ebe013ccf8acbbc0db7c5238eb9e4bc1f6ca/meta-openpli/recipes-support/scikit-build-core/scikit-build-core_0.10.7.bb

LecrisUT commented 10 hours ago

Doesn't yocto use a vendor approach to packaging? Something similar to homebrew?

I had some experience with yocto in the past, maybe I can help here. Could you point to the relevant branches containing rapidfuzz and scikit-build-core definitions? The most challenging part would be to make scikit-build-core pass the CMakeToolchains to the CMake backend, and I believe this would be the first distribution to support cross-compilation so we should work together to figure out what changes need to be done upstream.

LecrisUT commented 4 hours ago

A few things that I've gathered so far:

This is all I've managed to gather prior to setting up a yocto build and playing around with the actual definitions. @Hains if you have some contacts that could help with this process on the open-embedded side, it would be great to get together and see how to handle each one of these.