#20322 closed enhancement (fixed)
use SAGE_BANNER to propose a bare banner with no utf8
Reported by:  chapoton  Owned by:  

Priority:  major  Milestone:  sage7.2 
Component:  user interface  Keywords:  
Cc:  nthiery, embray, jdemeyer  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 6 years ago by
 Branch set to u/chapoton/20322
 Commit set to 465db90b975dbf2c2ada8d8b1c8f33ea4f048fcf
 Status changed from new to needs_review
comment:2 Changed 6 years ago by
 Cc embray jdemeyer added
comment:3 Changed 6 years ago by
 Component changed from PLEASE CHANGE to user interface
 Status changed from needs_review to needs_work
 Type changed from PLEASE CHANGE to enhancement
Can you use consistent indentation?
comment:4 Changed 6 years ago by
 Commit changed from 465db90b975dbf2c2ada8d8b1c8f33ea4f048fcf to c2d0ff3df2b0543fbab75c9c24e067d9bd6d48e4
Branch pushed to git repo; I updated commit sha1. New commits:
c2d0ff3  indentation

comment:5 Changed 6 years ago by
 Status changed from needs_work to needs_review
comment:6 Changed 6 years ago by
 Reviewers set to Jeroen Demeyer
 Status changed from needs_review to needs_work
I am adding a commit to make the bare and full banners more consistent...
comment:7 Changed 6 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 6 years ago by
 Branch changed from u/chapoton/20322 to u/jdemeyer/20322
comment:9 Changed 6 years ago by
 Commit changed from c2d0ff3df2b0543fbab75c9c24e067d9bd6d48e4 to 846e21de218ace07322ac24a64341ec71141578e
 Status changed from needs_work to needs_review
New commits:
846e21d  Make full and bare banner consistent

comment:10 Changed 6 years ago by
Running full doctests now with SAGE_BANNER=bare
...
comment:11 Changed 6 years ago by
 Commit changed from 846e21de218ace07322ac24a64341ec71141578e to 8b1c3697418043b7c1a6c4547d5e412a2ebe0e5f
Branch pushed to git repo; I updated commit sha1. New commits:
8b1c369  Ensure that SAGE_BANNER=no is not exported

comment:12 Changed 6 years ago by
Doctests pass: positive_review from my part.
comment:14 Changed 6 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 6 years ago by
@embray
This was just to help you to make the sagepatchbot docker container work.
comment:16 Changed 6 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 6 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 6 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 6 years ago by
 Status changed from positive_review to 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 6 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 6 years ago by
 Status changed from needs_work to positive_review
I guess this is yet another "the bug is not in the ticket but in the buildbot".
comment:22 Changed 6 years ago by
 Commit changed from 8b1c3697418043b7c1a6c4547d5e412a2ebe0e5f to 5b87f1aca7a38d0c5ef80b8e12b16822039a7360
 Status changed from positive_review to 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 6 years ago by
 Status changed from needs_review to positive_review
comment:24 Changed 6 years ago by
The last commit isn't really needed, it's just to make the buildbot happy.
comment:25 Changed 6 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): 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 6 years ago by
 Branch changed from u/jdemeyer/20322 to 5b87f1aca7a38d0c5ef80b8e12b16822039a7360
 Resolution set to fixed
 Status changed from positive_review to closed
comment:27 Changed 6 years ago by
 Commit 5b87f1aca7a38d0c5ef80b8e12b16822039a7360 deleted
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 6 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