Merge branch 'jk/test-send-sh-x-trace-elsewhere'

Running tests with '-x' option to trace the individual command
executions is a useful way to debug test scripts, but some tests
that capture the standard error stream and check what the command
said can be broken with the trace output mixed in.  When running
our tests under "bash", however, we can redirect the trace output
to another file descriptor to keep the standard error of programs
being tested intact.

* jk/test-send-sh-x-trace-elsewhere:
test-lib: set BASH_XTRACEFD automatically

diff --combined t/README
index 1dc908e43a993502bed611023448aa76fc9da6ad,009e339dbcd5f4826246f08ce00f0cf528c6ec13..76a0daa3ac00d298bc515c655f4548403095f5f3
--- 1/t/README
--- 2/t/README
+++ b/t/README
@@@ -84,9 -84,9 +84,9 @@@ appropriately before running "make"
  
  -x::
        Turn on shell tracing (i.e., `set -x`) during the tests
-       themselves. Implies `--verbose`. Note that this can cause
-       failures in some tests which redirect and test the
-       output of shell functions. Use with caution.
+       themselves. Implies `--verbose`. Note that in non-bash shells,
+       this can cause failures in some tests which redirect and test
+       the output of shell functions. Use with caution.
  
  -d::
  --debug::
@@@ -563,11 -563,6 +563,11 @@@ library for your script to use
     argument.  This is primarily meant for use during the
     development of a new test script.
  
 + - debug <git-command>
 +
 +   Run a git command inside a debugger. This is primarily meant for
 +   use when debugging a failing test script.
 +
   - test_done
  
     Your test script must have test_done at the end.  Its purpose

diff --combined t/test-lib.sh
index 286c5f33d1e7bac81045293f78d2adf69ce8378c,e99a5b64efa5a9bad7b62055579046cd34679d38..0055ebba46d539f30b9484335501571c3b4879bd
--- 1/t/test-lib.sh
--- 2/t/test-lib.sh
+++ b/t/test-lib.sh
@@@ -202,13 -202,13 +202,13 @@@ d
                }
                run_list=$1; shift ;;
        --run=*)
 -              run_list=$(expr "z$1" : 'z[^=]*=\(.*\)'); shift ;;
 +              run_list=${1#--*=}; shift ;;
        -h|--h|--he|--hel|--help)
                help=t; shift ;;
        -v|--v|--ve|--ver|--verb|--verbo|--verbos|--verbose)
                verbose=t; shift ;;
        --verbose-only=*)
 -              verbose_only=$(expr "z$1" : 'z[^=]*=\(.*\)')
 +              verbose_only=${1#--*=}
                shift ;;
        -q|--q|--qu|--qui|--quie|--quiet)
                # Ignore --quiet under a TAP::Harness. Saying how many tests
@@@ -222,15 -222,15 +222,15 @@@
                valgrind=memcheck
                shift ;;
        --valgrind=*)
 -              valgrind=$(expr "z$1" : 'z[^=]*=\(.*\)')
 +              valgrind=${1#--*=}
                shift ;;
        --valgrind-only=*)
 -              valgrind_only=$(expr "z$1" : 'z[^=]*=\(.*\)')
 +              valgrind_only=${1#--*=}
                shift ;;
        --tee)
                shift ;; # was handled already
        --root=*)
 -              root=$(expr "z$1" : 'z[^=]*=\(.*\)')
 +              root=${1#--*=}
                shift ;;
        --chain-lint)
                GIT_TEST_CHAIN_LINT=1
@@@ -322,6 -322,19 +322,19 @@@ els
        exec 4>/dev/null 3>/dev/null
  fi
  
+ # Send any "-x" output directly to stderr to avoid polluting tests
+ # which capture stderr. We can do this unconditionally since it
+ # has no effect if tracing isn't turned on.
+ #
+ # Note that this sets up the trace fd as soon as we assign the variable, so it
+ # must come after the creation of descriptor 4 above. Likewise, we must never
+ # unset this, as it has the side effect of closing descriptor 4, which we
+ # use to show verbose tests to the user.
+ #
+ # Note also that we don't need or want to export it. The tracing is local to
+ # this shell, and we would not want to influence any shells we exec.
+ BASH_XTRACEFD=4
+ 
  test_failure=0
  test_count=0
  test_fixed=0
@@@ -531,10 -544,6 +544,10 @@@ maybe_setup_valgrind () 
        fi
  }
  
 +want_trace () {
 +      test "$trace" = t && test "$verbose" = t
 +}
 +
  # This is a separate function because some tests use
  # "return" to end a test_expect_success block early
  # (and we want to make sure we run any cleanup like
@@@ -542,7 -551,7 +555,7 @@@
  test_eval_inner_ () {
        # Do not add anything extra (including LF) after '$*'
        eval "
 -              test \"$trace\" = t && set -x
 +              want_trace && set -x
                $*"
  }
  
@@@ -558,7 -567,7 +571,7 @@@ test_eval_ () 
        {
                test_eval_inner_ "$@" </dev/null >&3 2>&4
                test_eval_ret_=$?
 -              if test "$trace" = t
 +              if want_trace
                then
                        set +x
                        if test "$test_eval_ret_" != 0
@@@ -574,18 -583,13 +587,18 @@@ test_run_ () 
        test_cleanup=:
        expecting_failure=$2
  
 -      if test "${GIT_TEST_CHAIN_LINT:-0}" != 0; then
 +      if test "${GIT_TEST_CHAIN_LINT:-1}" != 0; then
 +              # turn off tracing for this test-eval, as it simply creates
 +              # confusing noise in the "-x" output
 +              trace_tmp=$trace
 +              trace=
                # 117 is magic because it is unlikely to match the exit
                # code of other programs
                test_eval_ "(exit 117) && $1"
                if test "$?" != 117; then
                        error "bug in the test script: broken &&-chain: $1"
                fi
 +              trace=$trace_tmp
        fi
  
        setup_malloc_check
@@@ -854,10 -858,10 +867,10 @@@ test -d "$GIT_BUILD_DIR"/templates/blt 
        error "You haven't built things yet, have you?"
  }
  
 -if ! test -x "$GIT_BUILD_DIR"/test-chmtime
 +if ! test -x "$GIT_BUILD_DIR"/t/helper/test-chmtime
  then
        echo >&2 'You need to build test-chmtime:'
 -      echo >&2 'Run "make test-chmtime" in the source (toplevel) directory'
 +      echo >&2 'Run "make t/helper/test-chmtime" in the source (toplevel) directory'
        exit 1
  fi
  
@@@ -907,11 -911,9 +920,11 @@@ yes () 
                y="$*"
        fi
  
 -      while echo "$y"
 +      i=0
 +      while test $i -lt 99
        do
 -              :
 +              echo "$y"
 +              i=$(($i+1))
        done
  }
  
@@@ -1000,7 -1002,7 +1013,7 @@@ test_i18ngrep () 
  test_lazy_prereq PIPE '
        # test whether the filesystem supports FIFOs
        case $(uname -s) in
 -      CYGWIN*)
 +      CYGWIN*|MINGW*)
                false
                ;;
        *)
@@@ -1056,28 -1058,20 +1069,28 @@@ test_lazy_prereq NOT_ROOT 
        test "$uid" != 0
  '
  
 -# On a filesystem that lacks SANITY, a file can be deleted even if
 -# the containing directory doesn't have write permissions, or a file
 -# can be accessed even if the containing directory doesn't have read
 -# or execute permissions, causing our tests that validate that Git
 -# works sensibly in such situations.
 +# SANITY is about "can you correctly predict what the filesystem would
 +# do by only looking at the permission bits of the files and
 +# directories?"  A typical example of !SANITY is running the test
 +# suite as root, where a test may expect "chmod -r file && cat file"
 +# to fail because file is supposed to be unreadable after a successful
 +# chmod.  In an environment (i.e. combination of what filesystem is
 +# being used and who is running the tests) that lacks SANITY, you may
 +# be able to delete or create a file when the containing directory
 +# doesn't have write permissions, or access a file even if the
 +# containing directory doesn't have read or execute permissions.
 +
  test_lazy_prereq SANITY '
        mkdir SANETESTD.1 SANETESTD.2 &&
  
        chmod +w SANETESTD.1 SANETESTD.2 &&
        >SANETESTD.1/x 2>SANETESTD.2/x &&
        chmod -w SANETESTD.1 &&
 +      chmod -r SANETESTD.1/x &&
        chmod -rx SANETESTD.2 ||
        error "bug in test sript: cannot prepare SANETESTD"
  
 +      ! test -r SANETESTD.1/x &&
        ! rm SANETESTD.1/x && ! test -f SANETESTD.2/x
        status=$?