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 sagelib/cysignals builds via distutils.cfg #31335

Closed
mkoeppe opened this issue Feb 4, 2021 · 90 comments

Comments

@mkoeppe
Copy link
Contributor

mkoeppe commented Feb 4, 2021

Follow-up from #31132:

It has been reported that it's still leaking with the upgraded python3 from homebrew that includes Homebrew/homebrew-core@784d292

The sagelib build collects many -I, -L directives through pkgconfig.
Such directives on the command line take precedence over the search paths set by sage-env in $CPATH, $LIBRARY_PATH.
Now if -I/usr/local/ or -L/usr/local (instead of more specific paths to /usr/local/Cellar/.../...) appear, then unwanted packages that happen to be installed can leak in.

However, here the problem is caused by homebrew installing a distutils.cfg that injects /usr/local/include into our include search path.
We fix the leak by switching to a modern configuration of setuptools.

This also affects cysignals, as reported in https://groups.google.com/g/sage-release/c/KdSKg6RdZok/m/91Ibwn8RAgAJ

Relevant tickets: #13348, #14709, #29562 (+), #29607 (+), #29697 (?), #31041 (), #30818 (), #30013 (~); possible follow-up: #31338

CC: @jhpalmieri @zlscherr @kiwifb @kliem @slel

Component: build

Author: Matthias Koeppe

Branch: e03c5ac

Reviewer: John Palmieri

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

@mkoeppe mkoeppe added this to the sage-9.3 milestone Feb 4, 2021
@mkoeppe
Copy link
Contributor Author

mkoeppe commented Feb 4, 2021

Dependencies: #30589, #31132, #31227

@mkoeppe

This comment has been minimized.

@mkoeppe

This comment has been minimized.

@mkoeppe

This comment has been minimized.

@mkoeppe

This comment has been minimized.

@mkoeppe

This comment has been minimized.

@mkoeppe

This comment has been minimized.

@mkoeppe

This comment has been minimized.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Feb 4, 2021

comment:9

To check whether this comes in through pkg-config, I used:

for a in $(pkg-config --list-all | sed 's/ .*//;'); do printf "$a: "; pkg-config --cflags $a; printf "$a: "; pkg-config --libs $a; done

However, on my system, the command line in the ticket description does not find any relevant packages that directly use /usr/local/{include,lib} (only lua and fuse do)

@mkoeppe

This comment has been minimized.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Feb 4, 2021

Changed dependencies from #30589, #31132, #31227 to #30589, #31132, #31227, #30770

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Feb 4, 2021

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Feb 4, 2021

comment:13

With the tickets listed in Dependencies merged (that's the branch on this ticket) and after brew install singular pari ecl, I get leakage from singular:

[sagelib-9.3.beta6] [ 1/13] gcc -Wno-unused-result -Wsign-compare -Wunreachable-code -fno-common -dynamic -DNDEBUG -g -fwrapv -O3 -Wall -isysroot /Library/Developer/CommandLineTools/SDKs/MacOSX10.15.sdk -O2 -g -march=native -I./sage/cpython -I/Users/mkoeppe/s/sage/sage-rebasing/worktree-algebraic-2018-spring/local/include/singular -I/Users/mkoeppe/s/sage/sage-rebasing/worktree-algebraic-2018-spring/build/pkgs/sagelib/src -I/usr/local/Cellar/[email protected]/3.9.1_7/Frameworks/Python.framework/Versions/3.9/include/python3.9 -I/Users/mkoeppe/s/sage/sage-rebasing/worktree-algebraic-2018-spring/local/lib/python3.9/site-packages/numpy/core/include -Ibuild/cythonized -I/usr/local/include -I/usr/local/opt/[email protected]/include -I/usr/local/opt/sqlite/include -I/usr/local/opt/tcl-tk/include -I/Users/mkoeppe/s/sage/sage-rebasing/worktree-algebraic-2018-spring/local/include -I/usr/local/Cellar/[email protected]/3.9.1_7/Frameworks/Python.framework/Versions/3.9/include/python3.9 -c build/cythonized/sage/algebras/letterplace/free_algebra_letterplace.cpp -o build/temp.macosx-10.15-x86_64-3.9/build/cythonized/sage/algebras/letterplace/free_algebra_letterplace.o -fno-strict-aliasing -DCYTHON_CLINE_IN_TRACEBACK=1 -DSING_NDEBUG -DOM_NDEBUG -I/Users/mkoeppe/s/sage/sage-rebasing/worktree-algebraic-2018-spring/local/include/singular -std=c++11
[sagelib-9.3.beta6] In file included from build/cythonized/sage/algebras/letterplace/free_algebra_letterplace.cpp:672:
[sagelib-9.3.beta6] In file included from /usr/local/include/singular/Singular/libsingular.h:7:
[sagelib-9.3.beta6] In file included from /Users/mkoeppe/s/sage/sage-rebasing/worktree-algebraic-2018-spring/local/include/singular/kernel/structs.h:23:
[sagelib-9.3.beta6] In file included from /Users/mkoeppe/s/sage/sage-rebasing/worktree-algebraic-2018-spring/local/include/singular/kernel/polys.h:15:
[sagelib-9.3.beta6] In file included from /Users/mkoeppe/s/sage/sage-rebasing/worktree-algebraic-2018-spring/local/include/singular/polys/monomials/ring.h:13:
[sagelib-9.3.beta6] In file included from /Users/mkoeppe/s/sage/sage-rebasing/worktree-algebraic-2018-spring/local/include/singular/coeffs/coeffs.h:19:
[sagelib-9.3.beta6] In file included from /usr/local/include/factory/factory.h:28:
[sagelib-9.3.beta6] /usr/local/include/factory/si_log2.h:6:19: error: redefinition of 'SI_LOG2'
[sagelib-9.3.beta6] static inline int SI_LOG2(int v)

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Feb 4, 2021

@mkoeppe

This comment has been minimized.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Feb 4, 2021

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Feb 4, 2021

Commit: 6d3409c

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Feb 4, 2021

comment:15

I think this is a specific issue with how we include singular headers.


Last 10 new commits:

32dfaddMerge branch 't/31227/accept__usr_bin_python3_from_xcode_12_3_on_macos_10_15__catalina_' into t/31335/homebrew__unused_packages__singular__pari_______in__usr_local_leak_into_sagelib_build
3049e53Use ecl-config to determine compiler/linker flags for ecl
1041128Rename ecl config variable
fd05a97Add fallback
2ac4cc8Merge branch 'develop' of git://github.com/sagemath/sage into public/build/ecl-config
6370c34src/sage/env.py: Do not use capture_output, which requires python 3.7
31f1bbbAlso use universal_newlines instead of text, for python 3.6 compatibility
11ad40aMerge branch 'public/build/ecl-config' of git://trac.sagemath.org/sage into public/build/ecl-config
046f72fMerge branch 't/31227/accept__usr_bin_python3_from_xcode_12_3_on_macos_10_15__catalina_' into t/30770/public/build/ecl-config
6d3409cMerge branch 't/30770/public/build/ecl-config' into t/31335/homebrew__unused_packages__singular__pari_______in__usr_local_leak_into_sagelib_build

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Feb 4, 2021

comment:16

-I/Users/mkoeppe/s/sage/sage-rebasing/worktree-algebraic-2018-spring/local/include/singular is (correctly) near the beginning of the command line,
but the problem is that -I/Users/mkoeppe/s/sage/sage-rebasing/worktree-algebraic-2018-spring/local/include only comes at the end -- after -I/usr/local/include (which probably comes in from CPATH set in .homebrew-build-env)

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Feb 4, 2021

comment:17

Replying to @mkoeppe:

after -I/usr/local/include (which probably comes in from CPATH set in .homebrew-build-env)

no, it's not coming from CPATH

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Feb 4, 2021

comment:18

it's also not coming in from sage.env.cython_aliases or sage.env.sage_include_directories

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Feb 4, 2021

comment:19
cat  /usr/local/opt/[email protected]/Frameworks/Python.framework/Versions/3.9/lib/python3.9/distutils/distutils.cfg 
[install]
prefix=/usr/local
[build_ext]
include_dirs=/usr/local/include:/usr/local/opt/[email protected]/include:/usr/local/opt/sqlite/include:/usr/local/opt/tcl-tk/include
library_dirs=/usr/local/lib:/usr/local/opt/[email protected]/lib:/usr/local/opt/sqlite/lib:/usr/local/opt/tcl-tk/lib

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Feb 4, 2021

comment:20

There we go, it's another distribution bug in homebrew. It installs a distutils.cfg that injects the paths into our build.

homebrew has been struggling with this since at least 2014 - Homebrew/legacy-homebrew#26272

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 4, 2021

Changed commit from 6d3409c to 017c351

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 4, 2021

Branch pushed to git repo; I updated commit sha1. Last 10 new commits:

6432727Merge tag '9.3.beta2' into t/30912/sagelib__update_metadata_for_pypi_deployment
4a693f2Move build/pkgs/sagelib/src/setup.cfg to SAGE_ROOT/src, replace by symlink
ea182d7Copy changes from build/pkgs/sagelib/src to src
a1a10b9src/VERSION.txt: New
5697335src/setup.cfg: Add license_file=LICENSE.txt
deb9eb3Merge tag '9.3.beta3' into t/30912/sagelib__update_metadata_for_pypi_deployment
7ad4c0eMerge tag '9.3.beta4' into t/30912/sagelib__update_metadata_for_pypi_deployment
62e1945Merge branch 't/30912/sagelib__update_metadata_for_pypi_deployment' into t/31335/homebrew__unused_packages__singular__pari_______in__usr_local_leak_into_sagelib_build
5c86b59trac 31134: update setuptools, setuptools_scm
017c351Merge commit '5c86b590e64d4c18d392a14abc2129205862d12e' of git://trac.sagemath.org/sage into t/31335/homebrew__unused_packages__singular__pari_______in__usr_local_leak_into_sagelib_build

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Feb 4, 2021

Changed dependencies from #30589, #31132, #31227, #30770 to #30589, #31132, #31227, #30770, #31134

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 7, 2021

Changed commit from 9f38a3b to a1b3abc

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 7, 2021

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

515f899sage_setup.docbuild.AllBuilder: stop the non-reference manual docs from being built in parallel
804ebd7sage_setup.dpcbuild.AllBuilder: Restrict workaround to macOS
b4ceee5sage_setup.docbuild: In the workaround, do not go through build_many to build serially
ae4ea55Merge branch 't/31344/homebrew__docbuild_crashes__libtcl_atforkprepare' into t/31335/homebrew__unused_packages__singular__pari_______in__usr_local_leak_into_sagelib_build

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 7, 2021

Changed commit from a1b3abc to ae4ea55

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 7, 2021

Changed commit from ae4ea55 to add7db4

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 7, 2021

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

add7db4Merge tag '9.3.beta7' into t/31335/homebrew__unused_packages__singular__pari_______in__usr_local_leak_into_sagelib_build

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Feb 9, 2021

comment:58

Replying to @zlscherr:

Replying to @mkoeppe:

When you ran this test, were the environment variables set by .homebrew-build-env set?

at testing time they were not set.

Shall we consider this issue -- .homebrew-build-env necessary to be set during testing -- one that can be addressed in follow-up tickets? (#31348, #31365)

@zlscherr
Copy link

zlscherr commented Feb 9, 2021

comment:59

That sounds fair. With .homebrew-build-env sourced I didn't have any out of the ordinary issues with make ptest, so it seems that things are working well modulo the runtime stuff.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 9, 2021

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

4e777e7Merge tag '9.3.beta8' into t/31335/homebrew__unused_packages__singular__pari_______in__usr_local_leak_into_sagelib_build

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 9, 2021

Changed commit from add7db4 to 4e777e7

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Mar 9, 2021

Changed dependencies from #30589, #31132, #31227, #30770, #30912, #31134, #31344 to none

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Mar 9, 2021

comment:61

All dependencies have been merged. Let's get this in?

@mkoeppe

This comment has been minimized.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Mar 14, 2021

comment:62

This is arguably another homebrew distribution bug.

@mkoeppe mkoeppe changed the title homebrew: Unused packages (singular, pari, ...) in /usr/local leak into sagelib build via distutils.cfg homebrew: Unused packages (singular, pari, ...) in /usr/local leak into sagelib/cysignals builds via distutils.cfg Mar 14, 2021
@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 14, 2021

Changed commit from 4e777e7 to bf433e8

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 14, 2021

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

bf433e8build/bin/sage-build-env-config.in: Set SETUPTOOLS_USE_DISTUTILS here, not in build/pkgs/sagelib/spkg-install

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 14, 2021

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

e03c5acbuild/bin/sage-build-env: Set SETUPTOOLS_USE_DISTUTILS here, not in build/pkgs/sagelib/spkg-install

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 14, 2021

Changed commit from bf433e8 to e03c5ac

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Mar 14, 2021

comment:65

Prompted by slelievre's report on sage-devel, applying this fix to all packages by putting the setting in build/bin/sage-build-env.
Needs review

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Mar 16, 2021

comment:66

Confirmed by @slel to fix the cysignals build in https://groups.google.com/g/sage-release/c/KdSKg6RdZok/m/O1VHLlUKAwAJ

@jhpalmieri
Copy link
Member

Reviewer: John Palmieri

@jhpalmieri
Copy link
Member

comment:67

I can confirm, too: if I install pari and singular via homebrew, then cysignals doesn't build without this fix.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Mar 16, 2021

comment:68

Thanks!

@vbraun
Copy link
Member

vbraun commented Mar 18, 2021

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Feb 8, 2022

Changed commit from e03c5ac to none

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Feb 8, 2022

comment:70

The problem with distutils.cfg has now been addressed by homebrew for their Python 3.10 package, see #31348 comment:38

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

4 participants