sagemath / binary-pkg

Tools for creating binary tarballs
Other
14 stars 11 forks source link

relocate-once.py is not compatible with Python 3 #16

Closed jhpalmieri closed 5 years ago

jhpalmieri commented 6 years ago

On systems where /usr/bin/env python runs Python 3, relocate-once.py breaks. The immediate fix is error checking when running relocate-once.py (see https://trac.sagemath.org/ticket/25668), but longer term, the script should be made compatible with Python 3.

There are two types of incompatibility:

  1. the line starting p('local/lib/libflint-13.5.2.dylib').patch(1552, 1684)... is very long, and more to the point (I think) has over 1500 method/attribute calls. In any case, that line yields

RecursionError: maximum recursion depth exceeded during compilation

  1. After removing that line, we get:
    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
vbraun commented 5 years ago

I'm fixing this in the py3-fixes branch

embray commented 5 years ago

I think that instead of writing one huge Python file as in https://github.com/sagemath/binary-pkg/commit/30c3352f77bc02b83d0e8fe82aad11474e9c477b#diff-27146f2eb04393561afea5f2e73053bb it might still make more sense if this patch data lived in a separate file. Then the script itself wouldn't even have to be generated, just the data (e.g. in a JSON file) that it reads in.

vbraun commented 5 years ago

Thats just an extra source of errors: now you need to find the data file, you can't atomically remove two files so state can be inconsistent, etc..

vbraun commented 5 years ago

Fixed

embray commented 5 years ago

My point was that this script could just live permanently in the Sage sources somewhere and consume a data file containing patches; then just the data file could be removed if needed. Outputting a gigantic generated script doesn't make a whole lot of sense IMO.

vbraun commented 5 years ago

The Sage repo is not in charge of creating/distributing binaries, the repo here is just one way one can distribute binaries. Docker and flatpak are others. I'm against splitting the binary-pkg logic across different repos.

slel commented 5 years ago

Fixed by the commits from 11 Dec 2018:

mkoeppe commented 5 years ago

With the binary package sage-8.7-OSX_10.11.6-x86_64 (from April 2019), I am still getting the RecursionError reported above. Was this binary package built with this fix? This is with a python 3 from Anaconda in the PATH.

(base) egret:~/s/sage/sage-8.7-OSX_10.11.6-x86_64 (master *)$ ./sage 
RecursionError: maximum recursion depth exceeded during compilation
Error running the script 'relocate-once.py'.
vbraun commented 5 years ago

Yes, 8.7 doesn't include it. Please try again with 8.8 when the binaries are out (hopefully in a few days)

smbraden commented 3 years ago

Hi, I've read that SageMath can run on Python3 ever since the 9.0 release. I have downloaded Sage 9.2 for Ubuntu_20.04, however I still get the same error.

/usr/bin/env: ‘python’: No such file or directory Error running the script 'relocate-once.py'.

If I install python2 will this do the trick? Otherwise, please let me know how I might be able to resolve the installation issue. Thanks

mkoeppe commented 3 years ago

This is an error that is unrelated to this error. See https://wiki.sagemath.org/ReleaseTours/sage-9.2#Binaries for how to fix it on your system.