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

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

Closed
mkoeppe opened this issue Jan 13, 2021 · 103 comments
Closed

Comments

@mkoeppe
Copy link
Contributor

mkoeppe commented Jan 13, 2021

(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 mkoeppe added this to the sage-9.3 milestone Jan 13, 2021
@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jan 13, 2021

Dependencies: #31132

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jan 13, 2021

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jan 14, 2021

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
Copy link
Contributor Author

mkoeppe commented Jan 14, 2021

Changed dependencies from #31132 to #31132, #31228

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jan 14, 2021

Commit: ce34810

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 23, 2021

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_

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 23, 2021

Changed commit from ce34810 to 67d66b7

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jan 23, 2021

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

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 23, 2021

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

3325a4ebuild/pkgs/python3/spkg-configure.m4: Actually test with CFLAGS=$CFLAGS_MARCH

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 23, 2021

Changed commit from 67d66b7 to 3325a4e

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jan 23, 2021

comment:8

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

@mkoeppe

This comment has been minimized.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jan 23, 2021

Author: Matthias Koeppe

@kliem
Copy link
Contributor

kliem commented Jan 25, 2021

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
Copy link
Contributor

kliem commented Jan 25, 2021

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

@kliem
Copy link
Contributor

kliem commented Jan 25, 2021

comment:10

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

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jan 25, 2021

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

@mkoeppe

This comment has been minimized.

@jhpalmieri
Copy link
Member

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
Copy link
Contributor Author

mkoeppe commented Jan 26, 2021

comment:14

Did you run bootstrap?

@jhpalmieri
Copy link
Member

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
Copy link
Member

Attachment: pillow-8.0.1.log

@jhpalmieri
Copy link
Member

comment:16

Attachment: cython-0.29.21.log

Oh, and gmpy2, also.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 1, 2021

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

0b3e70dbuild/pkgs/python3/spkg-configure.m4: If PYTHON_FOR_VENV is configured to build multiarch extensions, set SAGE_ARCHFLAGS to disable it

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Feb 1, 2021

comment:62

Here's a new version to try on big sur

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 1, 2021

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

372adcb.github/workflows/tox.yml: Update xcode versions

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 1, 2021

Changed commit from 0b3e70d to 372adcb

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Feb 1, 2021

comment:64

Started tests at https://github.com/mkoeppe/sage/actions/runs/526769308 but it looks like there is a long wait time to get macos instances

@mkoeppe

This comment has been minimized.

@mkoeppe

This comment has been minimized.

@mkoeppe mkoeppe changed the title Accept /usr/bin/python3 from XCode 12.3 on macOS 10.15 (Catalina) Accept /usr/bin/python3 from XCode 12.3 on macOS 10.15 (Catalina) and 11 (Big Sur) Feb 1, 2021
@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 1, 2021

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

fc8b676SAGE_CHECK_PYTHON_FOR_VENV: Rework with less nesting

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 1, 2021

Changed commit from 372adcb to fc8b676

@zlscherr
Copy link

zlscherr commented Feb 1, 2021

comment:69

For the record, I was able to successfully build sage and documentation with this ticket on Big Sur using

--with-python=/usr/bin/python3

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Feb 1, 2021

comment:70

Thanks for testing!

@jhpalmieri
Copy link
Member

comment:71

Works for me with the system Python 3 on both Big Sur and Catalina.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Feb 1, 2021

comment:72

Thanks for testing!

Let's deal with issue #31314 (messages from find_stale_files) in a follow up.

Ready for review?

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Feb 2, 2021

comment:73

Replying to @mkoeppe:

Started tests at https://github.com/mkoeppe/sage/actions/runs/526769308 but it looks like there is a long wait time to get macos instances

Some results have been coming in:

  • local-homebrew-macos-10.15-xcode-11.7-{minimal,standard}: clean
  • local-homebrew-macos-11.0-xcode-11.7-{minimal,standard}: Homebrew complains that xcode is too old

Still waiting for the tests with other xcode versions (12.2, 12.3, 12.4). No failures so far.

Note all runs are on x86_64 - we do not have test infrastructor for macos-arm64 yet.

@kliem
Copy link
Contributor

kliem commented Feb 2, 2021

Changed reviewer from https://github.com/kliem/sage/pull/36/checks, Jonathan Kliem to Jonathan Kliem

@kliem
Copy link
Contributor

kliem commented Feb 2, 2021

comment:74

I'm happy with this ticket, but I would like another opinion on this.

@kliem
Copy link
Contributor

kliem commented Feb 2, 2021

comment:75

Ok, I give it another 24 hours. If nobody speaks up, I'll give a positive review (after taking a final look of course).

@zlscherr
Copy link

zlscherr commented Feb 2, 2021

comment:76

I don't feel like I have enough experience to review this, but it certainly built on Big Sur and Catalina. I'm going to upgrade Big Sur to 11.2 later and see if this builds on 11.2 as well.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Feb 2, 2021

comment:77

Replying to @mkoeppe:

Started tests at https://github.com/mkoeppe/sage/actions/runs/526769308 but it looks like there is a long wait time to get macos instances

Some more results:

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Feb 2, 2021

comment:78

Let's take care of the scipy build failures in #31326.

I think this is sufficient improvement for one ticket.

@zlscherr
Copy link

zlscherr commented Feb 3, 2021

comment:80

Seems to work well on Big Sur 11.2 with standard homebrew packages. When running make ptest I get the annoying warnings:

ld: warning: dylib (/usr/local/lib/libgmp.dylib) was built for newer macOS version (10.15) than being linked (10.14.6)

which is probably because the MACOSX_DEPLOYMENT_TARGET for /usr/bin/python3 is set to 10.14.6 which doesn't match what Homebrew sets. It's easy enough to eliminate the warnings by setting MACOSX_DEPLOYMENT_TARGET=11.0 before doing all the testing.

Otherwise, everything seems to build/test correctly.

@kliem
Copy link
Contributor

kliem commented Feb 3, 2021

comment:81

Ok. Thanks for fixing all of this. I think the ticket is a good improvement as it is.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Feb 3, 2021

comment:82

Thanks!

@vbraun
Copy link
Member

vbraun commented Feb 20, 2021

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

No branches or pull requests

5 participants