Opened 7 years ago
Closed 4 years ago
#18438 closed defect (fixed)
Race between building and using python
Reported by:  vbraun  Owned by:  embray 

Priority:  critical  Milestone:  sage8.4 
Component:  build  Keywords:  random_fail 
Cc:  Merged in:  
Authors:  Volker Braun, Erik Bray  Reviewers:  Dima Pasechnik 
Report Upstream:  N/A  Work issues:  
Branch:  d84bfb6 (Commits, GitHub, GitLab)  Commit:  d84bfb620292949c4b84d10baf15e45888a6a390 
Dependencies:  Stopgaps: 
Description
I think what happened below is that the build system tried to use the sagedownloadfile script while Python was in the process of being installed. Rebuild works. We should probably use sagenativeexecute python:
Found local metadata for ntl6.2.1.p0 Attempting to download package ntl6.2.1.p0 Could not find platform dependent libraries <exec_prefix> Consider setting $PYTHONHOME to <prefix>[:<exec_prefix>] Traceback (most recent call last): File "/mnt/disk/home/buildslavesage/slave/sage_git/build/src/bin/sagedownloadfile", line 11, in <module> import contextlib File "/mnt/disk/home/buildslavesage/slave/sage_git/build/local/lib/python2.7/contextlib.py", line 4, in <module> from functools import wraps File "/mnt/disk/home/buildslavesage/slave/sage_git/build/local/lib/python2.7/functools.py", line 10, in <module> from _functools import partial, reduce ImportError: No module named _functools Error: spkg file '/mnt/disk/home/buildslavesage/slave/sage_git/build/upstream/ntl6.2.1.tar.bz2' not found. This shouldn't happen, it is a bug in the sagespkg script.
Change History (21)
comment:1 Changed 7 years ago by
 Branch set to u/vbraun/race_between_building_and_using_python
comment:2 Changed 7 years ago by
 Commit set to 7f5bbf96e9e3eec11dcb6b1adbfaff8f3893fd29
comment:3 Changed 7 years ago by
sagenativeexecute
doesn't get rid of the whole Sage environment, we need something slightly different here that keeps SAGE_ROOT
but resets PATH
. This is now named sagesystempython
comment:4 Changed 7 years ago by
There's way more necessary to use the system python! If you don't reset library paths and python search paths, you're setting yourself up for failure. #9386 is just some shstyle issues away from finalization and will provide proper environment reset tools, in a fairly efficient manner. Perhaps it's better to build on that? If you want to preserve SAGE_ROOT, you could remove it from the toberestored list, or set export SAGE_SAVED_SAGE_ROOT=$SAGE_ROOT
(which means we should perhaps document a process to tweak restores).
comment:5 Changed 7 years ago by
Well if #9386 can be finished then that would be great... but it didn't appear to be that close.
comment:6 Changed 7 years ago by
From experience I know that if you are going to run a different python, you'll have to set PYTHONPATH and PYTHONHOME to the appropiate (previous) values too. I ran into this problem with a startup script (in python) for magma. It didn't use anything fancy from Python, but at some point my system python and sage python (or their clibs) were sufficiently different that the sage python libraries (picked up via PYTHONPATH, which was normally unset on my system) prevented python from running successfully.
comment:7 Changed 7 years ago by
The system python ought to work without (=unset) PYTHONPATH/PYTHONHOME, which is what the sagesystempython script does. Of course in more complicated setups you'll need the right values...
comment:8 Changed 7 years ago by
Here is another failure with more context:
Compiling /mnt/disk/home/release/Sage/local/lib/python2.7/test/test_fnmatch.py ... Compiling /mnt/disk/home/release/Sage/local/lib/python2.7/test/test_fork1.py ... Compiling /mnt/disk/home/release/Sage/local/lib/python2.7/test/test_format.py ... Compiling /mnt/disk/home/release/Sage/local/lib/python2.7/test/test_fpformat.py ... Compiling /mnt/disk/home/release/Sage/local/lib/python2.7/test/test_fractions.pyFound local metadata for ntl6.2.1.p0 Could not find platform dependent libraries <exec_prefix> Consider setting $PYTHONHOME to <prefix>[:<exec_prefix>] Traceback (most recent call last): File "/mnt/disk/home/release/Sage/src/bin/sagedownloadfile", line 11, in <module> import contextlib File "/mnt/disk/home/release/Sage/local/lib/python2.7/contextlib.py", line 4, in <module> from functools import wraps File "/mnt/disk/home/release/Sage/local/lib/python2.7/functools.py", line 10, in <module> from _functools import partial, reduce ImportError: No module named _functools Makefile:1260: recipe for target '/mnt/disk/home/release/Sage/local/var/lib/sage/installed/ntl6.2.1.p0' failed make[2]: *** [/mnt/disk/home/release/Sage/local/var/lib/sage/installed/ntl6.2.1.p0] Error 1 make[2]: *** Waiting for unfinished jobs.... ... Compiling /mnt/disk/home/release/Sage/local/lib/python2.7/test/test_frozen.py ... Compiling /mnt/disk/home/release/Sage/local/lib/python2.7/test/test_ftplib.py ... Compiling /mnt/disk/home/release/Sage/local/lib/python2.7/test/test_funcattrs.py ...
comment:9 Changed 4 years ago by
 Milestone changed from sage6.7 to sage8.4
 Priority changed from major to critical
See also #26083 for a similar report with some additional context. This has been becoming more common as we integrate more Python tools into sagespkg
. I feel that long term sagespkg
should be rewritten entirely in Python, but even that would not completely fix this problem. I agree with the general approach of preferring to use the system Python for the build system; otherwise trying to use tools in $SAGE_LOCAL
to build $SAGE_LOCAL
is destined to result in problems.
comment:10 Changed 4 years ago by
 Owner changed from (none) to embray
The original branch for this doesn't totally make sense anymore, but I'm testing out a new implementation (which is actually much simpler now that Sage doesn't set PYTHONPATH, PYTHONHOME, LD_LIBRARY_PATH, etc.) But we also have more build scripts now that rely on Python so many of them also need to be updated.
comment:11 Changed 4 years ago by
 Branch changed from u/vbraun/race_between_building_and_using_python to u/embray/ticket18438
 Commit changed from 7f5bbf96e9e3eec11dcb6b1adbfaff8f3893fd29 to 39fa36329240bae075cd6c0116ce5b493ab557c7
 Status changed from new to needs_review
I've tested this a few times now on a 16 core machine with different variations of make j
from 8 to 16, so there should have been a high likelihood of reproducing the problem (as I confirmed against the unmodified 8.4.beta4), but with this patch I did not have any problems, as expected.
New commits:
39fa363  Trac #18438: Always use system Python for build toolchain scripts written in

comment:12 Changed 4 years ago by
 Status changed from needs_review to needs_work
Huh. I just tried this on a different system and ran into a teensy problem: It seems, for some reason, that sagepipinstall
is now using the system pip rather than the one installed in Sage. Which is odd because I deliberately did not update sagepipinstall
. It's supposed to use Sage's Python online.
comment:13 Changed 4 years ago by
 Commit changed from 39fa36329240bae075cd6c0116ce5b493ab557c7 to e6c18f1697ae33c024a006443eb04715569fe701
Branch pushed to git repo; I updated commit sha1. New commits:
e6c18f1  Trac #18438: Ensure that sagepipinstall runs pip with Sage's Python

comment:14 Changed 4 years ago by
 Status changed from needs_work to needs_review
I'm not wild about the workaround in sagepipinstall
and am open to other ideas. On the other hand, maybe it's not all bad, as it really emphasizes and ensures that sagepipinstall
is run with Sage's Python.
comment:15 Changed 4 years ago by
 Commit changed from e6c18f1697ae33c024a006443eb04715569fe701 to ddafa36338450833a5cdd86eb3fecb16ad0bff0e
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
ddafa36  Trac #18438: Always use system Python for build toolchain scripts written in

comment:16 Changed 4 years ago by
(Decided to squash to a single commit)
comment:17 Changed 4 years ago by
 Status changed from needs_review to needs_work
The sageflock
thing still causes things to break when installing cypari. Not sure why I never had this problem in earlier testing. Maybe I already had $SAGE_LOCAL
on my $PATH
to start with for some reason.
comment:18 Changed 4 years ago by
 Commit changed from ddafa36338450833a5cdd86eb3fecb16ad0bff0e to d84bfb620292949c4b84d10baf15e45888a6a390
Branch pushed to git repo; I updated commit sha1. New commits:
d84bfb6  Trac #18438: A different take on sagesystempython

comment:19 Changed 4 years ago by
 Status changed from needs_work to needs_review
I think this is a better middleground. It make sure to run the system Python, but otherwise keeps Sage's $PATH
configuration.
If/when this has otherwise positive review I'll squash the commits again.
comment:20 Changed 4 years ago by
 Reviewers set to Dima Pasechnik
 Status changed from needs_review to positive_review
looks good to me.
comment:21 Changed 4 years ago by
 Branch changed from u/embray/ticket18438 to d84bfb620292949c4b84d10baf15e45888a6a390
 Resolution set to fixed
 Status changed from positive_review to closed
New commits:
Merge #18417 into #18428
remove checksumming from sagespkg
Do not ignor errors in sagespkg
Only download mirror list when needed
sagespkg has to ignore some errors when building from scratch
Merge #18428 into #18438
Always use the system python for sagedownloadfile