From 2e76b6a44cccf5ad8aca96b127e06c2e81363b60 Mon Sep 17 00:00:00 2001 From: Ryan Roberts Date: Tue, 3 Jan 2023 17:04:57 +0000 Subject: [PATCH 1/4] utils: Remove red as a text colour from the logger Red implies an error has occured. So using red as a colour for an informational output stream could be confusing for the user. Remove it from the set of colours we use. Signed-off-by: Ryan Roberts --- shrinkwrap/utils/logger.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shrinkwrap/utils/logger.py b/shrinkwrap/utils/logger.py index 8f7d1d9..d7e4cca 100644 --- a/shrinkwrap/utils/logger.py +++ b/shrinkwrap/utils/logger.py @@ -7,7 +7,7 @@ import re termcolor = None _ansi_escape = re.compile(r'\x1B(?:[@-Z\\-_]|\[[0-?]*[ -/]*[@-~])') -_colors = ['blue', 'cyan', 'green', 'yellow', 'magenta', 'grey', 'red'] +_colors = ['blue', 'cyan', 'green', 'yellow', 'magenta', 'grey'] Data = namedtuple("Data", "tag color") -- GitLab From 2ced2d21865e54036b55ec0b1ffa40d59f5ed896 Mon Sep 17 00:00:00 2001 From: Ryan Roberts Date: Tue, 3 Jan 2023 17:41:00 +0000 Subject: [PATCH 2/4] build, clean: Squelch noisy output from git and dtc Git is chatty by default and so would previously output informtive messages even when shrinkwrap's --verbose was not specified. This would make the output less legible. So pass --quiet to git to shut if up when the user has not specified --verbose. Similarly, when injecting a CHOSEN node into a device tree, dtc would re-emit warnings that are already output from the original compile. Suppress these duplicates by passing -q to dtc when doing the recompile. Signed-off-by: Ryan Roberts --- config/dt-base.yaml | 2 +- shrinkwrap/utils/config.py | 12 +++++++----- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/config/dt-base.yaml b/config/dt-base.yaml index 2ba5814..2f0bba0 100644 --- a/config/dt-base.yaml +++ b/config/dt-base.yaml @@ -52,7 +52,7 @@ build: - if [ -z "$${CHOSEN}" ]; then - cp $${DTB_INTER} $${DTB_FINAL} - else - - ( dtc -O dts -I dtb $${DTB_INTER} ; echo -e "/ { chosen { $${CHOSEN} }; };" ) | dtc -O dtb -o $${DTB_FINAL} + - ( dtc -q -O dts -I dtb $${DTB_INTER} ; echo -e "/ { chosen { $${CHOSEN} }; };" ) | dtc -q -O dtb -o $${DTB_FINAL} - fi clean: diff --git a/shrinkwrap/utils/config.py b/shrinkwrap/utils/config.py index c5849ce..53766a3 100644 --- a/shrinkwrap/utils/config.py +++ b/shrinkwrap/utils/config.py @@ -724,6 +724,7 @@ def build_graph(configs, echo): build all the configs. """ graph = {} + gitargs = '' if echo else '--quiet ' pre = script_preamble(echo) @@ -783,10 +784,10 @@ def build_graph(configs, echo): g.append(f'\trm -rf {gitlocal} > /dev/null 2>&1 || true') g.append(f'\tmkdir -p {basedir}') g.append(f'\ttouch {sync}') - g.append(f'\tgit clone {gitremote} {gitlocal}') + g.append(f'\tgit clone {gitargs}{gitremote} {gitlocal}') g.append(f'\tpushd {gitlocal}') - g.append(f'\tgit checkout --force {gitrev}') - g.append(f'\tgit submodule update --init --checkout --recursive --force') + g.append(f'\tgit checkout {gitargs}--force {gitrev}') + g.append(f'\tgit submodule {gitargs}update --init --checkout --recursive --force') g.append(f'\tpopd') g.append(f'\trm {sync}') g.append(f'fi') @@ -835,6 +836,7 @@ def clean_graph(configs, echo, clean_repo): clean all the configs. """ graph = {} + gitargs = '' if echo else '--quiet ' pre = script_preamble(echo) @@ -882,8 +884,8 @@ def clean_graph(configs, echo, clean_repo): g.append(f'\tif [ -d "{gitlocal}/.git" ] && [ ! -f "{sync}" ]; then') g.append(f'\t\tpushd {gitlocal}') - g.append(f'\t\tgit clean -xdff') - g.append(f'\t\tgit reset --hard') + g.append(f'\t\tgit clean {gitargs}-xdff') + g.append(f'\t\tgit reset {gitargs}--hard') g.append(f'\t\tpopd') g.append(f'\tfi') -- GitLab From 8e97737b7391a710ec46f0af379ee39eda3f789c Mon Sep 17 00:00:00 2001 From: Ryan Roberts Date: Tue, 3 Jan 2023 18:04:28 +0000 Subject: [PATCH 3/4] build, clean: Make normal and verbose output more consistent Use labels of the same length for both normal and verbose output, and reserve the default tezt colour for normal output to distingush it from verbose prints. Signed-off-by: Ryan Roberts --- shrinkwrap/utils/graph.py | 2 +- shrinkwrap/utils/logger.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/shrinkwrap/utils/graph.py b/shrinkwrap/utils/graph.py index cd4c054..297ba20 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(20, colorize) + log = logger.Logger(27, colorize) ts = graphlib.TopologicalSorter(graph) def _pump(pm): diff --git a/shrinkwrap/utils/logger.py b/shrinkwrap/utils/logger.py index d7e4cca..793f626 100644 --- a/shrinkwrap/utils/logger.py +++ b/shrinkwrap/utils/logger.py @@ -7,7 +7,7 @@ import re termcolor = None _ansi_escape = re.compile(r'\x1B(?:[@-Z\\-_]|\[[0-?]*[ -/]*[@-~])') -_colors = ['blue', 'cyan', 'green', 'yellow', 'magenta', 'grey'] +_colors = ['blue', 'cyan', 'green', 'yellow', 'magenta'] Data = namedtuple("Data", "tag color") -- GitLab From 0bbf27d91f72a41b2d64f31480ba76fdcd326502 Mon Sep 17 00:00:00 2001 From: Ryan Roberts Date: Wed, 4 Jan 2023 13:57:00 +0000 Subject: [PATCH 4/4] run: Fix extra LFs and ignored CRs in output We were previously converting UART output to strings using Python's universal newline technique, where '\r', '\n' and '\r\n' were all replaced with '\n'. This caused a few issues; First, telnet would occasionally output '\r\r\n' which would get converted to 2 newlines, whereas only one was intended. Secondly, we were swallowing legitimate '\r's when working at the interactive shell and the cursor would occasionally get messed up. Solve this by never converting line endings when converting the binary data to string data. As a result we must be more carefull with '\r' since they may now exist in the stream. Signed-off-by: Ryan Roberts --- shrinkwrap/commands/run.py | 7 ++++--- shrinkwrap/utils/logger.py | 37 ++++++++++++++++++++++++++++++------- shrinkwrap/utils/process.py | 6 +++++- 3 files changed, 39 insertions(+), 11 deletions(-) diff --git a/shrinkwrap/commands/run.py b/shrinkwrap/commands/run.py index ab7454f..e6561d7 100644 --- a/shrinkwrap/commands/run.py +++ b/shrinkwrap/commands/run.py @@ -107,12 +107,13 @@ def dispatch(args): few lines of output, which is output by telnet. This is confusing for users since telnet is an implementation detail. """ - match = "Escape character is '^]'.\n" + + match = "Escape character is '^]'." pdata = proc.data - for line in data.splitlines(keepends=True): + for line in logger.splitlines(data): if len(pdata) >= 2 and terminals[pdata[1]]['strip']: - if line == match: + if line.find(match) >= 0: terminals[pdata[1]]['strip'] = False if all([not t['strip'] \ for t in terminals.values()]): diff --git a/shrinkwrap/utils/logger.py b/shrinkwrap/utils/logger.py index 793f626..c9e63db 100644 --- a/shrinkwrap/utils/logger.py +++ b/shrinkwrap/utils/logger.py @@ -11,6 +11,17 @@ _colors = ['blue', 'cyan', 'green', 'yellow', 'magenta'] Data = namedtuple("Data", "tag color") +def splitlines(string): + """ + Like str.splitlines(True) but preserves '\r'. + """ + lines = string.split('\n') + keepends = [l + '\n' for l in lines[:-1]] + if lines[-1] != '': + keepends.append(lines[-1]) + return keepends + + class Logger: def __init__(self, tag_size, colorize): self._tag_size = tag_size @@ -43,7 +54,9 @@ class Logger: on the left to identify the originating process. """ # Remove any ansi escape sequences since we are just outputting - # text to stdout. + # 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 @@ -56,7 +69,7 @@ class Logger: tag = tag[:self._tag_size-3] + '...' tag = f'{{:>{self._tag_size}}}'.format(tag) - lines = data.splitlines(keepends=True) + lines = splitlines(data) start = 0 # If the cursor is not at the start of a new line, then if the @@ -66,20 +79,30 @@ class Logger: # newline and add a tag for the new owner. if self._prev_char != '\n': if self._prev_tag == tag: - self.print(f'{lines[0]}', color, end='') + self.print(lines[0], tag, True, color, end='') start = 1 else: - print(f'\n', end='') + self.print('\n', tag, True, color, end='') for line in lines[start:]: - self.print(f'[ {tag} ] {line}', color, end='') + self.print(line, tag, False, color, end='') self._prev_tag = tag self._prev_char = lines[-1][-1] sys.stdout.flush() - def print(self, text, color=None, on_color=None, attrs=None, **kwargs): + 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} ] ' + text = text.replace('\r', f'\r{tag}') + + if not cont: + self._print(tag, color, on_color, attrs, end='') + + self._print(text, color, on_color, attrs, **kwargs) + + def _print(self, text, color=None, on_color=None, attrs=None, **kwargs): if self._colorize: text = termcolor.colored(text, color, on_color, attrs) - print(text, **kwargs) \ No newline at end of file + print(text, **kwargs) diff --git a/shrinkwrap/utils/process.py b/shrinkwrap/utils/process.py index d238412..0d88420 100644 --- a/shrinkwrap/utils/process.py +++ b/shrinkwrap/utils/process.py @@ -121,7 +121,11 @@ class ProcessManager: proc._stdin = io.open(master, 'wb', buffering=0) proc._stdout = io.open(master, 'rb', -1, closefd=False) - proc._stdout = io.TextIOWrapper(proc._stdout) + # Don't attempt to translate newlines. We need the '\r's + # to correctly return the carriage for interactive + # terminals. Telnet sometimes gives '\r\r\n' too, which + # would be incorrectly translated to '\n\n'. + proc._stdout = io.TextIOWrapper(proc._stdout, newline='') # stdout and stderr get merged into pty, so can't tell # them apart. This isn't a problem for the emit build # warnings use case. -- GitLab