Run relocate-once.py with Sage's Python2 and with error checking

[jhpalmieri]

Running Sage for the first time after installing from binary under
Linux or macOS leads to running relocate-once.py.

Before this ticket, when run by Python 3 (eg as installed by Anaconda),
it would lead to the following error.

$ ./sage
RecursionError: maximum recursion depth exceeded during compilation
************************************************************************
It seems that you are attempting to run Sage from an unpacked source
tarball, but you have not compiled it yet (or maybe the build has not
finished). You should run `make` in the Sage root directory first.
If you did not intend to build Sage from source, you should download
a binary tarball instead. Read README.txt for more information.
************************************************************************

which seems to come from Python3 choking on a very long line in
relocate-once.py, because of the Python3 bug described as
Python issue 32758.

See reports by Sage users at

• https://ask.sagemath.org/question/42736/linux-binary-seems-to-be-source/
• https://ask.sagemath.org/question/42562/recursionerror-when-installing-sagemath-82/
• https://ask.sagemath.org/question/35132/how-do-i-solve-this-installation-problem/

Reported in "upstream" binary-pkg at binary-pkg issue 16 -- see the fix there.

After that fix, relocate-once.py can run with Python 3, and this ticket adds
error-checking after running relocate-once.py, with a meaningful error
in case of failure.

The tentative solution at one point was to make relocate-once.py be run
with Sage's Python2, whence the now-inaccurate ticket summary.

Upstream: Reported upstream. No feedback yet.

CC: @fchapoton @embray @jhpalmieri @saraedum @slel @vbraun

Component: distribution

Keywords: Anaconda, RecursionError

Author: John Palmieri

Branch: abb2a89

Reviewer: Jeroen Demeyer, Volker Braun

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

[jhpalmieri]

comment:1

According to "j.c." on ask.sagemath.org, the solution to installing a binary with Ananconda installed is:

On the off-chance you're using Anaconda, try commenting out the export 
PATH="/anaconda/bin:$PATH" line in the .bash_profile file (and restarting 
your terminal) before you run Sage for the first time.

You can uncomment it again after sage successfully runs (until you have 
to rebuild or upgrade Sage).

[fchapoton]

comment:2

this probably deserves the "critical" status, given the number of people trapped by the error message..

[slel]

Changed keywords from none to Anaconda, RecursionError

[slel]

Branch: u/slelievre/binary_releases__give_better_error_message_if_anaconda_is_present

[slel]

Commit: c374e9a

[slel]

New commits:

c374e9a

Improve error message when Sage fails to start with RecursionError

[slel]

Author: Samuel Lelièvre

[jhpalmieri]

comment:6

This is catching all errors, not just a RecursionError, I think. Is it worth trying to catch just that one?

[jhpalmieri]

comment:7

And why does the if block if [ ! -x "$SAGE_LOCAL/bin/sage" ]; then ... run at all? This is bash, not Python, so how is anaconda interfering? What is causing the recursion error?

[jdemeyer]

comment:8

Replying to @jhpalmieri:

And why does the if block if [ ! -x "$SAGE_LOCAL/bin/sage" ]; then ... run at all? This is bash, not Python, so how is anaconda interfering? What is causing the recursion error?

Exactly my thought too! The only reasonable explanation is that anaconda itself sets some $SAGE_XXX variables.

[jhpalmieri]

comment:9

First, I think we should catch an error if relocate-once.py does not run successfully, and in particular that's when the message about Anaconda should be printed.

Second, there are several problems in relocate-once.py:

• Related to anaconda and/or Python 3: the RecursionError comes from the line starting

p('local/lib/libflint-13.5.2.dylib').patch(1552, 1684)...

That line is 61957 characters long (!), and I wonder if its length is causing problems. Certainly if I delete just that line, I no longer get the RecursionError.

• related to Python 3, if I delete the aforementioned line, I get

Rewriting paths for your new installation directory
===================================================

This might take a few minutes but only has to be done once.

Traceback (most recent call last):
  File "/Users/jpalmier/Downloads/SageMath/relocate-once.py", line 145, in <module>
    p('build/make/Makefile-auto').substitute().save()
  File "/Users/jpalmier/Downloads/SageMath/relocate-once.py", line 133, in __call__
    filename = os.path.join(self.root_path, filename)
  File "/anaconda3/lib/python3.6/posixpath.py", line 94, in join
    genericpath._check_arg_types('join', a, *p)
  File "/anaconda3/lib/python3.6/genericpath.py", line 151, in _check_arg_types
    raise TypeError("Can't mix strings and bytes in path components") from None
TypeError: Can't mix strings and bytes in path components

[jdemeyer]

comment:10

We really need a test machine where this error occurs to see what the value of $SAGE_LOCAL is at that point.

[jdemeyer]

comment:11

OK, so the problem is with relocate-once.py. That wasn't obvious at all so I guess the error handler there should be improved...

[jhpalmieri]

comment:12

Replying to @jdemeyer:

OK, so the problem is with relocate-once.py. That wasn't obvious at all so I guess the error handler there should be improved...

You mean, there should be an error handler there?

[jhpalmieri]

comment:13

By the way, I have now installed Anaconda on OS X, and it's easy enough to add or remove the bad parts in my PATH to test things. Others should feel free to do the same.

[jdemeyer]

comment:14

Is the problem just that the script is run with Python 3 instead of Python 2?

[jdemeyer]

comment:15

Replying to @jhpalmieri:

You mean, there should be an error handler there?

I'm just wondering who is displaying the message RecursionError: maximum recursion depth exceeded during compilation? That's not how Python displays errors by default.

[jhpalmieri]

comment:16

If I run relocate-once.py using Sage's Python 3.6.6, I do not get the recursion error. (I get the string/bytes error, of course.) So maybe Anaconda's Python 3.6.5 is badly configured?

[jhpalmieri]

comment:17

Further debugging: if I put some echo statements in the sage script before and after running relocate-once.py, they both execute. If I put a print statement in relocate-once.py, it doesn't print. I am really wondering if the long line makes anaconda's Python 3 barf before it even loads the file.

[jhpalmieri]

comment:18

It looks something like the issue discussed in https://bugs.python.org/issue32758.

[slel]

comment:19

Replying to @jdemeyer:

I'm just wondering who is displaying the message

RecursionError: maximum recursion depth exceeded during compilation

That's not how Python displays errors by default.

Searching the web for [ "maximum recursion depth exceeded during compilation" ]
yields many results, including the following one not related to Sage:

• https://www.reddit.com/r/learnpython/comments/3zusdy/eval_python_beginner/

It might indicate that the error has something to do with a call to eval(?).

This other result shows that the error can occur even just running sage --version:

• infinite recursion error during sage compilation Homebrew/homebrew-cask#18279

[jdemeyer]

comment:20

Replying to @slel:

It might indicate that the error has something to do with a call to eval(?).

Or more generally, just the Python parser. All the evidence points to Python crashing while parsing the .py file, before it even executes anything. So it crashes with just the error message without a traceback.

[jdemeyer]

comment:21

One obvious fix in Sage is to add error checking in the top-level sage script:

# If this is a freshly-unpacked binary tarball then run the installer
# Note: relocate-once.py deletes itself upon successful completion
if [ -x "$SAGE_ROOT/relocate-once.py" ]; then
    "$SAGE_ROOT/relocate-once.py"
fi

[jhpalmieri]

comment:41

OS X – it is the only system I have easy access to.

[fchapoton]

comment:42

shall we put this into "positive review" now ?

[videlec]

comment:43

update milestone 8.3 -> 8.4

[jdemeyer]

Reviewer: Jeroen Demeyer

[jhpalmieri]

comment:45

Any progress on this? I would very much like it to get merged in 8.4, since it has been a common issue, at least on ask.sagemath.org.

[jhpalmieri]

comment:47

Ping?

[slel]

comment:48

Volker, would you review this?

[embray]

comment:49

Maybe it works, but it seems a bit sketchy to use the Python in Sage to run relocate-once.py. I admit, I don't quite understand exactly what it is relocate-once.py does, but if the Python in Sage and the libraries it depends on haven't been "relocated" yet, how can you guarantee they'll work properly?

I think it would make more sense to modify relocate-once.py to just work better on more Pythons. In particular, I don't think it's necessary for it be implemented in such a way that it does not contain extremely long expressions that tax the parser/peephole optimizers in Python.

[vbraun]

comment:50

This is a rather fragile workaround, run Sage's python2 while the rpaths in the binary point to the wrong place and pray that none of those shared libraries end up being used by relocate-once.

The thing that needs to be done eventually is making relocate-once python3 compatible, including not emitting very long chains of method calls.

[jhpalmieri]

comment:51

A compromise would be to use the system Python 2 if we can find it (via something like python2 relocate-once.py || python2.7 relocate-once.py || ...) and otherwise call Sage's Python 2 and hope that it works. Is it better to have something that you know will break if the system python runs Python 3, or to have something that might work in that situation, even if it's fragile?

Otherwise, this puts the ball in the court of the people who maintain the Sage binary distributions. I seem to have opened up sagemath/binary-pkg#16 6 months ago, but no progress yet. I don't have the time to figure out how relocate-once.py is generated and then to redo it to avoid the long chains of methods.

[vbraun]

comment:52

I'm fixing this at sagemath/binary-pkg#16

[embray]

comment:53

I don't know if this has ever happened specifically with the relocate-once.py script, but I've also had problems in the past with Python 2.7 bytecode compiler crashing on extremely long expressions, particularly arithmetic expressions like a = 1; a + a + a + a + ... repeating a few thousand times (incidentally only if a is a variable; if it's a 1 literal I think those constant expressions get optimized before reaching the compiler).

In fact it was only a couple years ago I think that they fixed this to at least raise a RuntimeError instead of segfaulting.

[sagetrac-git]

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

3b0bacb	trac 25668: run relocate-once.py with Sage's python2.
abb2a89	trac 25668: add error-checking after running relocate-once.py.

[sagetrac-git]

Changed commit from 030c1bc to abb2a89

[jhpalmieri]

comment:55

Replying to @vbraun:

I'm fixing this at sagemath/binary-pkg#16

Thanks!

For this ticket, the only thing left to do is error-checking. Ready for review.

---

New commits:

3b0bacb	trac 25668: run relocate-once.py with Sage's python2.
abb2a89	trac 25668: add error-checking after running relocate-once.py.

[vbraun]

Changed reviewer from Jeroen Demeyer to Jeroen Demeyer, Volker Braun

[vbraun]

Changed branch from u/jhpalmieri/python2-relocate-once to abb2a89

[slel]

Changed commit from abb2a89 to none

[slel]

comment:59

(Editing ticket description to reflect that the fix is not the one
suggested by the ticket summary.)