Re: How should we deal with actual hush odd behavior?

[Francis Laniel <francis.laniel@amarulasolutions.com>]

Hi.


Le lundi 23 août 2021, 13:20:10 CEST Wolfgang Denk a écrit :
> Dear Francis,
> 
> In message <2787647.e9J7NaK4W3@pwmachine> you wrote:
> > Porting 2021 Busybox hush to U-Boot seems, for me, to be a good idea as we
> > would benefit from Busybox bug fixes as well as being compatible with
> > actual hush (in theory).
> > We could also add new features to U-Boot hush, like functions, as they
> > were
> > added to Busybox.
> 
> Thanks a lot, much appreciated.
> 
> > Nonetheless, the idea of this port is to be compatible.
> 
> My recommendation is not to try to be bug-compatible, i. e. is the
> current U-Boot version of hush behaves different than the recent
> one, we should check how other POSIX compatible shells behave.
> I would expect that in most cases the old hush in U-Boot has a bug,
> which has been fixed in the recent version.
> 
> In such a situation, we should accept the bug fix and not try tokeep
> the old, buggy behaviour of the old version.
> 
> > In practice, I noted some cases when this is actually not the case.
> > The first one can be related to how && and || operators were handled in
> > hush. So, the following: false && false || true
> > Returns 0 on Busybox 2021 hush and 1 on U-Boot.
> > 
> > The behavior of 2021 is coherent with the definition of these operators 
[4]:
> > >    The return status of AND and OR lists is the exit
> > >    status of the last command executed in the list.
> 
> This is a clear bug in the old version.  It's good to see it fixed
> in the recent code.
> 
> > An other example concerns variable expansion, where foo='bar "quux" is
> > expanded to bar quux in U-Boot and bar "quux in Busybox.
> 
> I guess you mean   foo='bar "quux'   ?  [ foo='bar "quux"  is
> missing the closing apostrophe.]

Sorry, I indeed missed the closing apostrophe.

> 
> Again, this is a bug in U-Boots command line parsing.
> 
> > I do not have a real opinion on the second one, as I think local variable
> > set in U-Boot scripts are quite simple as people do not try to do:
> > foo="bar \"quux 'quuz' \"\"\"corge".
> > The first one is maybe more problematic.
> > Grepping "if test" shows me that the more complex if condition seems to be
> > under the form:
> > if first_test_ AND/OR second_test
> > Here also, people seems to no try to write complex expression like: foo ||
> > bar; echo quux && quuz.
> 
> We should port the recent version of hush without hesitating about
> bug compatibility of expected use cases.  We can'tknoww if peple
> didn't use afeature because the ran into problems with it, or
> because it's just not their programming style.
> 
> > So, porting Busybox 2021 hush can solve bugs we have currently in U-Boot,
> > but what if fixing these bugs lead to a board script failing and so a
> > device not booting...
> 
> The new version should be optional in any case, at least for some
> longer migration period to give users time to test their scripts.
> If someone selects the new version in his board configuration, he
> probably has run enough tests to make sure his scripts are working
> fine in the new environment.

I added a choice in Kconfig to permits user to select the parser they want to 
use (either the old or the new one).
By default, the old one is selected.

More generally, I see it seems to be a consensus over going with the new 2021 
behavior (which contains bug fixes compared to old one) rather than sticking 
with the old (and sometime buggy) behavior.

Thank you a lot for all your comments!

> 
> Best regards,
> 
> Wolfgang Denk