Re: [RFC PATCH v5 09/24] cli: Add menu for hush parser

[Tom Rini <trini@konsulko.com>]

[-- Attachment #1: Type: text/plain, Size: 2577 bytes --]

On Mon, Nov 07, 2022 at 08:28:42AM -0700, Simon Glass wrote:
> Hi Patrick,
> 
> On Mon, 7 Nov 2022 at 05:32, Patrick DELAUNAY
> <patrick.delaunay@foss.st.com> wrote:
> >
> > Hi,
> >
> > On 11/1/22 20:20, Francis Laniel wrote:
> > > For the moment, the menu contains only entry: HUSH_OLD_PARSER which is the
> > > default.
> > > The goal is to prepare the field to add a new hush parser which guarantees
> > > actual behavior is still correct.
> > >
> > > Signed-off-by: Francis Laniel <francis.laniel@amarulasolutions.com>
> > > ---
> > >   cmd/Kconfig     | 21 +++++++++++++++++++++
> > >   common/Makefile |  3 ++-
> > >   2 files changed, 23 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/cmd/Kconfig b/cmd/Kconfig
> > > index 3f6bc70d43..c15d7c51f7 100644
> > > --- a/cmd/Kconfig
> > > +++ b/cmd/Kconfig
> > > @@ -23,6 +23,27 @@ config HUSH_PARSER
> > >         If disabled, you get the old, much simpler behaviour with a somewhat
> > >         smaller memory footprint.
> > >
> > > +menu "Hush flavor to use"
> > > +     depends on HUSH_PARSER
> > > +
> > > +     config HUSH_OLD_PARSER
> > > +             bool "Use hush old parser"
> > > +             default y
> > > +             help
> > > +               This option enables the old flavor of hush based on hush Busybox from
> > > +               2005.
> > > +
> > > +               It is actually the default U-Boot shell when decided to use hush as shell.
> > > +
> > > +     config HUSH_2021_PARSER
> > > +             bool "Use hush 2021 parser"
> > > +             help
> > > +               This option enables the new flavor of hush based on hush Busybox from
> > > +               2021.
> > > +
> > > +               For the moment, it is highly experimental and should be used at own risks.
> > > +endmenu
> > > +
> >
> >
> > I think "choice" can be made sense here
> >
> > => only one version is used
> >
> >
> > choice
> >      prompt "Hush flavor to use"
> >      default HUSH_OLD_PARSER
> >
> >      depends on HUSH_PARSER
> >
> >
> > config HUSH_OLD_PARSER
> >
> >      bool  "Use hush old parser"
> >
> > config HUSH_2021_PARSER
> >
> >    bool "Use hush 2021 parser"
> >
> > endchoice
> 
> We need to be able to build both and then select the correct one at
> runtime, at least for sandbox. Otherwise we would need yet another
> sandbox build. So I think what we have here makes sense.

I think choice is fine, as that's for testing. Once we're ready to merge
this we'll not keep both around for long.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]