On Fri, Jun 18, 2021 at 08:29:57PM +0100, Guillaume Tucker wrote:
There's a bit more to it... The lkdtm tests make use of the process substitution feature with the <() syntax which is specific to Bash. The tests run by KernelCI use Debian, where /bin/sh points to /bin/dash by default which doesn't support this feature. So one way to fix it would be:
(/bin/bash -c 'cat <(echo '"$test"') >'"$TRIGGER")
Argh. I always forget that <() is a bash-ism. Thank you for tracking this down!
However, this might break others' workflows.
In fact the LAVA jobs run by KernelCI do define the $SHELL environment variable except it's defined to be /bin/sh - and that means /bin/dash gets called and we're back to the issue explained above.
I've manually run a modified test job which defines SHELL=/bin/bash and that works:
Yay!!
So to avoid hitting the same issue in other places, as it seems like there is an implicit dependency on Bash, we can just change KernelCI kselftest jobs to always export SHELL=/bin/bash.
I suppose an even better fix would be to use standard shell features that would work with any /bin/sh implementation, but this is there to kill the sub-shell rather than the main script process so I'm not entirely sure if we can easily do that differently. Maybe we can pipe the output to cat rather than the substitution syntax, e.g.:
(/bin/sh -c '(echo '"$test"') | cat >'"$TRIGGER") || true
Yeah, this is the right fix. There's no reason anything should depend on bash; I was just not thinking when I wrote this originally. :)
So I think the "safest" solution is to not change the kselftest script and export SHELL=/bin/bash in the KernelCI jobs. If the pipe approach is good enough at catching signals then it could be done on top of this patch as it's standard and should work with any /bin/sh implementation. What do you think?
If you set SHELL=/bin/bash for now, the lkdtm tests should work as they are, and once the v2 patch lands, they'll continue to work, and SHELL=/bin/bash can be removed.
Thank you so much!
-Kees