Fix src/build/cythonized/build/cythonized directory

[jdemeyer]

Inside $SAGE_ROOT, I see a file like

src/build/cythonized/build/cythonized/sage/libs/eclib/wrap.cpp

The duplication of build/cythonized is not how it's supposed to be.

There is also an issue with dependency checking: when src/sage/libs/eclib/wrap.cpp is edited and Sage is rebuilt, the new contents of that file are not taken into account.

This started with #27040, which accidentally removed the top-level source directory SAGE_SRC from the include directories. This way, the include was only found in the build directory, confusing Cython.

[jhpalmieri]

This seems to be recent: I don't see it in Sage 8.6.

[jhpalmieri]

Is it a problem to have src/build/cythonized/setup.py, a copy of src/setup.py? That's been there for some time, I think.

[jhpalmieri]

It looks to me like the build/cythonized/build/cythonized problem first arises in 8.7.beta2.

[jhpalmieri]

Could the problem be #27041?

[jhpalmieri]

No, it's #27040, and in particular, the directories are created when cythonize is run in sage_build_cython.run. The dependencies for the elements in module_list are wrong: before #27040, I see (among other things):

/path/to/SAGE_ROOT/local/include/python2.7/pythread.h
/path/to/SAGE_ROOT/src/sage/cpython/cython_metaclass.h
/path/to/SAGE_ROOT/src/sage/libs/ntl/ntlwrap.h
/path/to/SAGE_ROOT/src/sage/libs/ntl/ntlwrap_impl.h
/path/to/SAGE_ROOT/src/setup.py

while after #27040, I see

/path/to/SAGE_ROOT/local/include/python2.7/pythread.h
build/cythonized/sage/cpython/cython_metaclass.h
build/cythonized/sage/libs/ntl/ntlwrap.h
build/cythonized/sage/libs/ntl/ntlwrap_impl.h
build/cythonized/setup.py

(I put a print statement in the Cython.Build.Dependencies.cythonize function: after module_list, module_metadata = create_extension_list(..., I added

    for m in module_list:
        for dep in m.depends:
            print(dep)

) Does that clarify anything?

[jdemeyer]

New commits:

​a5c7119

Put back SAGE_SRC/SAGE_LIB in sage_include_directories()

[embray]

Just to be clear, this is only an oddity when building in source, right? Does it actually break anything? You mentioned in the ticket something about it breaking dependency checking when modifying some files. Did this fix it?

[jdemeyer]

Replying to embray:

Just to be clear, this is only an oddity when building in source, right?

What do you mean with "building in source"?

Does it actually break anything? You mentioned in the ticket something about it breaking dependency checking when modifying some files. Did this fix it?

Yes, this is needed to fix dependency checking because the build directory build/cythonize is also in the list of include directories (see line 421 of src/setup.py). Certain auto-generated files end up in build/cythonize, so that's needed. Normal C/C++ source files like sage/cpython/cython_metaclass.h appear in the source directory $SAGE_SRC. They are also copied by Cython to build/cythonized. But the dependency should be on the file in $SAGE_SRC which means that this directory is needed in the list of include directories and it should come before build/cythonized in that list.

[jhpalmieri]

With this branch in place, will existing build/cythonized/build/cythonized directories cause any problems? Is there any reason to clean them up?

[jdemeyer]

Replying to jhpalmieri:

With this branch in place, will existing build/cythonized/build/cythonized directories cause any problems?

I don't think so.

[jhpalmieri]

First: with this branch, the build/cythonized/build/ directory is not created.

Second: without this branch (and is this what Erik was asking?), if I use ./configure --prefix=/path/to/somewhere/else, then build/cythonized/build is still created in SAGE_ROOT/src.

[jhpalmieri]

I'm ready to give this a positive review. I'll wait a day to see if there are any objections.