polm / fugashi

A Cython MeCab wrapper for fast, pythonic Japanese tokenization and morphological analysis.
MIT License
389 stars 31 forks source link

Fix tests on manylinux2014 #52

Closed lambdadog closed 2 years ago

lambdadog commented 2 years ago

To do this, I've adjusted the methodology of test_manylinux to that of the manylinux builds a little, citing entrypoint.sh.

Instead of setting the PATH in Dockerfile (which was causing the "no valid C compiler" errors) I've adjusted test_manylinux.yaml to use the same substitution methodology seen in entrypoint.sh to get the python executable irrespective of PATH. I also needed to add the LD_LIBRARY_PATH hack in order for libmecab.so to be found.

This also means that the versions being tested for don't have to live both in Dockerfile AND in test_manylinux.yaml, which they did previously.

Working tests run: https://github.com/lambdadog/fugashi/actions/runs/1603485583

Overall, there are a couple of changes I'd still like to see here (caching of Mecab and sharing of resources between the manylinux builds and tests, most notably), but if I was going to do any CI refactors it would be to the entirety layout of the CI-relevant files so for now I intend to wait on my existing PRs being merged then hopefully dive into it all with a scalpel.


Closes #48 since manylinux2014 was already working for builds.

polm commented 2 years ago

Thanks, this looks good too!