spkg-configure.m4 for python3

dimpase commented 5 years ago

Do not install python3 if a sufficiently new version is available in $PATH. This could be a system python3, or the python3 from within a venv.

As suggested in #29032, we make $SAGE_LOCAL a venv (https://docs.python.org/3/library/venv.html) over the system python3 -- if a suitable system python3 is found -- by using the venv.EnvBuilder API.

This is done by the new script build/bin/sage-venv, which we use both during configure (to make sure that we select a system python3 for which venvs work correctly) and during make.

The venv does not include the system site-packages: We continue to install all Python packages into our venv using the existing build infrastructure. We keep the task of using system site-packages for the follow-up ticket #29023.

(Note, we use venv (new since Python 3.3), not virtualenv. So this change is limited to Python 3 builds of Sage.)

CC: @jdemeyer @embray @vbraun @kiwifb @jhpalmieri @videlec

Component: build: configure

Author: Matthias Koeppe, Erik Bray

Branch: 1c845a4

Reviewer: Dima Pasechnik

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

dimpase commented 5 years ago

Description changed:

--- 
+++ 
@@ -2,4 +2,15 @@

 At the moment I don't know how feasible it is - one small obstacle might be that we really do not want to mess with two pythons at the same time.

-
+while it should be possible/desirable to use a system python, any additional Python packages should optionally be installable into a virtualenv instead of the system site-packages, including sagelib itself unless the system *happens* to meet all of sagelib's Python dependencies already.  Listing some relevant Python packages below:
+* cysignals
+* cython
+* ipython
+* jinja2
+* jupyter_core
+* numpy
+* scipy
+* sympy
+* pip
+* setuptools
+* six

dimpase commented 5 years ago

Description changed:

--- 
+++ 
@@ -13,4 +13,5 @@
 * sympy
 * pip
 * setuptools
+* pkgconfig
 * six

embray commented 5 years ago

comment:3

As the Sage-8.8 release milestone is pending, we should delete the sage-8.8 milestone for tickets that are not actively being worked on or that still require significant work to move forward. If you feel that this ticket should be included in the next Sage release at the soonest please set its milestone to the next release milestone (sage-8.9).

embray commented 5 years ago

comment:4

For Python packages when installing using the system Python we definitely need a well thought out approach, likely with different options for handling questions such as where to install additional Python packages.

$SAGE_LOCAL already acts as a sort-of virtualenv, but we might still consider using virtualenv/venv in $SAGE_LOCAL (with the --system-site-packages option) as a way of extending sys.path while still using as many system Python packages as possible.

mkoeppe commented 4 years ago

comment:5

See #29013 - "In a python3 build, install all Python packages into a venv"

mkoeppe commented 4 years ago

Description changed:

--- 
+++ 
@@ -1,17 +1,5 @@
-do not install pythonX if we are in the virtenv of pythonX.
+Do not install python3 if a sufficiently new version is available in `$PATH`. This could be a system python3, or the python3 from within a venv.

-At the moment I don't know how feasible it is - one small obstacle might be that we really do not want to mess with two pythons at the same time.
+Via #29013, we install all of our own Python packages into a separate venv that includes the system site-packages.

-while it should be possible/desirable to use a system python, any additional Python packages should optionally be installable into a virtualenv instead of the system site-packages, including sagelib itself unless the system *happens* to meet all of sagelib's Python dependencies already.  Listing some relevant Python packages below:
-* cysignals
-* cython
-* ipython
-* jinja2
-* jupyter_core
-* numpy
-* scipy
-* sympy
-* pip
-* setuptools
-* pkgconfig
-* six
+

mkoeppe commented 4 years ago

Dependencies: #29013

mkoeppe commented 4 years ago

comment:6

Narrowed the ticket to py3.

mkoeppe commented 4 years ago

Branch: public/27824-python3-spkg-configure

mkoeppe commented 4 years ago

comment:8

Here's a version that works. The venv is created in build/make/Makefile target (in #29013, I created the venv in the spkg-postinst script of the python3 spkg.)

New commits:

`1363425`	`In a python3 build, install all Python packages into a venv`
`a394268`	`sage-pip-install: Use PYTHON=sage-python23`
`a84815b`	`spkg-configure.m4 for python3`

mkoeppe commented 4 years ago

Commit: a84815b

mkoeppe commented 4 years ago

Author: Matthias Koeppe

dimpase commented 4 years ago

comment:10

how about checking for Py3 deps: sqlite libpng bzip2 xz libffi ? (I skipped in this list ones that are 2nd order, so no need to check them)

dimpase commented 4 years ago

comment:11

On Gentoo Python 3 does not have sqlite module by default. I had to do $ pip install pysqlite3 --user, and still elliptic_curves spkg installation errors out.

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

Changed commit from a84815b to 0d0495f

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

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

0d0495f build/pkgs/python3/spkg-configure.m4: Add SAGE_SPKG_DEPCHECK for sqlite libpng bzip2 xz libffi

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

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

98dff9d Remove use of --system-site-packages

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

Changed commit from 0d0495f to 98dff9d

mkoeppe commented 4 years ago

comment:14

I've reduced the scope of the ticket, excluding the use of system site packages.

mkoeppe commented 4 years ago

Description changed:

--- 
+++ 
@@ -1,5 +1,7 @@
 Do not install python3 if a sufficiently new version is available in `$PATH`. This could be a system python3, or the python3 from within a venv.

-Via #29013, we install all of our own Python packages into a separate venv that includes the system site-packages.
+Via #29013, we install all of our own Python packages into a separate venv.
+
+The venv does not include the system site-packages; we keep that task for a follow-up ticket.

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

Changed commit from 98dff9d to 6180132

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

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

6180132 build/pkgs/python3/spkg-configure.m4: Find @PYTHON_FOR_VENV@ that has sqlite3 module

dimpase commented 4 years ago

comment:16

I tried this on gentoo (after a usual struggle to get sqlite3 module built in Python) and it broke at docbuild, with

RuntimeError: libSingular not found--a working Singular install in $SAGE_LOCAL is required for Sage to work

even though libSingular has built just fine (it's some magic with from sage.env import SINGULAR_SO that is broken, so SINGULAR_SO is None)

mkoeppe commented 4 years ago

comment:17

Thanks for testing!

dimpase commented 4 years ago

comment:18

Replying to @mkoeppe:

I've reduced the scope of the ticket, excluding the use of system site packages.

do you mean pip/etc-installed site-packages of system's Python?

mkoeppe commented 4 years ago

comment:19

Replying to @dimpase:

Replying to @mkoeppe:

I've reduced the scope of the ticket, excluding the use of system site packages.

do you mean pip/etc-installed site-packages of system's Python?

Yes.

mkoeppe commented 4 years ago

Changed dependencies from #29013 to #29013, #29019

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

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

`61d7822`	`Trac #29002: lower the version bound on the system sqlite check.`
`3ab4515`	`Merge branch 'u/mjo/ticket/29002' of git://trac.sagemath.org/sage into t/27824/public/27824-python3-spkg-configure`
`c7b9ae4`	`Add sqlite depcheck`
`961f850`	`Trac #29002: lower the version bound on the system sqlite check (again).`
`d321948`	`Trac #29002: drop the readline dependency check for system sqlite.`
`14d15fb`	`Merge branch 'u/mjo/ticket/29002' of git://trac.sagemath.org/sage into t/27824/public/27824-python3-spkg-configure`
`258d112`	`build/pkgs/pillow/patches/setup.py.patch: Don't fail if Py_MACOS_SYSROOT sysconfig variable does not exist`
`43673c0`	`Merge branch 't/29019/make_patch_to_pillow_more_robust__do_not_depend_on__py_macos_sysroot__' into t/27824/public/27824-python3-spkg-configure`
`56b1618`	`build/pkgs/pillow/spkg-install: Help setup.py find zlib.h on macOS; show more build output`
`5930b4e`	`Merge branch 't/29019/make_patch_to_pillow_more_robust__do_not_depend_on__py_macos_sysroot__' into t/27824/public/27824-python3-spkg-configure`

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

Changed commit from 6180132 to 5930b4e

mkoeppe commented 4 years ago

comment:22

Now this is giving

[pillow-5.3.0.p0] gcc -Wno-unused-result -Wsign-compare -Wunreachable-code -fno-common -dynamic -DNDEBUG -g -fwrapv -O3 -Wall -I/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.15.sdk/usr/include -I/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.15.sdk/System/Library/Frameworks/Tk.framework/Versions/8.5/Headers -g -DHAVE_OPENJPEG -DHAVE_LIBZ -DHAVE_LIBTIFF -DPILLOW_VERSION="5.3.0" -I/usr/local/Cellar/openjpeg/2.3.1/include/openjpeg-2.3 -I/Users/mkoeppe/s/sage/sage-rebasing/worktree-venv/local/var/tmp/sage/build/pillow-5.3.0.p0/src/src/libImaging -I/usr/local/Cellar/jpeg/9c/include -I/usr/local/Cellar/libtiff/4.1.0/include -I/usr/local/Cellar/freetype/2.10.1/include/freetype2 -I/usr/local/Cellar/little-cms2/2.9/include -I/Users/mkoeppe/s/sage/sage-rebasing/worktree-venv/local/include -I/Library/Developer/CommandLineTools/SDKs/MacOSX10.15.sdk/usr/include -I/Users/mkoeppe/s/sage/sage-rebasing/worktree-venv/local/lib/sage/venv/sage/include -I/usr/local/include -I/usr/local/Cellar/freetype/2.10.1/include -I/usr/local/include -I/usr/local/opt/openssl@1.1/include -I/usr/local/opt/sqlite/include -I/Users/mkoeppe/s/sage/sage-rebasing/worktree-venv/local/lib/sage/venv/sage/include -I/usr/local/Cellar/python/3.7.6_1/Frameworks/Python.framework/Versions/3.7/include/python3.7m -c src/_imaging.c -o build/temp.macosx-10.15-x86_64-3.7/src/_imaging.o
[pillow-5.3.0.p0] error: $MACOSX_DEPLOYMENT_TARGET mismatch: now "10.9" but "10.15" during configure
[pillow-5.3.0.p0] ********************************************************************************

This is coming from the fake MACOSX_DEPLOYMENT_TARGET set in sage-env. See #18272, #7095, #18254, #16312.

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

Changed commit from 5930b4e to 7bd8746

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

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

`3b34e2e`	`src/bin/sage-env: Don't set MACOSX_DEPLOYMENT_TARGET if we use system python3`
`7bd8746`	`_get_shared_lib_filename: Do not assume Python sysconfig paths are in SAGE_LOCAL`

mkoeppe commented 4 years ago

comment:24

Replying to @dimpase:

I tried this on gentoo (after a usual struggle to get sqlite3 module built in Python) and it broke at docbuild, with
RuntimeError: libSingular not found--a working Singular install in $SAGE_LOCAL is required for Sage to work
even though libSingular has built just fine (it's some magic with from sage.env import SINGULAR_SO that is broken, so SINGULAR_SO is None)

Fixed now.

mkoeppe commented 4 years ago

Description changed:

--- 
+++ 
@@ -2,6 +2,6 @@

 Via #29013, we install all of our own Python packages into a separate venv.

-The venv does not include the system site-packages; we keep that task for a follow-up ticket.
+The venv does not include the system site-packages; we keep that task for the follow-up ticket #29023.

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

Changed commit from 7bd8746 to cd864e8

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

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

`d4fbe40`	`_get_shared_lib_filename: Do not assume Python sysconfig paths are in SAGE_LOCAL`
`bba08ec`	`Create module src/sage/env_config.py from src/sage/env_config.py.in, defining SAGE_LOCAL`
`d1779cc`	`Merge branch 't/29022/create_module_src_sage_env_config_py_from_src_sage_env_config_py_in__defining_variables_for_use_in_sage_env' into t/29013/public/29013-use-py3-venv`
`cd864e8`	`Merge branch 't/29013/public/29013-use-py3-venv' into t/27824/public/27824-python3-spkg-configure`

mkoeppe commented 4 years ago

comment:27

Compiles OK on macOS with homebrew. make ptestlong mostly good except for

GLPK output (spkg-configure should really reject a libglpk that talks too much) - it makes it hard to see actual failures
and numerical failures in src/sage/matrix/matrix2.pyx, src/sage/finance/time_series.pyx, and src/sage/schemes/affine/affine_homset.py as reported in #29013

sage -t --long src/sage/matrix/matrix2.pyx  # 2 doctests failed
sage -t --long src/sage/finance/time_series.pyx  # 5 doctests failed
sage -t --long src/sage/schemes/affine/affine_homset.py  # 1 doctest failed

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

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

`85147f9`	`build/pkgs/numpy/lapack_conf.py: Add a [DEFAULT] section to site.cfg`
`93bae6f`	`Merge branch 't/29025/numpy__site_cfg_needs_a__default__section' into t/29013/public/29013-use-py3-venv`
`669b1df`	`Merge branch 't/29013/public/29013-use-py3-venv' into t/27824/public/27824-python3-spkg-configure`

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

Changed commit from cd864e8 to 669b1df

dimpase commented 4 years ago

comment:29

Replying to @mkoeppe:

Replying to @dimpase:
I tried this on gentoo (after a usual struggle to get sqlite3 module built in Python) and it broke at docbuild, with
RuntimeError: libSingular not found--a working Singular install in $SAGE_LOCAL is required for Sage to work
even though libSingular has built just fine (it's some magic with from sage.env import SINGULAR_SO that is broken, so SINGULAR_SO is None)
Fixed now.

after this fix, it builds and passes most tests on Gentoo (some errors related to cryptominisat that I had installed, and it probably got broken in some way)

dimpase commented 4 years ago

comment:30

I see a problem with cryptominisat spkg on this branch (which otherwise builds and passes most tests (not related to this packages) on Gentoo)

sage -t --warn-long 88.1 src/sage/sat/solvers/cryptominisat.py
**********************************************************************
File "src/sage/sat/solvers/cryptominisat.py", line 49, in sage.sat.solvers.cryptominisat.CryptoMiniSat
Failed example:
    solver = CryptoMiniSat()                                  # optional - cryptominisat
Exception raised:
    Traceback (most recent call last):
      File "/mnt/opt/Sage/sage-dev/local/lib/sage/venv/sage/lib/python3.7/site-packages/sage/sat/solvers/cryptominisat.py", line 69, in __init__
        from pycryptosat import Solver
    ModuleNotFoundError: No module named 'pycryptosat'

I wonder whether it uses a convoluted way to create python extensions, which got broken.

dimpase commented 4 years ago

comment:31

I see (on a system where cryptominisat package works) that it is one of few packages that get installed without a sub-directory named the same as package in site-packages:

sage: import pycryptosat
sage: print(pycryptosat.__file__)
/home/scratch2/dimpase/sage/sage/local/lib/python3.7/site-packages/pycryptosat.cpython-37m-x86_64-linux-gnu.so
sage: import numpy
sage: print(numpy.__file__)
/home/scratch2/dimpase/sage/sage/local/lib/python3.7/site-packages/numpy/__init__.py

Not sure whether this is a bug on the package, or a bug in this branch. On this branch I see (with local=SAGE_LOCAL)

$ ls local/lib/python3.7/site-packages/*
local/lib/python3.7/site-packages/pycryptosat-0.2.0-py3.7.egg-info  local/lib/python3.7/site-packages/pycryptosat.cpython-37m-x86_64-linux-gnu.so

So it seems that this branch does not pick up anything from that location.

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

Changed commit from 669b1df to 61a9e37

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

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

`2ed5d07`	`Add cryptominisat patch for virtual envs`
`61a9e37`	`Merge branch 't/29013/public/29013-use-py3-venv' into t/27824/public/27824-python3-spkg-configure`

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

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

`27d945a`	`moved patch to the right place`
`8c546cf`	`Merge branch 't/29013/public/29013-use-py3-venv' into t/27824/public/27824-python3-spkg-configure`

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

Changed commit from 61a9e37 to 8c546cf

embray commented 4 years ago

comment:34

What's the deal with config_venv? Why is any of that necessary? I looked at the commit that added it but it wasn't clear. It seems to me another reason to avoid all the venv stuff...

embray commented 4 years ago

comment:35

So, all that $VIRTUAL_ENV/bin/activate does, essentially, is to set 3 environment variables: PATH (to which it inserts $VIRTUAL_ENV/bin in the front), PS1 (which Sage also already sets when activating the sage environment), and the $VIRTUAL_ENV environment variable itself.

Other than that, the only thing that really makes a directory a virtualenv (with Python 3 + venv, I'm not counting the old virtualenv) is the presence of pyvenv.cfg in the install prefix. The format of this file is more-or-less documented (albeit not very clearly) in https://docs.python.org/3/library/venv.html. So all we really need to do to turn $SAGE_LOCAL into a virtualenv of a system Python 3 is to write the appropriate pyvenv.cfg into $SAGE_LOCAL, and not mess around with the full venv command at all. That, and make $SAGE_LOCAL/bin/python3 a symlink to the real python3.

Although not strictly necessary, it is also possibly helpful to third-party tools to set the $VIRTUAL_ENV environment variable.

mkoeppe commented 4 years ago

comment:36

Replying to @embray:

What's the deal with config_venv? Why is any of that necessary? I looked at the commit that added it but it wasn't clear.

It is used in a configure test, build/pkgs/python3/spkg-configure.m4 to make sure that the system python has all optional modules that we need.

mkoeppe commented 4 years ago

comment:37

Replying to @embray:

So all we really need to do to turn $SAGE_LOCAL into a virtualenv of a system Python 3 is to write the appropriate pyvenv.cfg into $SAGE_LOCAL, and not mess around with the full venv command at all.

This amounts to trying to upgrade python3's venv to something like a conda environment. This is what I avoid in #29013 on purpose - the venv is only used in a clean, documented way, using the documented tools.

sagemath / sage

spkg-configure.m4 for python3 #27824