Opened 7 years ago

Closed 3 years ago

#18438 closed defect (fixed)

Race between building and using python

Reported by: vbraun Owned by: embray
Priority: critical Milestone: sage-8.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:

Status badges

Description

I think what happened below is that the build system tried to use the sage-download-file script while Python was in the process of being installed. Rebuild works. We should probably use sage-native-execute python:

Found local metadata for ntl-6.2.1.p0
Attempting to download package ntl-6.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/buildslave-sage/slave/sage_git/build/src/bin/sage-download-file", line 11, in <module>
    import contextlib
  File "/mnt/disk/home/buildslave-sage/slave/sage_git/build/local/lib/python2.7/contextlib.py", line 4, in <module>
    from functools import wraps
  File "/mnt/disk/home/buildslave-sage/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/buildslave-sage/slave/sage_git/build/upstream/ntl-6.2.1.tar.bz2' not found.
This shouldn't happen, it is a bug in the sage-spkg script.

Change History (21)

comment:1 Changed 7 years ago by vbraun

  • Branch set to u/vbraun/race_between_building_and_using_python

comment:2 Changed 7 years ago by vbraun

  • Authors set to Volker Braun
  • Commit set to 7f5bbf96e9e3eec11dcb6b1adbfaff8f3893fd29

New commits:

d5bbfc0Merge #18417 into #18428
e280943remove checksumming from sage-spkg
aa24229Do not ignor errors in sage-spkg
49aa3c5Only download mirror list when needed
fd0b5ecsage-spkg has to ignore some errors when building from scratch
84c4620Merge #18428 into #18438
7f5bbf9Always use the system python for sage-download-file

comment:3 Changed 7 years ago by vbraun

sage-native-execute 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 sage-system-python

comment:4 Changed 7 years ago by nbruin

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 sh-style 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 to-be-restored 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 vbraun

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 nbruin

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 start-up 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 c-libs) 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 vbraun

The system python ought to work without (=unset) PYTHONPATH/PYTHONHOME, which is what the sage-system-python script does. Of course in more complicated setups you'll need the right values...

comment:8 Changed 6 years ago by vbraun

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 ntl-6.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/sage-download-file", 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/ntl-6.2.1.p0' failed
make[2]: *** [/mnt/disk/home/release/Sage/local/var/lib/sage/installed/ntl-6.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 3 years ago by embray

  • Milestone changed from sage-6.7 to sage-8.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 sage-spkg. I feel that long term sage-spkg 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 3 years ago by embray

  • 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 3 years ago by embray

  • Authors changed from Volker Braun to Volker Braun, Erik Bray
  • Branch changed from u/vbraun/race_between_building_and_using_python to u/embray/ticket-18438
  • 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:

39fa363Trac #18438: Always use system Python for build toolchain scripts written in

comment:12 Changed 3 years ago by embray

  • 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 sage-pip-install is now using the system pip rather than the one installed in Sage. Which is odd because I deliberately did not update sage-pip-install. It's supposed to use Sage's Python online.

comment:13 Changed 3 years ago by git

  • Commit changed from 39fa36329240bae075cd6c0116ce5b493ab557c7 to e6c18f1697ae33c024a006443eb04715569fe701

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

e6c18f1Trac #18438: Ensure that sage-pip-install runs pip with Sage's Python

comment:14 Changed 3 years ago by embray

  • Status changed from needs_work to needs_review

I'm not wild about the workaround in sage-pip-install and am open to other ideas. On the other hand, maybe it's not all bad, as it really emphasizes and ensures that sage-pip-install is run with Sage's Python.

comment:15 Changed 3 years ago by git

  • Commit changed from e6c18f1697ae33c024a006443eb04715569fe701 to ddafa36338450833a5cdd86eb3fecb16ad0bff0e

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

ddafa36Trac #18438: Always use system Python for build toolchain scripts written in

comment:16 Changed 3 years ago by embray

(Decided to squash to a single commit)

comment:17 Changed 3 years ago by embray

  • Status changed from needs_review to needs_work

The sage-flock 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 3 years ago by git

  • Commit changed from ddafa36338450833a5cdd86eb3fecb16ad0bff0e to d84bfb620292949c4b84d10baf15e45888a6a390

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

d84bfb6Trac #18438: A different take on sage-system-python

comment:19 Changed 3 years ago by embray

  • Status changed from needs_work to needs_review

I think this is a better middle-ground. 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 3 years ago by dimpase

  • Reviewers set to Dima Pasechnik
  • Status changed from needs_review to positive_review

looks good to me.

comment:21 Changed 3 years ago by vbraun

  • Branch changed from u/embray/ticket-18438 to d84bfb620292949c4b84d10baf15e45888a6a390
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.