Re: [LTP] [PATCH v7 2/4] ci: add patchwork communication script

[Petr Vorel <pvorel@suse.cz>]

Hi Dennis,

> On 4/15/25 7:35 PM, Petr Vorel wrote:
> > Hi Andrea,
> > You may have noticed in tst_test.sh, that local variable never uses $(...).
> > It assign single value, but never call $(...). This is for a reason.
> > [...]
> > What happen? $? is assigned from result of local keyword,
> > it overwrite previous result from $(...). Note even '#!/bin/sh -e'
> > would not cause it to fail early.

> Here is the corresponding documentation:
> https://www.gnu.org/savannah-checkouts/gnu/bash/manual/bash.html#index-local

> > The return status is zero unless local is used outside a function, an
> invalid name is supplied, or name is a readonly variable

> So:
> If you are interested in return statuses then don't *mask them* with local.

> Use this pattern instead:

> local my_var
> my_var=$(my_expr)

Thanks for much better explanation than I provided + link to the doc. I'll Cc
you on my shell related patches :).

I remember this since 87a82a62ce ("lib/tst_test.sh: fix ROD_SILENT command
return status check") [1], another related is e267a022cd ("tst_test.sh: Fix $@
usage on older dash releases") [2].

Because shell has quirks (both POSIX portable syntax or bash or whatever shell
extension) we've had many fixes for shell code, e.g. 0c0076fbaf ("tst_test.sh:
Fix ROD_SILENT() quoting") [3], although some of them are non-bash specific,
e.g. 0bb01e67b3 ("shell: Fix handling default parameters in tst_mkfs()") [4]
(just to list very few).

The biggest problem is that some things aren't easily done with plain shell,
that's why we need to use C helpers in testcases/lib (some of them are just
reusing C API to avoid reimplementing code, but e.g.  testcases/lib/tst_timeout_kill.c
was really needed after few attempts to write code in bash). Also cgroup tests
aren't reliable when written in shell. Therefore we're trying to get rid of
shell API (rewritten tests into C, or make them to use testcases/lib/tst_run_shell.c,
which makes the layer really thin).

Kind regards,
Petr

[1] https://github.com/linux-test-project/ltp/commit/87a82a62ce3fcdaf174d6e4529e0324742e13684
[2] https://github.com/linux-test-project/ltp/commit/e267a022cd2deb0d5e7280570c711420927ad817
[3] https://github.com/linux-test-project/ltp/commit/0c0076fbaf6e0059b470fadff6240fc56952c218
[4] https://github.com/linux-test-project/ltp/commit/0bb01e67b3ba079f8f069a99422d783fe6da4287

> Kind regards,
> Dennis

> > (Deliberately test with bash to demonstrate local behaves oddly not even in dash
> > or 'busybox sh' but even with bash. And yes, given how many errors we caught
> > with this script and generate_arch.sh and generate_syscalls.sh due shell strange
> > syntax and behavior makes me wonder if we really want to use shell scripts for
> > anything longer than 5 lines.)

> > Kind regards,
> > Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp