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

[Tom Rini]

On Thu, Apr 24, 2025 at 12:32:30PM -0600, Simon Glass wrote:
> Hi Quentin,
> 
> On Thu, 24 Apr 2025 at 02:14, Quentin Schulz <quentin.schulz at cherry.de> wrote:
> >
> > 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?
> 
> It depends on the size of the series, but if the change log is not in
> the patch then I struggle to match it to the patch and in practice I
> don't really take much notice of a change log in the cover letter, or
> perhaps of the patch in general.
> 
> But I'm not looking for people to do manual work.
> 
> My question was really whether b4 could add this feature, if people
> are starting to use it more, as I had understood that someone is now
> working full-time on Linux tooling.
> 
> The feature is in patman, which also keeps the change logs in sync
> between the cover letter and the patches. It isn't that hard to do.

You're welcome to bring this up as a feature request with the b4
maintainer, but it's not a requirement for U-Boot. But I believe what
Quentin was noting was that you could get the information you require
instead via b4 diff.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20250424/4ca80fa1/attachment.sig>