scripts: runners: error on missing non-elf outputs

The RunnerConfig class stores the locations of the Zephyr output files
in various formats (elf, hex, bin). A longstanding issue with the
representation is that these might not exist if the corresponding
Kconfig options are not set. For example, if
CONFIG_BUILD_OUTPUT_BIN=n, there is no .bin file.

Change this so the type system knows these are Optional[str], not str.

Fix the runners that use non-ELF outputs so they check for the
existence of the relevant file before using it, mostly using a new
ZephyrBinaryRunner.ensure_output helper.

I'm not going to bother with checking for the ELF file itself; that's
always there as far as I can tell.

Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>

scripts/west_commands/run_common.py

@@ -346,9 +346,7 @@ def get_runner_config(build_dir, yaml_path, runners_yaml, args=None):
             # directory containing the runners.yaml file.
             return fspath(yaml_dir / from_yaml)
 
-        # FIXME these RunnerConfig values really ought to be
+        return None
-        # Optional[str], but some runners rely on them being str.
-        return ''
 
     def config(attr):
         return getattr(args, attr, None) or yaml_config.get(attr)

scripts/west_commands/runners/bossac.py

@@ -155,6 +155,7 @@ class BossacBinaryRunner(ZephyrBinaryRunner):
             self.check_call(cmd_stty)
 
     def make_bossac_cmd(self):
+        self.ensure_output('bin')
         cmd_flash = [self.bossac, '-p', self.port, '-R', '-e', '-w', '-v',
                      '-b', self.cfg.bin_file]
 

scripts/west_commands/runners/canopen_program.py

@@ -101,6 +101,7 @@ class CANopenBinaryRunner(ZephyrBinaryRunner):
 
     def flash(self, **kwargs):
         '''Download program to flash over CANopen'''
+        self.ensure_output('bin')
         self.logger.info('Using Node ID %d, program number %d',
                          self.downloader.node_id,
                          self.downloader.program_number)

scripts/west_commands/runners/core.py

@@ -233,9 +233,9 @@ class RunnerConfig(NamedTuple):
     '''
     build_dir: str              # application build directory
     board_dir: str              # board definition directory
-    elf_file: str               # zephyr.elf path
+    elf_file: Optional[str]     # zephyr.elf path, or None
-    hex_file: str               # zephyr.hex path
+    hex_file: Optional[str]     # zephyr.hex path, or None
-    bin_file: str               # zephyr.bin path
+    bin_file: Optional[str]     # zephyr.bin path, or None
     gdb: Optional[str] = None   # path to a usable gdb
     openocd: Optional[str] = None  # path to a usable openocd
     openocd_search: Optional[str] = None  # add this to openocd search path
@@ -586,3 +586,26 @@ class ZephyrBinaryRunner(abc.ABC):
             return _DebugDummyPopen()  # type: ignore
 
         return subprocess.Popen(cmd, creationflags=cflags, preexec_fn=preexec)
+
+    def ensure_output(self, output_type: str) -> None:
+        '''Ensure self.cfg has a particular output artifact.
+
+        For example, ensure_output('bin') ensures that self.cfg.bin_file
+        refers to an existing file. Errors out if it's missing or undefined.
+
+        :param output_type: string naming the output type
+        '''
+        output_file = getattr(self.cfg, f'{output_type}_file', None)
+
+        if output_file is None:
+            err = f'{output_type} file location is unknown.'
+        elif not os.path.isfile(output_file):
+            err = f'{output_file} does not exist.'
+        else:
+            return
+
+        if output_type in ('elf', 'hex', 'bin'):
+            err += f' Try enabling CONFIG_BUILD_OUTPUT_{output_type.upper()}.'
+
+        # RuntimeError avoids a stack trace saved in run_common.
+        raise RuntimeError(err)

scripts/west_commands/runners/dfu.py

@@ -106,6 +106,7 @@ class DfuUtilBinaryRunner(ZephyrBinaryRunner):
 
     def do_run(self, command, **kwargs):
         self.require(self.cmd[0])
+        self.ensure_output('bin')
 
         if not self.find_device():
             raise RuntimeError('device not found')

scripts/west_commands/runners/jlink.py

@@ -179,8 +179,7 @@ class JLinkBinaryRunner(ZephyrBinaryRunner):
 
     def flash(self, **kwargs):
         self.require(self.commander)
-        if self.bin_name is None:
+        self.ensure_output('bin')
-            raise ValueError('Cannot flash; bin_name is missing')
 
         lines = ['r'] # Reset and halt the target
 

scripts/west_commands/runners/nios2.py

@@ -4,8 +4,6 @@
 
 '''Runner for NIOS II, based on quartus-flash.py and GDB.'''
 
-import os
-
 from runners.core import ZephyrBinaryRunner, NetworkPortHelper
 
 
@@ -58,10 +56,7 @@ class Nios2BinaryRunner(ZephyrBinaryRunner):
             raise ValueError('Cannot flash; --quartus-flash not given.')
         if self.cpu_sof is None:
             raise ValueError('Cannot flash; --cpu-sof not given.')
-        if not os.path.isfile(self.hex_name):
+        self.ensure_output('hex')
-            raise ValueError('Cannot flash; hex file ({}) does not exist. '.
-                             format(self.hex_name) +
-                             'Try enabling CONFIG_BUILD_OUTPUT_HEX.')
 
         self.logger.info('Flashing file: {}'.format(self.hex_name))
         cmd = [self.quartus_py,

scripts/west_commands/runners/nrfjprog.py

@@ -5,7 +5,6 @@
 
 '''Runner for flashing with nrfjprog.'''
 
-import os
 import shlex
 import subprocess
 import sys
@@ -294,11 +293,8 @@ class NrfJprogBinaryRunner(ZephyrBinaryRunner):
     def do_run(self, command, **kwargs):
         self.require('nrfjprog')
         self.build_conf = BuildConfiguration(self.cfg.build_dir)
-        if not os.path.isfile(self.hex_):
-            raise RuntimeError(
-                f'Cannot flash; hex file ({self.hex_}) does not exist. '
-                'Try enabling CONFIG_BUILD_OUTPUT_HEX.')
 
+        self.ensure_output('hex')
         self.ensure_snr()
         self.ensure_family()
         self.check_force_uicr()

scripts/west_commands/runners/openocd.py

@@ -133,10 +133,7 @@ class OpenOcdBinaryRunner(ZephyrBinaryRunner):
             self.do_debugserver(**kwargs)
 
     def do_flash(self, **kwargs):
-        if not path.isfile(self.hex_name):
+        self.ensure_output('hex')
-            raise ValueError('Cannot flash; hex file ({}) does not exist. '.
-                             format(self.hex_name) +
-                             'Try enabling CONFIG_BUILD_OUTPUT_HEX.')
         if self.load_cmd is None:
             raise ValueError('Cannot flash; load command is missing')
         if self.verify_cmd is None:

scripts/west_commands/runners/stm32cubeprogrammer.py

@@ -196,4 +196,8 @@ class STM32CubeProgrammerBinaryRunner(ZephyrBinaryRunner):
 
         # flash image and run application
         dl_file = self.cfg.elf_file if self._use_elf else self.cfg.hex_file
+        if dl_file is None:
+            raise RuntimeError(f'cannot flash; no download file was specified')
+        elif not os.path.isfile(dl_file):
+            raise RuntimeError(f'download file {dl_file} does not exist')
         self.check_call(cmd + ["--download", dl_file, "--start"])

scripts/west_commands/runners/stm32flash.py

@@ -87,6 +87,7 @@ class Stm32flashBinaryRunner(ZephyrBinaryRunner):
 
     def do_run(self, command, **kwargs):
         self.require('stm32flash')
+        self.ensure_output('bin')
 
         bin_name = self.cfg.bin_file
         bin_size = path.getsize(bin_name)

scripts/west_commands/tests/test_bossac.py

@@ -5,6 +5,7 @@
 # SPDX-License-Identifier: Apache-2.0
 
 import argparse
+import os
 import platform
 from unittest.mock import patch, call
 
@@ -160,6 +161,11 @@ def bcfg_check_cond5(item):
 def bcfg_get_cond5(item):
     return dict(BC_DICT_COND5)[item]
 
+def os_path_isfile_patch(filename):
+    if filename == RC_KERNEL_BIN:
+        return True
+    return os.path.isfile(filename)
+
 
 @patch('runners.bossac.BossacBinaryRunner.supports',
 	return_value=False)
@@ -193,7 +199,8 @@ def test_bossac_init(cc, req, bcfg_ini, bcfg_check, bcfg_val,
 	no --offset
     """
     runner = BossacBinaryRunner(runner_config, port=TEST_BOSSAC_PORT)
-    runner.run('flash')
+    with patch('os.path.isfile', side_effect=os_path_isfile_patch):
+        runner.run('flash')
     assert cc.call_args_list == [call(x) for x in EXPECTED_COMMANDS]
 
 
@@ -233,7 +240,8 @@ def test_bossac_create(cc, req, bcfg_ini, bcfg_check, bcfg_val,
     BossacBinaryRunner.add_parser(parser)
     arg_namespace = parser.parse_args(args)
     runner = BossacBinaryRunner.create(runner_config, arg_namespace)
-    runner.run('flash')
+    with patch('os.path.isfile', side_effect=os_path_isfile_patch):
+        runner.run('flash')
     assert cc.call_args_list == [call(x) for x in EXPECTED_COMMANDS]
 
 
@@ -275,7 +283,8 @@ def test_bossac_create_with_speed(cc, req, bcfg_ini, bcfg_check, bcfg_val,
     BossacBinaryRunner.add_parser(parser)
     arg_namespace = parser.parse_args(args)
     runner = BossacBinaryRunner.create(runner_config, arg_namespace)
-    runner.run('flash')
+    with patch('os.path.isfile', side_effect=os_path_isfile_patch):
+        runner.run('flash')
     assert cc.call_args_list == [call(x) for x in EXPECTED_COMMANDS_WITH_SPEED]
 
 
@@ -319,7 +328,8 @@ def test_bossac_create_with_flash_address(cc, req, bcfg_ini, bcfg_check,
     BossacBinaryRunner.add_parser(parser)
     arg_namespace = parser.parse_args(args)
     runner = BossacBinaryRunner.create(runner_config, arg_namespace)
-    runner.run('flash')
+    with patch('os.path.isfile', side_effect=os_path_isfile_patch):
+        runner.run('flash')
     assert cc.call_args_list == [
         call(x) for x in EXPECTED_COMMANDS_WITH_FLASH_ADDRESS
     ]
@@ -360,7 +370,8 @@ def test_bossac_create_with_omit_address(cc, req, bcfg_ini, bcfg_check,
 	no --offset
     """
     runner = BossacBinaryRunner(runner_config, port=TEST_BOSSAC_PORT)
-    runner.run('flash')
+    with patch('os.path.isfile', side_effect=os_path_isfile_patch):
+        runner.run('flash')
     assert cc.call_args_list == [call(x) for x in EXPECTED_COMMANDS]
 
 
@@ -398,7 +409,8 @@ def test_bossac_create_with_arduino(cc, req, bcfg_ini, bcfg_check,
 	--offset
     """
     runner = BossacBinaryRunner(runner_config, port=TEST_BOSSAC_PORT)
-    runner.run('flash')
+    with patch('os.path.isfile', side_effect=os_path_isfile_patch):
+        runner.run('flash')
     assert cc.call_args_list == [call(x) for x in EXPECTED_COMMANDS_WITH_EXTENDED]
 
 @patch('runners.bossac.BossacBinaryRunner.supports',
@@ -435,7 +447,8 @@ def test_bossac_create_with_adafruit(cc, req, bcfg_ini, bcfg_check,
 	--offset
     """
     runner = BossacBinaryRunner(runner_config, port=TEST_BOSSAC_PORT)
-    runner.run('flash')
+    with patch('os.path.isfile', side_effect=os_path_isfile_patch):
+        runner.run('flash')
     assert cc.call_args_list == [call(x) for x in EXPECTED_COMMANDS_WITH_EXTENDED]
 
 
@@ -472,7 +485,8 @@ def test_bossac_create_with_oldsdk(cc, req, bcfg_ini, bcfg_check,
     """
     runner = BossacBinaryRunner(runner_config)
     with pytest.raises(RuntimeError) as rinfo:
-        runner.run('flash')
+        with patch('os.path.isfile', side_effect=os_path_isfile_patch):
+            runner.run('flash')
     assert str(rinfo.value) == "This version of BOSSA does not support the" \
                                " --offset flag. Please upgrade to a newer" \
                                " Zephyr SDK version >= 0.12.0."
@@ -511,7 +525,8 @@ def test_bossac_create_error_missing_dt_info(cc, req, bcfg_ini, bcfg_check,
     """
     runner = BossacBinaryRunner(runner_config)
     with pytest.raises(RuntimeError) as rinfo:
-        runner.run('flash')
+        with patch('os.path.isfile', side_effect=os_path_isfile_patch):
+            runner.run('flash')
     assert str(rinfo.value) == "The device tree zephyr,code-partition" \
                                " chosen node must be defined."
 
@@ -550,7 +565,8 @@ def test_bossac_create_error_missing_kconfig(cc, req, bcfg_ini, bcfg_check,
     """
     runner = BossacBinaryRunner(runner_config)
     with pytest.raises(RuntimeError) as rinfo:
-        runner.run('flash')
+        with patch('os.path.isfile', side_effect=os_path_isfile_patch):
+            runner.run('flash')
     assert str(rinfo.value) == \
         "There is no CONFIG_USE_DT_CODE_PARTITION Kconfig defined at " \
         + TEST_BOARD_NAME + "_defconfig file.\n This means that" \

scripts/west_commands/tests/test_dfu_util.py

@@ -4,6 +4,7 @@
 # SPDX-License-Identifier: Apache-2.0
 
 import argparse
+import os
 from unittest.mock import patch, call
 
 import pytest
@@ -54,6 +55,11 @@ def find_device_patch():
 def require_patch(program):
     assert program in [DFU_UTIL, TEST_EXE]
 
+def os_path_isfile_patch(filename):
+    if filename == RC_KERNEL_BIN:
+        return True
+    return os.path.isfile(filename)
+
 def id_fn(tc):
     return 'exe={},alt={},dfuse_config={},img={}'.format(*tc)
 
@@ -75,7 +81,8 @@ def test_dfu_util_init(cc, req, find_device, tc, runner_config):
     exe, alt, dfuse_config, img = tc
     runner = DfuUtilBinaryRunner(runner_config, TEST_PID, alt, img, exe=exe,
                                  dfuse_config=dfuse_config)
-    runner.run('flash')
+    with patch('os.path.isfile', side_effect=os_path_isfile_patch):
+        runner.run('flash')
     assert find_device.called
     assert req.called_with(exe)
     assert cc.call_args_list == [call(EXPECTED_COMMAND[tc])]
@@ -119,7 +126,8 @@ def test_dfu_util_create(cc, req, gfa, find_device, bcfg, tc, runner_config):
     DfuUtilBinaryRunner.add_parser(parser)
     arg_namespace = parser.parse_args(args)
     runner = DfuUtilBinaryRunner.create(runner_config, arg_namespace)
-    runner.run('flash')
+    with patch('os.path.isfile', side_effect=os_path_isfile_patch):
+        runner.run('flash')
 
     if dfuse:
         cfg = DfuSeConfig(address=TEST_DFUSE_ADDR, options=modifiers or '')

scripts/west_commands/tests/test_stm32flash.py

@@ -65,6 +65,11 @@ def os_path_getsize_patch(filename):
         return TEST_BIN_SIZE
     return os.path.isfile(filename)
 
+def os_path_isfile_patch(filename):
+    if filename == RC_KERNEL_BIN:
+        return True
+    return os.path.isfile(filename)
+
 @pytest.mark.parametrize('action', EXPECTED_COMMANDS)
 @patch('runners.core.ZephyrBinaryRunner.require', side_effect=require_patch)
 @patch('runners.core.ZephyrBinaryRunner.check_call')
@@ -80,7 +85,8 @@ def test_stm32flash_init(cc, req, action, runner_config):
                  serial_mode=TEST_SERIAL_MODE, reset=TEST_RESET, verify=TEST_VERIFY)
 
     with patch('os.path.getsize', side_effect=os_path_getsize_patch):
-        runner.run('flash')
+        with patch('os.path.isfile', side_effect=os_path_isfile_patch):
+            runner.run('flash')
     assert cc.call_args_list == [call(x) for x in EXPECTED_COMMANDS[action]]
 
 @pytest.mark.parametrize('action', EXPECTED_COMMANDS)
@@ -99,5 +105,6 @@ def test_stm32flash_create(cc, req, action, runner_config):
     arg_namespace = parser.parse_args(args)
     runner = Stm32flashBinaryRunner.create(runner_config, arg_namespace)
     with patch('os.path.getsize', side_effect=os_path_getsize_patch):
-        runner.run('flash')
+        with patch('os.path.isfile', side_effect=os_path_isfile_patch):
+            runner.run('flash')
     assert cc.call_args_list == [call(x) for x in EXPECTED_COMMANDS[action]]