From dbcd42706bbcdcd3737a00dd4d2f4cd15de5fa8c Mon Sep 17 00:00:00 2001 From: Ryan Roberts Date: Fri, 24 Feb 2023 11:47:57 +0000 Subject: [PATCH 1/5] utils: Use unique id for proc streams rather than friendly name We are about to make the friendly name optional to avoid the need to prepend the tag on each line of output. So in preparation, assign a unique id to each process stream and use that to determine if we have stream interleving mid-line. Since multiple streams could soon choose an empty friendly name, we can no longer use that as a unique id. Signed-off-by: Ryan Roberts --- shrinkwrap/utils/logger.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/shrinkwrap/utils/logger.py b/shrinkwrap/utils/logger.py index c9e63db..0233f21 100644 --- a/shrinkwrap/utils/logger.py +++ b/shrinkwrap/utils/logger.py @@ -8,7 +8,7 @@ termcolor = None _ansi_escape = re.compile(r'\x1B(?:[@-Z\\-_]|\[[0-?]*[ -/]*[@-~])') _colors = ['blue', 'cyan', 'green', 'yellow', 'magenta'] -Data = namedtuple("Data", "tag color") +Data = namedtuple("Data", "id tag color") def splitlines(string): @@ -26,8 +26,8 @@ class Logger: def __init__(self, tag_size, colorize): self._tag_size = tag_size self._colorize = colorize - self._color_next = 0 - self._prev_tag = None + self._id_next = 0 + self._prev_id = None self._prev_char = '\n' if self._colorize: @@ -41,11 +41,10 @@ class Logger: log() is called. Includes the tag for the process and an allocated colour. """ - idx = self._color_next - self._color_next += 1 - self._color_next %= len(_colors) - color = _colors[idx] - return Data(tag, color) + id = self._id_next + self._id_next += 1 + color = _colors[id % len(_colors)] + return Data(id, tag, color) def log(self, pm, proc, data, streamid): """ @@ -61,6 +60,7 @@ class Logger: if len(data) == 0: return + id = proc.data[0].id tag = proc.data[0].tag color = proc.data[0].color @@ -78,7 +78,7 @@ class Logger: # the first part of the line has a different owner, insert a # newline and add a tag for the new owner. if self._prev_char != '\n': - if self._prev_tag == tag: + if self._prev_id == id: self.print(lines[0], tag, True, color, end='') start = 1 else: @@ -87,7 +87,7 @@ class Logger: for line in lines[start:]: self.print(line, tag, False, color, end='') - self._prev_tag = tag + self._prev_id = id self._prev_char = lines[-1][-1] sys.stdout.flush() -- GitLab From fb48fe50602da167a4ec815d2d18779ba2ebc92c Mon Sep 17 00:00:00 2001 From: Rob Herring Date: Fri, 10 Feb 2023 18:22:15 +0000 Subject: [PATCH 2/5] config: Make empty 'friendly' disable stdout prefix The line prefix for stdout interferes with interactive terminal usage such as shell command line editing. Allow 'friendly' to be an empty string to disable the prefix on that particular output. For example, FVP output is prefixed, but Linux is not. Signed-off-by: Rob Herring --- documentation/userguide/config.rst | 2 +- shrinkwrap/utils/logger.py | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/documentation/userguide/config.rst b/documentation/userguide/config.rst index c750190..32e84a7 100644 --- a/documentation/userguide/config.rst +++ b/documentation/userguide/config.rst @@ -206,7 +206,7 @@ terminal section =========== =========== =========== key type description =========== =========== =========== -friendly string Label to display against the terminal when muxing to stdout. +friendly string Label to display against the terminal when muxing to stdout. An empty string disables the prefix for the output. port_regex string Regex to use to find the TCP port of the terminal when parsing the FVP stdout. Must have single capture group. type enum-string Terminal type. See below for options. =========== =========== =========== diff --git a/shrinkwrap/utils/logger.py b/shrinkwrap/utils/logger.py index 0233f21..54b1521 100644 --- a/shrinkwrap/utils/logger.py +++ b/shrinkwrap/utils/logger.py @@ -67,7 +67,8 @@ class Logger: # Make the tag. if len(tag) > self._tag_size: tag = tag[:self._tag_size-3] + '...' - tag = f'{{:>{self._tag_size}}}'.format(tag) + if len(tag): + tag = f'{{:>{self._tag_size}}}'.format(tag) lines = splitlines(data) start = 0 @@ -94,7 +95,8 @@ class Logger: def print(self, text, tag, cont, color=None, on_color=None, attrs=None, **kwargs): # Ensure that any '\r's only rewind to the end of the tag. - tag = f'[ {tag} ] ' + if len(tag): + tag = f'[ {tag} ] ' text = text.replace('\r', f'\r{tag}') if not cont: -- GitLab From 1b3e1d519b6951d7ec7c5a757a2d73fa1798289a Mon Sep 17 00:00:00 2001 From: Ryan Roberts Date: Mon, 27 Feb 2023 11:31:18 +0000 Subject: [PATCH 3/5] config: Set primary terminal to not use a friendly tag This reduces the amount of screen real-estate we need to use for the common case. Signed-off-by: Ryan Roberts --- config/bootwrapper.yaml | 2 +- config/cca-3world.yaml | 2 +- config/ns-edk2-acpi.yaml | 2 +- config/ns-preload.yaml | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/config/bootwrapper.yaml b/config/bootwrapper.yaml index f1f7e30..848f78c 100644 --- a/config/bootwrapper.yaml +++ b/config/bootwrapper.yaml @@ -36,5 +36,5 @@ run: terminals: bp.terminal_0: - friendly: linux + friendly: '' type: stdinout diff --git a/config/cca-3world.yaml b/config/cca-3world.yaml index 079558c..64d9435 100644 --- a/config/cca-3world.yaml +++ b/config/cca-3world.yaml @@ -114,7 +114,7 @@ run: terminals: bp.terminal_0: - friendly: tfa+linux + friendly: '' type: stdinout bp.terminal_1: diff --git a/config/ns-edk2-acpi.yaml b/config/ns-edk2-acpi.yaml index fc68c99..04e189f 100644 --- a/config/ns-edk2-acpi.yaml +++ b/config/ns-edk2-acpi.yaml @@ -92,7 +92,7 @@ run: terminals: bp.terminal_0: - friendly: tfa+edk2+linux + friendly: '' type: stdinout bp.terminal_1: friendly: edk2 diff --git a/config/ns-preload.yaml b/config/ns-preload.yaml index ea1ae8c..c9e4802 100644 --- a/config/ns-preload.yaml +++ b/config/ns-preload.yaml @@ -79,5 +79,5 @@ run: terminals: bp.terminal_0: - friendly: tfa+linux + friendly: '' type: stdinout -- GitLab From 82d7c98dd33eb189117131d20a1bbb5823f1a3db Mon Sep 17 00:00:00 2001 From: Ryan Roberts Date: Fri, 24 Feb 2023 12:50:50 +0000 Subject: [PATCH 4/5] config: Configure color coding on a per-terminal basis Applying a color coding to each terminal's output can interfere with coloring/escape sequences that an interactive shell running within the FVP may want to apply. Although we already have the --no-color command line option, this is not fine-grained enough, so allow each terminal to specify a no_color property. We then set this property for all terminals within the standard set of configs that are stdinout. The result is that the main interactive terminal is now the default text color, other terminals are still colored differently, and any (e.g.) `ls` output is now colored correctly. Signed-off-by: Ryan Roberts --- config/bootwrapper.yaml | 1 + config/cca-3world.yaml | 1 + config/ns-edk2-acpi.yaml | 1 + config/ns-preload.yaml | 1 + documentation/userguide/config.rst | 1 + shrinkwrap/commands/run.py | 29 +++++++++++++++++++---------- shrinkwrap/utils/graph.py | 4 ++-- shrinkwrap/utils/logger.py | 30 ++++++++++++++++++++---------- 8 files changed, 46 insertions(+), 22 deletions(-) diff --git a/config/bootwrapper.yaml b/config/bootwrapper.yaml index 848f78c..3cb0c76 100644 --- a/config/bootwrapper.yaml +++ b/config/bootwrapper.yaml @@ -38,3 +38,4 @@ run: bp.terminal_0: friendly: '' type: stdinout + no_color: true diff --git a/config/cca-3world.yaml b/config/cca-3world.yaml index 64d9435..9d4515c 100644 --- a/config/cca-3world.yaml +++ b/config/cca-3world.yaml @@ -116,6 +116,7 @@ run: bp.terminal_0: friendly: '' type: stdinout + no_color: true bp.terminal_1: friendly: tfa-rt diff --git a/config/ns-edk2-acpi.yaml b/config/ns-edk2-acpi.yaml index 04e189f..183d089 100644 --- a/config/ns-edk2-acpi.yaml +++ b/config/ns-edk2-acpi.yaml @@ -94,5 +94,6 @@ run: bp.terminal_0: friendly: '' type: stdinout + no_color: true bp.terminal_1: friendly: edk2 diff --git a/config/ns-preload.yaml b/config/ns-preload.yaml index c9e4802..8505a6b 100644 --- a/config/ns-preload.yaml +++ b/config/ns-preload.yaml @@ -81,3 +81,4 @@ run: bp.terminal_0: friendly: '' type: stdinout + no_color: true diff --git a/documentation/userguide/config.rst b/documentation/userguide/config.rst index 32e84a7..113c2c2 100644 --- a/documentation/userguide/config.rst +++ b/documentation/userguide/config.rst @@ -209,6 +209,7 @@ key type description friendly string Label to display against the terminal when muxing to stdout. An empty string disables the prefix for the output. port_regex string Regex to use to find the TCP port of the terminal when parsing the FVP stdout. Must have single capture group. type enum-string Terminal type. See below for options. +no_color boolean Optional (defaults to false, only applies to ['stdout', 'stdinout'] types): If true, output from this terminal is not color-coded. If this terminal carries the interactive shell, it is advised to set this to true to prevent interferring with the shell's escape sequences. --no-color command line option causes this to behave as if set to true. =========== =========== =========== Terminal types: diff --git a/shrinkwrap/commands/run.py b/shrinkwrap/commands/run.py index e6561d7..b57c537 100644 --- a/shrinkwrap/commands/run.py +++ b/shrinkwrap/commands/run.py @@ -99,7 +99,7 @@ def dispatch(args): if len(t['friendly']) > name_field: name_field = min(len(t['friendly']), max_name_field) - log = logger.Logger(name_field, not args.no_color) + log = logger.Logger(name_field) def _strip_telnet_header(pm, proc, data, streamid): """ @@ -121,6 +121,14 @@ def dispatch(args): else: log.log(pm, proc, line, streamid) + def _colorize(global_no_color, terminal): + if global_no_color: + return False + elif 'no_color' in terminal: + return not terminal['no_color'] + else: + return True + def _find_term_ports(pm, proc, data, streamid): """ Initial handler function called by ProcessManager. When the fvp @@ -155,19 +163,20 @@ def dispatch(args): name = t['friendly'] type = t['type'] port = t["port"] + colorize = _colorize(args.no_color, t) if type in ['stdout']: cmd = f'nc localhost {port}' pm.add(process.Process(cmd, - False, - (log.alloc_data(name), k), - False)) + False, + (log.alloc_data(name, colorize), k), + False)) if type in ['stdinout']: cmd = f'telnet localhost {port}' pm.add(process.Process(cmd, - True, - (log.alloc_data(name), k), - False)) + True, + (log.alloc_data(name, colorize), k), + False)) t['strip'] = True strip = True if type in ['xterm']: @@ -221,9 +230,9 @@ def dispatch(args): # the fvp has gone. pm = process.ProcessManager(_find_term_ports, _complete) pm.add(process.Process(f'bash {tmpfilename}', - False, - (log.alloc_data('fvp'),), - True)) + False, + (log.alloc_data('fvp', not args.no_color),), + True)) rt_ip = runtime.get().ip_address() diff --git a/shrinkwrap/utils/graph.py b/shrinkwrap/utils/graph.py index 297ba20..b80f4fc 100644 --- a/shrinkwrap/utils/graph.py +++ b/shrinkwrap/utils/graph.py @@ -113,7 +113,7 @@ def execute(graph, tasks, verbose=False, colorize=True): queue = [] active = 0 - log = logger.Logger(27, colorize) + log = logger.Logger(27) ts = graphlib.TopologicalSorter(graph) def _pump(pm): @@ -127,7 +127,7 @@ def execute(graph, tasks, verbose=False, colorize=True): frag.config, frag.component, frag.summary + '...') - data = (log.alloc_data(str(frag)), []) + data = (log.alloc_data(str(frag), colorize), []) _run_script(pm, data, frag) active += 1 diff --git a/shrinkwrap/utils/logger.py b/shrinkwrap/utils/logger.py index 54b1521..c890c2d 100644 --- a/shrinkwrap/utils/logger.py +++ b/shrinkwrap/utils/logger.py @@ -6,6 +6,13 @@ from collections import namedtuple import re termcolor = None + +def _import_termcolor(): + global termcolor + import termcolor as tc + termcolor = tc + + _ansi_escape = re.compile(r'\x1B(?:[@-Z\\-_]|\[[0-?]*[ -/]*[@-~])') _colors = ['blue', 'cyan', 'green', 'yellow', 'magenta'] Data = namedtuple("Data", "id tag color") @@ -23,27 +30,30 @@ def splitlines(string): class Logger: - def __init__(self, tag_size, colorize): + def __init__(self, tag_size): self._tag_size = tag_size - self._colorize = colorize self._id_next = 0 + self._color_next = 0 self._prev_id = None self._prev_char = '\n' - if self._colorize: - global termcolor - import termcolor as tc - termcolor = tc - - def alloc_data(self, tag): + def alloc_data(self, tag, colorize): """ Returns the object that should be stashed in proc.data[0] when log() is called. Includes the tag for the process and an allocated colour. """ + if colorize: + _import_termcolor() + color = self._color_next + self._color_next += 1 + color = _colors[color % len(_colors)] + else: + color = None + id = self._id_next self._id_next += 1 - color = _colors[id % len(_colors)] + return Data(id, tag, color) def log(self, pm, proc, data, streamid): @@ -105,6 +115,6 @@ class Logger: self._print(text, color, on_color, attrs, **kwargs) def _print(self, text, color=None, on_color=None, attrs=None, **kwargs): - if self._colorize: + if color: text = termcolor.colored(text, color, on_color, attrs) print(text, **kwargs) -- GitLab From 30d284f7039eb6830cd1badb4e052d0938abc718 Mon Sep 17 00:00:00 2001 From: Ryan Roberts Date: Mon, 27 Feb 2023 11:14:44 +0000 Subject: [PATCH 5/5] run: Stop removing escape sequences by default Our attempt to remove escape sequences from the terminal output streams was motivated by wanting to stop EDK2 from clearing over previous output (and generally leaving the terminal in a bad state for future output), as well as not wanting the escape sequences to conflict with shrinkwrap's use of color coding for different output sources. But it was very broken, and meant that legitimate escape sequence generators (e.g.) bash, ls, etc were being punished and the UX was diminished. Make it opt-in to remove escape sequences on a per-terminal basis. This is specified in the config, using the new terminal::::no_escapes property. By default this is disabled. But for EDK2 based configs, we set it to be enabled during boot until handoff to the kernel, which will disable it. Its still not perfect; if an escape sequence gets split across read buffers, we will not notice it. But given we are attempting to strip escape sequences far less often than we used to (only for configs with EDK2 and only until EDK2 hands off to the kernel), perhaps no body will notice. Signed-off-by: Ryan Roberts --- config/ns-edk2-acpi.yaml | 1 + documentation/userguide/config.rst | 1 + shrinkwrap/commands/run.py | 11 ++++++-- shrinkwrap/utils/logger.py | 44 ++++++++++++++++++++++-------- 4 files changed, 44 insertions(+), 13 deletions(-) diff --git a/config/ns-edk2-acpi.yaml b/config/ns-edk2-acpi.yaml index 183d089..2200739 100644 --- a/config/ns-edk2-acpi.yaml +++ b/config/ns-edk2-acpi.yaml @@ -95,5 +95,6 @@ run: friendly: '' type: stdinout no_color: true + no_escapes: 'EFI stub: Booting Linux Kernel...' bp.terminal_1: friendly: edk2 diff --git a/documentation/userguide/config.rst b/documentation/userguide/config.rst index 113c2c2..c75c4df 100644 --- a/documentation/userguide/config.rst +++ b/documentation/userguide/config.rst @@ -210,6 +210,7 @@ friendly string Label to display against the terminal when muxing to std port_regex string Regex to use to find the TCP port of the terminal when parsing the FVP stdout. Must have single capture group. type enum-string Terminal type. See below for options. no_color boolean Optional (defaults to false, only applies to ['stdout', 'stdinout'] types): If true, output from this terminal is not color-coded. If this terminal carries the interactive shell, it is advised to set this to true to prevent interferring with the shell's escape sequences. --no-color command line option causes this to behave as if set to true. +no_escapes bool/string Optional (defaults to false, only applies to ['stdout', 'stdinout'] types): If true, strips any escape sequences from the output stream before forwarding to the terminal. If a string, behaves as if true until the string is found in the output, which sets it to false. Useful to expunge escape sequences from EDK2 during boot. =========== =========== =========== Terminal types: diff --git a/shrinkwrap/commands/run.py b/shrinkwrap/commands/run.py index b57c537..92f4e71 100644 --- a/shrinkwrap/commands/run.py +++ b/shrinkwrap/commands/run.py @@ -129,6 +129,12 @@ def dispatch(args): else: return True + def _escape(terminal): + if 'no_escapes' in terminal: + return terminal['no_escapes'] + else: + return False + def _find_term_ports(pm, proc, data, streamid): """ Initial handler function called by ProcessManager. When the fvp @@ -164,18 +170,19 @@ def dispatch(args): type = t['type'] port = t["port"] colorize = _colorize(args.no_color, t) + escape = _escape(t) if type in ['stdout']: cmd = f'nc localhost {port}' pm.add(process.Process(cmd, False, - (log.alloc_data(name, colorize), k), + (log.alloc_data(name, colorize, escape), k), False)) if type in ['stdinout']: cmd = f'telnet localhost {port}' pm.add(process.Process(cmd, True, - (log.alloc_data(name, colorize), k), + (log.alloc_data(name, colorize, escape), k), False)) t['strip'] = True strip = True diff --git a/shrinkwrap/utils/logger.py b/shrinkwrap/utils/logger.py index c890c2d..233c4c2 100644 --- a/shrinkwrap/utils/logger.py +++ b/shrinkwrap/utils/logger.py @@ -15,7 +15,19 @@ def _import_termcolor(): _ansi_escape = re.compile(r'\x1B(?:[@-Z\\-_]|\[[0-?]*[ -/]*[@-~])') _colors = ['blue', 'cyan', 'green', 'yellow', 'magenta'] -Data = namedtuple("Data", "id tag color") +Data = namedtuple("Data", "id tag color noesc escbuf") + + +class MatchBuf: + def __init__(self, match): + self._match = match + self._buf = '' + + def match(self, data): + self._buf += data + found = self._buf.find(self._match) >= 0 + self._buf = self._buf[1 - len(self._match):] + return found def splitlines(string): @@ -37,7 +49,7 @@ class Logger: self._prev_id = None self._prev_char = '\n' - def alloc_data(self, tag, colorize): + def alloc_data(self, tag, colorize, no_escapes=False): """ Returns the object that should be stashed in proc.data[0] when log() is called. Includes the tag for the process and an @@ -54,7 +66,14 @@ class Logger: id = self._id_next self._id_next += 1 - return Data(id, tag, color) + if type(no_escapes) == str: + noesc = True + escbuf = MatchBuf(no_escapes) + else: + noesc = no_escapes + escbuf = None + + return Data(id, tag, color, [noesc], escbuf) def log(self, pm, proc, data, streamid): """ @@ -62,17 +81,20 @@ class Logger: terminals) to the terminal. Text is colored and a tag is added on the left to identify the originating process. """ - # Remove any ansi escape sequences since we are just outputting - # text to stdout. This defends against EDK2's agregious use of - # screen clearing. But it does have the side effect that - # legitimate shell usage can get a bit wonky. - data = _ansi_escape.sub('', data) - if len(data) == 0: - return - id = proc.data[0].id tag = proc.data[0].tag color = proc.data[0].color + noesc = proc.data[0].noesc + escbuf = proc.data[0].escbuf + + # Remove any ansi escape sequences if requested. + if noesc[0]: + if escbuf and escbuf.match(data): + noesc[0] = False + else: + data = _ansi_escape.sub('', data) + if len(data) == 0: + return # Make the tag. if len(tag) > self._tag_size: -- GitLab