Use setuptools instead of distutils

[oscarbenjamin]

Partial (short term) fix for gh-52

Uses setuptools instead of distutils.

[oscarbenjamin]

@deinst presumably something better needs to go here but I'm not sure what. I think that you said you worked out some code for this...

[deinst]

Ive spent a bit of time rummaging about in the numpy.distutils code to see what it does to fill the default_*_dirs variables. The above works for me, but that is because I keep my flint in a non standard place and need to supply the directory to setup.py.

On the unix/osx side, this is easy, just make a list of the usual suspects, and filter out the ones that don't exist. On the windows side things are more complicated and I'm working on figuring out what is needed.

I believe you still need get config_vars for the python dirs, but you can get it from sysconfig instead of distutils.sysconfig

[oscarbenjamin]

I believe you still need get config_vars for the python dirs, but you can get it from sysconfig instead of distutils.sysconfig

In the version of setup.py on master this is only used to set the OPT environment variable:

python-flint/setup.py

Lines 38 to 39 in 2fd4fd6

	(opt,) = get_config_vars('OPT')
	os.environ['OPT'] = " ".join(flag for flag in opt.split() if flag != '-Wstrict-prototypes')

What uses that environment variable?

The value I have locally on Linux is:

In [4]: from distutils.sysconfig import get_config_vars

In [5]: get_config_vars('OPT')
Out[5]: ['-DNDEBUG -g -fwrapv -O3 -Wall']

[deinst]

As I understand it OPT supplies default options to gcc. All setup.py does is removes -Wstrict-prototypes from it. Considering the number of compiler warnings already generated, it seems that filtering out -Wstrict_prototypes is probably not particularly helpful.

[oscarbenjamin]

With Python 3.12 I have:

In [2]: from sysconfig import get_config_vars

In [3]: get_config_vars('OPT')
Out[3]: ['-DNDEBUG -g -O3 -Wall']

Either way there does not seem to be any -Wstrict-prototypes.

Probably that can be removed but I'll leave it there for now while getting the main problems fixed.

I think I might have get the setup.py working for all Python version or OS combinations. Fingers crossed that the Windows build is about to succeed in CI...

The only thing currently missing is default_include_dirs and default_lib_dirs but I'm not sure why we should need to get those anyway. I would have thought that it was setuptools job to handle the default dirs.

[deinst]

If setuptools can get the directories correctly that is great. If not, I think I have gotten the requisite code from numpy.distutils.system_info to generate the default paths. I have only a foggy notion of exactly what the windows code is doing as things like vcpkg are inovations that postdate my working on code for windows.

[oscarbenjamin]

I think I have gotten the requisite code from numpy.distutils.system_info to generate the default paths.

I think let's get this PR in for now because it at least means we have CI built wheels for 3.12 and that building a development checkout for 3.12 now works.

We can follow up with another PR for generating the paths. I don't know whether there is actually a reason for doing something other than just trusting setuptools to do it for us though.

Longer term if we can move to meson and meson-python then they should take care of default paths and library discovery etc so in principle it should not be a concern for us.

[oscarbenjamin]

Everything here seems to be working here except Windows. One of the things that I worried about with using setuptools is that its support for mingw is supposedly not as good as numpy.distutils and mingw is used for building the Windows wheels.

The build failure on Windows is:

        gcc -mdll -O -Wall -ID:\a\python-flint\python-flint\.local\include -IC:\Users\runneradmin\AppData\Local\Temp\cibw-run-5hwr_lyj\cp39-win_amd64\build\venv\include -IC:\Users\runneradmin\AppData\Local\pypa\cibuildwheel\Cache\nuget-cpython\python.3.9.13\tools\include -IC:\Users\runneradmin\AppData\Local\pypa\cibuildwheel\Cache\nuget-cpython\python.3.9.13\tools\Include -c src/flint/pyflint.c -o build\temp.win-amd64-cpython-39\Release\src\flint\pyflint.o
        src/flint/pyflint.c:428:41: warning: division by zero [-Wdiv-by-zero]
             enum { __pyx_check_sizeof_voidp = 1 / (int)(SIZEOF_VOID_P == sizeof(void*)) };
                                                 ^
        src/flint/pyflint.c:428:12: error: enumerator value for '__pyx_check_sizeof_voidp' is not an integer constant
             enum { __pyx_check_sizeof_voidp = 1 / (int)(SIZEOF_VOID_P == sizeof(void*)) };
                    ^~~~~~~~~~~~~~~~~~~~~~~~
        src/flint/pyflint.c: In function '__Pyx_ImportType_3_0_4':
        src/flint/pyflint.c:6670:13: warning: unknown conversion type character 'z' in format [-Wformat=]
                     "%s.%s size changed, may indicate binary incompatibility. "
                     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
        src/flint/pyflint.c:6671:24: note: format string is defined here
                     "Expected %zd from C header, got %zd from PyObject",
                                ^
        src/flint/pyflint.c:6670:13: warning: unknown conversion type character 'z' in format [-Wformat=]
                     "%s.%s size changed, may indicate binary incompatibility. "
                     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
        src/flint/pyflint.c:6671:47: note: format string is defined here
                     "Expected %zd from C header, got %zd from PyObject",
                                                       ^
        src/flint/pyflint.c:6670:13: warning: too many arguments for format [-Wformat-extra-args]
                     "%s.%s size changed, may indicate binary incompatibility. "
                     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
        error: command 'C:\\mingw64\\bin\\gcc.exe' failed with exit code 1

Those exact errors are reported as a general Cython+mingw issue here:
cython/cython#3405

The comments here indicate that mingw support in setuptools has improved: cython/cython#4470 (comment). Apparently though it is still necessary to apply a patch to CPython's header files for Python < 3.12.

We don't see this working for 3.12 working here because it fails building the 3.9 wheel before attempting the 3.12 wheel. I'm going to push a commit that only builds 3.12 wheels to see if at least that works.

[oscarbenjamin]

I'm going to push a commit that only builds 3.12 wheels to see if at least that works.

It does work so what works is:

• Building with setuptools for Linux/OSX in all Python versions
• Building with setuptools and mingw on Windows for Python 3.12
• Building with numpy.distutils and mingw on Windows for Python < 3.12

What does not work is:

• Building with setuptools and mingw on Windows for Python < 3.12
• Building with numpy.distutils on Python 3.12

To build with setuptools we would apparently need to apply this patch to CPython:

diff --git a/PC/pyconfig.h b/PC/pyconfig.h
index 1a33d4c5a1e4f..1d8408b363a66 100644
--- a/PC/pyconfig.h
+++ b/PC/pyconfig.h
@@ -209,6 +209,16 @@ typedef int pid_t;
 
 #endif /* _MSC_VER */
 
+/* ------------------------------------------------------------------------*/
+/* mingw and mingw-w64 define __MINGW32__ */
+#ifdef __MINGW32__
+
+#ifdef _WIN64
+#define MS_WIN64
+#endif
+
+#endif /* __MINGW32__*/
+
 /* ------------------------------------------------------------------------*/
 /* egcs/gnu-win32 defines __GNUC__ and _WIN32 */
 #if defined(__GNUC__) && defined(_WIN32)

I am more tempted though just to add some Python-version logic to setup.py. We have a setup that has been working for Python < 3.12 and I think I have just found a setup that works for Python 3.12 at least for development builds and for building wheels in CI.

Not having the default paths added in setup.py is a problem but at least being able to build at all on Python 3.12 is an improvement. This check passes in CI so it at least seems to pick up Flint in /usr/local on Linux:

python-flint/bin/pip_install_ubuntu.sh

Lines 24 to 56 in 2fd4fd6

	# This will default to installing in /usr/local. If you want to install in a
	# non-standard location then configure flint with
	# ./configure --disable-static --prefix=$PREFIX
	# If $PREFIX is no in default search paths, then at build time set
	# export C_INCLUDE_PATH=$PREFIX/include
	# and at runtime set
	# export LD_LIBRARY_PATH=$PREFIX/lib
	curl -O -L https://www.flintlib.org/flint-$FLINTVER.tar.gz
	tar xf flint-$FLINTVER.tar.gz
	cd flint-$FLINTVER
	./bootstrap.sh
	./configure --disable-static
	make -j
	sudo make install
	cd ..
	ls -l /usr/local/lib
	sudo ldconfig /usr/local/lib
	# Python build requirements. Ideally these would be in pyprojec.toml, but
	# first need to migrate from setup.py to pyproject.toml.
	pip install 'cython>=3' numpy wheel
	# Install from checkout (or sdist).
	echo -----------------------------------------------------------
	echo
	echo Running:
	echo $ pip install --no-build-isolation $1
	echo
	echo -----------------------------------------------------------
	pip install --no-build-isolation $1

[oscarbenjamin]

So it is working for all Python versions and for Windows as well as OSX and Linux.

Note that the pip install python-flint jobs are expected to fail. They are testing building from source with pip install python-flint after building Flint 3. Until a new sdist is pushed to PyPI that will fail because the sdist currently on PyPI expects Flint 2.9 and Arb. The only reason those jobs pass on master is because they are installing the binary wheel from PyPI but I've fixed that here which then means that those CI jobs (correctly) fail.

[oscarbenjamin]

I think this has to be merged before Cirrus CI will start generating 3.12 wheels for OSX ARM64. I think everything else is working here now so I'll merge and then see what CI does on the merge build.