Fix expansion of application to target.restrict_size #14994
Conversation
ciarmcom
added
the
release-type: patch
|
@icsys-omh, thank you for your changes. |
|
This pull request has automatically been marked as stale because it has had no recent activity. @ARMmbed/mbed-os-maintainers, @ARMmbed/mbed-os-core, please complete review of the changes to move the PR forward. Thank you for your contributions. |
|
Can you rebase your branch (icsys-omh:expand-application-to-restrict_size) ? I don't see here Github actions (travis was removed). The test should be failing as this is touching frozen tools files (or regions.py was not?). |
Supporting custom padding bytes complicates the code a lot, and appears to be unused internally. IntelHex will pad binary files as neccessary when writing, and supports a custom byte for padding. However, merge_region_list() supported a custom byte range for padding, which required a custom implementation. The reason for removing was that this functionality complicated fixing the expanding of the application to restrict_size, to fix ARMmbed#9949.
Without this, the binary for an application is not expanded to target.restrict_size. This means that if building a bootloader and an application, the application will not be placed correctly when combined with the bootloader binary, since the binary will be too small. With this change, the bootloader binary is correctly expanded, and the application is hence located at the expected address. This fixes ARMmbed#9949.
icsys-omh
force-pushed
the
expand-application-to-restrict_size
branch
from
71b8acc to
6cd9e87
Compare
|
@0xc0170 I just tried to rebase. Let me know if I didn't do it correctly. |
|
@0xc0170 What should I do to move this PR forward? |
|
@ARMmbed/mbed-os-core will need to review as tools were frozen (see https://os.mbed.com/blog/entry/Introducing-the-new-Mbed-Tools/ Step 1). |
|
This pull request has automatically been marked as stale because it has had no recent activity. @ARMmbed/mbed-os-core, please complete review of the changes to move the PR forward. Thank you for your contributions. |
|
@icsys-omh This PR unfortunately modifies files that are part of the old Mbed tools, which we've frozen bugs and all, so we can't accept this contribution. You can read more about why in this blog post.. I understand this is frustrating, as the replacement tools don't support yet bootloaders. We'd appreciate any support with adding bootloader support to the mbed-tools. Thanks! |
mergify
bot
removed
needs: review
component: tools
release-type: patch
Summary of changes
Fix expansion of application to target.restrict_size (#9949)
Without this, the binary for an application is not expanded to target.restrict_size. This means that if building a bootloader and an application, the application will not be placed correctly when combined with the bootloader binary, since the binary will be too small.
With this change, the bootloader binary is correctly expanded, and the application is hence located at the expected address, and buildling applications.
Inspired by #11043.
Impact of changes
I've also removed support for passing custom bytes to be used as padding from merge_region_list(), since it complicates this fix. This functionality appears unused internally, and I guess users are likely to use external tools if they need that kind of functionality.
Migration actions required
Documentation
None. This bring the actual functionality back to what's described in the bootloader docs.
Pull request type
Test results
Reviewers