Re: [PATCH v1.1] lib-submodule-update: pass OVERWRITING_FAIL

[Junio C Hamano <gitster@pobox.com>]

Denton Liu <liu.denton@gmail.com> writes:

> We are using `test_must_fail $command`. However, $command is not
> necessarily a git command; it could be a test helper function.
>
> In an effort to stop using test_must_fail with non-git commands, instead
> of invoking `test_must_fail $command`, run
> `OVERWRITING_FAIL=test_must_fail $command` instead.

This description alone does not make much sense to me as a reader.
For "when OVERWRITING_FAIL environment variable is set to
test_must_fail, something magical happens while running $command" to
be true, $command (which may be "a git command", or "a test helper
function") must be aware of, and must be told how to react to the
OVERWRITING_FAIL environment variable.  For example,

	test_must_fail test a = b
	test_must_fail git cat-file leaf HEAD

may succeed because "test a = b" and "git cat-file leaf HEAD" fail
and test_must_fail would notice that these commands exited with
non-zero status without crashing.

But how would the same happen for these commands,

	OVERWRITING_FAIL=test_must_fail test a = b
	OVERWRITING_FAIL=test_must_fail git cat-file leaf HEAD

which is what the above paragraph tells me to write "instead of
using test_must_fail".  There is something gravely missing from the
description.  

Is it that $command is *NEVER* a 'git' command, but just a selected
few helper functions know how to honor this convention?  If that is
the case, perhaps can we teach these helper functions an *option* to
expect a failure instead of expecting a success?  These are all
speculations, because the above description is too vague as a
starting point for clear thinking.

> +			OVERWRITING_FAIL=test_must_fail $command replace_sub1_with_directory &&
> +			OVERWRITING_FAIL= &&

If we have given up the "single-shot environment export" for
compatibility reasons (which is a sound decision to follow), we
should make sure it is clear to our readers that we are not using
that shell feature.  I.e.

			export OVERWRITING_FAIL=test_must_fail &&
			$command replace_sub1_with_directory &&
			unset OVERWRITING_FAIL &&

I still do not understand how you are forcing an arbitrary $command
to honor this environment variable, though.