Skip to content

Allow a 'paths-relative-to' key in the firmware config #70

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Allow a 'paths-relative-to' key in the firmware config #70

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2c74d21
snagrecover: config: Check if each fw_config is a dict instead of the…
Salamandar Jul 23, 2025
a3804c3
snagrecover: config: Allow a 'paths-relative-to' key in the firmware
Salamandar Jul 23, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 22 additions & 5 deletions src/snagrecover/config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
# along with this program; if not, write to the Free Software
# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.

import os
import yaml
from snagrecover.utils import (
cli_error,
Expand Down Expand Up @@ -76,6 +77,20 @@ def check_soc_model(soc_model: str):
return None


def patch_firmware_image_paths(fw_config: dict, this_file_path: str) -> dict:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function doesn't actually need to return anything. It just modifies the dict in-place.

paths_relative_to_conf = fw_config.pop("paths-relative-to", "CWD")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a user interface change, so it needs to be documented. You can describe this configuration option in docs/fw_binaries.md, in the introduction section.

if paths_relative_to_conf == "CWD":
path_relative_to = os.getcwd()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As mentioned in your PR message, I'd prefer we didn't modify the path at all if "CWD" is specified. I'm pretty sure it wouldn't change anything but I'd rather not take an unnecessary risk.

elif paths_relative_to_conf == "THIS_FILE":
path_relative_to = os.path.dirname(this_file_path)
else:
path_relative_to = path_relative_to_conf

for binary in fw_config.keys():
if "path" in fw_config[binary]:
fw_config[binary]["path"] = os.path.join(path_relative_to, fw_config[binary]["path"])


def init_config(args: list):
# this is the only time that config.recovery_config should be modified!
# get soc model
Expand Down Expand Up @@ -122,11 +137,13 @@ def init_config(args: list):
# get firmware configs
for path in args.firmware_file:
with open(path, "r") as file:
fw_configs = {**fw_configs, **yaml.safe_load(file)}
if not isinstance(fw_configs, dict):
cli_error(
f"firmware config passed to CLI did not evaluate to dict: {fw_configs}"
)
fw_config_file = yaml.safe_load(file)
if not isinstance(fw_config_file, dict):
cli_error(
f"firmware config passed to CLI did not evaluate to dict: {fw_configs}"
)
fw_config_file = patch_firmware_image_paths(fw_config_file, path)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I'd prefer "complete_fw_paths()"
also, as mentioned above, this function doesn't return anything.

fw_configs = {**fw_configs, **fw_config_file}
recovery_config["firmware"] = fw_configs

# store input arguments in config
Expand Down
Loading