[PATCH v2 0/4] Replace use of `test <expr> -o/a <expr>`

Hi,

this is the second version of my patch series that replaces all uses of `test <expr> -o/a <expr`.

Changes compared to v1:

    - I've expanded a bit on why we want to do these conversions in the
      first place in the first commit message.

    - Dropped a needless subshell and added missing quoting while at it.

    - Explained why we need to decompose the asserts in the second patch
      into two asserts.

Thanks!

Patrick

Patrick Steinhardt (4):
  global: convert trivial usages of `test <expr> -a/-o <expr>`
  contrib/subtree: stop using `-o` to test for number of args
  contrib/subtree: convert subtree type check to use case statement
  Makefile: stop using `test -o` when unlinking duplicate executables

 GIT-VERSION-GEN                |  2 +-
 Makefile                       |  2 +-
 configure.ac                   |  2 +-
 contrib/subtree/git-subtree.sh | 34 +++++++++++++++++++++++-----------
 t/perf/perf-lib.sh             |  2 +-
 t/perf/run                     |  9 +++++----
 t/valgrind/valgrind.sh         |  2 +-
 7 files changed, 33 insertions(+), 20 deletions(-)

Range-diff against v1:
1:  c5e627eb3fe ! 1:  2967c8ebb46 global: convert trivial usages of `test <expr> -a/-o <expr>`
    @@ Commit message
         these to instead instead concatenate multiple invocations of `test` via
         `&&` and `||`, respectively.
     
    +    While not all of the converted instances can cause ambiguity, it is
    +    worth getting rid of all of them regardless:
    +
    +        - It becomes easier to reason about the code as we do not have to
    +          argue why one use of `-a`/`-o` is okay while another one isn't.
    +
    +        - We don't encourage people to use these expressions.
    +
         Signed-off-by: Patrick Steinhardt <ps@pks.im>
     
      ## GIT-VERSION-GEN ##
    @@ GIT-VERSION-GEN: LF='
      then
      	VN=$(cat version) || VN="$DEF_VER"
     -elif test -d ${GIT_DIR:-.git} -o -f .git &&
    -+elif ( test -d ${GIT_DIR:-.git} || test -f .git ) &&
    ++elif { test -d "${GIT_DIR:-.git}" || test -f .git; } &&
      	VN=$(git describe --match "v[0-9]*" HEAD 2>/dev/null) &&
      	case "$VN" in
      	*$LF*) (exit 1) ;;
2:  b1ea45b8a88 ! 2:  977132d2236 contrib/subtree: stop using `-o` to test for number of args
    @@ Commit message
         of arguments we assert this via a single call to `test` with `-o`, which
         is discouraged by our coding guidelines.
     
    -    Convert these cases to stop doing so.
    +    Convert these cases to stop doing so. This requires us to decompose
    +    assertions of the style `assert test $# = 2 -o $# = 3` into two calls
    +    because we have no easy way to logically chain statements passed to the
    +    assert function.
     
         Signed-off-by: Patrick Steinhardt <ps@pks.im>
     
3:  7c54d9070fa = 3:  761cde1b341 contrib/subtree: convert subtree type check to use case statement
4:  bc9489ca5b8 = 4:  5326d86888a Makefile: stop using `test -o` when unlinking duplicate executables

base-commit: dadef801b365989099a9929e995589e455c51fed