jamesturk / jellyfish

🪼 a python library for doing approximate and phonetic matching of strings.
https://jamesturk.github.io/jellyfish/
MIT License
2.04k stars 157 forks source link

fixed python 3.11 on musllinux #184

Closed MartinoMensio closed 1 year ago

MartinoMensio commented 1 year ago

Hi, After doing some tests, I discovered that the wheel available for musllinux python 3.11 does not work due to an issue with pyo3/maturin. On lower python versions or on normal linux instead it works.

https://github.com/PyO3/maturin/issues/1559

This generates the following error:

import jellyfish
  File "/app/.venv/lib/python3.11/site-packages/jellyfish/__init__.py", line 3, in <module>
    from ._rustyfish import *
ModuleNotFoundError: No module named 'jellyfish._rustyfish'

To reproduce on the latest jellyfish I am following this script:

# set up quemu for architecture emulation
docker run --rm --privileged multiarch/qemu-user-static --reset -p yes

# x64
docker run --rm -it --platform=linux/x86_64 -v `pwd`/script.sh:/app/script.sh -v `pwd`/tests:/app/tests -v `pwd`/testdata:/app/testdata python:3.11-slim sh /app/script.sh 
docker run --rm -it --platform=linux/x86_64 -v `pwd`/script.sh:/app/script.sh -v `pwd`/tests:/app/tests -v `pwd`/testdata:/app/testdata python:3.10-slim sh /app/script.sh 
docker run --rm -it --platform=linux/x86_64 -v `pwd`/script.sh:/app/script.sh -v `pwd`/tests:/app/tests -v `pwd`/testdata:/app/testdata python:3.9-slim sh /app/script.sh 
docker run --rm -it --platform=linux/x86_64 -v `pwd`/script.sh:/app/script.sh -v `pwd`/tests:/app/tests -v `pwd`/testdata:/app/testdata python:3.8-slim sh /app/script.sh 
docker run --rm -it --platform=linux/x86_64 -v `pwd`/script.sh:/app/script.sh -v `pwd`/tests:/app/tests -v `pwd`/testdata:/app/testdata python:3.7-slim sh /app/script.sh 

# x86
docker run --rm -it --platform=linux/aarch64 -v `pwd`/script.sh:/app/script.sh -v `pwd`/tests:/app/tests -v `pwd`/testdata:/app/testdata python:3.11-slim sh /app/script.sh 
docker run --rm -it --platform=linux/aarch64 -v `pwd`/script.sh:/app/script.sh -v `pwd`/tests:/app/tests -v `pwd`/testdata:/app/testdata python:3.10-slim sh /app/script.sh 
docker run --rm -it --platform=linux/aarch64 -v `pwd`/script.sh:/app/script.sh -v `pwd`/tests:/app/tests -v `pwd`/testdata:/app/testdata python:3.9-slim sh /app/script.sh 
docker run --rm -it --platform=linux/aarch64 -v `pwd`/script.sh:/app/script.sh -v `pwd`/tests:/app/tests -v `pwd`/testdata:/app/testdata python:3.8-slim sh /app/script.sh 
docker run --rm -it --platform=linux/aarch64 -v `pwd`/script.sh:/app/script.sh -v `pwd`/tests:/app/tests -v `pwd`/testdata:/app/testdata python:3.7-slim sh /app/script.sh  

# musllinux

# 3.11: ImportError: cannot import name '_rustyfish' in import jellyfish
docker run --rm -it --platform=linux/x86_64 -v `pwd`/script.sh:/app/script.sh -v `pwd`/tests:/app/tests -v `pwd`/testdata:/app/testdata python:3.11-alpine sh /app/script.sh 
docker run --rm -it --platform=linux/x86_64 -v `pwd`/script.sh:/app/script.sh -v `pwd`/tests:/app/tests -v `pwd`/testdata:/app/testdata python:3.10-alpine sh /app/script.sh 
docker run --rm -it --platform=linux/x86_64 -v `pwd`/script.sh:/app/script.sh -v `pwd`/tests:/app/tests -v `pwd`/testdata:/app/testdata python:3.9-alpine sh /app/script.sh 
docker run --rm -it --platform=linux/x86_64 -v `pwd`/script.sh:/app/script.sh -v `pwd`/tests:/app/tests -v `pwd`/testdata:/app/testdata python:3.8-alpine sh /app/script.sh 
docker run --rm -it --platform=linux/x86_64 -v `pwd`/script.sh:/app/script.sh -v `pwd`/tests:/app/tests -v `pwd`/testdata:/app/testdata python:3.7-alpine sh /app/script.sh 

# x86
# 3.11: ImportError: cannot import name '_rustyfish' in import jellyfish
docker run --rm -it --platform=linux/386 -v `pwd`/script.sh:/app/script.sh -v `pwd`/tests:/app/tests -v `pwd`/testdata:/app/testdata python:3.11-alpine sh /app/script.sh 
docker run --rm -it --platform=linux/386 -v `pwd`/script.sh:/app/script.sh -v `pwd`/tests:/app/tests -v `pwd`/testdata:/app/testdata python:3.10-alpine sh /app/script.sh 
docker run --rm -it --platform=linux/386 -v `pwd`/script.sh:/app/script.sh -v `pwd`/tests:/app/tests -v `pwd`/testdata:/app/testdata python:3.9-alpine sh /app/script.sh 
docker run --rm -it --platform=linux/386 -v `pwd`/script.sh:/app/script.sh -v `pwd`/tests:/app/tests -v `pwd`/testdata:/app/testdata python:3.8-alpine sh /app/script.sh 
docker run --rm -it --platform=linux/386 -v `pwd`/script.sh:/app/script.sh -v `pwd`/tests:/app/tests -v `pwd`/testdata:/app/testdata python:3.7-alpine sh /app/script.sh 

We should find a way to test the generated wheels in github actions with docker, to be sure to generate working wheels.

Best, Martino

MartinoMensio commented 1 year ago

This is currently being investigated within maturin, so just ignore/discard this PR at the moment.

Martino

MartinoMensio commented 1 year ago

An update on this issue and how it has been solved:

  1. maturin v1.0.0-beta.7 selected for musl targets, that fixes the mismatch of EXT_SUFFIX from gnu to musl happening with musl and CPython3.11
  2. switching to the beta, made it necessary to move the declaration of the rust module name from Cargo.toml to pyproject.toml. Before the beta it was already deprecated but was not causing issues (see warning message in builds). This was generating ModuleNotFoundError: No module named 'jellyfish._rustyfish' https://github.com/PyO3/maturin/blob/main/Changelog.md#01416---2023-03-26

Now the build of the wheels works and I tested them via docker. No more import errors are generated.

Only downside: for now the CI.yaml has to specify the beta maturin version because it's not yet in stable. Probably in a month we will need to remove that line.

MartinoMensio commented 1 year ago

We still need to wait the v0.14.17 to appear in https://github.com/PyO3/maturin-action/blob/main/versions-manifest.json

MartinoMensio commented 1 year ago

Now no downsides, maturin stable v0.14.17 fixes it. This PR currently only contains a change targetting the deprecated rust module name from Cargo.toml to pyproject.toml