| Message ID | 20221101192029.10231-10-francis.laniel@amarulasolutions.com |
|---|---|
| State | RFC |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | Modernize U-Boot shell | expand |
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 Regards Patrick > config CMDLINE_EDITING > bool "Enable command line editing" > depends on CMDLINE > diff --git a/common/Makefile b/common/Makefile > index 20addfb244..360a155af3 100644 > --- a/common/Makefile > +++ b/common/Makefile > @@ -8,7 +8,8 @@ ifndef CONFIG_SPL_BUILD > obj-y += init/ > obj-y += main.o > obj-y += exports.o > -obj-$(CONFIG_HUSH_PARSER) += cli_hush.o > +obj-$(CONFIG_HUSH_OLD_PARSER) += cli_hush.o > +obj-$(CONFIG_HUSH_2021_PARSER) += cli_hush_2021.o > obj-$(CONFIG_AUTOBOOT) += autoboot.o > > # # boards
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. Regards, Simon
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.
Hi, On Tue, 8 Nov 2022 at 08:21, Tom Rini <trini@konsulko.com> wrote: > > 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. Oh that's good. I heard people worrying about compatibility and size of the new shell, so thought we might need both. Regards, Simon
Hi. Le mardi 8 novembre 2022, 21:15:12 CET Simon Glass a écrit : > Hi, > > On Tue, 8 Nov 2022 at 08:21, Tom Rini <trini@konsulko.com> wrote: > > 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. > > Oh that's good. I heard people worrying about compatibility and size > of the new shell, so thought we might need both. Thank all of you for your feedback! I think being able to change the shell at run time with "parser set" could also be useful for people to test if they are worried their board will not work with the new one. So, they can test a feature with the old shell, change at runtime to the new one and see if everything is correct. > Regards, > Simon Best regards.
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 + config CMDLINE_EDITING bool "Enable command line editing" depends on CMDLINE diff --git a/common/Makefile b/common/Makefile index 20addfb244..360a155af3 100644 --- a/common/Makefile +++ b/common/Makefile @@ -8,7 +8,8 @@ ifndef CONFIG_SPL_BUILD obj-y += init/ obj-y += main.o obj-y += exports.o -obj-$(CONFIG_HUSH_PARSER) += cli_hush.o +obj-$(CONFIG_HUSH_OLD_PARSER) += cli_hush.o +obj-$(CONFIG_HUSH_2021_PARSER) += cli_hush_2021.o obj-$(CONFIG_AUTOBOOT) += autoboot.o # # boards
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(-)