From 10461443a16d3453f6474349b2d28044c71a110b Mon Sep 17 00:00:00 2001 From: Michael Spang Date: Mon, 29 Nov 2021 23:10:59 -0500 Subject: [PATCH] Fix unexpected firmware selection by flashing scripts (#12345) Currently the flashing scripts will flash a firmware .bin from the current directory, if it exists with the right filename, and otherwise finds the firmware in the same directory as the wrapper. It is confusing that changing directories influences the firmware selection. Downloading a built firmware archive, unpacking it, and running the wrapper should always flash the firmware that was unpacked. Remove the search logic. Similarly, remove the concept of detection of "optional" firmware constituents by file presence. The command line should determine the requisite artifacts; continuing when expected files are missing is not desirable. Finally, fix rebuilds of flashing scripts after the python generator's imports change by adding those to inputs in the build file. --- build/toolchain/flashable_executable.gni | 3 ++ scripts/flashing/efr32_firmware_utils.py | 4 +-- scripts/flashing/esp32_firmware_utils.py | 15 ++++---- scripts/flashing/firmware_utils.py | 35 ++++++------------- scripts/flashing/nrfconnect_firmware_utils.py | 4 +-- scripts/flashing/p6_firmware_utils.py | 4 +-- scripts/flashing/qpg_firmware_utils.py | 4 +-- third_party/efr32_sdk/efr32_executable.gni | 9 ++--- third_party/p6/p6_executable.gni | 10 +++--- third_party/qpg_sdk/qpg_executable.gni | 10 +++--- 10 files changed, 48 insertions(+), 50 deletions(-) diff --git a/build/toolchain/flashable_executable.gni b/build/toolchain/flashable_executable.gni index afd155547a12a3..fdcb716cbaeb6e 100644 --- a/build/toolchain/flashable_executable.gni +++ b/build/toolchain/flashable_executable.gni @@ -57,6 +57,7 @@ template("gen_flashing_script") { [ "flashing_script_generator", "flashing_script_name", + "flashing_script_inputs", "flashing_options", "deps", "data_deps", @@ -72,6 +73,7 @@ template("gen_flashing_script") { ] script = flashing_script_generator + inputs = flashing_script_inputs } } @@ -131,6 +133,7 @@ template("flashable_executable") { gen_flashing_script("$target_name.flashing") { flashing_script_generator = invoker.flashing_script_generator + flashing_script_inputs = invoker.flashing_script_inputs flashing_script_name = "$root_out_dir/${invoker.flashing_script_name}" if (defined(invoker.flashing_options)) { flashing_options = invoker.flashing_options diff --git a/scripts/flashing/efr32_firmware_utils.py b/scripts/flashing/efr32_firmware_utils.py index 35812f847c3507..d04e106f894c6d 100755 --- a/scripts/flashing/efr32_firmware_utils.py +++ b/scripts/flashing/efr32_firmware_utils.py @@ -153,8 +153,8 @@ def actions(self): if self.erase().err: return self - application = self.optional_file(self.option.application) - if application: + if self.option.application: + application = self.option.application if self.flash(application).err: return self if self.option.verify_application: diff --git a/scripts/flashing/esp32_firmware_utils.py b/scripts/flashing/esp32_firmware_utils.py index bbbabb79d1d2bd..a61e93097a174e 100755 --- a/scripts/flashing/esp32_firmware_utils.py +++ b/scripts/flashing/esp32_firmware_utils.py @@ -73,6 +73,7 @@ """ import os +import pathlib import sys import firmware_utils @@ -222,7 +223,8 @@ 'help': 'Bootloader image', 'default': None, 'argparse': { - 'metavar': 'FILE' + 'metavar': 'FILE', + 'type': pathlib.Path, }, }, 'bootloader_offset': { @@ -237,7 +239,8 @@ 'help': 'Partition table image', 'default': None, 'argparse': { - 'metavar': 'FILE' + 'metavar': 'FILE', + 'type': pathlib.Path, }, }, 'partition_offset': { @@ -408,11 +411,11 @@ def actions(self): if self.erase().err: return self - bootloader = self.optional_file(self.option.bootloader) - application = self.optional_file(self.option.application) - partition = self.optional_file(self.option.partition) + if self.option.application: + application = self.option.application + bootloader = self.option.bootloader + partition = self.option.partition - if bootloader or application or partition: # Collect the flashable items. flash = [] if bootloader: diff --git a/scripts/flashing/firmware_utils.py b/scripts/flashing/firmware_utils.py index fcc312b07a4467..52391efe018c34 100644 --- a/scripts/flashing/firmware_utils.py +++ b/scripts/flashing/firmware_utils.py @@ -15,6 +15,7 @@ """Utitilies to flash or erase a device.""" import argparse +import pathlib import errno import locale import os @@ -62,7 +63,8 @@ 'help': 'Flash an image', 'default': None, 'argparse': { - 'metavar': 'FILE' + 'metavar': 'FILE', + 'type': pathlib.Path, }, }, 'verify_application': { @@ -320,8 +322,8 @@ def format_command(self, template, args=None, opt=None): ↦ ρᵢ if opt[name]==σᵢ ρ otherwise """ - if isinstance(template, str): - result = [template.format_map(opt)] + if isinstance(template, str) or isinstance(template, pathlib.Path): + result = [str(template).format_map(opt)] elif isinstance(template, list): result = [] for i in template: @@ -362,26 +364,6 @@ def format_command(self, template, args=None, opt=None): raise ValueError('Unknown: {}'.format(template)) return result - def find_file(self, filename, dirs=None): - """Resolve a file name; also checks the script directory.""" - if os.path.isabs(filename) or os.path.exists(filename): - return filename - dirs = dirs or [] - if self.argv0: - dirs.append(os.path.dirname(self.argv0)) - for directory in dirs: - name = os.path.join(directory, filename) - if os.path.exists(name): - return name - raise FileNotFoundError(errno.ENOENT, os.strerror(errno.ENOENT), - filename) - - def optional_file(self, filename, dirs=None): - """Resolve a file name, if present.""" - if filename is None: - return None - return self.find_file(filename, dirs) - def parse_argv(self, argv): """Handle command line options.""" self.argv0 = argv[0] @@ -426,10 +408,15 @@ def make_wrapper(self, argv): defaults = [] for key, value in vars(args).items(): if key in self.option and value != getattr(self.option, key): - defaults.append(' {}: {},'.format(repr(key), repr(value))) + if isinstance(value, pathlib.Path): + defaults.append(' {}: os.path.join(os.path.dirname(sys.argv[0]), {}),'.format( + repr(key), repr(str(value)))) + else: + defaults.append(' {}: {},'.format(repr(key), repr(value))) script = """ import sys + import os.path DEFAULTS = {{ {defaults} diff --git a/scripts/flashing/nrfconnect_firmware_utils.py b/scripts/flashing/nrfconnect_firmware_utils.py index ea7d68faaede25..1d8a90baac37d2 100755 --- a/scripts/flashing/nrfconnect_firmware_utils.py +++ b/scripts/flashing/nrfconnect_firmware_utils.py @@ -140,8 +140,8 @@ def actions(self): if self.erase().err: return self - application = self.optional_file(self.option.application) - if application: + if self.option.application: + application = self.option.application if self.flash(application).err: return self if self.option.verify_application: diff --git a/scripts/flashing/p6_firmware_utils.py b/scripts/flashing/p6_firmware_utils.py index 098adb77e982ec..0fe247247b4070 100755 --- a/scripts/flashing/p6_firmware_utils.py +++ b/scripts/flashing/p6_firmware_utils.py @@ -127,8 +127,8 @@ def actions(self): if self.erase().err: return self - application = self.optional_file(self.option.application) - if application: + if self.option.application: + application = self.option.application if self.flash(application).err: return self if self.option.verify_application: diff --git a/scripts/flashing/qpg_firmware_utils.py b/scripts/flashing/qpg_firmware_utils.py index c5df781c89c894..525a931c35fb80 100644 --- a/scripts/flashing/qpg_firmware_utils.py +++ b/scripts/flashing/qpg_firmware_utils.py @@ -111,8 +111,8 @@ def actions(self): if self.erase().err: return self - application = self.optional_file(self.option.application) - if application: + if self.option.application: + application = self.option.application if self.flash(application).err: return self if self.option.verify_application: diff --git a/third_party/efr32_sdk/efr32_executable.gni b/third_party/efr32_sdk/efr32_executable.gni index 62934ccc961c0c..702c9fa567f3e5 100644 --- a/third_party/efr32_sdk/efr32_executable.gni +++ b/third_party/efr32_sdk/efr32_executable.gni @@ -30,11 +30,12 @@ template("efr32_executable") { # or in different containers. flashing_runtime_target = target_name + ".flashing_runtime" + flashing_script_inputs = [ + "${chip_root}/scripts/flashing/efr32_firmware_utils.py", + "${chip_root}/scripts/flashing/firmware_utils.py", + ] copy(flashing_runtime_target) { - sources = [ - "${chip_root}/scripts/flashing/efr32_firmware_utils.py", - "${chip_root}/scripts/flashing/firmware_utils.py", - ] + sources = flashing_script_inputs outputs = [ "${root_out_dir}/{{source_file_part}}" ] } diff --git a/third_party/p6/p6_executable.gni b/third_party/p6/p6_executable.gni index d7a824ddbe603e..bec0a3e3835a5c 100644 --- a/third_party/p6/p6_executable.gni +++ b/third_party/p6/p6_executable.gni @@ -31,11 +31,13 @@ template("p6_executable") { #or in different containers. flashing_runtime_target = target_name + ".flashing_runtime" + flashing_script_inputs = [ + "${chip_root}/scripts/flashing/firmware_utils.py", + "${chip_root}/scripts/flashing/p6_firmware_utils.py", + ] + copy(flashing_runtime_target) { - sources = [ - "${chip_root}/scripts/flashing/firmware_utils.py", - "${chip_root}/scripts/flashing/p6_firmware_utils.py", - ] + sources = flashing_script_inputs outputs = [ "${root_out_dir}/{{source_file_part}}" ] } diff --git a/third_party/qpg_sdk/qpg_executable.gni b/third_party/qpg_sdk/qpg_executable.gni index ab9b6a1461065a..068c857a6f9f16 100644 --- a/third_party/qpg_sdk/qpg_executable.gni +++ b/third_party/qpg_sdk/qpg_executable.gni @@ -30,11 +30,13 @@ template("qpg_executable") { # or in different containers. flashing_runtime_target = target_name + ".flashing_runtime" + flashing_script_inputs = [ + "${chip_root}/scripts/flashing/firmware_utils.py", + "${chip_root}/scripts/flashing/qpg_firmware_utils.py", + ] + copy(flashing_runtime_target) { - sources = [ - "${chip_root}/scripts/flashing/firmware_utils.py", - "${chip_root}/scripts/flashing/qpg_firmware_utils.py", - ] + sources = flashing_script_inputs outputs = [ "${root_out_dir}/{{source_file_part}}" ] }