[PATCH v2 2/2] binman: etype: u_boot_spl_pubkey_dtb: provide more explicit error for key-name-hint with path

[Quentin Schulz]

Hi Simon,

On 4/23/25 2:27 PM, Simon Glass wrote:
> Hi Quentin,
> 
> On Tue, 22 Apr 2025 at 02:53, Quentin Schulz <quentin.schulz at cherry.de> wrote:
>>
>> Hi Simon,
>>
>> On 4/18/25 7:19 PM, Simon Glass wrote:
>>> On Fri, 18 Apr 2025 at 05:26, Quentin Schulz <foss+uboot at 0leil.net> wrote:
>>>>
>>>> From: Quentin Schulz <quentin.schulz at cherry.de>
>>>>
>>>> key-name-hint property in u-boot-spl-pubkey-dtb binman entry may contain
>>>> a path instead of a filename due to user mistake.
>>>>
>>>> Because we currently assume it is a filename instead of a path, binman
>>>> will find the full path to the key based on that path, and return the
>>>> dirname of the full path but keeps the path in key-name-hint instead of
>>>> stripping the directories from it.
>>>>
>>>> This means mkimage will fail with the following error message if we have
>>>> key-name-hint set to keys/dev:
>>>>
>>>> binman: Error 1 running 'fdt_add_pubkey -a sha256,rsa2048 -k /home/qschulz/work/upstream/u-boot/keys -n keys/dev -r conf /home/qschulz/work/upstream/u-boot/build/ringneck/u-boot-spl-dtbdhsfx3mf': Couldn't open RSA certificate: '/home/qschulz/work/upstream/u-boot/keys/keys/dev.crt': No such file or directory
>>>>
>>>> Let's make it a bit more obvious what the error is by erroring out in
>>>> binman if a path is provided in key-name-hint (it is named key-name-hint
>>>> and not key-path-hint after all).
>>>>
>>>> Fixes: 5609843b57a4 ("binman: etype: Add u-boot-spl-pubkey-dtb etype")
>>>> Signed-off-by: Quentin Schulz <quentin.schulz at cherry.de>
>>>> ---
>>>>    tools/binman/etype/u_boot_spl_pubkey_dtb.py              |  2 ++
>>>>    tools/binman/ftest.py                                    |  7 +++++++
>>>>    .../binman/test/348_key_name_hint_dir_spl_pubkey_dtb.dts | 16 ++++++++++++++++
>>>>    3 files changed, 25 insertions(+)
>>>>
>>>
>>> Reviewed-by: Simon Glass <sjg at chromium.org>
>>>
>>> The change log seems to be missing?
>>
>> The change log?
>>
>> Between v1 and v2? It's in the cover letter of the series, this is how
>> b4 handles it, not per-patch change log.
> 
> Oh dear, that's harder to work with. I believe someone is working on
> b4 so perhaps that feature could be added?
> 

https://lore.kernel.org/tools/CAOiHx=m8tPANa+5A2dohEdkz48aPgH1g7ZXvF4uLmEu+SCo6DA@mail.gmail.com/t/#u

seems to indicate it's possible to do this manually within the editor 
when doing a git commit, by adding stuff after a --- line. I checked, 
seems to be working fine indeed?

The cover letter will still have a new changes entry, so I guess this 
will somehow duplicate the work, with having to list the changelog in 
two different places and making sure they keep in sync somehow?

Note that you can also do a

b4 diff --compare-versions 1 2

which will do a range-diff of both versions and you'll see differences 
for yourself. Note that the same limitations as for the typical 
range-diff apply, namely that if the changes are too big, it won't 
attempt at comparing the commit diffs, during rebases the git context 
may pollute a bit the output.

This seems like it's bothering you, but you're the first to complain 
since I've started to use b4 a while ago. I don't mind doing it, but it 
would be nice to know for which subsystems/projects I need to do this 
(and hopefully remember doing it :) ). Should we have some documentation 
for that maybe? And not specific to a specific tool used for contributing?

Cheers,
Quentin