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

homebrew: Unused packages (singular, pari, ...) in /usr/local leak into build when using homebrew's python3 #31132

Closed
mkoeppe opened this issue Dec 29, 2020 · 80 comments

Comments

@mkoeppe
Copy link
Contributor

mkoeppe commented Dec 29, 2020

As previously discussed in #30745, /usr/local is leaking into our build, through wrong orders of include and/or library directives.

In #30745, this problem was not fixed but rather some more packages were found to be usable on homebrew, so distro information was added.

The problem has been reported again with pari (building cypari2) in #31029/#30589.

This is caused by a misconfigured python3. In this ticket, we add code to detect this problem when looking for the system python at configure time.

Checking whether SageMath should install SPKG python3...
checking whether any of libpng bzip2 xz libffi is installed as or will be installed as SPKG... no
checking for python3 >= 3.6.0, < 3.10.0 with modules sqlite3, ctypes, math, hashlib, crypt, readline, socket, zlib, distutils.core... 
checking ... whether /usr/local/bin/python3 is good... no, this is a misconfigured Python whose sysconfig compiler/linker flags contain -I or -L options, which may cause wrong versions of libraries to leak into the build of Python packages - see https://github.com/sagemath/sage-prod/issues/31132; to use it anyway, use ./configure --with-python=/usr/local/bin/python3
checking ... whether /usr/bin/python3 is good... yes
checking for python3 >= 3.6.0, < 3.10.0 with modules sqlite3, ctypes, math, hashlib, crypt, readline, socket, zlib, distutils.core... /usr/bin/python3
configure: will use system package and not install SPKG python3

When --with-python=... is used, only a warning is issued.

See also:

Related homebrew issues and pull requests:

Depends on #30589

Upstream: Reported upstream. No feedback yet.

CC: @orlitzky @jhpalmieri @zlscherr @kiwifb @NathanDunfield @kliem

Component: build

Author: Matthias Koeppe

Branch: 76d5fd1

Reviewer: Zachary Scherr

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

@mkoeppe mkoeppe added this to the sage-9.3 milestone Dec 29, 2020
@zlscherr
Copy link

comment:1

I discovered another annoying leak while testing. I have octave installed via homebrew and one of its dependencies is qhull. With qhull installed via homebrew if I build sage and run

./sage -tp 8 --long --warn-long 31.1 --random-seed=0 src/sage/plot/plot3d/list_plot3d.py

then it crashes with

sage: m = matrix(RDF, 6, [sin(i^2 + j^2) for i in [0,pi/5,..,pi] for j in [0,pi/5,..,pi]]) ## line 379 ##
sage: list_plot3d(m, color='yellow', interpolation_type='linear', num_points=5) # indirect doctest ## line 380 ##
QH6249 qh_lib_check: Incorrect qhull library called.  Size of qhT for caller is 2896, but for library is 2792.
QH6256 qh_lib_check: Cannot continue.  Library 'qhull 7.2.0 (2015.2 2016/01/18)' uses a static qhT (e.g., libqhull.so)

I was able to track the problem down to matplotlib. If I do:

make matplotlib-clean

brew unlink qhull

make matplotlib

brew link qhull

then the tests pass without any issues.

@zlscherr
Copy link

comment:2

I think I'll open an issue with matplotlib since it's not really a sage problem.

@zlscherr
Copy link

zlscherr commented Jan 2, 2021

comment:3

I created #31148 to use system qhull when building matplotlib if it is present. This should fix these issues.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jan 7, 2021

comment:4

In #31180, @NathanDunfield pointed out that if we use python3 from homebrew, /usr/local/include ends up on the front of the include search path via sysconfig CFLAGS.

>>> import sysconfig
>>> sysconfig.get_config_var('CFLAGS')
'-Wno-unused-result -Wsign-compare -Wunreachable-code -fno-common -dynamic -DNDEBUG -g -fwrapv -O3 -Wall -I/usr/local/include -isysroot /Library/Developer/CommandLineTools/SDKs/MacOSX10.15.sdk -I/Library/Developer/CommandLineTools/SDKs/MacOSX10.15.sdk/usr/include -I/Library/Developer/CommandLineTools/SDKs/MacOSX10.15.sdk/System/Library/Frameworks/Tk.framework/Versions/8.5/Headers'

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jan 7, 2021

comment:5

Happening in setuptools._distutils.sysconfig.customize_compiler (https://github.com/pypa/setuptools/blob/main/setuptools/_distutils/sysconfig.py#L185) via __init_posix

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jan 7, 2021

comment:6

By setting environment variables, one cannot override these flags, only append.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jan 7, 2021

comment:7

I think this is really a packaging bug in homebrew's python3.

@NathanDunfield
Copy link
Contributor

comment:8

I agree that this is a packaging bug in homebrew. I checked four other Python 3 .* packages (Ubuntu 20.04, CentOS 7, Python.org's macOS installer, Conda) and none of them have -Iincludes in sysconfig.get_config_var('CFLAGS').

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jan 7, 2021

comment:9

homebrew issue: Homebrew/homebrew-core#68352

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jan 7, 2021

Upstream: Reported upstream. No feedback yet.

@mkoeppe mkoeppe changed the title homebrew: Unused packages (singular, pari, ...) in /usr/local leak into build homebrew: Unused packages (singular, pari, ...) in /usr/local leak into build when using homebrew's python3 Jan 7, 2021
@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jan 8, 2021

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jan 8, 2021

Commit: 2c60025

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jan 8, 2021

Author: Matthias Koeppe

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jan 8, 2021

comment:13

Let's see what the homebrew maintainers do with this.

For now the best thing we can do is detect the issue and print a warning (or we could reject the homebrew python with this issue entirely.)


New commits:

2c60025m4/sage_check_python_for_venv.m4: Warn if python3 is misconfigured like it is currently in homebrew

@mkoeppe

This comment has been minimized.

@jhpalmieri
Copy link
Member

comment:16

This warning is buried in config.log. Would it be better to print it at the end, along with the instructions for what additional packages should be installed, or is that too hard to implement.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 8, 2021

Changed commit from 2c60025 to ee679bd

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 8, 2021

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

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=...

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jan 8, 2021

comment:18

Here's a different approach. Now actually rejecting the broken homebrew python3.

For any package, to see the reasons why a system package is rejected, one needs to scroll up in the configure output (or look into config.log)

@mkoeppe

This comment has been minimized.

@jhpalmieri
Copy link
Member

comment:20

Looks okay to me. It took one machine about 30 seconds longer to build Sage when it had to build its own Python (just one iteration, so not very scientific), so avoiding homebrew's Python for now is not placing a huge burden on developers.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jan 10, 2021

comment:21

On my machine (I'm still running Catalina), it accepts /usr/bin/python3.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jan 10, 2021

comment:22

Let's get this in please

@zlscherr
Copy link

comment:23

looks good to me on Mac. Big Sur also has /usr/bin/python3 which gets picked up and accepted.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jan 14, 2021

comment:53

Replying to @zlscherr:

I think this is caused by build/pkgs/python3/spkg-configure.m4. It appears to set

SAGE_MACOSX_DEPLOYMENT_TARGET='legacy'

which is then picked up by sage-env.

That's right, this mechanism deactivates the old code in sage-env when we use system python3 instead of building our own python.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jan 14, 2021

comment:54

Thanks!

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jan 14, 2021

comment:55

The Homebrew python maintainers are still struggling to fix up their python builds. I have added some links to the ticket description to track these efforts.

@mkoeppe

This comment has been minimized.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jan 16, 2021

comment:56

Replying to @mkoeppe:

Replying to @mkoeppe:

Replying to @zlscherr:

on Big Sur, configure allowed me to use /usr/bin/python3 but then Cython failed to build.

  gcc -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 -O2 -g -march=native -I/Users/zscherr/sage/develop/local/include -I/Applications/Xcode.app/Contents/Developer/Library/Frameworks/Python3.framework/Versions/3.8/include/python3.8 -c /private/var/folders/5f/z870t4kd5dlctkcrs984nh8r0000gn/T/pip-req-build-p2i1cj72/Cython/Plex/Scanners.c -o build/temp.macosx-10.14.6-x86_64-3.8/private/var/folders/5f/z870t4kd5dlctkcrs984nh8r0000gn/T/pip-req-build-p2i1cj72/Cython/Plex/Scanners.o
  clang: error: the clang compiler does not support '-march=native'
  error: command 'gcc' failed with exit status 1

This looks like a complication that may have come in through #27122

I have opened #31228 for this.

See also #30725.

@vbraun
Copy link
Member

vbraun commented Jan 24, 2021

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Feb 3, 2021

Changed commit from 76d5fd1 to none

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Feb 3, 2021

comment:58

A fix has just been merged into homebrew master Homebrew/homebrew-core@784d292

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Feb 3, 2021

comment:59

And indeed the upgrade of python3 to 3.9.1_7 has repaired it.

@jhpalmieri
Copy link
Member

comment:60

brew info python@3.9 tells me that I have 3.9.1_7 installed, but I still see this in config.log:

	CFLAGS = "-Wno-unused-result -Wsign-compare -Wunreachable-code -fno-common -dynamic -DNDEBUG -g -fwrapv -O3 -Wall -I/usr/local/include -isysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk -I/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include"
	LDFLAGS = "-L/usr/local/lib -isysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk"
configure:32994: result: no, this is a misconfigured Python whose sysconfig compiler/linker flags contain -I or -L options, which may cause wrong versions of libraries to leak into the build of Python packages - see https://github.com/sagemath/sage-prod/issues/31132; to use it anyway, use ./configure --with-python=/usr/local/bin/python3

@zlscherr
Copy link

zlscherr commented Feb 3, 2021

comment:61

very bizarre. I had to do:

brew reinstall python@3.9 --build-from-source

and that fixed it for me. Maybe the bottle hasn't been updated yet?

@zlscherr
Copy link

zlscherr commented Feb 3, 2021

comment:62

Although I'm not sure if I needed the --build-from-source. Maybe just try brew reinstall first?

@jhpalmieri
Copy link
Member

comment:63

I tried brew reinstall python@3.9 and that seems to have worked.

@zlscherr
Copy link

zlscherr commented Feb 3, 2021

comment:64

I mentioned this on the thread and it’s because they forgot to bump the version number I guess from 7 to 8. The next time it upgrades everything should be good without needing to reinstall.

@zlscherr
Copy link

zlscherr commented Feb 4, 2021

comment:65

Has anyone else tried building sage with the newest homebrew python? Configure now accepts it, but I still have issues with /usr/local leaking into the builds. Not sure why that happens now that python3-config no longer remembers /usr/local.

@zlscherr
Copy link

zlscherr commented Feb 4, 2021

comment:66

Although I should add that cypari build fine even with homebrew's pari was installed. The problem was that singular and pari were leaking into building sagelib, and then ecl leaked into building the manifolds documentation.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Feb 4, 2021

comment:67

Follow up in #31335.

@jhpalmieri
Copy link
Member

comment:68

Replying to @jhpalmieri:

I tried brew reinstall python@3.9 and that seems to have worked.

This worked on one machine (Big Sur) but not another (Catalina). I deleted the cached download and it still didn't work, and neither did brew reinstall python@3.9 -s. Oh well, maybe 3.9.1_8 will work on this machine.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Feb 4, 2021

comment:69

I think I had to do brew update first, which displayed some instructions regarding unshallowing, after which I ran brew update again.

@jhpalmieri
Copy link
Member

comment:70

Doing brew upgrade and then forcing the upgrade to Python worked, although maybe that's because it's now version 3.9.1_8. (Pro-tip: if you've forced Sage to build with homebrew's Python, don't upgrade that Python in the middle of doctesting, at least if you want doctesting to go well.)

@orlitzky
Copy link
Contributor

comment:71

This is now rejecting the system python on Gentoo, I guess because of the -L in LDFLAGS:

$ python3 -m sysconfig
...
CONFIGURE_LDFLAGS = "-Wl,-O1 -Wl,--as-needed -L."
LDFLAGS = "-Wl,-O1 -Wl,--as-needed -L."

That is added to our python build for obsolete reasons (there is no Gentoo/FreeBSD anymore):

# Set LDFLAGS so we link modules with -lpython3.2 correctly.                                                                                                             
# Needed on FreeBSD unless Python 3.2 is already installed.                                                                                                              
# Please query BSD team before removing this!                                                                                                                            
append-ldflags "-L."

Can someone briefly summarize the problem that this -L. causes, so that I can get it removed?

@NathanDunfield
Copy link
Contributor

comment:73

Replying to @orlitzky:

Can someone briefly summarize the problem that this -L. causes, so that I can get it removed?

When building a C/C++ extension module for Python in the standard way (distutils/setuptools), compiler and linker flags are pulled from sysconfig. While you can specify additional flags for such a module, the ones in sysconfig come first in the compile/link commands, meaning that they take precedence. In the present case, that means if there is a copy of a library in . there is no way to instead use a version located elsewhere, e.g. in /usr/lib.

@orlitzky
Copy link
Contributor

comment:74

Great, thanks. Fix is in progress.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Feb 11, 2021

comment:75

Exception for -L. was done in #31358

@isuruf
Copy link
Member

isuruf commented Sep 24, 2021

comment:76

Can't this be fixed by stripping CFLAGS and LDSHARED of -I and -L flags and then appending them to CFLAGS and LDFLAGS env variables?

@isuruf
Copy link
Member

isuruf commented Sep 24, 2021

comment:77

-I can't be stripped, but -L can be stripped by setting LDSHARED env variable. This would fix the conda-forge issue as conda-forge only use -L and doesn't use -I.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Sep 25, 2021

comment:78

Let's discuss this in #31539

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

7 participants