Re: [PATCH v3 1/2] difftool: add a skeleton for the upcoming builtin

Hi Peff,

On Fri, 25 Nov 2016, Jeff King wrote:

On Fri, Nov 25, 2016 at 06:41:23PM +0100, Johannes Schindelin wrote:

quoted

quoted

Ah, I didn't realize that was a requirement. If this is going to be part
of a release and real end-users are going to see it, that does make me
think the config option is the better path (than the presence of some
file), as it's our standard way of tweaking run-time behavior.

So how do you easily switch back and forth between testing the old vs the
new difftool via the test suite?

If it's for a specific tool, I'd consider teaching the test suite to run
the whole script twice: once with the flag set and once without.

That is sometimes more complicated, though, if the script creates many
sub-repos. An environment variable might be more natural. If you already
support flipping the default via config, you can probably do:

  GIT_CONFIG_PARAMETERS="'difftool.usebuiltin=true'"
  export GIT_CONFIG_PARAMETERS

Except that that does not work, of course. To figure out why, apply this
diff:

-- snip --

diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh
index 17f3008277..27159f65f3 100755
--- a/t/t7800-difftool.sh
+++ b/t/t7800-difftool.sh

@@ -10,6 +10,9 @@ Testing basic diff tool invocation
 
 . ./test-lib.sh
 
+echo "config $(git config difftool.usebuiltin)." >&2
+exit 1
+
 difftool_test_setup ()
 {
 	test_config diff.tool test-tool &&

diff --git a/t/test-lib.sh b/t/test-lib.sh
index 9980a46133..0ddeded92b 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh

@@ -86,6 +86,7 @@ EDITOR=:
 # /usr/xpg4/bin/sh and /bin/ksh to bail out.  So keep the unsets
 # deriving from the command substitution clustered with the other
 # ones.
+echo "before $(git config difftool.usebuiltin)." >&2
 unset VISUAL EMAIL LANGUAGE COLUMNS $("$PERL_PATH" -e '
 	my @env = keys %ENV;
 	my $ok = join("|", qw(

@@ -104,6 +105,7 @@ unset VISUAL EMAIL LANGUAGE COLUMNS $("$PERL_PATH" -e

'
 	my @vars = grep(/^GIT_/ && !/^GIT_($ok)/o, @env);
 	print join("\n", @vars);
 ')
+echo "after $(git config difftool.usebuiltin)." >&2
 unset XDG_CONFIG_HOME
 unset GITPERLLIB
 GIT_AUTHOR_EMAIL=author@example.com
-- snap --

and then weep at this output:

GIT_CONFIG_PARAMETERS="'difftool.usebuiltin=true'"; export GIT_CONFIG_PARAMETERS; bash t7800-difftool.sh -i -v -x
before true.
after .
Initialized empty Git repository in
/home/virtualbox/git/git-for-windows/t/trash
directory.t7800-difftool/.git/
config .
FATAL: Unexpected exit with code 1

In other words, GIT_CONFIG_PARAMETERS is *explicitly scrubbed* from the
environment when we run our tests (by the code block between the "before"
and the "after" statements in the diff above).

Ciao,
Dscho