Jeff King <peff@xxxxxxxx> writes:
> Yeah, I am totally fine with this direction. My reservations were:
>
> 1. It was work somebody had to do. But now you've done it.
>
> 2. It's _possible_ that some script somewhere is depending on it. But
> I think the "--" in the name plus the lack of documentation means
> that it's unlikely, and that we're morally absolved if somebody's
> script does break.
That's how we burned some folks who depend on submodule--helper,
isn't it? ;-)
> The patch itself looks obviously correct to me.
>> - test_must_fail git env--helper --type=bool --default=false --exit-code MISSING >actual.out 2>actual.err &&
>> + test_must_fail test-tool env-helper --type=bool --default=false --exit-code MISSING >actual.out 2>actual.err &&
>
> Long lines like these made me wonder if it should simply be "test-tool
> env", which is shorter. We do not need "helper" to avoid polluting the
> main git-command namespace, and everything in test-tool is a helper
> anyway. But it probably doesn't matter much either way. It's not like
> that line wasn't already overly long. :)
I agree that the "-helper" looks a bit irritating, not because the
line is long, but because test-tool is by definition about helpers
used in tests, so the suffix is redundant.
Let's have a small update before queuing it.
> If we do take this, then my t/interop patch can be dropped, though we
> might want to salvage the error message bit:
Yup, that does make sense. Will queue what is below the scissors
line.
Thanks, both.
>
> -- >8 --
> Subject: [PATCH] t/interop: report which vanilla git command failed
>
> The interop test library sets up wrappers "git.a" and "git.b" to
> represent the two versions to be tested. It also wraps vanilla "git" to
> report an error, with the goal of catching tests which accidentally fail
> to use one of the version-specific wrappers (which could invalidate the
> tests in a very subtle way).
>
> But when it catches an invocation of vanilla git, it doesn't give any
> details, which makes it very hard to debug exactly which invocation is
> responsible (especially if it's buried in a function invocation, etc).
> Let's report the arguments passed to git, which helps narrow it down.
>
> Signed-off-by: Jeff King <peff@xxxxxxxx>
> ---
> t/interop/interop-lib.sh | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/t/interop/interop-lib.sh b/t/interop/interop-lib.sh
> index 3e0a2911d4..62f4481b6e 100644
> --- a/t/interop/interop-lib.sh
> +++ b/t/interop/interop-lib.sh
> @@ -68,7 +68,7 @@ generate_wrappers () {
> wrap_git .bin/git.a "$DIR_A" &&
> wrap_git .bin/git.b "$DIR_B" &&
> write_script .bin/git <<-\EOF &&
> - echo >&2 fatal: test tried to run generic git
> + echo >&2 fatal: test tried to run generic git: $*
> exit 1
> EOF
> PATH=$(pwd)/.bin:$PATH
