Re: [RFC PATCH 00/28] cli: Add a new shell

[Sean Anderson <seanga2@gmail.com>]

On 7/7/21 11:49 PM, Heiko Schocher wrote:
> Hello Sean,
> 
> On 01.07.21 08:15, Sean Anderson wrote:
>> Well, this has been sitting on my hard drive for too long without feedback
>> ("Release early, release often"), so here's the first RFC. This is not ready to
>> merge (see the "Future work" section below), but the shell is functional and at
>> least partially tested.
>>
>> The goal is to have 0 bytes gained over Hush. Currently we are around 800 bytes
>> over on sandbox.
>>
>> add/remove: 90/54 grow/shrink: 3/7 up/down: 12834/-12042 (792)
> 
> Thanks for this comparison. Here just my thoughts, but I have currently to
> low time to follow here completly, so sorry if I miss stuff already
> discussed.
> 
>  From my side of view we can add a second shell beside hush, and as you
> already mentioned it may is even possible to start LIL from hush. This
> would be cool.
> 
> But I disagree in the following points:
> 
> - I do not see why LIL should *replace* hush. 

This is the eventual goal. This series just adds an option at build time
(CONFIG_LIL) which uses LIL as the shell instead of Hush. But anyone who
writes a series like this must be convinced that it is better than what
already exists. So I write that my goal is to replace hush, even if I
think that it will take a long while.

>    This may break a lot of current boards. Also I see no need to do
>    this, as if you do complex stuff in u-boot shell it may is the
>    wrong way and you have to think about... and I see no big
>    discussions here about BUGS in our current hush implementation ...
>    may it makes sense to bring them up and fix?

I have been told not to work on the current shell; that it is too old
and there is no point in improving it. Since we will be starting afresh
anyway, and I do not think that the Hush model is particularly good, I
chose to work from a different base.

> - Is LIL really so good code, that it is done? Or is there only no community
>    which develop further?

AFAIK there is not too much of a LIL community. The code quality is
something I found out after working on it for a while. From a cursory
inspection, it was around the same as all the other shells I looked at
(no comments, etc). FWIW the current Hush shell does not really handle
errors either. For example, on (x)malloc failure, we print and hang.
This could easily be done for LIL, but it is not so nice when your
language has recursion. So I have tried to add error checking where
possible. And of course, Hush ignores missing variables as well.

> - Do we really want code, which does no error checking? Commentless code...
> 
>    you wrote:
>> - There is a serious error handling problem. Most original LIL code never
>>    checked errors. In almost every case, errors were silently ignored, even
>>    malloc failures! While I have designed new code to handle errors properly,
> 
>    Which really let me think, nobody is interested in this code ... this
>    sounds like crap... and this brings me to the next point
> 
> - Do we not have the same problem as with hush, if we change LIL code?

I think the primary problem with Hush was that we expected to backport
updates at all. I think such a core UI feature should not have so many
ifdefs in it when no backport was ever made. It is difficult to modify
Hush because not only do you have to understand how U-Boot uses some
feature, you also have to understand how Busybox uses some feature. And
Hush of course has less comments as LIL does (after all patches in this
series are applied).

>    Do you plan to upstream your changes?

I don't view it as especially important. I view LIL as a starting point
which can be modified to suit U-Boot. It may end up that each individual
component is changed in some way, but starting with a working shell has
been very helpful for development.

> So please do not understand me wrong, I see you invested much time here,
> but I see LIL not in the short term as a replacement for hush.
> 
> I would accept to add LIL as an option and we will see, how good it works.

That is what I intend to do.

> And if we have in lets say 2-3 years more boards which use LIL instead of
> hush *and* also active maintained LIL code, we may can think of getting rid
> of hush... and may have a "hush compatibility" layer...
> 
> I have no idea of current hush mainline code and how many work it will be
> to update U-Boot to a current hush version. But may this would be make more
> sense as hush is in active development. Also may to discuss with hush maintainers
> to upstream U-Boot changes... this would make in my eyes more sense.

AFAICT most of the U-Boot changes are removal of features, workarounds
for unimplemented functionality (e.g. anywhere Hush does fork/exec), and
some small additions like command repetition which cannot be upstreamed
because it would break any semblance of POSIX compatibility.

--Sean

>> = Why Lil?
>>
>> When looking for a suitable replacement shell, I evaluated implementations using
>> the following criteria:
>>
>> - It must have a GPLv2-compatible license.
>> - It must be written in C, and have no major external dependencies.
>> - It must support bare function calls. That is, a script such as 'foo bar'
>>    should invoke the function 'foo' with the argument 'bar'. This preserves the
>>    shell-like syntax we expect.
>> - It must be small. The eventual target is that it compiles to around 10KiB with
>>    -Os and -ffunction-sections.
>> - There should be good tests. Any tests at all are good, but a functioning suite
>>    is better.
>> - There should be good documentation
>> - There should be comments in the source.
>> - It should be "finished" or have only slow development. This will hopefully
>>    make it easier to port changes.
>>
>> Notably absent from the above list is performance. Most scripts in U-Boot will
>> be run once on boot. As long as the time spent evaluating scripts is kept under
>> a reasonable threshold (a fraction of the time spend initializing hardware or
>> reading data from persistant storage), there is no need to optimize for speed.
>>
>> In addition, I did not consider updating Hush from Busybox. The mismatch in
>> computing environment expectations (as noted in the "New shell" section above)
>> still applies. IMO, this mismatch is the biggest reason that things like
>> functions and command substitution have been excluded from the U-Boot's Hush.
>>
>> == lil
>>
>> - zLib
>> - TCL
>> - Compiles to around 10k with no builtins. To 25k with builtins.
>> - Some tests, but not organized into a suite with expected output. Some evidence
>>    that the author ran APL, but no harness.
>> - Some architectural documentation. Some for each functions, but not much.
>> - No comments :l
>> - 3.5k LoC
>>
>> == picol
>>
>> - 2-clause BSD
>> - TCL
>> - Compiles to around 25k with no builtins. To 80k with builtins.
>> - Tests with suite (in-language). No evidence of fuzzing.
>> - No documentation :l
>> - No comments :l
>> - 5k LoC
>>
>> == jimtcl
>>
>> - 2-clause BSD
>> - TCL
>> - Compiles to around 95k with no builtins. To 140k with builtins. Too big...
>>
>> == boron
>>
>> - LGPLv3+ (so this is right out)
>> - REBOL
>> - Compiles to around 125k with no builtins. To 190k with builtins. Too big...
>>
>> == libmawk
>>
>> - GPLv2
>> - Awk
>> - Compiles to around 225k. Too big...
>>
>> == libfawk
>>
>> - 3-clause BSD
>> - Uses bison+yacc...
>> - Awk; As it turns out, this has parentheses for function calls.
>> - Compiles to around 24-30k. Not sure how to remove builtins.
>> - Test suite (in-language). No fuzzing.
>> - Tutorial book. No function reference.
>> - No comments
>> - Around 2-4k LoC
>>
>> == MicroPython
>>
>> - MIT
>> - Python (but included for completeness)
>> - Compiles to around 300k. Too big...
>>
>> == mruby/c
>>
>> - 3-clause BSD
>> - Ruby
>> - Compiles to around 85k without builtins and 120k with. Too big...
>>
>> == eLua
>>
>> - MIT
>> - Lua
>> - Build system is a royal pain (custom and written in Lua with external deps)
>> - Base binary is around 250KiB and I don't want to deal with reducing it
>>
>> So the interesting/viable ones are
>> - lil
>> - picol
>> - libfawk (maybe)
> 
> Very nice overview, thanks!
> 
> bye,
> Heiko
>