Fix the Sage banner

[jdemeyer]

Something has broken the Sage banner.

In 8.2.beta8:

┌────────────────────────────────────────────────────────────────────┐
│ SageMath version 8.2.beta8, Release Date: 2018-03-10               │
│ Type "notebook()" for the browser-based notebook interface.        │
│ Type "help()" for help.                                            │
└────────────────────────────────────────────────────────────────────┘
┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓
┃ Warning: this is a prerelease version, and it may be unstable.     ┃
┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛

In 8.2.rc0:

SageMath version 8.2.rc0, Release Date: 2018-03-28

This is despite the fact that printing the banner works:

SageMath version 8.2.rc0, Release Date: 2018-03-28
sage: from sage.misc.banner import banner
sage: banner()
┌────────────────────────────────────────────────────────────────────┐
│ SageMath version 8.2.rc0, Release Date: 2018-03-28                 │
│ Type "notebook()" for the browser-based notebook interface.        │
│ Type "help()" for help.                                            │
└────────────────────────────────────────────────────────────────────┘
┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓
┃ Warning: this is a prerelease version, and it may be unstable.     ┃
┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛

CC: @embray

Component: user interface

Author: Jeroen Demeyer, Erik Bray

Branch: u/vbraun/36d438055e2e9bd5399152e510b6d233fc591ffe

Reviewer: Erik Bray, Jeroen Demeyer

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

[jdemeyer]

Author: Jeroen Demeyer

[embray]

comment:6

It seems like the release manager is running sage-update-version in some context where unicode is just broken...

[embray]

Changed author from Jeroen Demeyer to none

[embray]

comment:7

Why is it stored in src/bin anyways? It's not an executable at all...

[jdemeyer]

Branch: u/jdemeyer/fix_the_sage_banner

[jdemeyer]

Author: Jeroen Demeyer

[jdemeyer]

Commit: 58b4913

[jdemeyer]

New commits:

58b4913

Generate the Sage banner at runtime

[embray]

Reviewer: Erik Bray

[embray]

comment:11

+1

Let me just make sure that the banner still gets printed pretty quickly on slower platforms (i.e. Cygwin).

On that note I've also been really wanting to add like a spinner or something while it imports sage, but that's another matter...

[embray]

comment:12

It does come up noticeably a little more slowly, but it's still subtle. I'm also currently running a debug build which probably accounts for it being slow to begin with, so I'm not too concerned about it right now.

[vbraun]

comment:13

**********************************************************************
File "src/sage/misc/lazy_attribute.pyx", line 88, in sage.misc.lazy_attribute._lazy_attribute._sage_src_lines_
Failed example:
    src[0]
Expected:
    'def banner(full=None):\n'
Got:
    'def banner():\n'
**********************************************************************
File "src/sage/misc/lazy_attribute.pyx", line 90, in sage.misc.lazy_attribute._lazy_attribute._sage_src_lines_
Failed example:
    lines
Expected:
    87
Got:
    81
**********************************************************************
1 item had failures:
   2 of   6 in sage.misc.lazy_attribute._lazy_attribute._sage_src_lines_
    [130 tests, 2 failures, 2.79 s]
----------------------------------------------------------------------
sage -t --long src/sage/misc/abstract_method.py  # 2 doctests failed
sage -t --long src/sage/misc/lazy_attribute.pyx  # 2 doctests failed

[embray]

comment:14

I suggest those tests be rewritten around a dummy function that is not going to actually be changed...

[vbraun]

comment:15

The banner was broken in #24825, now the output of the full banner can't be redirected

$ sage --python -c "import sage.misc.banner; sage.misc.banner.banner(full=True)" | cat
SageMath version 8.2.rc1, Release Date: 2018-03-31

Pipes that don't lead to the terminal do need to be encoded, check with isatty or always encode.

[vbraun]

comment:16

Also cold starts can be slow (especially over nfs), which is why historically we want to print something before Python starts up.

[jdemeyer]

comment:17

Well, starting up Python should only a fraction of the time of starting up Sage.

[fchapoton]

comment:18

maybe we should just undo #24825, in order to allow the release of 8.2 ?

[embray]

comment:30

How about this: If you want to switch back to your branch (and just add the fix to the tests) I don't mind. My only goal here was to fix a regression. At the same time, as long as Volker agrees to the rest of your changes (getting rid of the pre-written banner files) then it's kind of a moot point.

The rest of the confusion over this mostly stems from the use of "print()" instead of, say, providing an interface explicitly for writing the banner to a text file in some specific encoding (which I agree kind of sucks anyways because it's not necessarily the right encoding for the user's terminal).

[vbraun]

comment:31

I'm not going to play referee, please figure something out that works amongst yourselves :-)

[jdemeyer]

Changed branch from u/embray/fix_the_sage_banner to u/jdemeyer/fix_the_sage_banner

[jdemeyer]

comment:33

I just removed the commit changing back the banner on Python 2.

---

New commits:

a928654	Generate the Sage banner at runtime
f572d40	Fix this test to account for changes to 'banner'

[jdemeyer]

Changed commit from e98cc89 to f572d40

[embray]

comment:34

That's fine--I was just trying to restore some "backwards compatibility", but I think we can both agree it was bad to begin with, and that it's a moot point now.

[vbraun]

comment:35

Still get

sage -t --long src/sage/misc/abstract_method.py  # 2 doctests failed

[embray]

comment:36

That reminds me to change that silly test.

[embray]

comment:37

I see; this is a different test that also depends on the source code of that particular function...

[embray]

Changed commit from f572d40 to 36d4380

[embray]

Changed branch from u/jdemeyer/fix_the_sage_banner to u/embray/fix_the_sage_banner

[embray]

New commits:

36d4380

fix this test

[kiwifb]

comment:39

Thank you for annoying sage-on-distros by using SAGE_ROOT in the main sage script. SAGE_ROOT outside of the build system is just a pain in the ass. If you have to use a text file to store the version, name it uniquely and ship to /etc, or put it in a folder exclusively belonging to sage.

I guess it is too late at this stage, we'll deal with it later.

[vbraun]

Changed branch from u/embray/fix_the_sage_banner to 36d4380

[embray]

comment:41

Replying to @kiwifb:

Thank you for annoying sage-on-distros by using SAGE_ROOT in the main sage script. SAGE_ROOT outside of the build system is just a pain in the ass. If you have to use a text file to store the version, name it uniquely and ship to /etc, or put it in a folder exclusively belonging to sage.

I guess it is too late at this stage, we'll deal with it later.

I don't think the sarcasm is necessary--I see your point but it's not completely obvious, and you had the opportunity here to set it back to "needs_work" or something--I wouldn't mind. Please open a new ticket for this and CC me and set it to "blocker". An easy fix for this would be to ensure "VERSION.txt" is installed somewhere sensinble under $SAGE_LOCAL (like, not bin/ :) and use it from there if possible.

[embray]

Changed commit from 36d4380 to none

[kiwifb]

comment:42

Replying to @embray:

Replying to @kiwifb:

Thank you for annoying sage-on-distros by using SAGE_ROOT in the main sage script. SAGE_ROOT outside of the build system is just a pain in the ass. If you have to use a text file to store the version, name it uniquely and ship to /etc, or put it in a folder exclusively belonging to sage.

I guess it is too late at this stage, we'll deal with it later.

I don't think the sarcasm is necessary--I see your point but it's not completely obvious, and you had the opportunity here to set it back to "needs_work" or something--I wouldn't mind. Please open a new ticket for this and CC me and set it to "blocker". An easy fix for this would be to ensure "VERSION.txt" is installed somewhere sensinble under $SAGE_LOCAL (like, not bin/ :) and use it from there if possible.

I noticed this ticket because Volker included it on his branch. Rather than sarcasm, that's my exasperation at some stuff that bleeds out. I spent quite a bit of time over the years reducing the use of SAGE_ROOT. People just forget. For info I ship VERSION.txt as /etc/sage-version.txt which seems sensible but may not be the best place. I should ask what the other do.

[embray]

comment:43

I know, that's fair. I also want to do with any and all runtime dependency on $SAGE_ROOT--it's really annoying.

[jdemeyer]

comment:44

The problem with VERSION.txt looks very similar to COPYING.txt. So maybe we should deal with both in #21571?

[vbraun]

Changed branch from 36d4380 to u/vbraun/36d438055e2e9bd5399152e510b6d233fc591ffe