On Thu, Apr 24, 2025 at 10:14:19AM +0200, Quentin Schulz 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.sch...@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+ub...@0leil.net>
> > > > wrote:
> > > > >
> > > > > From: Quentin Schulz <quentin.sch...@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.sch...@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 <s...@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+sco...@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 would be good to update the U-Boot documentation to reflect using b4 for most cases, and then how to use things like "b4 diff" as above for reviewers. I personally do not care where the changelog is, as long as it exists. -- Tom
signature.asc
Description: PGP signature
