Opened 21 months ago
Last modified 4 months ago
#29013 closed enhancement
In a python3 build, install all Python packages into a venv different from SAGE_LOCAL — at Version 54
Reported by:  mkoeppe  Owned by:  

Priority:  major  Milestone:  sage9.4 
Component:  build  Keywords:  sd111 
Cc:  dimpase, jdemeyer, embray, vbraun, jhpalmieri  Merged in:  
Authors:  Matthias Koeppe, Erik Bray  Reviewers:  
Report Upstream:  N/A  Work issues:  
Branch:  public/29013usepy3venv (Commits, GitHub, GitLab)  Commit:  208c7e68c24c1fe896a5e6573c6aae5399056431 
Dependencies:  #29362  Stopgaps: 
Description (last modified by )
#27824 makes $SAGE_LOCAL
a venv if a suitable system python3 is found.
On this ticket, we create a venv (https://docs.python.org/3/library/venv.html) instead at $SAGE_LOCAL/lib/sage/venv/sage/
, run the standard activate
script in the sageenv
script, and install all Python packages (including sagelib
) into this venv.
(This used to be preparation for #27824; but the approach of that has been changed.)
Change History (54)
comment:1 Changed 21 months ago by
 Description modified (diff)
comment:2 Changed 21 months ago by
 Branch set to public/29013usepy3venv
comment:3 Changed 21 months ago by
 Cc vbraun added
 Commit set to a3942688c3398af5c7bcc62f3fb3dde191a9b2c9
 Status changed from new to needs_review
comment:4 Changed 21 months ago by
In the branch #27824, the venv is created by slightly more complicated code in build/make/Makefile
. This complexity is not necessary for the present ticket.
comment:5 Changed 21 months ago by
Does this ticket make sense separately, or only as part of #27824 ?
comment:6 Changed 21 months ago by
#27824 is the use case that I have in mind. This ticket is a less intrusive change and should go through the betas first.
comment:7 Changed 21 months ago by
 Dependencies set to #29002
comment:8 followup: ↓ 10 Changed 21 months ago by
I tried building with this branch on OS X, and I got doctest failures:
sage t long src/sage/plot/plot.py # 1 doctest failed 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/misc/trace.py # 4 doctests failed sage t long src/sage/env.py # 1 doctest failed sage t long src/sage/repl/configuration.py # 1 doctest failed sage t long src/sage/schemes/affine/affine_homset.py # 1 doctest failed
For example
File "src/sage/plot/plot.py", line 521, in sage.plot.plot Failed example: os.system("sage c \"if 'matplotlib' in sys.modules: sys.exit(1)\"") # long time Expected: 0 Got: Traceback (most recent call last): File "/Users/jpalmier/Desktop/Sage/sage_builds/TESTING/sage9.1.beta0/local/bin/sageeval", line 4, in <module> from sage.all import * ModuleNotFoundError: No module named 'sage' 256
and
********************************************************************** File "src/sage/matrix/matrix2.pyx", line 1127, in sage.matrix.matrix2.Matrix.pseudoinverse Failed example: M.pseudoinverse() # tol 1e15 Expected: [0.0620518477661335 0.0206839492553778 0.0124103695532267] [ 0.124103695532267 0.0413678985107557 0.0248207391064534] [ 0.186155543298400 0.0620518477661335 0.0372311086596801] Got: [0.0620518477661334 0.0206839492553778 0.0124103695532267] [ 0.124103695532267 0.0413678985107556 0.0248207391064534] [ 0.186155543298400 0.0620518477661335 0.0372311086596801] Tolerance exceeded in 2 of 9: 0.0620518477661335 vs 0.0620518477661334, tolerance 2e15 > 1e15 0.0413678985107557 vs 0.0413678985107556, tolerance 3e15 > 1e15 ********************************************************************** File "src/sage/matrix/matrix2.pyx", line 1131, in sage.matrix.matrix2.Matrix.pseudoinverse Failed example: M.pseudoinverse(algorithm="numpy") # tol 1e15 Expected: [0.0620518477661335 0.0206839492553778 0.0124103695532267] [ 0.124103695532267 0.0413678985107557 0.0248207391064534] [ 0.186155543298400 0.0620518477661335 0.0372311086596801] Got: [0.0620518477661334 0.0206839492553778 0.0124103695532267] [ 0.124103695532267 0.0413678985107556 0.0248207391064534] [ 0.186155543298400 0.0620518477661335 0.0372311086596801] Tolerance exceeded in 2 of 9: 0.0620518477661335 vs 0.0620518477661334, tolerance 2e15 > 1e15 0.0413678985107557 vs 0.0413678985107556, tolerance 3e15 > 1e15 ********************************************************************** 1 item had failures: 2 of 25 in sage.matrix.matrix2.Matrix.pseudoinverse [2472 tests, 2 failures, 9.06 s]
comment:9 Changed 21 months ago by
Thanks for testing!
comment:10 in reply to: ↑ 8 ; followup: ↓ 13 Changed 21 months ago by
 Dependencies changed from #29002 to #29002, #29022
Replying to jhpalmieri:
sage t long src/sage/matrix/matrix2.pyx # 2 doctests failed
Not sure what's causing the numerical failures.
sage t long src/sage/env.py # 1 doctest failed
For this one, I have created #29022.
comment:11 Changed 21 months ago by
 Commit changed from a3942688c3398af5c7bcc62f3fb3dde191a9b2c9 to d4fbe4082890ac0aa09559b87393544678592c94
Branch pushed to git repo; I updated commit sha1. New commits:
d4fbe40  _get_shared_lib_filename: Do not assume Python sysconfig paths are in SAGE_LOCAL

comment:12 Changed 21 months ago by
 Commit changed from d4fbe4082890ac0aa09559b87393544678592c94 to d1779cc2ff56d8dd5f58ebd1f5322e32499a3970
Branch pushed to git repo; I updated commit sha1. New commits:
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/29013usepy3venv

comment:13 in reply to: ↑ 10 Changed 21 months ago by
comment:14 Changed 21 months ago by
The numerical problems seems to happen because numpy does not find BLAS.
comment:15 Changed 21 months ago by
Actually, it does not find openblas or atlas, but it does find the "Accelerate framework":
accelerate_info: FOUND: extra_compile_args = ['msse3', 'I/System/Library/Frameworks/vecLib.framework/Headers'] extra_link_args = ['Wl,framework', 'Wl,Accelerate'] define_macros = [('NO_ATLAS_INFO', 3), ('HAVE_CBLAS', None)] FOUND: extra_compile_args = ['msse3', 'I/System/Library/Frameworks/vecLib.framework/Headers'] extra_link_args = ['Wl,framework', 'Wl,Accelerate'] define_macros = [('NO_ATLAS_INFO', 3), ('HAVE_CBLAS', None)]
comment:16 Changed 21 months ago by
 Commit changed from d1779cc2ff56d8dd5f58ebd1f5322e32499a3970 to 93bae6f990fa685b12f26a2b2736d45f7c605ce6
comment:17 Changed 21 months ago by
 Dependencies changed from #29002, #29022 to #29002, #29022, #29025
Merged #29025, fixes the numerical doctests.
comment:18 followup: ↓ 22 Changed 21 months ago by
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 warnlong 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/sagedev/local/lib/sage/venv/sage/lib/python3.7/sitepackages/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.
comment:19 Changed 21 months ago by
oops, comment:18 belongs to #27824
comment:20 followup: ↓ 21 Changed 21 months ago by
Minor bikeshed, but why the long path "lib/sage/venv/sage"? What about something like just "lib/sageX.Y"; analogously to "lib/pythonX.Y"? We've never had a "lib/sage" before in the first place, so I don't see any immediate need to make the path so much more complicated.
comment:21 in reply to: ↑ 20 ; followup: ↓ 44 Changed 21 months ago by
Replying to embray:
Minor bikeshed, but why the long path "lib/sage/venv/sage"? What about something like just "lib/sageX.Y"; analogously to "lib/pythonX.Y"? We've never had a "lib/sage" before in the first place, so I don't see any immediate need to make the path so much more complicated.
1) "venv" has to be in the path to indicate that it's a venv in the technical sense and not a random place with invitation to install stuff in it.
2) Definitely no versioning.
3) I suppose I could drop the final "sage", making it lib/sage/venv
comment:22 in reply to: ↑ 18 Changed 21 months ago by
Replying to dimpase:
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 warnlong 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/sagedev/local/lib/sage/venv/sage/lib/python3.7/sitepackages/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.
Upstream merged pull request https://github.com/msoos/cryptominisat/pull/551
to take care of a virtualenv
installation.
comment:23 Changed 21 months ago by
I haven't tested whether this patch helps for our venv
. The patch is not in the current release 5.6.8 (which we have).
comment:24 Changed 21 months ago by
 Commit changed from 93bae6f990fa685b12f26a2b2736d45f7c605ce6 to 2ed5d071cf9825df2a2dbf6d54b3a6f09153be3a
Branch pushed to git repo; I updated commit sha1. New commits:
2ed5d07  Add cryptominisat patch for virtual envs

comment:25 Changed 21 months ago by
Try with this patch please. I can't compile cryptominisat because of an unrelated reason.
comment:26 Changed 21 months ago by
patch landed into a wrong place, it should be one level down, in patches/
testing now, after correction of this.
comment:27 Changed 21 months ago by
Sorry, thanks.
comment:28 Changed 21 months ago by
 Commit changed from 2ed5d071cf9825df2a2dbf6d54b3a6f09153be3a to 27d945a8d41354234c123f02e2e1d41d287a7541
Branch pushed to git repo; I updated commit sha1. New commits:
27d945a  moved patch to the right place

comment:29 Changed 21 months ago by
the patch worked, it's all good with tests now.
comment:30 Changed 21 months ago by
Thank you!
comment:31 followup: ↓ 32 Changed 21 months ago by
I might have missed some discussion elsewhere, but I still don't see the advantage of this extra complication over having the sage environment just extend sys.path
where necessary. I say this as a big fan of virtualenvs in general, and having suggested doing something like this in the past. But the more I think about it the less I see any advantage in this case over having an additional sys.path
entry.
comment:32 in reply to: ↑ 31 Changed 21 months ago by
Replying to embray:
I say this as a big fan of virtualenvs in general, and having suggested doing something like this in the past.
Yes, of course this ticket was inspired by various insights and discussions shared by you and others on trac tickets.
But the more I think about it the less I see any advantage in this case over having an additional
sys.path
entry.
Previously, with our python2 build, there was no clear way to do it. But now with python 3 there is a standard and clean tool: that's venv. Installing into it is supported by standard tools.
comment:33 followups: ↓ 34 ↓ 35 Changed 21 months ago by
Or, at the very least, what I've suggested in the past was to make $SAGE_LOCAL
itself a virtualenv, as virtualenvs already have a sortof root filesystem, including among other things lib/pythonX.Y/sitepackages
.
With this approach, there will now be (depending whether or not the system Python is used) both:
SAGE_LOCAL/lib/python3.7/sitepackages
SAGE_LOCAL/lib/sage/venv/sage/lib/python3.7/sitepackages
If SAGE_LOCAL
itself (if it exists) were a virtualenv already that would simplify matters greatly.
comment:34 in reply to: ↑ 33 ; followup: ↓ 36 Changed 21 months ago by
Replying to embray:
Or, at the very least, what I've suggested in the past was to make
$SAGE_LOCAL
itself a virtualenv, as virtualenvs already have a sortof root filesystem, including among other thingslib/pythonX.Y/sitepackages
.
No, the present ticket explicitly avoids that.
The reason is that venv
has no documented mechanism for adding nonPython stuff to it, in particular to its activation scripts.
comment:35 in reply to: ↑ 33 Changed 21 months ago by
Replying to embray:
With this approach, there will now be (depending whether or not the system Python is used) both:
SAGE_LOCAL/lib/python3.7/sitepackages
SAGE_LOCAL/lib/sage/venv/sage/lib/python3.7/sitepackages
That's correct. In a fresh install, the first one will be empty.
In an install upgraded from earlier versions, there will be packages in both. When there's an update, the old version if removed from the first one and the new version is added to the second one.
comment:36 in reply to: ↑ 34 Changed 21 months ago by
Replying to mkoeppe:
Replying to embray:
Or, at the very least, what I've suggested in the past was to make
$SAGE_LOCAL
itself a virtualenv, as virtualenvs already have a sortof root filesystem, including among other thingslib/pythonX.Y/sitepackages
.No, the present ticket explicitly avoids that. The reason is that
venv
has no documented mechanism for adding nonPython stuff to it, in particular to its activation scripts.
There's no reason you can'tI've often install C libraries into a virtualenv by setting the path to it in ./configure prefix
.
comment:37 followup: ↓ 39 Changed 21 months ago by
I really hate to be a pain in the ass because I know you put a lot of work into this, but I really don't get it... It really seems complicated and messy to both activate the Sage environment, and then within that activate a virtualenv. I don't think that's what I ever had in mind when I suggested using a virtualenv for Sage (or if I did, I take it back...)
I'd still like to know what problem this is solving. You say that it's required for #27824, but why? What specific problems related to #27824 is it solving? Because most likely there's another way.
comment:38 followup: ↓ 40 Changed 21 months ago by
comment:39 in reply to: ↑ 37 Changed 21 months ago by
Replying to embray:
I don't think that's what I ever had in mind when I suggested using a virtualenv for Sage
Don't worry. I'm prepared to take full credit for this specific version of the idea, if necessary.
comment:40 in reply to: ↑ 38 ; followup: ↓ 41 Changed 21 months ago by
Replying to dimpase:
Well, the separation of #29013 and #27824 is artificial, but otherwise, they do the job. venv removes an extra complication of sagelib being very far from pipinstallable.
How? Why? There's really nothing that magical about virtualenvs. It's just an alternate install prefix plus (as of Python 3 and the venv module) some small support bits in the interpreter to report a different sys.prefix
, etc.
comment:41 in reply to: ↑ 40 ; followup: ↓ 42 Changed 21 months ago by
Replying to embray:
Replying to dimpase:
Well, the separation of #29013 and #27824 is artificial, but otherwise, they do the job. venv removes an extra complication of sagelib being very far from pipinstallable.
How? Why? There's really nothing that magical about virtualenvs. It's just an alternate install prefix plus (as of Python 3 and the venv module) some small support bits in the interpreter to report a different
sys.prefix
, etc.
Part of the magic is to use the standard instead of projectspecific tricks.
comment:42 in reply to: ↑ 41 Changed 21 months ago by
Replying to mkoeppe:
Replying to embray:
Replying to dimpase:
Well, the separation of #29013 and #27824 is artificial, but otherwise, they do the job. venv removes an extra complication of sagelib being very far from pipinstallable.
How? Why? There's really nothing that magical about virtualenvs. It's just an alternate install prefix plus (as of Python 3 and the venv module) some small support bits in the interpreter to report a different
sys.prefix
, etc.Part of the magic is to use the standard instead of projectspecific tricks.
What "tricks" are you avoiding?
comment:43 Changed 21 months ago by
 Commit changed from 27d945a8d41354234c123f02e2e1d41d287a7541 to f2af47873732f2142fc4b6b3bf6b4aed94a2503a
Branch pushed to git repo; I updated commit sha1. New commits:
08ec551  build/make/deps: Make python3_venv a .PHONY target

b2ab6f3  src/bin/sageenv: Simplify the venv path to $SAGE_LOCAL/lib/sage/venv

7a43544  src/sage/env.py, envconfig.py.in: Add MAXIMA; src/sage/interfaces/maxima.py: Use it

f2af478  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/29013usepy3venv

comment:44 in reply to: ↑ 21 Changed 21 months ago by
comment:45 Changed 21 months ago by
comment:46 Changed 21 months ago by
 Commit changed from f2af47873732f2142fc4b6b3bf6b4aed94a2503a to 199a72ed50ae52a1024872e1c417fb3919a11eb8
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
6b28ac1  In a python3 build, install all Python packages into a venv

981f65e  sagepipinstall: Use PYTHON=sagepython23

a6854bd  _get_shared_lib_filename: Do not assume Python sysconfig paths are in SAGE_LOCAL

11ada01  build/pkgs/numpy/lapack_conf.py: Add a [DEFAULT] section to site.cfg

3bd10e4  Add cryptominisat patch for virtual envs

da13a9e  moved patch to the right place

c9d067a  src/bin/sageenv: Simplify the venv path to $SAGE_LOCAL/lib/sage/venv

199a72e  src/sage/env.py: Rework _get_shared_lib_filename, fix for Cygwin: it would privilege system DLLs over DLLs in SAGE_LOCAL

comment:47 Changed 21 months ago by
comment:48 Changed 20 months ago by
 Status changed from needs_review to needs_work
 Work issues set to Redo on top of current #29022; #29038 instead of #29022
comment:49 Changed 20 months ago by
 Dependencies changed from #29002, #29022, #29025 to #29002, #29022, #29025, #29038
comment:50 Changed 20 months ago by
I would be interested in your comments on #29032. By making $SAGE_LOCAL
itself into a virtualenvwhich it already essentially is in all other respectsno other weird hacks are needed.
Other than some things like the OSX fixes I don't see what this solves that my branch doesn't.
comment:51 Changed 19 months ago by
 Commit changed from 199a72ed50ae52a1024872e1c417fb3919a11eb8 to 208c7e68c24c1fe896a5e6573c6aae5399056431
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
980f444  In a python3 build, install all Python packages into a venv

90bf090  sagepipinstall: Use PYTHON=sagepython23

9edf068  _get_shared_lib_filename: Do not assume Python sysconfig paths are in SAGE_LOCAL

ead2d0f  Add cryptominisat patch for virtual envs

56e3daa  src/bin/sageenv: Simplify the venv path to $SAGE_LOCAL/lib/sage/venv

208c7e6  src/sage/env.py: Rework _get_shared_lib_filename, fix for Cygwin: it would privilege system DLLs over DLLs in SAGE_LOCAL

comment:52 Changed 19 months ago by
 Dependencies #29002, #29022, #29025, #29038 deleted
 Work issues Redo on top of current #29022; #29038 instead of #29022 deleted
comment:53 Changed 19 months ago by
 Dependencies set to #29362
comment:54 Changed 18 months ago by
 Description modified (diff)
 Milestone changed from sage9.1 to sagewishlist
 Summary changed from In a python3 build, install all Python packages into a venv to In a python3 build, install all Python packages into a venv different from SAGE_LOCAL
A simple change. Compiles from a fresh checkout and runs without problems on macOS.
New commits:
In a python3 build, install all Python packages into a venv
sagepipinstall: Use PYTHON=sagepython23