On Fri, Dec 31, 2021 at 05:13:06PM +0100, Francis Laniel wrote: > Hi. > > First I hope you are fine and the same for your relatives. > > During 2021 summer, Sean Anderson wrote a contribution to add a new shell, based > on LIL, to U-Boot [1][2]. > While one of the goals of this contribution was to address the fact actual > U-Boot shell, which is based on Busybox hush, is old there was a discussion > about adding a new shell versus updating the actual one [3][4]. > > So, in this series, with Harald Seiler, we updated the actual U-Boot shell to > reflect what is currently in Busybox source code. > Basically, this contribution is about taking a snapshot of Busybox shell/hush.c > file (as it exists in commit 37460f5da) and adapt it to suit U-Boot needs. > > This contribution was written to be as backward-compatible as possible to avoid > breaking the existing. > So, the 2021 hush flavor offers the same as the actual, that is to say: > 1. Variable expansion. > 2. Instruction lists (;, && and ||). > 3. If, then and else. > 4. Loops (for, while and until). > No new features offered by Busybox hush were implemented (e.g. functions). > > In terms of testing, new unit tests were added to ut to ensure the new behavior > is the same as the old one and it does not add regression. > Nonetheless, if old behavior was buggy and fixed upstream, the fix is then added > to U-Boot [5]. > In sandbox, all of these tests pass smoothly: > 2021> printenv board > board=sandbox > 2021> ut hush > Running 20 hush tests > ... > Failures: 0 > Thanks to the effort of Harald Seiler, I was successful booting a board: > 2021> printenv board_rev > board_rev=iMX8MP > 2021> boot > ... > root@iMX8MPboard:~# fw_printenv board_rev > board_rev=iMX8MP > > I marked this contribution as RFC to indeed collect your opinion. > My goal is not to change suddenly actual shell to this one, we clearly need a > transition period to think about it. > I think it is better to see this contribution as a proof of concept which shows > it is possible to update the actual shell. > > If you want to review it - your review will really be appreciated - here are > some information regarding the commits: > * commits marked as "test:" deal with unit tests. > * commit "cli: Add Busybox upstream hush.c file." copies Busybox shell/hush.c > into U-Boot tree, this explain why this commit contains around 12000 additions. > * commit "cli: Port Busybox 2021 hush to U-Boot." modifies previously added file > to permit us to use this as new shell. > The really good idea of #include'ing Busybox code into a wrapper file to define > some particular functions while minimizing modifications to upstream code comes > from Harald Seiler. > * Other commits focus on enabling features we need (e.g. if). > > Francis Laniel (21): > test: Add framework to test hush behavior. > test: hush: Test hush if/else. > test/py: hush_if_test: Remove the test file. > test: hush: Test hush variable expansion. > test: hush: Test hush commands list. > test: hush: Test hush loops. > cli: Add Busybox upstream hush.c file. > cli: Port Busybox 2021 hush to U-Boot. > cli: Add choice for hush parser. > cli: Add HUSH_2021_PARSER to hush parser choice . > cli: Enables using hush 2021 parser as command line parser. > cli: hush_2021: Enable variables expansion for hush 2021. > cli: hush_2021: Add functions to be called from run_command(). > cli: Modify run_command() to add hush 2021 as parser. > test: hush: Fix instructions list tests for hush 2021. > test: hush: Fix variable expansion tests for hush 2021. > cli: hush_2021: Enable using \< and \> as string compare operators. > cli: hush_2021: Enable if keyword. > test: hush: Fix if tests for hush 2021. > cli: hush_2021: Enable loops. > test: hush: Fix loop tests for hush 2021. > > cmd/Kconfig | 22 + > common/Makefile | 3 +- > common/cli.c | 27 +- > common/cli_hush_2021.c | 309 + > common/cli_hush_2021_upstream.c | 12791 +++++++++++++++++++++++++++ > include/cli_hush.h | 8 + > include/test/hush.h | 15 + > include/test/suites.h | 1 + > test/Makefile | 3 + > test/cmd_ut.c | 6 + > test/hush/Makefile | 10 + > test/hush/cmd_ut_hush.c | 20 + > test/hush/dollar.c | 242 + > test/hush/if.c | 351 + > test/hush/list.c | 127 + > test/hush/loop.c | 84 + > test/py/tests/test_hush_if_test.py | 184 - > 17 files changed, 14015 insertions(+), 188 deletions(-) > create mode 100644 common/cli_hush_2021.c > create mode 100644 common/cli_hush_2021_upstream.c > create mode 100644 include/test/hush.h > create mode 100644 test/hush/Makefile > create mode 100644 test/hush/cmd_ut_hush.c > create mode 100644 test/hush/dollar.c > create mode 100644 test/hush/if.c > create mode 100644 test/hush/list.c > create mode 100644 test/hush/loop.c > delete mode 100644 test/py/tests/test_hush_if_test.py > > > Best regards, thank you in advance for your reviews and happy new year! Sorry for the extremely delayed response here. I'm glad you did this. The first thing I see is that there's still some fail to build problems to sort out: https://source.denx.de/u-boot/u-boot/-/commits/TEST/RFC-modernize-u-boot-shell and doing a world build locally shows a few more also fall out so please give https://u-boot.readthedocs.io/en/latest/develop/ci_testing.html a read on how to get at least an Azure run for the next iteration. When enabled by default there's a few unused function warnings (I enabled the option for all boards for a test) that also need to be addressed. Then the next step would be to get all of the current tests to pass, for me I get: FAILED test/py/tests/test_md.py::test_md_repeat - AssertionError: assert '00200040: ' in '' FAILED test/py/tests/test_ut.py::test_ut[ut_hush_hush_test_simple_dollar] - Exception: Bad p... FAILED test/py/tests/test_ut.py::test_ut[ut_lib_lib_test_hush_echo] - Exception: Bad pattern... after dropping the forced SYS_PROMPT. Again, thanks for doing this and I do look forward to the next posting. -- Tom