Jonathan Nieder <jrnieder@xxxxxxxxx> writes:
> +test_expect_success 'set up ssh wrapper' '
> + cp "$GIT_BUILD_DIR/t/helper/test-fake-ssh$X" \
> + "$TRASH_DIRECTORY/ssh$X" &&
> + GIT_SSH="$TRASH_DIRECTORY/ssh$X" &&
> + export GIT_SSH &&
> + export TRASH_DIRECTORY &&
> + >"$TRASH_DIRECTORY"/ssh-output
> +'
>
> copy_ssh_wrapper_as () {
> cp "$TRASH_DIRECTORY/ssh$X" "${1%$X}$X" &&
> + test_when_finished "rm -f ${1%$X}$X" &&
> GIT_SSH="${1%$X}$X" &&
As we can clearly see in the context, this is not a new issue, but I
find the users of this helper that benefit from the "${1%$X}$X"
magic somewhat iffy.
There are callers of this helper that pass "somedir/plink" and
"somedir/plink.exe", but behind these callers that _think_ they are
testing the variant with and without the trailing ".exe", the helper
always add ".exe" (after stripping an existing one) on $X=.exe
platforms, ending up in testing the same thing twice. On platforms
with $X='', testing two different command names may have "some"
value, but I wonder if it is cleaner to use a much less magical
"$1$X" here, and skip the test with a caller that gives ".exe"
variant using a test prerequisite on $X=.exe platforms to avoid
redundant tests?
This is totally outside the scope of this series; I mention this
only because this may be a possible #leftoverbits.
Thanks.
