Skip to content
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.

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

Add hook for post-processing filesystem #1042

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

Conversation

lawik
Copy link
Contributor

@lawik lawik commented Jan 8, 2025

Providing the config for :nerves, :firmware and the key post_processing_script will now allow mix firmware to pass a flag to rel2fw.sh that tells it what script to run when the filesystem is finalized.

This is the hook required to do filesystem signing for dm-verity and any other similar schemes.

Added some info about it to the advanced configuration guide. Ideally expanded to be a signing guide later when that has been more fully built out.

Providing the config for :nerves, :firmware and the key
`post_processing_script` will now allow `mix firmware` to pass a flag
to rel2fw.sh that tells it what script to run when the filesystem is
finalized.

This is the hook required to do filesystem signing for dm-verity and
any other similar schemes.

Added some info about it to the advanced configuration guide. Ideally
expanded to be a signing guide later when that has been more fully
built out.
@lawik lawik requested review from fhunleth and jjcarstens January 8, 2025 14:11
@lawik
Copy link
Contributor Author

lawik commented Jan 8, 2025

Requires this: nerves-project/nerves_system_br#832

@lawik
Copy link
Contributor Author

lawik commented Jan 8, 2025

CI hit a cyclomatic complexity limit. I am not convinced the code would be better by breaking something out into a separate function. It works pretty much in line with the rest. It is certainly possible to address it but it feels like it would take some kind of odd work?

Happy for input.

@lawik
Copy link
Contributor Author

lawik commented Jan 15, 2025

Okay, found a decent way of reducing the cyclomatic complexity while retaining the clarity of what piece of config one is pulling in.

@fhunleth
Copy link
Member

Cool. I was hoping @jjcarstens would be able to get some time to review. He's been busy, so if he doesn't pop up, I'll make a pass and get it in a release.

@@ -296,4 +288,31 @@ defmodule Mix.Tasks.Firmware do
""")
end
end

defp config_arg(:rootfs_overlay, firmware_config) do
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
defp config_arg(:rootfs_overlay, firmware_config) do
defp rootfs_overlay_arg(firmware_config) do

LGTM! Though I would probably suggest descriptive purpose built functions for these args rather than a generic config_arg that has variable key arg

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I liked the single function where all the config keys were exposed to find. But since I had to reduce complexity for the cyclomat I wanted to do something that made it visually simpler but kept the keys visible. Okay to do the suggested change but it becomes a bit of a hunt to figure out what's needed or involved when diving in I think.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants