-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
build_ext does not properly track dependencies (and should be deprecated / removed) #3541
Comments
Thanks for the report. The thing is, What might work, I think, is to copy the current configuration options from PR very welcome. |
Here is how the options for the old_build_ext (on the left hand side) roughly map to cythonize parameters (right hand side) used by the new_build_ext: cython_cplus : language Given how incompatible the two build_exts are, is it worth making the new_build_ext backwards compatible with the old_build_ext, or should we just switch to the new_build_ext as a possibly breaking change for Cython 3.0? |
This also solves a difficulty with the Cython import in setuptools' build_ext. We need to inherit from the one in distutils, so that setuptools can inherit from us. That leads to a circular dependency that goes either way depending on which gets imported first by users, and in what way (from-import or module import). This is built to match the code in https://github.com/pypa/setuptools/blob/9f1822ee910df3df930a98ab99f66d18bb70659b/setuptools/command/build_ext.py#L14-L21 Closes #3541
@matusvalo, thanks for the fix. |
Hello up there. In the context of getting gevent miscompiled the question of proper dependency tracking on Cython side was raised. While original cause for that particular gevent miscompilation seems to be #1428, other issues were also found.
This issue is probably related to #1436 and shows that
Cython.Distutils.build_ext
does not properly track build dependencies.In 2016
Cython.Distutils.build_ext
was changed to usecythonize
and the old implementation was moved intoCython.Distutils.old_build_ext
for exactly particular reason thatold_build_ext
was not tracking dependencies properly: cb55c11. A warning corresponding to the move was added:cython/Cython/Distutils/old_build_ext.py
Lines 37 to 42 in 3de7a4b
However right after that
Cython.Distutils.build_ext
was reverted to useold_build_ext
and new implementation became available asnew_build_ext
(4ecdd3e) with the idea toAs of today (2020 April)
Cython.Distutils.build_ext
still points toold_build_ext
and the warning is somehow not printed even ifold_build_ext
is imported directly(*):This way many projects still use
old_build_ext
=from Cython.Build import build_ext
to compile their pyx files and miss both proper dependency tracking and the warning.As a fix I propose to make the deprecation warnings effective and to make
Cython.Build.build_ext
to refer tonew_build_ext
which uses cythonize.The rest of this issue demonstrates that
Cython.Build.build_ext
dependency handling is broken.setup.py
a.pyx
b.pxd
b.pyx
( first compile - ok )
now change
b.pxd
andb.pyx
to add argument to bfunc:and recompile - only
b
is rebuilt - nota
:which is WRONG because
a
will useb
module via outdated pxd.( In this particular case it should be giving compilation error at Cython level:
)
Cython
3.0a1-79-g3de7a4b8f
Thanks beforehand,
Kirill
/cc @robertwb
P.S.
Cython documentation says to use
Cython.Build.new_build_ext
which should be usingcythonize
internally:however Cython.Build does not have it:
because
new_build_ext
is defined only in Cython.Distutils.build_ext(*) probably due to some logic error in _check_stack calls. if I patch Cython locally with
the warning is printed:
The text was updated successfully, but these errors were encountered: