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