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:
- Variable expansion.
- Instruction lists (;, && and ||).
- If, then and else.
- 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).
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.