#20322 closed enhancement (fixed)
use SAGE_BANNER to propose a bare banner with no utf8
Reported by:  Frédéric Chapoton  Owned by:  

Priority:  major  Milestone:  sage7.2 
Component:  user interface  Keywords:  
Cc:  Nicolas M. Thiéry, Erik Bray, Jeroen Demeyer  Merged in:  
Authors:  Frédéric Chapoton  Reviewers:  Jeroen Demeyer 
Report Upstream:  N/A  Work issues:  
Branch:  5b87f1a (Commits, GitHub, GitLab)  Commit:  
Dependencies:  Stopgaps: 
Description
The pretty colored banner for sage uses utf8, and this poses problem for the docker patchbot.
Let us use SAGE_BANNER to propose a shorten banner when needed.
Change History (28)
comment:1 Changed 7 years ago by
Authors:  → Frédéric Chapoton 

Branch:  → u/chapoton/20322 
Commit:  → 465db90b975dbf2c2ada8d8b1c8f33ea4f048fcf 
Status:  new → needs_review 
comment:2 Changed 7 years ago by
Cc:  Erik Bray Jeroen Demeyer added 

comment:3 Changed 7 years ago by
Component:  PLEASE CHANGE → user interface 

Status:  needs_review → needs_work 
Type:  PLEASE CHANGE → enhancement 
Can you use consistent indentation?
comment:4 Changed 7 years ago by
Commit:  465db90b975dbf2c2ada8d8b1c8f33ea4f048fcf → c2d0ff3df2b0543fbab75c9c24e067d9bd6d48e4 

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

comment:5 Changed 7 years ago by
Status:  needs_work → needs_review 

comment:6 Changed 7 years ago by
Reviewers:  → Jeroen Demeyer 

Status:  needs_review → needs_work 
I am adding a commit to make the bare and full banners more consistent...
comment:7 Changed 7 years ago by
oh well, bare means "without bells and dingles"
all what is needed to have the tests pass in the docker container is that it contains the text of VERSION.txt
comment:8 Changed 7 years ago by
Branch:  u/chapoton/20322 → u/jdemeyer/20322 

comment:9 Changed 7 years ago by
Commit:  c2d0ff3df2b0543fbab75c9c24e067d9bd6d48e4 → 846e21de218ace07322ac24a64341ec71141578e 

Status:  needs_work → needs_review 
New commits:
846e21d  Make full and bare banner consistent

comment:11 Changed 7 years ago by
Commit:  846e21de218ace07322ac24a64341ec71141578e → 8b1c3697418043b7c1a6c4547d5e412a2ebe0e5f 

Branch pushed to git repo; I updated commit sha1. New commits:
8b1c369  Ensure that SAGE_BANNER=no is not exported

comment:14 Changed 7 years ago by
Out of curiosity, in what context did this come up? I reported the following issue upstream to Docker: https://github.com/docker/docker/issues/21323 But AFAICT this only affects the Docker client on Windows.
comment:15 Changed 7 years ago by
@embray
This was just to help you to make the sagepatchbot docker container work.
comment:16 Changed 7 years ago by
I just haven't had this problem specifically since I'm not trying to run the sagepatchbot container on Windows. In the meantime I just disabled the SAGE_BANNER for the docker images in sagemath/dockerimages@2c90e030 . So I was just curious how this came about since I wasn't explicitly asking for it (not that I'm against it).
comment:17 Changed 7 years ago by
This is the disabling of the banner that causes some failing doctests:
the other failing doctest is in sagedev, but not yet understood.
SO PLEASE, once the next release is out, use SAGE_BANNER=bare instead of "no"
comment:18 Changed 7 years ago by
Ah, I see. Though maybe those tests should check the actual value of the environment variable and skip if it's different, or just set it explicitly beforehand.
comment:19 Changed 7 years ago by
Status:  positive_review → needs_work 

sage t long src/sage/tests/cmdline.py ********************************************************************** File "src/sage/tests/cmdline.py", line 106, in sage.tests.cmdline.test_executable Failed example: out.find(version()) >= 0 Expected: True Got: False ********************************************************************** File "src/sage/tests/cmdline.py", line 114, in sage.tests.cmdline.test_executable Failed example: out.find(version()) >= 0 Expected: True Got: False ********************************************************************** 1 item had failures: 2 of 235 in sage.tests.cmdline.test_executable [234 tests, 2 failures, 63.86 s]
comment:20 Changed 7 years ago by
Hum, I think this just needs a banner and VERSION.txt refreshment to work.
So this is a rather special kind of ticket. Maybe to merge just before a beta release ?
comment:21 Changed 7 years ago by
Status:  needs_work → positive_review 

I guess this is yet another "the bug is not in the ticket but in the buildbot".
comment:22 Changed 7 years ago by
Commit:  8b1c3697418043b7c1a6c4547d5e412a2ebe0e5f → 5b87f1aca7a38d0c5ef80b8e12b16822039a7360 

Status:  positive_review → needs_review 
Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:
5b87f1a  Updated SageMath version to 7.2.beta1

comment:23 Changed 7 years ago by
Status:  needs_review → positive_review 

comment:24 Changed 7 years ago by
The last commit isn't really needed, it's just to make the buildbot happy.
comment:25 Changed 7 years ago by
I still think that if a test is expecting something particular about the environment that it should set that environment for the test. For example something like

src/sage/tests/cmdline.py
diff git a/src/sage/tests/cmdline.py b/src/sage/tests/cmdline.py index 534436d..1d0395e 100644
a b from subprocess import * 57 57 import os, select 58 58 59 59 60 def test_executable(args, input="", timeout=100.0, **kwds):60 def test_executable(args, input="", timeout=100.0, env={}, **kwds): 61 61 r""" 62 62 Run the program defined by ``args`` using the string ``input`` on 63 63 the standard input. … … def test_executable(args, input="", timeout=100.0, **kwds): 102 102 103 103 Run Sage itself with various options:: 104 104 105 sage: (out, err, ret) = test_executable(["sage"] )105 sage: (out, err, ret) = test_executable(["sage"], env={'SAGE_BANNER': 'bare'}) 106 106 sage: out.find(version()) >= 0 107 107 True 108 108 sage: err … … def test_executable(args, input="", timeout=100.0, **kwds): 110 110 sage: ret 111 111 0 112 112 113 sage: (out, err, ret) = test_executable(["sage"], "3^33\n" )113 sage: (out, err, ret) = test_executable(["sage"], "3^33\n", env={'SAGE_BANNER': 'bare'}) 114 114 sage: out.find(version()) >= 0 115 115 True 116 116 sage: out.find("5559060566555523") >= 0 … … def test_executable(args, input="", timeout=100.0, **kwds): 120 120 sage: ret 121 121 0 122 122 123 sage: (out, err, ret) = test_executable(["sage", "q"], "3^33\n" )123 sage: (out, err, ret) = test_executable(["sage", "q"], "3^33\n", env={'SAGE_BANNER': 'bare'}) 124 124 sage: out.find(version()) >= 0 125 125 False 126 126 sage: out.find("5559060566555523") >= 0 … … def test_executable(args, input="", timeout=100.0, **kwds): 489 489 ....: os.open(os.ctermid(), os.O_RDONLY) 490 490 ....: return True 491 491 ....: except OSError: 492 ....: return False 492 ....: return False 493 493 sage: (out, err, ret) = test_executable(["sage", "ecl"], "(* 12345 54321)\n") 494 494 sage: out.find("Embeddable CommonLisp") >= 0 495 495 True … … def test_executable(args, input="", timeout=100.0, **kwds): 728 728 del pexpect_env["TERM"] 729 729 except KeyError: 730 730 pass 731 732 pexpect_env.update(env) 733 731 734 p = Popen(args, stdin=PIPE, stdout=PIPE, stderr=PIPE, env=pexpect_env, **kwds) 732 735 if input: 733 736 p.stdin.write(input)
comment:26 Changed 7 years ago by
Branch:  u/jdemeyer/20322 → 5b87f1aca7a38d0c5ef80b8e12b16822039a7360 

Resolution:  → fixed 
Status:  positive_review → closed 
comment:27 Changed 7 years ago by
Commit:  5b87f1aca7a38d0c5ef80b8e12b16822039a7360 

Incidentally, the patchbot has its own unicode banner that is causing Docker client to crash: https://github.com/robertwb/sagepatchbot/blob/master/src/patchbot.py#L455 I will open a separate issue.
comment:28 Changed 7 years ago by
I have submitted an issue regarding the patchbot's banner to https://github.com/robertwb/sagepatchbot/pull/78
setting SAGE_BANNER to "bare" should give the expected result
New commits:
trac #20322 quick solution for a shorten banner