Re: Dangerous commands (was:[ANNOUNCE] fstests: for-next branch updated to v2024.02.04)

[Eric Biggers <ebiggers@kernel.org>]

On Thu, Feb 29, 2024 at 07:44:52PM +0100, David Sterba wrote:
> > I worried about this a long time ago, and tried running shellcheck on
> > the entire codebase.  Thousands of error messages about sloppy quoting
> > later I gave up.  Later I turned that into a patch in djwong-wtf that
> > runs it only on the files changed by the head commit.
> > 
> > It **didn't notice** the cleanup error!  So that wouldn't have saved me.
> > 
> > Long term we ought to rewrite fstests in any language that isn't as much
> > of a foot gun.  Or someone starts a project to set -e -u and deals with
> > the massive treewide change that's going to be.
> 
> I see from your and Dave's response that the problems are known and that
> everybody tried to fix it in some way. What I also see that it's kind of
> futile so that people carry their patches in own branches for testing.
> The amount of treewide changes is probably killing the idea or this
> would have to be coordinated or done in sprints or idk.

At the risk of being blamed for posting a "redundant" response again:

What I tend to do in new shell scripts outside xfstests is use
'set -e -u -o pipefail', and also make sure they pass shellcheck.

For example, I did that for the tests I wrote for the fscrypt command-line tool:
https://github.com/google/fscrypt/tree/master/cli-tests.  And I made the
shellcheck be enforced by GitHub Actions, so new warnings don't get introduced.

So, the point is, I'm very much in favor of 'set -e -u -o pipefail' plus
shellcheck as a modern, less error-prone way to write shell scripts.

What can realistically be done with the existing 170000 (?) lines of shell
script in xfstests is another question.  Perhaps common/ could be updated first,
and then individual tests could opt into strict mode as they get updated too.
It would be a lot of work in any case.

- Eric