Re: [PATCH v8 4/9] sandbox: Build the mkeficapsule tool for the sandbox variants

[Takahiro Akashi <takahiro.akashi@linaro.org>]

On Fri, Aug 11, 2023 at 07:54:11PM +0530, Sughosh Ganu wrote:
> hi Simon,
> 
> On Fri, 11 Aug 2023 at 19:07, Simon Glass <sjg@chromium.org> wrote:
> >
> > Hi Sughosh,
> >
> > On Fri, 11 Aug 2023 at 05:23, Sughosh Ganu <sughosh.ganu@linaro.org> wrote:
> > >
> > > hi Simon,
> > >
> > > On Thu, 10 Aug 2023 at 22:57, Simon Glass <sjg@chromium.org> wrote:
> > > >
> > > > Hi,
> > > >
> > > > On Thu, 10 Aug 2023 at 09:52, Tom Rini <trini@konsulko.com> wrote:
> > > > >
> > > > > On Thu, Aug 10, 2023 at 07:53:33PM +0530, Sughosh Ganu wrote:
> > > > >
> > > > > > Build the mkeficapsule tool for all the sandbox variants. This tool
> > > > > > will be used subsequently for testing capsule generation in binman.
> > > > > >
> > > > > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> > > > > > ---
> > > > > > Changes since V7: None
> > > > > >
> > > > > >  tools/Kconfig | 6 +++---
> > > > > >  1 file changed, 3 insertions(+), 3 deletions(-)
> > > > > >
> > > > > > diff --git a/tools/Kconfig b/tools/Kconfig
> > > > > > index 6e23f44d55..353a855243 100644
> > > > > > --- a/tools/Kconfig
> > > > > > +++ b/tools/Kconfig
> > > > > > @@ -91,10 +91,10 @@ config TOOLS_SHA512
> > > > > >         Enable SHA512 support in the tools builds
> > > > > >
> > > > > >  config TOOLS_MKEFICAPSULE
> > > > > > -     bool "Build efimkcapsule command"
> > > > > > -     default y if EFI_CAPSULE_ON_DISK
> > > > > > +     bool "Build mkeficapsule tool"
> > > > > > +     default y if EFI_CAPSULE_ON_DISK || SANDBOX
> > > > > >       help
> > > > > > -       This command allows users to create a UEFI capsule file and,
> > > > > > +       This tool allows users to create a UEFI capsule file and,
> > > > > >         optionally sign that file. If you want to enable UEFI capsule
> > > > > >         update feature on your target, you certainly need this.
> > > > >
> > > > > Sorry, what is this fixing exactly?
> > > >
> > > > s/command/tool/ is mixed in with this commit, but the main purpose is
> > > > to enable it on sandbox.
> > >
> > > Sorry, I did not understand this statement. The changes made here are
> > > using the same nomenclature(tool) for referring to mkeficapsule.
> > >
> > > >
> > > > The commit message really should mention both changes.
> > >
> > > Which two changes? The commit message states what the commit is doing,
> > > and then states the reason for the change. What more information is
> > > needed in the commit message?
> >
> > The two changes are:
> >
> > 1. The one the commit message mentions
> > 2. Changing 'command' to 'tool' in the Kconfig
> 
> Okay, will put in a mention for the second point as well. Thanks.

There is another use of 'command' in CONFIG_TOOLS_MKFWUDATA.
So it would be better to separate (2) from (1) and to fix
both 'command'.

-Takahiro Akashi


> 
> -sughosh