rapido-linux / rapido

Quickly test Linux kernel changes
GNU Lesser General Public License v2.1
22 stars 22 forks source link

reduce global variables by passing reference to _rt_require_* helpers #205

Closed ddiss closed 1 year ago

ddiss commented 1 year ago

A common cut script pattern that we currently have is something like:

_rt_require_foo
_rt_require_bar
...
$DRACUT --install "... $FOO_BINS $BAR_BINS"

With each new component tested via cut / autorun, we tend to gather new _rt_require_X() helpers, and corresponding global variables. To avoid these global variables getting too out of control, I think it makes sense to use Bash pass-by-reference functionality instead, i.e.:

req_bins=()
_rt_require_foo req_bins
_rt_require_bar req_bins
...
$DRACUT --install "... ${req_bins[*]}"

The corresponding helper functions then look something like:

_rt_require_foo() {
        declare -n req_paths_ref="$1"

        req_paths_ref+=( $(type -P mkfs.foo fooer) ) \
                || _fail "path $p doesn't provide foo binaries"
}

I think It's much cleaner, as it's clearer by looking at the caller where the binary paths will end up and it reduces the number of variables needed - the req_bins pass-by-ref variable above is appended with foo and bar paths.

We're already using Bash pass-by-reference functionality elsewhere, so it shouldn't change our (recent) version dependency.

ddiss commented 1 year ago

One thing I forgot to mention - converting existing _rt_require_X helpers to use this new pass-by-reference pattern would mean breaking out-of-tree testcases, so we should at the very least print a helpful message if the parameter is missing.

ddiss commented 1 year ago

Merged