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

src/update_handler: support *.bin for u-boot emmc-boot image #1201

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

hthiery
Copy link
Contributor

@hthiery hthiery commented Aug 1, 2023

Since a common name for U-Boot binaries is flash.bin or u-boot.bin add support for the "*.bin" suffix for the emmc-boot type.

Since a common name for U-Boot binaries is flash.bin or u-boot.bin add
support for the "*.bin" suffix for the emmc-boot type.

Signed-off-by: Heiko Thiery <[email protected]>
Copy link
Member

@ejoerns ejoerns left a comment

Choose a reason for hiding this comment

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

Sounds reasonable to me.

@a3f
Copy link
Contributor

a3f commented Aug 1, 2023

This would resolve #1009, but @jluebbe had concerns about this :

As mentioned in #1009 (comment), we don't want to add more broad matches here. The easy fix is to name rename the images to .img in your build system. Even if we allow .bin now, that would only set a precedent for things like .raw or any other file extension that people like to use for images.

@codecov
Copy link

codecov bot commented Aug 1, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (966a86b) 80.28% compared to head (789385f) 80.28%.
Report is 4 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1201   +/-   ##
=======================================
  Coverage   80.28%   80.28%           
=======================================
  Files          58       58           
  Lines       18000    18000           
=======================================
  Hits        14451    14451           
  Misses       3549     3549           
Files Changed Coverage Δ
src/update_handler.c 64.12% <ø> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Paiusco
Copy link

Paiusco commented Aug 1, 2023

@a3f I think would also add precedent towards #1177 as well...

@ejoerns
Copy link
Member

ejoerns commented Aug 1, 2023

I would differentiate between the renaming applied by integration tools and very common original names set by the generating tools. We allow .ext4 for ext4 images since this is the common extension.
I guess arguing that U-Boot is some kind of widespread embedded bootloader is not fully wrong... thus one could argue that .bin is a very common extension for bootloaders.
But on the other hand, I do not have a very strong opinion on this.

Also, if I understand @jluebbe in #1009 correctly, he was referring to 'broad matches' like "*" for "boot-emmc" or "*.bin" for all slot types. Explicitly handling "*.bin" for "boot-emmc" does not appear to be a 'broad match' to me.

@jluebbe
Copy link
Member

jluebbe commented Aug 3, 2023

Instead of adding more and more extension matches, perhaps an explicit option in the manifest to set the file type would be better.

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.

None yet

5 participants