Opened 4 years ago
Closed 4 years ago
#22764 closed defect (fixed)
Remove the link SAGE_LOCAL/lib/python
Reported by: | jhpalmieri | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-8.0 |
Component: | build | Keywords: | |
Cc: | Merged in: | ||
Authors: | John Palmieri | Reviewers: | Volker Braun |
Report Upstream: | N/A | Work issues: | |
Branch: | fb90d5e (Commits, GitHub, GitLab) | Commit: | fb90d5e3667237f2d9fc7ba982b466bc52802b1d |
Dependencies: | Stopgaps: |
Description
We do not need the link SAGE_LOCAL/lib/python
, pointing to python2.7
, and it can interfere with the Python 3 build, at least on OS X. (See #22756.)
Change History (27)
comment:1 Changed 4 years ago by
- Branch set to u/jhpalmieri/no-local-lib-python
comment:2 Changed 4 years ago by
- Commit set to 0e90d5294f2dd1f95f2e96bf34830d1256a3f211
- Status changed from new to needs_review
comment:3 Changed 4 years ago by
- Commit changed from 0e90d5294f2dd1f95f2e96bf34830d1256a3f211 to f33e86f76463c02dee30513fd5553ebbe5a228d5
Branch pushed to git repo; I updated commit sha1. New commits:
f33e86f | trac 22764: replace a few instances of
|
comment:4 Changed 4 years ago by
- Commit changed from f33e86f76463c02dee30513fd5553ebbe5a228d5 to b02ecf34018f48b24012623c5388a34d589bb3d2
Branch pushed to git repo; I updated commit sha1. New commits:
b02ecf3 | trac 22764: in a few comments and print statements, fix a few
|
comment:5 Changed 4 years ago by
see also #22638
comment:7 Changed 4 years ago by
- Commit changed from b02ecf34018f48b24012623c5388a34d589bb3d2 to df6a763f1c6190cbcb33501fdf7b6f4e5ee46400
Branch pushed to git repo; I updated commit sha1. New commits:
df6a763 | trac 22764: rebase to Sage 8.0.beta1
|
comment:8 Changed 4 years ago by
- Status changed from needs_work to needs_review
This should apply now. The old version changed src/sage/all.py
to fix a doctest. The conflict replaced the old doctest with the one from Sage 8.0.beta1 that looks more robust to me. I haven't built with this change, though, so I can't promise no doctest failures.
comment:9 Changed 4 years ago by
- Commit changed from df6a763f1c6190cbcb33501fdf7b6f4e5ee46400 to 7cd0d62ad1fe72747813b665682fea591aaf2a80
Branch pushed to git repo; I updated commit sha1. New commits:
7cd0d62 | trac 22764: merge with 8.0.beta1
|
comment:10 Changed 4 years ago by
With the branch here, I am having problems building matplotlib
and fpylll
. For fpylll
, the log file consists entirely of
Traceback (most recent call last): File "setup.py", line 131, in <module> long_description=open('README.rst').read(), File "/Users/palmieri/Desktop/Sage_stuff/sage_builds/TESTING/sage-8.0.beta0/local/lib/python3.5/encodings/ascii.py", line 26, in decode return codecs.ascii_decode(input, self.errors)[0] UnicodeDecodeError: 'ascii' codec can't decode byte 0xe2 in position 2249: ordinal not in range(128) Error: could not determine package name
The matplotlib
log starts with
NOTE: Set SAGE_MATPLOTLIB_GUI to anything but 'no' to try to build the Matplotlib GUI. Not building any matplotlib graphical backends. Traceback (most recent call last): File "make-setup-config.py", line 34, in <module> config.write(configfile) File "/Users/palmieri/Desktop/Sage_stuff/sage_builds/TESTING/sage-8.0.beta0/local/lib/python3.5/configparser.py", line 916, in write self._sections[section].items(), d) File "/Users/palmieri/Desktop/Sage_stuff/sage_builds/TESTING/sage-8.0.beta0/local/lib/python3.5/configparser.py", line 920, in _write_section fp.write("[{}]\n".format(section_name)) TypeError: a bytes-like object is required, not 'str' Warning: This package has a badly-behaved setup.py which outputs more than the package name for 'setup.py --name'; using the last line as the package name: matplotlib Installing package matplotlib using pip Waiting for shared lock to run pip install --ignore-installed --verbose --no-deps --no-index --isolated . ... ok Ignoring indexes: https://pypi.python.org/simple Processing /Users/palmieri/Desktop/Sage_stuff/sage_builds/TESTING/sage-8.0.beta0/local/var/tmp/sage/build/matplotlib-1.5.1.p0/src Running setup.py (path:/var/folders/cp/n8wtqs490tq5psknff1hv9qr0000gn/T/pip-9u0y0hgx-build/setup.py) egg_info for package from file:///Users/palmieri/Desktop/Sage_stuff/sage_builds/TESTING/sage-8.0.beta0/local/var/tmp/sage/build/matplotlib-1.5.1.p0/src Running command python setup.py egg_info
and then proceeds to try to build. Then later it has this:
gcc -Wno-unused-result -Wsign-compare -Wunreachable-code -DNDEBUG -g -fwrapv -O3 -Wall -DPY_ARRAY_UNIQUE_SYMBOL=MPL_matplotlib_backends__macosx_ARRAY_API -DNPY_NO_DEPRECATED_API=NPY_1_7_API_VERSION -I/Users/palmieri/Desktop/Sage_stuff/sage_builds/TESTING/sage-8.0.beta0/local/lib/python3.5/site-packages/numpy/core/include -I/Users/palmieri/Desktop/Sage_stuff/sage_builds/TESTING/sage-8.0.beta0/local/lib/python3.5/site-packages/numpy/core/include -I/usr/local/include -I/usr/include -I/usr/X11/include -I/opt/X11/include -I. -Iextern/agg24-svn/include -I/Users/palmieri/Desktop/Sage_stuff/sage_builds/TESTING/sage-8.0.beta0/local/include/python3.5m -c src/_macosx.m -o build/temp.macosx-10.9-x86_64-3.5/src/_macosx.o gcc: error: src/_macosx.m: Objective-C compiler not installed on this system error: command 'gcc' failed with exit status 1
I don't know if these are tied to the branch here or not, since I haven't been able to build python3 without the changes here.
comment:11 Changed 4 years ago by
- Status changed from needs_review to needs_work
I don't agree with the change to src/bin/sage
. It needs to check whether Sage was built inside $SAGE_LOCAL
and the new code doesn't do that (it could potentially import a different version of Sage installed on the system).
comment:12 Changed 4 years ago by
I wasn't completely satisfied with that change, either. We could do something like this instead:
-
src/bin/sage
diff --git a/src/bin/sage b/src/bin/sage index cc6e41cbed..b26618a3b8 100755
a b fi 383 383 # Prepare for running Sage, either interactively or non-interactively. 384 384 sage_setup() { 385 385 # Check that we're not in a source tarball which hasn't been built yet (#13561). 386 if ! python -c 'import sage.version' 2> /dev/null ; then 386 if [ ! -x "$SAGE_LOCAL/bin/python" ]; then 387 built=no 388 else 389 PYTHON_VERSION=$("$SAGE_LOCAL/bin/python" -c 'import sys; print("%d.%d" % sys.version_info[:2])') 390 if [ ! -d "$SAGE_LOCAL/lib/python$PYTHON_VERSION/site-packages/sage" ]; then 391 built=no 392 fi 393 fi 394 if [ $built = no ]; then 387 395 echo >&2 '************************************************************************' 388 396 echo >&2 'It seems that you are attempting to run Sage from an unpacked source' 389 397 echo >&2 'tarball, but you have not compiled it yet (or maybe the build has not'
How about that?
comment:13 Changed 4 years ago by
Can't we just check for the presence of SAGE_LOCAL/bin/sage instead of messing around with python minutae?
comment:14 Changed 4 years ago by
I think that SAGE_LOCAL/bin/sage
can get installed well before the Sage library.
comment:15 Changed 4 years ago by
Yes, but so what? Its only a sanity check that you don't try to run an unpacked source tarball.
comment:16 Changed 4 years ago by
Either way is fine with me: check for local/lib/python$PYTHON_VERSION/site-packages/sage/
or for local/bin/sage
.
comment:17 Changed 4 years ago by
- Commit changed from 7cd0d62ad1fe72747813b665682fea591aaf2a80 to ab49413b6ee1f7eebcc9044caf2c0abc45aad746
Branch pushed to git repo; I updated commit sha1. New commits:
ab49413 | trac 22764: change how check whether Sage is built.
|
comment:18 Changed 4 years ago by
- Status changed from needs_work to needs_review
I decided that I like the more involved check. It is not complicated and it is more in line with the printed message, especially the part "(or maybe the build has not finished)".
comment:19 follow-ups: ↓ 20 ↓ 21 Changed 4 years ago by
Thats hardcoding the assumption that python is a symlink to python3 when built with SAGE_PYTHON3=yes and would conflict with #22582
comment:20 in reply to: ↑ 19 Changed 4 years ago by
Replying to vbraun:
Thats hardcoding the assumption that python is a symlink to python3 when built with SAGE_PYTHON3=yes
Yes, you're right.
and would conflict with #22582
It would conflict with your current branch at #22582, but there is another option: making a reliable symlink python -> python3
when SAGE_PYTHON3=yes
. As I said at #22582, I would like to hear some other opinions about those two options. (As I also said at #22582, maybe SAGE_PYTHON3
should control some run-time aspects of Sage, not just build-time. If such were available and did not just include the symlink, we could use those for this check.)
comment:21 in reply to: ↑ 19 Changed 4 years ago by
Replying to vbraun:
Thats hardcoding the assumption that python is a symlink to python3 when built with SAGE_PYTHON3=yes and would conflict with #22582
Another way of looking at this: if we're not really thinking about how to run Sage using python3, that it's premature to do so, then: if you build with SAGE_PYTHON3=yes
, the build will not complete because Sage isn't ready for Python 3 yet, so no matter what the symlink points to, you will get this message. If you build without SAGE_PYTHON3=yes
, then the symlink should point to python2
and this check will behave as it always has.
If we just check whether local/bin/sage
exists, then you will get a less clear error message if Sage hasn't finished building yet, for whatever reason.
Once we figure out whether SAGE_PYTHON3
has run-time effects or not, we can modify this check.
comment:22 Changed 4 years ago by
- Commit changed from ab49413b6ee1f7eebcc9044caf2c0abc45aad746 to fb90d5e3667237f2d9fc7ba982b466bc52802b1d
comment:23 Changed 4 years ago by
But I don't really care that much.
comment:24 Changed 4 years ago by
Should we bump the version numbers for python2 and python3, to make sure their spkg-install files get run, so the link actually gets removed when people upgrade?
comment:25 Changed 4 years ago by
- Reviewers set to Volker Braun
- Status changed from needs_review to positive_review
Its not actually harmful, so I'm fine with leaving the old link hang around...
comment:26 Changed 4 years ago by
The link can break the python3 build on OS X, but #22756 bumps the version number on python3 to try to address this. It still may break the first time, but in such a way that the build will get far enough to delete the link. After that it should work the next time.
comment:27 Changed 4 years ago by
- Branch changed from u/jhpalmieri/no-local-lib-python to fb90d5e3667237f2d9fc7ba982b466bc52802b1d
- Resolution set to fixed
- Status changed from positive_review to closed
New commits:
Trac 22764: remove the link SAGE_LOCAL/lib/python -> python2.7