Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix broken Cython compilation due to baked C++ files #131

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

KeyWeeUsr
Copy link

Having C++ files for users without Cython is nice, but when people actually use Cython it introduces multiple things:

  • incompatibilities on Cython version change
  • out-dated generated code which is by default skipped in the cythonization step as there already is a generated file (thus requires --force without a relevant reason in this case)
  • if anything changes in an upstream (dependency, CPython, etc), people can't just tell pip to utilize source code because the source code is already broken by the generated code even if the repo itself already contains a fix

What's happening is basically this:

  • on the first hash the repo is without change, thus you'll get just C++ error due to missing symbol
  • on the second hash, the state is after the unsuccessful compilation and it can be seen that the C++ code was untouched
  • on the third hash the generated file is removed, thus cythonization + C++ compilation proceeds as it should
  • on the fourth hash a dumbed-down patch was utilized, cythonization overwrote the C++ file and C++ compilation succeeded
docker run -it python:alpine /bin/sh -c '
    apk add git g++ && \
    git clone https://github.com/seomoz/reppy && \
    cd reppy && \
    git submodule update --init --recursive && \
    git status && \
    pip install cython && \
    sha256sum reppy/robots.cpp >> /hashes.txt && \
    pip install -v -e . || \
    sha256sum reppy/robots.cpp >> /hashes.txt && \
    git clean -dxf && \
    git status && \
    rm reppy/robots.cpp && \
    pip install -v -e . && \
    sha256sum reppy/robots.cpp >> /hashes.txt && \
    pip uninstall -y reppy && \
    git clean -dxf && \
    git status && \
    sed -i -e "43i\ \ \ \ from Cython.Build import cythonize as c;c(ext_files[-1], force=True,language=\"c++\")" setup.py && \
    git diff > diff.txt && cat diff.txt && rm diff.txt && \
    pip install -v -e . && \
    sha256sum reppy/robots.cpp >> /hashes.txt && \
    cat /hashes.txt' |tee reppy-output.txt

This might have implications on caching of the result when developing the package when having multiple files, however since there is pretty much a single .pyx file I believe this implication is redundant and the overall dev-experience improves by having a straightforward solution.

Attached is the tee-d output: reppy-output.txt

Closes #122

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

python 3.9-dev getting an error
1 participant