From 145ec14c9dfb5de4f49170248815fb70195968b9 Mon Sep 17 00:00:00 2001 From: Ryan Roberts Date: Wed, 13 Mar 2024 09:43:15 +0000 Subject: [PATCH 1/6] run: Fix broken user ack when using telnet terminal If any terminal type is set to 'telnet', Shrinkwrap prints the required telnet command that the user needs to connect, and waits for the user to acknowledge by pressing enter. It uses the standard Python input() function for this. However, the process manager has already redirected stdin so input() never receives any input and never returns. Fix this by explicitly requesting the process manager deactivates stdin around the input() call. Fixes: c99dc74 ("run: Overhaul user input handling.") Signed-off-by: Ryan Roberts --- shrinkwrap/commands/run.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/shrinkwrap/commands/run.py b/shrinkwrap/commands/run.py index 3357dc8..326f188 100644 --- a/shrinkwrap/commands/run.py +++ b/shrinkwrap/commands/run.py @@ -199,8 +199,11 @@ def dispatch(args): print(f'To start {name} terminal, run:') print(f' telnet {ip} {port}') if wait: + # Temporarily restore sys.stdin for input(). + pm._stdin_deactivate() print() input("Press Enter to continue...") + pm._stdin_activate() if strip: pm.set_handler(_strip_telnet_header) -- GitLab From bac54fc7b0255c68522ff3c83981ba3d8a6f6acc Mon Sep 17 00:00:00 2001 From: Ryan Roberts Date: Wed, 13 Mar 2024 11:19:48 +0000 Subject: [PATCH 2/6] build, clean, process, run: Allow json overlay on command line The --overlay option is supported by most commands and allows passing a yaml file to use as an overlay on top of the main config. Sometimes the desired overlay is very small and it is onerous to create a file. So let's allow a json-encoded overlay to be passed directly on the command line. If the overlay string starts with a '{' it is interpreted as a json object, otherwise it is treated as a yaml file name. This allows (e.g.): $ shrinkwrap run --overlay '{"run":{"params":{"-C bp.pl011_uart0.out_file":"out0.log"}}}' ... Signed-off-by: Ryan Roberts --- shrinkwrap/utils/config.py | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/shrinkwrap/utils/config.py b/shrinkwrap/utils/config.py index 46e1907..cbc27ee 100644 --- a/shrinkwrap/utils/config.py +++ b/shrinkwrap/utils/config.py @@ -3,6 +3,7 @@ import graphlib import io +import json import os import re import yaml @@ -355,7 +356,7 @@ def filename(name, rel=os.getcwd()): Given a config name, finds the path to the config on disk. If the config name exists relative to rel, we return that since it is a user config. Else, if the config name exists relative to the config store then we - return that. If neither exist, then we return the filepath option, since + return that. If neither exist, then we return name unmodified, since that will generate the most useful error. """ fpath = os.path.abspath(os.path.join(rel, name)) @@ -366,18 +367,25 @@ def filename(name, rel=os.getcwd()): elif cpath: return os.path.abspath(os.path.join(cpath, name)) else: - return fpath + return name def load(file_name, overlays=[], friendly=None): """ Load a config from disk and return it as a dictionary. The config is - fully normalized, validated and merged. + fully normalized, validated and merged. If file_name starts with '{' it + is treated as a json config instead of a file name and is parsed + directly. This allows passing json config snippets as overlays on the + command line. """ def _config_load(file_name): - with open(file_name) as file: - config = yaml.safe_load(file) - config_dir = os.path.dirname(file_name) + if file_name[0] == '{': + config = json.loads(file_name) + config_dir = os.getcwd() + else: + with open(file_name) as file: + config = yaml.safe_load(file) + config_dir = os.path.dirname(file_name) config = _config_normalize(config) _config_validate(config) -- GitLab From b1188ab9acb4d87dd26b6c88b43d2b541328cfa9 Mon Sep 17 00:00:00 2001 From: Ryan Roberts Date: Wed, 13 Mar 2024 11:54:18 +0000 Subject: [PATCH 3/6] docs: Recipe for passing json overlay on command line Show how a short overlay can be passed on the command line without the need for a file. While we are at it, remove the "Example Linux Feature Development Use Case" recipe, which was marked todo. Signed-off-by: Ryan Roberts --- documentation/userguide/quickstart.rst | 21 ++++++++------- documentation/userguide/recipes.rst | 36 ++++++++++++++++++++++---- 2 files changed, 42 insertions(+), 15 deletions(-) diff --git a/documentation/userguide/quickstart.rst b/documentation/userguide/quickstart.rst index f86d4d6..eb46e6d 100644 --- a/documentation/userguide/quickstart.rst +++ b/documentation/userguide/quickstart.rst @@ -748,16 +748,17 @@ Alternatively, you could have passed ``--dry-run`` to see the FVP invocation scr

Overlays are an important concept for Shrinkwrap. An overlay is a config -fragment (yaml file) that can be passed separately on the command line and forms -the top layer of the config. In this way, it can override or add any required -configuration. You could achive the same effect by creating a new config and -specifying the main config as a layer in that new config, but with an overlay, -you can apply a config fragment to many different existing configs without the -need to write a new config file each time. You can see overlays being using in -the above commands to target a specific Arm architecture revision (v9.3 in the -example). You can change the targetted architecture just by changing the -overlay. There are many other places where overlays come in handy. See -:ref:`userguide/recipes:Shrinkwrap Recipes` for more examples. +fragment (either a yaml file or a json-encoded string) that can be passed +separately on the command line and forms the top layer of the config. In this +way, it can override or add any required configuration. You could achive the +same effect by creating a new config and specifying the main config as a layer +in that new config, but with an overlay, you can apply a config fragment to many +different existing configs without the need to write a new config file each +time. You can see overlays being using in the above commands to target a +specific Arm architecture revision (v9.3 in the example). You can change the +targetted architecture just by changing the overlay. There are many other places +where overlays come in handy. See :ref:`userguide/recipes:Shrinkwrap Recipes` +for more examples. You will notice in the examples above, that only ``build`` commands include the overlay and ``run`` commands don't specify it. This is because the final config diff --git a/documentation/userguide/recipes.rst b/documentation/userguide/recipes.rst index 15dec34..c093d8e 100644 --- a/documentation/userguide/recipes.rst +++ b/documentation/userguide/recipes.rst @@ -243,9 +243,35 @@ comand line: shrinkwrap run ns-edk2.yaml --rtvar=KERNEL=path/to/Image --rtvar=CMDLINE="console=ttyAMA0 earlycon=pl011,0x1c090000 root=/dev/vda ip=dhcp acpi=force" -****************************************** -Example Linux Feature Development Use Case -****************************************** +************************************* +Pass Overlay Directly on Command Line +************************************* + +All of the previous examples that utilize overlays, put the overlay in a yaml +file and pass the file name on the command line. Here is an example that runs +the FVP as normal but saves all output from UART0 to uart0.log: + +Create a file called ``my-overlay.yaml``: + +.. code-block:: yaml + + run: + params: + -C bp.pl011_uart0.out_file: uart0.log + +Now run the FVP, passing in the overlay: + +.. code-block:: shell + + shrinkwrap run ns-edk2.yaml --rtvar=KERNEL=path/to/Image --overlay=my-overlay.yaml + +However, it is also possible to pass an overlay, encoded as json, directly on +the command line, without the need for a file. This is useful when the overlay +is small. JSON allows the entire content to be encoded on a single line without +having to care about whitespace, so is suited to this purpose. + +This is equivalent: + +.. code-block:: shell -.. todo:: - Add commentary on the config created to develop FEAT_LPA2. + shrinkwrap run ns-edk2.yaml --rtvar=KERNEL=path/to/Image --overlay='{"run":{"params":{"-C bp.pl011_uart0.out_file":"uart0.log"}}}' -- GitLab From 64a6758c87ea3356d7d5ad2c1ebc9e22ce8dce3b Mon Sep 17 00:00:00 2001 From: Ryan Roberts Date: Wed, 13 Mar 2024 15:01:29 +0000 Subject: [PATCH 4/6] logger: Add free_data() hook To use the logger, the caller must first allocate instance data using Logger.alloc_data(). Previously all of this data was trivial so had no hook to free it. However, we are about to add a file descriptor to it, so we need a hook to free the file when it goes out of scope. Signed-off-by: Ryan Roberts --- shrinkwrap/commands/run.py | 2 ++ shrinkwrap/utils/graph.py | 6 ++++-- shrinkwrap/utils/logger.py | 3 +++ 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/shrinkwrap/commands/run.py b/shrinkwrap/commands/run.py index 326f188..61aa435 100644 --- a/shrinkwrap/commands/run.py +++ b/shrinkwrap/commands/run.py @@ -211,6 +211,8 @@ def dispatch(args): pm.set_handler(log.log) def _complete(pm, proc, retcode): + log.free_data(proc.data[0]) + # If the FVP exits with non-zero exit code, we propagate that # error so that shrinkwrap also exits with non-zero exit code. if retcode not in [0, None] and proc.run_to_end: diff --git a/shrinkwrap/utils/graph.py b/shrinkwrap/utils/graph.py index 3554eb1..3ab24c2 100644 --- a/shrinkwrap/utils/graph.py +++ b/shrinkwrap/utils/graph.py @@ -165,7 +165,8 @@ def execute(graph, tasks, verbose=False, colorize=True): nonlocal active nonlocal ts - data = proc.data[1] + data = proc.data[0] + err = proc.data[1] frag = proc.data[2] tmpdir = proc.data[3] logfile = proc.data[4] @@ -173,6 +174,7 @@ def execute(graph, tasks, verbose=False, colorize=True): if logfile: logfile.close() + log.free_data(data) shutil.rmtree(tmpdir) if retcode is None: @@ -183,7 +185,7 @@ def execute(graph, tasks, verbose=False, colorize=True): if retcode: if not verbose: print('\n== error start ' + ('=' * 65)) - print(''.join(data)) + print(''.join(err)) print('== error end ' + ('=' * 67) + '\n') raise Exception(f"Failed to execute '{frag}'") diff --git a/shrinkwrap/utils/logger.py b/shrinkwrap/utils/logger.py index 233c4c2..1b3dfc9 100644 --- a/shrinkwrap/utils/logger.py +++ b/shrinkwrap/utils/logger.py @@ -75,6 +75,9 @@ class Logger: return Data(id, tag, color, [noesc], escbuf) + def free_data(self, data): + pass + def log(self, pm, proc, data, streamid): """ Logs text data from one of the processes (FVP or one of its uart -- GitLab From 9f724a3c9d812eb069d52d55fbbe9ec385de8b4a Mon Sep 17 00:00:00 2001 From: Ryan Roberts Date: Wed, 13 Mar 2024 17:04:30 +0000 Subject: [PATCH 5/6] build, clean: Refactor per-component file logging to logger Let's move the per-component file logging out of the graph executor and into the logger. This is a common location where we can reuse this functionality more easily (for logging per-FVP-terminals to file - see next commit). No behavioural changes intended. Signed-off-by: Ryan Roberts --- shrinkwrap/utils/graph.py | 44 +++++++++++++++----------------------- shrinkwrap/utils/logger.py | 25 +++++++++++++++++----- 2 files changed, 37 insertions(+), 32 deletions(-) diff --git a/shrinkwrap/utils/graph.py b/shrinkwrap/utils/graph.py index 3ab24c2..1d0e7c1 100644 --- a/shrinkwrap/utils/graph.py +++ b/shrinkwrap/utils/graph.py @@ -100,20 +100,10 @@ def _run_script(pm, data, script): with open(tmpfilename, 'w') as tmpfile: tmpfile.write(script.commands()) - if script.config and script.component: - logname = os.path.join(workspace.build, - 'log', - script.config, - f'{script.component}.log') - os.makedirs(os.path.dirname(logname), exist_ok=True) - logfile = open(logname, 'w', buffering=1) - else: - logfile = None - # Start the process asynchronously. pm.add(process.Process(f'bash {tmpfilename}', False, - (*data, script, tmpdir, logfile), + (*data, script, tmpdir), True)) @@ -132,33 +122,37 @@ def execute(graph, tasks, verbose=False, colorize=True): nonlocal log while len(queue) > 0 and active < tasks: frag = queue.pop() + logname = None + if frag.config and frag.component: + logname = os.path.join(workspace.build, + 'log', + frag.config, + f'{frag.component}.log') + os.makedirs(os.path.dirname(logname), exist_ok=True) _update_labels(labels, mask, frag.config, frag.component, frag.summary + '...') - data = (log.alloc_data(str(frag), colorize), []) + data = (log.alloc_data(str(frag), colorize, False, logname), []) _run_script(pm, data, frag) active += 1 def _should_log(proc, data, streamid): - if streamid == process.STDERR and \ + if verbose or \ + (streamid == process.STDERR and \ (not proc.data[2].stderrfilt or \ - 'warning' in data or 'error' in data): + 'warning' in data or 'error' in data)): return True return False def _log(pm, proc, data, streamid): - logfile = proc.data[4] - if logfile: - logfile.write(data) - if verbose: - log.log(pm, proc, data, streamid) - else: + logstd = _should_log(proc, data, streamid) + if not verbose: proc.data[1].append(data) - if _should_log(proc, data, streamid): - log.log(pm, proc, data, streamid) - lc.skip_overdraw_once() + log.log(pm, proc, data, streamid, logstd) + if logstd: + lc.skip_overdraw_once() def _complete(pm, proc, retcode): nonlocal queue @@ -169,10 +163,6 @@ def execute(graph, tasks, verbose=False, colorize=True): err = proc.data[1] frag = proc.data[2] tmpdir = proc.data[3] - logfile = proc.data[4] - - if logfile: - logfile.close() log.free_data(data) shutil.rmtree(tmpdir) diff --git a/shrinkwrap/utils/logger.py b/shrinkwrap/utils/logger.py index 1b3dfc9..0892760 100644 --- a/shrinkwrap/utils/logger.py +++ b/shrinkwrap/utils/logger.py @@ -15,7 +15,7 @@ def _import_termcolor(): _ansi_escape = re.compile(r'\x1B(?:[@-Z\\-_]|\[[0-?]*[ -/]*[@-~])') _colors = ['blue', 'cyan', 'green', 'yellow', 'magenta'] -Data = namedtuple("Data", "id tag color noesc escbuf") +Data = namedtuple("Data", "id tag color noesc escbuf logfile") class MatchBuf: @@ -49,7 +49,7 @@ class Logger: self._prev_id = None self._prev_char = '\n' - def alloc_data(self, tag, colorize, no_escapes=False): + def alloc_data(self, tag, colorize, no_escapes=False, logname=None): """ Returns the object that should be stashed in proc.data[0] when log() is called. Includes the tag for the process and an @@ -73,12 +73,18 @@ class Logger: noesc = no_escapes escbuf = None - return Data(id, tag, color, [noesc], escbuf) + if logname: + logfile = open(logname, 'w', buffering=1) + else: + logfile = None + + return Data(id, tag, color, [noesc], escbuf, logfile) def free_data(self, data): - pass + if data.logfile: + data.logfile.close() - def log(self, pm, proc, data, streamid): + def log(self, pm, proc, data, streamid, logstd=True): """ Logs text data from one of the processes (FVP or one of its uart terminals) to the terminal. Text is colored and a tag is added @@ -89,6 +95,15 @@ class Logger: color = proc.data[0].color noesc = proc.data[0].noesc escbuf = proc.data[0].escbuf + logfile = proc.data[0].logfile + + # Write out to file if requested. + if logfile: + logfile.write(data) + + # Write to stdout if requested. + if not logstd: + return # Remove any ansi escape sequences if requested. if noesc[0]: -- GitLab From 4343d54b73c7751e1da23806f62c8256427f320e Mon Sep 17 00:00:00 2001 From: Ryan Roberts Date: Wed, 13 Mar 2024 17:11:28 +0000 Subject: [PATCH 6/6] run: Log each FVP terminal output into its own log file Add an optional 'logfile' field for each terminal in the run.terminals yaml section. If a path is specified (and if type is stdout or stdinout), the output for the terminal is also saved to the logfile. Signed-off-by: Ryan Roberts --- documentation/userguide/configmodel.rst | 1 + shrinkwrap/commands/run.py | 17 +++++++++++++++-- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/documentation/userguide/configmodel.rst b/documentation/userguide/configmodel.rst index b50e4b1..fc16e19 100644 --- a/documentation/userguide/configmodel.rst +++ b/documentation/userguide/configmodel.rst @@ -229,6 +229,7 @@ port_regex string Regex to use to find the TCP port of the terminal when p 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. +logfile string Optional (defaults to none, only applies to ['stdout', 'stdinout'] types): Specifies path to a log file where all output to the terminal will be duplicated. =========== =========== =========== Terminal types: diff --git a/shrinkwrap/commands/run.py b/shrinkwrap/commands/run.py index 61aa435..7bc9522 100644 --- a/shrinkwrap/commands/run.py +++ b/shrinkwrap/commands/run.py @@ -138,6 +138,13 @@ def dispatch(args): else: return False + def _logfile(terminal): + if terminal['type'] in ['stdinout', 'stdout']: + logfile = terminal.get('logfile') + return logfile if logfile else None + else: + return None + def _find_term_ports(pm, proc, data, streamid): """ Initial handler function called by ProcessManager. When the fvp @@ -179,13 +186,13 @@ def dispatch(args): cmd = f'nc localhost {port}' pm.add(process.Process(cmd, False, - (log.alloc_data(name, colorize, escape), k), + (log.alloc_data(name, colorize, escape, _logfile(t)), k), False)) if type in ['stdinout']: cmd = f'telnet localhost {port}' pm.add(process.Process(cmd, True, - (log.alloc_data(name, colorize, escape), k), + (log.alloc_data(name, colorize, escape, _logfile(t)), k), False)) t['strip'] = True strip = True @@ -225,6 +232,12 @@ def dispatch(args): for rtvar in resolver['run']['rtvars'].values(): if rtvar['type'] == 'path': rt.add_volume(rtvar['value']) + for t in terminals.values(): + logfile = _logfile(t) + if logfile: + logdir = os.path.abspath(os.path.dirname(logfile)) + os.makedirs(logdir, exist_ok=True) + rt.add_volume(logdir) rt.add_volume(workspace.package) rt.start() -- GitLab