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

From: Junio C Hamano <gitster@pobox.com>
To: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Cc: Jeff King <peff@peff.net>,
	git@vger.kernel.org, David Aguilar <davvid@gmail.com>,
	Dennis Kaarsemaker <dennis@kaarsemaker.net>
Subject: Re: [PATCH v3 1/2] difftool: add a skeleton for the upcoming builtin
Date: Mon, 28 Nov 2016 11:27:56 -0800	[thread overview]
Message-ID: <xmqqa8cjjtfn.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <alpine.DEB.2.20.1611281830040.117539@virtualbox> (Johannes Schindelin's message of "Mon, 28 Nov 2016 18:34:17 +0100 (CET)")

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> However, I have been bitten time and again by problems that occurred only
> in production, our test suite (despite taking already waaaaaay too long to
> be truly useful in my daily development) was simply not good enough.
>
> So my plan was different: to let end users opt-in to test this new beast
> thoroughly, more thoroughly than any review would.

I agree with that 100%.  

We need to ensure "fallback to known working code" escape hatch is
robust for that plan to work well, and that is why (1) I have been
more focused on getting 1/2 right, and (2) I do not think it should
be Windows-only like in your early plan, and (3) I do not think it
would be "this will merely be there for only a month or so", like
you said earlier.

> And for that, environment variables are just not an option. I need
> something that can be configured in a portable application, so that the
> main Git for Windows installation is unaffected.

I am not sure I follow here.  

Are you saying that the users who are opting into the experiment
will keep two installations, one for daily use that avoids getting
hit by the experimental code and the other that is used for testing?
How are they switching between the two?  By using different %PATH%?
I am not sure how it is different from setting an environment
$GIT_TEST_BUILTIN_DIFFTOOL.

In any case, I do not have strong preference between environment and
configuration.  If you can make 1/2 robust with configuration, that
is just as well.  My message you are responding to was merely to
suggest another possibility.

The latter two points in my four-bullet list are hopefully still
viable if you go with the configuration; it may go something like:

 - The bulk of the tests is moved into a common dot-sourced file,
   with (1) PERL prerequite stripped and (2) "git difftool" replaced
   with $git_difftool

 - Two test files do one of

    git_difftool="git difftool"
    git_difftool="git -c difftool.useBuiltin=true difftool"

   and include the dot-sourced file.  The one that does the former
   needs to give up early depending on PERL prerequisite.

perhaps.

> My original "create a file in libexec/git-core/" was simple, did the job
> reliably, and worked also for testing.

It may have been OK for quick-and-dirty hack during development, but
I do not think it was good in anything released.