Hi!
Le dimanche 10 décembre 2023, 00:16:31 CET Tom Rini a écrit : > On Fri, Dec 08, 2023 at 11:30:01PM +0100, Francis Laniel wrote: > > Hi. > > > > > > 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 modern 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). > > > > It is possible to change the parser at runtime using the "cli" command: > > => cli print > > old > > => cli set modern > > => cli print > > modern > > => cli set old > > The default parser is the old one. > > Note that to use both parser, you would need to set both > > CONFIG_HUSH_MODERN_PARSER and CONFIG_HUSH_OLD_PARSER. > > > > 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: > > => printenv board > > board=sandbox > > => ut hush > > Running 20 hush tests > > ... > > Failures: 0 > > => cli set modern > > => ut hush > > Running 20 hush tests > > ... > > Failures: 0 > > > > Thanks to the effort of Harald Seiler, I was successful booting a board: > > => printenv fdtfile > > fdtfile=amlogic/meson-gxl-s905x-libretech-cc.dtb > > => cli get > > old > > => boot > > ... > > root@lepotato:~# > > root@lepotato:~# reboot > > ... > > => cli set modern > > => cli get > > modern > > => printenv fdtfile > > fdtfile=amlogic/meson-gxl-s905x-libretech-cc.dtb > > => boot > > ... > > root@lepotato:~# > > > > This contribution indeed adds a lot of code and there were concern about > > its size [6, 7]. > > With regard to the amount of code added, the cli_hush_upstream.c is 13030 > > lines long but it seems a smaller subset is really used: > > gcc -D__U_BOOT__ -E common/cli_hush_upstream.c | wc -l > > 2870 > > Despite this, it is better to still have the whole upstream code for the > > sake of easing maintenance. > > With regard to memory size, I conducted some experiments for version 8 of > > this series and for a subset of arm64 boards and found the worst case to > > be 4K [8]. Tom Rini conducted more research on this and also found the > > increase to be acceptable [9]. > > > > 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. > > * commit "cmd: Add new parser command" adds a new command which permits > > selecting parser at runtime. > > I am not really satisfied with the fact it calls cli_init() and cli_loop() > > each time the parser is set, so your reviews would be welcomed. > > * Other commits focus on enabling features we need (e.g. if). > > > > Changes since: > > v2: > > * Added a small fix to compile sandbox with NO_SDL=1. > > * Added a command to change parser at runtime. > > * Added 2021 parser function to all run_command*(). > > > > v3: > > * Various bug fixes pointed by the CI. > > * Added upstream busybox hush commits until 6th February 2022. > > > > v4: > > * Various cleaning. > > * Modified python test to accept failure output when the test are > > designed to fail. > > * Bumped upstream busybox hush commits until 24h March 2022. > > > > v5: > > * Bumped upstream busybox hush commits until 30th January 2023. > > * Fix how hush interprets '<' and '>', indeed we needed to escape them > > but I removed this behavior as tests are handled by test command and > > not hush itself. This permitted to have the ut fdt to pass. > > * Fix a problem with how exit was handled. This was reported by the ut > > exit > > test. > > > > v6: > > * There was no v6 and I got mixed up with version. > > > > v7: > > * Bumped upstream busybox hush commits until 9th May 2023. > > * Renamed parser command to change parser at runtime to cli and added > > documentation. > > * Added better separation of patches. > > * Removed code about __gnu_thumb1_case_si as it was merged in another > > series. * Various cleaning. > > > > v8: > > * Bumped upstream busybox hush commits until 25th May 2023. > > > > v9: > > * Bumped upstream busybox hush commits until 2nd October 2023. > > > > v10: > > * Fixed a build error in commit adding cli command. > > * Add new CONFIG_HUSH_SELECTABLE to only build cli command if the two > > shell > > flavors are selected. > > > > v11: > > * Renamed everything containing 2021 to modern. > > * Fixed cmd Kconfig indentation. > > * Set modern parser as default for majority of boards except some. > > Thanks, this looks good overall and I'll put things in CI soon and aim > to get this in to -next and v2024.04. Thank you! In the meanwhile, I will take a look time to time to upstream busybox to keep our version of hush up to date! I will also take a look to enabling function, but I guarantee anything on this side! Best regards.