How should we deal with actual hush odd behavior?
Hi.
I hope you are fine and the same for your family and friends.
In July, a proposal to add a new shell for U-Boot was posted on the mailing list [1]. The community discussed a lot about this changes, some people did not agree with it because the new shell is not compatible with the actual one (hush) [2]. So, a proposal to update U-Boot actual hush to follow what they currently have in Busybox was made [3].
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.
Nonetheless, the idea of this port is to be compatible. 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.
An other example concerns variable expansion, where foo='bar "quux" is expanded to bar quux in U-Boot and bar "quux in Busybox.
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.
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... I would like to have the opinion of the community on this question.
Best regards.
--- [1] https://lists.denx.de/pipermail/u-boot/2021-July/453347.html [2] https://lists.denx.de/pipermail/u-boot/2021-July/453790.html [3] https://lists.denx.de/pipermail/u-boot/2021-July/453848.html [4] https://linux.die.net/man/1/bash
Hi Francis,
On Fri, 20 Aug 2021 at 10:12, Francis Laniel francis.laniel@amarulasolutions.com wrote:
Hi.
I hope you are fine and the same for your family and friends.
In July, a proposal to add a new shell for U-Boot was posted on the mailing list [1]. The community discussed a lot about this changes, some people did not agree with it because the new shell is not compatible with the actual one (hush) [2]. So, a proposal to update U-Boot actual hush to follow what they currently have in Busybox was made [3].
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.
Nonetheless, the idea of this port is to be compatible. 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.
An other example concerns variable expansion, where foo='bar "quux" is expanded to bar quux in U-Boot and bar "quux in Busybox.
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.
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... I would like to have the opinion of the community on this question.
My feeling is that we should go with the newer (correct?) behaviour. Boards not booting can be found with the existing release process.
Also if we keep the old hush around for a while people can still use it, particularly if it is much smaller.
Regards, SImon
On Fri, Aug 20, 2021 at 12:22:22PM -0600, Simon Glass wrote:
Hi Francis,
On Fri, 20 Aug 2021 at 10:12, Francis Laniel francis.laniel@amarulasolutions.com wrote:
Hi.
I hope you are fine and the same for your family and friends.
In July, a proposal to add a new shell for U-Boot was posted on the mailing list [1]. The community discussed a lot about this changes, some people did not agree with it because the new shell is not compatible with the actual one (hush) [2]. So, a proposal to update U-Boot actual hush to follow what they currently have in Busybox was made [3].
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.
Nonetheless, the idea of this port is to be compatible. 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.
An other example concerns variable expansion, where foo='bar "quux" is expanded to bar quux in U-Boot and bar "quux in Busybox.
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.
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... I would like to have the opinion of the community on this question.
My feeling is that we should go with the newer (correct?) behaviour. Boards not booting can be found with the existing release process.
Also if we keep the old hush around for a while people can still use it, particularly if it is much smaller.
I would at this point echo this sentiment as well. Thanks!
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.]
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.
Best regards,
Wolfgang Denk
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
participants (4)
-
Francis Laniel -
Simon Glass -
Tom Rini -
Wolfgang Denk