search

sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.37k stars 466 forks source link

Accept /usr/bin/python3 from XCode 12.3 on macOS 10.15 (Catalina) and 11 (Big Sur) #31227

Closed mkoeppe closed 3 years ago

mkoeppe commented 3 years ago

(from #31132)

27122 determines whether CC accepts -march=native and then adds it globally to CFLAGS in build/bin/sage-build-env.

30725 fixes this for macOS, where some packages are explicitly built with clang.

However, there is another mechanism how clang can come in: Python extension modules are compiled with the compiler listed in sysconfig. On macOS with homebrew, using /usr/bin/python3, this could again be clang, which does not accept -march=native in combination with -arch arm64 (which is provided even on x86_64 if ARCHFLAGS is unset).

In this ticket, we fix this by checking whether the compiler used by distutils accepts -march=native (or more generally, the flags determined earlier by configure). If it does not, then we disable the use of these flags.

In order to test this with /usr/bin/python3 from XCode 12.3 (on macOS 10.15 (Catalina)), we fix another issue: This python3 (3.8.2) is not able to build extension modules because it emits -arch arm64 -arch x86_64 unless ARCHFLAGS is set explicitly

$ /usr/bin/python3 -m sysconfig |grep '\bCFLAGS ='
    CFLAGS = "-Wno-unused-result -Wsign-compare -Wunreachable-code -fno-common -dynamic -DNDEBUG -g -fwrapv -O3 -Wall -iwithsysroot/System/Library/Frameworks/System.framework/PrivateHeaders -iwithsysroot/Applications/Xcode.app/Contents/Developer/Library/Frameworks/Python3.framework/Versions/3.8/Headers -arch arm64 -arch x86_64"

$ ARCHFLAGS="" /usr/bin/python3 -m sysconfig |grep '\bCFLAGS ='
    CFLAGS = "-Wno-unused-result -Wsign-compare -Wunreachable-code -fno-common -dynamic -DNDEBUG -g -fwrapv -O3 -Wall -iwithsysroot/System/Library/Frameworks/System.framework/PrivateHeaders -iwithsysroot/Applications/Xcode.app/Contents/Developer/Library/Frameworks/Python3.framework/Versions/3.8/Headers  -arch x86_64 "

In this ticket, if the distutils test fails, we try if setting ARCHFLAGS="" fixes it. Also, after accepting a system python and ARCHFLAGS so far has not been set, we test whether the system python wants to do the multi-arch build.

In either of these two cases, we store a configuration variable that causes sage-env to set ARCHFLAGS as well.

This is somewhat complicated logic - we are tiptoeing around previous breakage that had been caused by setting ARCHFLAGS unconditionally (#29408).

Depends on #31132 Depends on #30725

CC: @jhpalmieri @zlscherr @dimpase @kliem @vbraun

Component: build: configure

Author: Matthias Koeppe

Branch/Commit: fc8b676

Reviewer: Jonathan Kliem

Issue created by migration from https://trac.sagemath.org/ticket/31227

mkoeppe commented 3 years ago

Dependencies: #31132

mkoeppe commented 3 years ago

Branch: u/mkoeppe/acceptusr_bin_python3_from_xcode_12_3_on_macos_10_15catalina_

mkoeppe commented 3 years ago

New commits:

2c60025m4/sage_check_python_for_venv.m4: Warn if python3 is misconfigured like it is currently in homebrew
ee679bdbuild/pkgs/python3/spkg-configure.m4: Do not check sysconfig CPPFLAGS so that /usr/bin/python3 is accepted on macOS; reject broken homebrew python3 unless requested explicitly with configgure --with-python=...
29aee87build/pkgs/python3: Update to 3.9.0rc2
e61d464build/pkgs/python3: Update to 3.9.0
f8bb56dbuild/pkgs/python3: Update to 3.9.1
0e3513fbuild/pkgs/pip: Update to 20.3.3
41d7e7aMerge branch 'u/jhpalmieri/new_wheel' of git://trac.sagemath.org/sage into t/30589/upgrade-python-3.9
76d5fd1Merge branch 't/30589/upgrade-python-3.9' into t/31132/homebrew__unused_packages__singular__pari_______in__usr_local_leak_into_build_when_using_homebrew_s_python3
ce34810m4/sage_check_python_for_venv.m4: Use empty ARCHFLAGS while testing distutils
mkoeppe commented 3 years ago

Changed dependencies from #31132 to #31132, #31228

mkoeppe commented 3 years ago

Commit: ce34810

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Branch pushed to git repo; I updated commit sha1. This was a forced push. Last 10 new commits:

1b16a8cbuild/pkgs/pari/spkg-install.in: Do not recompute MACOSX_VERSION
c6e30eatox.ini: Add configuration factors gcc_10, gcc_11
9b0eb15build/pkgs/{cmake,curl,psutil,python3,r}: If setting CC=clang, also set CFLAGS
05985d8Merge branch 't/30725/macos__spkg_install__remove_outdated__building_with_clang__code___which_conflicts_with___march_native_' into t/31228/the___march_native__flag_vs__the_compiler_used_when_python_extensions_are_built
02c2684R is already compiled without -march=native
24b4fccset EXTRACFLAGS instead of CFLAGS
44be967make code a bit more stable against future changes
f56e65efix CFLAGS for python3/spkg-build.in by running testcflags.sh after determination of the compiler
09b03d2Merge branch 'u/gh-kliem/macos__spkg_install__remove_outdated__building_with_clang__code___which_conflicts_with___march_native_' of git://trac.sagemath.org/sage into t/31228/the___march_native__flag_vs__the_compiler_used_when_python_extensions_are_built
67d66b7Merge branch 't/31228/the___march_native__flag_vs__the_compiler_used_when_python_extensions_are_built' into t/31227/accept__usr_bin_python3_from_xcode_12_3_on_macos_10_15__catalina_
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Changed commit from ce34810 to 67d66b7

mkoeppe commented 3 years ago

Changed dependencies from #31132, #31228 to #31132, #30725, #31228

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Branch pushed to git repo; I updated commit sha1. New commits:

3325a4ebuild/pkgs/python3/spkg-configure.m4: Actually test with CFLAGS=$CFLAGS_MARCH
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Changed commit from 67d66b7 to 3325a4e

mkoeppe commented 3 years ago
comment:8

This is the branch that I had. I won't have time to work on it over the next week.

mkoeppe commented 3 years ago

Description changed:

--- 
+++ 
@@ -10,5 +10,6 @@
 egret:~/s/sage/sage-rebasing/worktree-algebraic-2018-spring (t/31132/homebrew__unused_packages__singular__pari_______in__usr_local_leak_into_build_when_using_homebrew_s_python3 *

-In this ticket, we set ARCHFLAGS during this configure step... +In this ticket, we set ARCHFLAGS during this configure step.

mkoeppe commented 3 years ago

Author: Matthias Koeppe

kliem commented 3 years ago
comment:9

It was proposed to mark #31228 as invalid and do it all here. In this case (and it is in fact already done here).

In this case, this ticket here is really ready for review (and the depency of #31228 could be removed).

kliem commented 3 years ago

Reviewer: https://github.com/kliem/sage/pull/36/checks, Jonathan Kliem

kliem commented 3 years ago
comment:10

I'm happy with this ticket, but I will have to wait and see if things work out in practice.

mkoeppe commented 3 years ago

Changed dependencies from #31132, #30725, #31228 to #31132, #30725

mkoeppe commented 3 years ago

Description changed:

--- 
+++ 
@@ -1,5 +1,14 @@
 (from #31132)

+#27122 determines whether CC accepts `-march=native` and then adds it globally to `CFLAGS` in `build/bin/sage-build-env`.
+
+#30725 fixes this for macOS, where some packages are explicitly built with `clang`.
+
+However, there is another mechanism how `clang` can come in: Python extension modules are compiled with the compiler listed in sysconfig. On macOS with homebrew, using `/usr/bin/python3`, this could again be `clang`, which does not accept `-march=native`.
+
+In this ticket, we fix this by checking whether the compiler used by distutils accepts `-march=native` (or more generally, the flags determined earlier by `configure`). If it does not, then we disable the use of these flags.
+
+In order to test this with `/usr/bin/python3` from XCode 12.3 (on macOS 10.15 (Catalina)), we fix another issue:
 This python3 (3.8.2) is not able to build extension modules because it emits `-arch arm64 -arch x86_64` unless `ARCHFLAGS` is set explicitly
jhpalmieri commented 3 years ago
comment:13

With this branch, ./configure --with-python=/usr/bin/python3 fails for me on OS X Catalina with Xcode 12.3. See attached config.log.

mkoeppe commented 3 years ago
comment:14

Did you run bootstrap?

jhpalmieri commented 3 years ago
comment:15

Replying to @mkoeppe:

Did you run bootstrap?

No, sorry for the false report. Having run ./bootstrap, and with local/bin/python3 a symlink pointing to /Applications/Xcode.app/Contents/Developer/usr/bin/python3, the build has now failed twice: Pillow and cython have both failed to build.

jhpalmieri commented 3 years ago

Attachment: pillow-8.0.1.log

jhpalmieri commented 3 years ago
comment:16

Attachment: cython-0.29.21.log

Oh, and gmpy2, also.

jhpalmieri commented 3 years ago

Attachment: gmpy2-2.1.0b5.log

mkoeppe commented 3 years ago
comment:17

OK, looks like we have to set ARCHFLAGS="" in more places. But this needs to be done very carefully - see #29408

jhpalmieri commented 3 years ago
comment:18

I ran make -k to get a fuller report, and it ended with:

* package:         gmpy2-2.1.0b5
  last build time: Jan 26 15:52
  log file:        /Users/jpalmier/Desktop/Sage/git/sage/logs/pkgs/gmpy2-2.1.0b5.log
  build directory: /Users/jpalmier/Desktop/Sage/git/sage/local/var/tmp/sage/build/gmpy2-2.1.0b5

* package:         cython-0.29.21
  last build time: Jan 26 15:52
  log file:        /Users/jpalmier/Desktop/Sage/git/sage/logs/pkgs/cython-0.29.21.log
  build directory: /Users/jpalmier/Desktop/Sage/git/sage/local/var/tmp/sage/build/cython-0.29.21

* package:         psutil-5.2.0.p2
  last build time: Jan 26 15:52
  log file:        /Users/jpalmier/Desktop/Sage/git/sage/logs/pkgs/psutil-5.2.0.p2.log
  build directory: /Users/jpalmier/Desktop/Sage/git/sage/local/var/tmp/sage/build/psutil-5.2.0.p2

* package:         pillow-8.0.1
  last build time: Jan 26 15:52
  log file:        /Users/jpalmier/Desktop/Sage/git/sage/logs/pkgs/pillow-8.0.1.log
  build directory: /Users/jpalmier/Desktop/Sage/git/sage/local/var/tmp/sage/build/pillow-8.0.1

* package:         kiwisolver-1.0.1
  last build time: Jan 26 15:52
  log file:        /Users/jpalmier/Desktop/Sage/git/sage/logs/pkgs/kiwisolver-1.0.1.log
  build directory: /Users/jpalmier/Desktop/Sage/git/sage/local/var/tmp/sage/build/kiwisolver-1.0.1

* package:         pynac-0.7.26.sage-2020-04-03.p0
  last build time: Jan 26 15:53
  log file:        /Users/jpalmier/Desktop/Sage/git/sage/logs/pkgs/pynac-0.7.26.sage-2020-04-03.p0.log
  build directory: /Users/jpalmier/Desktop/Sage/git/sage/local/var/tmp/sage/build/pynac-0.7.26.sage-2020-04-03.p0

* package:         cffi-1.14.4
  last build time: Jan 26 15:53
  log file:        /Users/jpalmier/Desktop/Sage/git/sage/logs/pkgs/cffi-1.14.4.log
  build directory: /Users/jpalmier/Desktop/Sage/git/sage/local/var/tmp/sage/build/cffi-1.14.4

All show the same error, "clang: error: the clang compiler does not support '-march=native'".

jhpalmieri commented 3 years ago
comment:19

Replying to @mkoeppe:

OK, looks like we have to set ARCHFLAGS="" in more places. But this needs to be done very carefully - see #29408

How can we detect whether we're using a Python which requires this? And where is the right place to do it? build/bin/sage-build-env-config.in? There are lots of affected packages, so doing it globally would be easier. (pyzmq, cysignals, numpy, and others also seem to fail with the same error, if I recklessly add export ARCHFLAGS="" to the install scripts for the other packages which were failing.)

mkoeppe commented 3 years ago
comment:20

Replying to @jhpalmieri:

All show the same error, "clang: error: the clang compiler does not support '-march=native'".

Oh, this is a different error. Let me try to fix this

mkoeppe commented 3 years ago
comment:21

Could you post config.log from this run?

jhpalmieri commented 3 years ago

Attachment: config.log

jhpalmieri commented 3 years ago
comment:22

Replying to @mkoeppe:

Could you post config.log from this run?

Yes. I replaced the old config.log, since that was a result of my error, not a problem with this ticket.

mkoeppe commented 3 years ago
comment:23

Thanks. The crazy thing is that passing -march=native only seems to result in the error clang: error: the clang compiler does not support '-march=native' if also -arch arm64 is passed. The current test in python3/spkg-configure.m4 does not catch this.

kliem commented 3 years ago
comment:24

What I find a bit confusing is the following:

We do not test, whether setting ARCHFLAGS="" was necesarry in order to build this test extension during configure

Why do we expect that in real life situations (when actually compiling python extensions for sage) ARCHFLAGS="" is not a requirement anymore?

jhpalmieri commented 3 years ago
comment:25

For what it's worth, setting ARCHFLAGS="" in sage-build-env-config.in let me build Sage. Doctesting didn't go that well: various failures because of "command 'gcc' failed with exit status 1" and "#error Unsupported architecture" and "#error architecture not supported".

mkoeppe commented 3 years ago
comment:27

Replying to @jhpalmieri:

For what it's worth, setting ARCHFLAGS="" in sage-build-env-config.in let me build Sage. Doctesting didn't go that well: various failures because of "command 'gcc' failed with exit status 1" and "#error Unsupported architecture" and "#error architecture not supported".

If we set ARCHFLAGS, we will have to do this in sage-env so that it also affects runtime use of the compiler (some of which is tested in doctests) and when users install Python packages with sage -pip.

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Branch pushed to git repo; I updated commit sha1. New commits:

be3196dbuild/pkgs/python3/spkg-configure.m4, src/bin/sage-env[-config.in]: Prepare conditional setting of ARCHFLAGS
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Changed commit from 3325a4e to be3196d

mkoeppe commented 3 years ago

Description changed:

--- 
+++ 
@@ -4,7 +4,7 @@

 #30725 fixes this for macOS, where some packages are explicitly built with `clang`.

-However, there is another mechanism how `clang` can come in: Python extension modules are compiled with the compiler listed in sysconfig. On macOS with homebrew, using `/usr/bin/python3`, this could again be `clang`, which does not accept `-march=native`.
+However, there is another mechanism how `clang` can come in: Python extension modules are compiled with the compiler listed in sysconfig. On macOS with homebrew, using `/usr/bin/python3`, this could again be `clang`, which does not accept `-march=native` in combination with `-arch arm64` (which is provided even on x86_64 if `ARCHFLAGS` is unset).

 In this ticket, we fix this by checking whether the compiler used by distutils accepts `-march=native` (or more generally, the flags determined earlier by `configure`). If it does not, then we disable the use of these flags.
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Branch pushed to git repo; I updated commit sha1. New commits:

4c1596cbuild/pkgs/python3/spkg-configure.m4, m4/sage_check_python_for_venv.m4: Refactor through new macro SAGE_PYTHON_CHECK_DISTUTILS
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Changed commit from be3196d to 4c1596c

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Changed commit from 4c1596c to dc5e225

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Branch pushed to git repo; I updated commit sha1. New commits:

dc5e225build/pkgs/python3/spkg-configure.m4: On macOS, if the distutils test fails, try using empty ARCHFLAGS
mkoeppe commented 3 years ago
comment:32

Here's a new version that may be worth testing

mkoeppe commented 3 years ago

Description changed:

--- 
+++ 
@@ -19,6 +19,7 @@
 egret:~/s/sage/sage-rebasing/worktree-algebraic-2018-spring (t/31132/homebrew__unused_packages__singular__pari_______in__usr_local_leak_into_build_when_using_homebrew_s_python3 *

-In this ticket, we set ARCHFLAGS during this configure step. +In this ticket, if the distutils test fails, we try if setting ARCHFLAGS="" fixes it. In that case, we store a configuration variable that causes sage-env to set ARCHFLAGS as well. +

mkoeppe commented 3 years ago
comment:34

Builds successfully for me with tox -e local-homebrew-macos-standard-python3_xcode

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Branch pushed to git repo; I updated commit sha1. New commits:

df17ad4tox.ini: Add configuration factors for specific homebrew python3.x
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Changed commit from dc5e225 to df17ad4

kliem commented 3 years ago
comment:36

What is the purpose of this line?

+    AC_SUBST([ARCHFLAGS], [unset])
kliem commented 3 years ago
comment:37

I think the second reason is wrong here. Isn't this the case when building a C extension fails?

dnl distutils test
AC_DEFUN([SAGE_PYTHON_CHECK_DISTUTILS], [
    m4_pushdef([PYTHON_EXE], [$1])
    m4_pushdef([COMMANDS_IF_DISTUTILS_GOOD], [$2])
    m4_pushdef([COMMANDS_IF_DISTUTILS_NOT_GOOD], [$3])
    SAGE_PYTHON_DISTUTILS_C_CONFTEST
    dnl (echo "***ENV***:"; env; echo "***SYSCONFIG***"; conftest_venv/bin/python3 -m sysconfig) >& AS_MESSAGE_LOG_FD
    echo PYTHON_EXE conftest.py --verbose build --build-base=conftest.dir >& AS_MESSAGE_LOG_FD
    AS_IF([PYTHON_EXE conftest.py --verbose build --build-base=conftest.dir >& AS_MESSAGE_LOG_FD 2>&1 ], [
        SAGE_PYTHON_DISTUTILS_CXX_CONFTEST
        echo PYTHON_EXE conftest.py --verbose build --build-base=conftest.dir >& AS_MESSAGE_LOG_FD 2>&1
        AS_IF([PYTHON_EXE conftest.py --verbose build --build-base=conftest.dir >& AS_MESSAGE_LOG_FD 2>&1 ], [
            COMMANDS_IF_DISTUTILS_GOOD], [
            reason="distutils cannot build a C++ 11 extension"
            COMMANDS_IF_DISTUTILS_NOT_GOOD
        ])
    ], [
       reason="distutils cannot build a C++ 11 extension"
       COMMANDS_IF_DISTUTILS_NOT_GOOD
    ])
    m4_popdef([PYTHON_EXE])
    m4_popdef([COMMANDS_IF_DISTUTILS_GOOD])
    m4_popdef([COMMANDS_IF_DISTUTILS_NOT_GOOD])
])
mkoeppe commented 3 years ago
comment:38

Also tox -e local-homebrew-usrlocal-minimal-python3.8 still works.