[RFC PATCH v2 00/21] Modernize U-Boot shell

[Tom Rini]

On Sun, Feb 06, 2022 at 07:36:56PM +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 at 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).
> 
> Changes since v1:
> * Renamed cli_hush_2021_upstream.c to cli_hush_upstream.c.
> * Addressed reviews regarding variable expansion unit test.

With some quick hacks to silence warnings and make this the default
parser, the first thing I see is that these tests on sandbox reliably
fail now:
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...

And while I'm not entirely sure about ut_hush_hush_test_simple_dollar as
that seems like it might be an intentional parsing change, the other two
are likely problems to resolve in the port?  The other thing I see is
that sandbox has substantial size growth, like something else is doing
on?  It's 17KiB whereas most non-LTO platform are a bit more than 4KiB
and LTO platforms are a bit more than 2KiB.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20220218/8e11ccbd/attachment.sig>