subsy/bootloader/cmake/debug_keys: fix SB_SIGNING_KEY supply #16173
Conversation
TBD Signed-off-by: Andrzej Puzdrowski <[email protected]>
github-actions
bot
added
the
changelog-entry-required
Test specificationCI/Jenkins/NRF
CI/Jenkins/integration
Detailed information of selected test modules Note: This message is automatically posted and updated by the CI |
|
@tejlmand |
| if(IS_ABSOLUTE ${CONFIG_SB_SIGNING_KEY_FILE}) | ||
| set(SIGNATURE_PRIVATE_KEY_FILE ${CONFIG_SB_SIGNING_KEY_FILE}) | ||
| set(SIGNATURE_PRIVATE_KEY_FILE ${_X_SIGNATURE_PRIVATE_KEY_FILE}) |
There was a problem hiding this comment.
you are checking if CONFIG_SB_SIGNING_KEY_FILE is absolute, but you are setting the value of SIGNATURE_PRIVATE_KEY_FILE to the value of the internal _X_SIGNATURE_PRIVATE_KEY_FILE.
| @@ -22,17 +22,17 @@ if (DEFINED ENV{SB_SIGNING_KEY_FILE} AND NOT SB_SIGNING_KEY_FILE) | |||
| if (NOT EXISTS "$ENV{SB_SIGNING_KEY_FILE}") | |||
| message(FATAL_ERROR "ENV points to non-existing PEM file '$ENV{SB_SIGNING_KEY_FILE}'") | |||
| else() | |||
| set(SIGNATURE_PRIVATE_KEY_FILE $ENV{SB_SIGNING_KEY_FILE}) | |||
| set(_X_SIGNATURE_PRIVATE_KEY_FILE $ENV{SB_SIGNING_KEY_FILE}) | |||
There was a problem hiding this comment.
variables intended for local use should be in lower case, and please remove the leading _, as we don't use that in CMake, especially because it has a special meaning for functions.
There was a problem hiding this comment.
This has not been working for several NCS releases, so therefore this functionality was removed for sysbuild here: #16154
If we consider this to be a bug that we want to fix for parent/child images, then we should support the same for sysbuild.
@nordicjm feel free to give additional comments.
Also, if this PR should be merged then a proper commit message should be written.
|
I do not think this should be fixed, it should be listed as a known issue and references to it removed from the help text. Fixing this gives a false sense of security of the feature working for users that might go back to older versions and be aware that this is a silent failure. The feature has never worked, let's just remove it. |
|
This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time. |
ref:NCSDK-28124