Re: [PATCH 05/12] selftests/mm: fix invocation of tests that are run via shell scripts

From: John Hubbard
Date: Fri Jun 02 2023 - 15:19:39 EST


On 6/2/23 08:34, Peter Xu wrote:
On Thu, Jun 01, 2023 at 06:33:51PM -0700, John Hubbard wrote:
We cannot depend upon git to reliably retain the executable bit on shell
scripts, or so I was told several years ago while working on this same
run_vmtests.sh script. And sure enough, things such as test_hmm.sh are
lately failing to run, due to lacking execute permissions.

A nice clean way to fix this would have been to use TEST_PROGS instead
of TEST_FILES for the .sh scripts here. That tells the selftest
framework to run these (and emit a warning if the files are not
executable, but still run them anyway).

Unfortunately, run_vmtests.sh has its own run_test() routine, which does
*not* do the right thing for shell scripts.

Fix this by explicitly adding "bash" to each of the shell script
invocations. Leave fixing the overall approach to another day.

Is it possible someone just doesn't have "bash" at all? I used to only use

Well, maybe [1]. But that someone won't be running these tests as-is, because
the tests explicitly require bash, even before this patch.

"sh" without bash installed I think, but that was not on Linux, so I'm not
sure how much that applies..

sh invocations are for when you want to express that this script should
avoid using bash-specific things, in order to ensure portability to
other environments.

But here, the run_vmtests.sh file requires bash already, as per the
first line:

#!/bin/bash

...which is ultimately why I decided to use bash, rather than sh here.


Maybe use $(SHELL)? I saw a bunch of usage in the tree too.


That's more of a Makefile construct that you are seeing, and only in a
few odd Makefiles. Recall that in Make, $(SHELL) has the same effect
that ${SHELL} has in bash/sh, by the way: dereferencing a variable.

And Make's "$(shell ...)" command is what is normally used to *run* a
shell command, in the kernel's build system.

Having said all that, I will take a quick look at what it would take
to shift over to the selftest framework's run_test() instead, in
order to avoid this ugly "fix".


[1] https://www.bleepingcomputer.com/news/linux/kali-linux-20204-switches-the-default-shell-from-bash-to-zsh/

thanks,
--
John Hubbard
NVIDIA