threads / discuss / 51346

[2.22.0] difftool no longer passes through to git diff if diff.tool is unset

Subject: [2.22.0] difftool no longer passes through to git diff if diff.tool is unset

## tl;dr

8 messages between Jun 19, 2019 and Jun 26, 2019.

replies: 7people: 3as markdown or json

Pugh, Logan· Jun 19, 2019, 23:54 UTC · lore

Note: This issue was originally discussed on this StackOverflow thread: https://stackoverflow.com/q/56675863

Prior to Git version 2.22.0 I was able to use git difftool without configuring diff.tool or merge.tool and it would show the diff using git diff.

E.g. with Git 2.21.0:

~/gits/src/git$ git difftool --no-index color.c color.h

diff --git a/color.c b/color.h
index ebb222ec33..98894d6a17 100644

In Git version 2.22.0 an error message about diff.tool not being configured is displayed:

~/gits/src/git$ git difftool --no-index color.c color.h

This message is displayed because 'diff.tool' is not configured.
See 'git difftool --tool-help' or 'git help config' for more details.
'git difftool' will now attempt to use one of the following tools:
kompare emerge vimdiff

The following commit is thought to have caused the regression: https://github.com/git/git/commit/05fb8726cccc74908853c166248ff9b6abdafae5

Please let me know if I can provide any more info.

Thanks,

Logan

Denton Liu· Jun 20, 2019, 01:17 UTC · re: Pugh, Logan · lore

Re: [2.22.0] difftool no longer passes through to git diff if diff.tool is unset

Hi Logan,

On Wed, Jun 19, 2019 at 11:54:22PM +0000, Pugh, Logan wrote:

Show 26 quoted lines

> Note: This issue was originally discussed on this StackOverflow thread: https://stackoverflow.com/q/56675863
> 
> Prior to Git version 2.22.0 I was able to use git difftool without configuring diff.tool or merge.tool and it would show the diff using git diff.
> 
> E.g. with Git 2.21.0:
> 
> ~/gits/src/git$ git difftool --no-index color.c color.h
> diff --git a/color.c b/color.h
> index ebb222ec33..98894d6a17 100644
> 
> In Git version 2.22.0 an error message about diff.tool not being configured is displayed:
> 
> ~/gits/src/git$ git difftool --no-index color.c color.h
> 
> This message is displayed because 'diff.tool' is not configured.
> See 'git difftool --tool-help' or 'git help config' for more details.
> 'git difftool' will now attempt to use one of the following tools:
> kompare emerge vimdiff
> 
> The following commit is thought to have caused the regression: https://github.com/git/git/commit/05fb8726cccc74908853c166248ff9b6abdafae5
> 
> Please let me know if I can provide any more info.
> 
> Thanks,
> 
> Logan

Thanks for the report. I'll take a look at this later today.

Denton Liu· Jun 20, 2019, 02:45 UTC · re: Denton Liu · lore

Re: [2.22.0] difftool no longer passes through to git diff if diff.tool is unset

On Wed, Jun 19, 2019 at 09:17:44PM -0400, Denton Liu wrote:

Show 31 quoted lines

> Hi Logan,
> 
> On Wed, Jun 19, 2019 at 11:54:22PM +0000, Pugh, Logan wrote:
> > Note: This issue was originally discussed on this StackOverflow thread: https://stackoverflow.com/q/56675863
> > 
> > Prior to Git version 2.22.0 I was able to use git difftool without configuring diff.tool or merge.tool and it would show the diff using git diff.
> > 
> > E.g. with Git 2.21.0:
> > 
> > ~/gits/src/git$ git difftool --no-index color.c color.h
> > diff --git a/color.c b/color.h
> > index ebb222ec33..98894d6a17 100644
> > 
> > In Git version 2.22.0 an error message about diff.tool not being configured is displayed:
> > 
> > ~/gits/src/git$ git difftool --no-index color.c color.h
> > 
> > This message is displayed because 'diff.tool' is not configured.
> > See 'git difftool --tool-help' or 'git help config' for more details.
> > 'git difftool' will now attempt to use one of the following tools:
> > kompare emerge vimdiff
> > 
> > The following commit is thought to have caused the regression: https://github.com/git/git/commit/05fb8726cccc74908853c166248ff9b6abdafae5
> > 
> > Please let me know if I can provide any more info.
> > 
> > Thanks,
> > 
> > Logan
> 
> Thanks for the report. I'll take a look at this later today.

Using the following command on git.git,

	$ HOME=/dev/null ./git --exec-path=. difftool --no-index color.c color.h

I did a quick bisect on the issue and it seems like the cause of this bug is actually 287ab28bfa (diff: reuse diff setup for --no-index case, 2019-02-16). I'll do a bit more digging tomorrow.

(CCing Peff into this)

Jeff King· Jun 20, 2019, 05:21 UTC · re: Denton Liu · lore

Re: [2.22.0] difftool no longer passes through to git diff if diff.tool is unset

On Wed, Jun 19, 2019 at 10:45:17PM -0400, Denton Liu wrote:

Show 7 quoted lines

> Using the following command on git.git,
> 
> 	$ HOME=/dev/null ./git --exec-path=. difftool --no-index color.c color.h
> 
> I did a quick bisect on the issue and it seems like the cause of this
> bug is actually 287ab28bfa (diff: reuse diff setup for --no-index case,
> 2019-02-16). I'll do a bit more digging tomorrow.

I don't know much about how git-difftool works, but it looks like it sets GIT_EXTERNAL_DIFF=git-difftool--helper.

Prior to 287ab28bfa, we would not have respected any external diff command when running git-diff. But after it, we do.

In the case that he user has not provided --no-index, then this all works as I guess difftool is meant to: it runs the helper and says "hey, you have not configured this".

It seems like the behavior of the above command prior to 287ab28bfa was not intentional. It would run git-diff, expecting it to trigger the helper, but it never did (and instead just did a normal no-index diff).

So it seems like the new behavior is actually the right thing, as it makes the --no-index case consistent with the regular one? I'm not at all clear why you would run "difftool" here if you it is not configured and you just want the straight diff output.

-Peff

Pugh, Logan· Jun 20, 2019, 19:29 UTC · re: Jeff King · lore

Re: [2.22.0] difftool no longer passes through to git diff if diff.tool is unset

Show 30 quoted lines

> On Wed, Jun 19, 2019 at 10:45:17PM -0400, Denton Liu wrote:
> 
>> Using the following command on git.git,
>> 
>> 	$ HOME=/dev/null ./git --exec-path=. difftool --no-index color.c color.h
>> 
>> I did a quick bisect on the issue and it seems like the cause of this
>> bug is actually 287ab28bfa (diff: reuse diff setup for --no-index case,
>> 2019-02-16). I'll do a bit more digging tomorrow.
> 
> I don't know much about how git-difftool works, but it looks like it
> sets GIT_EXTERNAL_DIFF=git-difftool--helper.
> 
> Prior to 287ab28bfa, we would not have respected any external diff
> command when running git-diff. But after it, we do.
> 
> In the case that he user has not provided --no-index, then this all
> works as I guess difftool is meant to: it runs the helper and says "hey,
> you have not configured this".
> 
> It seems like the behavior of the above command prior to 287ab28bfa was
> not intentional. It would run git-diff, expecting it to trigger the
> helper, but it never did (and instead just did a normal no-index diff).
> 
> So it seems like the new behavior is actually the right thing, as it
> makes the --no-index case consistent with the regular one? I'm not
> at all clear why you would run "difftool" here if you it is not
> configured and you just want the straight diff output.
> 
> -Peff

Hi Peff,

Thanks for the explanation. It sounds like I was under the incorrect assumption that I could use the difftool command the same way as the diff command. Part of my confusion could be blamed on the git-difftool documentation (https://git-scm.com/docs/git-difftool) which near the top states:

 > git difftool is a frontend to git diff and accepts the same options 
and arguments. See git-diff[1].

My use case is a CLI program I've written that processes and then compares two arbitrary files using the git difftool apparatus as configured by the end user, leaving the choice to them whether to use the internal diff tool or an external tool.

Now, if I'm understanding correctly, I should not rely on the behavior of git difftool --no-index passing through to git diff. I could add another CLI switch and code path to my program that calls git diff directly instead of git difftool but the passthrough behavior seemed more elegant at the time.

Ideally, in my mind, git difftool should work as it says on the tin, as a straight up passthrough to git diff *unless* explicitly configured to use external tools (e.g. diff.tool and diff.guitool).

Hopefully that makes sense, please let me know if I am off-base.

Thanks,

Logan

Jeff King· Jun 25, 2019, 21:35 UTC · re: Pugh, Logan · lore

Re: [2.22.0] difftool no longer passes through to git diff if diff.tool is unset

On Thu, Jun 20, 2019 at 07:29:36PM +0000, Pugh, Logan wrote:

Show 5 quoted lines

> Thanks for the explanation. It sounds like I was under the incorrect 
> assumption that I could use the difftool command the same way as the 
> diff command. Part of my confusion could be blamed on the git-difftool 
> documentation (https://git-scm.com/docs/git-difftool) which near the top 
> states:

Well, it _is_ true that you can use it the same way. It's just that you need to configure it to use whatever 3rd-party tool you want (and if you do not want to configure a tool, then you are better off just using git-diff directly). It was only due to a bug/historical accident that it behaved just like git-diff in the no-index case (but not in the regular case -- AFAICT, that would have been broken for your script always).

Show 14 quoted lines

> My use case is a CLI program I've written that processes and then 
> compares two arbitrary files using the git difftool apparatus as 
> configured by the end user, leaving the choice to them whether to use 
> the internal diff tool or an external tool.
> 
> Now, if I'm understanding correctly, I should not rely on the behavior 
> of git difftool --no-index passing through to git diff. I could add 
> another CLI switch and code path to my program that calls git diff 
> directly instead of git difftool but the passthrough behavior seemed 
> more elegant at the time.
> 
> Ideally, in my mind, git difftool should work as it says on the tin, as 
> a straight up passthrough to git diff *unless* explicitly configured to 
> use external tools (e.g. diff.tool and diff.guitool).

That does make some sense to me for your use case. But I'm worried it would be a worse experience for people new to difftool (they run it and scratch their heads why it does not do anything different, whereas now they get walked through an interactive configuration).

I dunno. I do not use difftool myself, so I don't have strong opinions.

In the meantime, I think you can probably switch behavior in your script by checking if the diff.tool config is set. It might be nice if difftool had a better way to query that without you having to know if it's configured. Or in your case I suppose even better would just be an option like "--if-not-configured-just-use-regular-diff". Then it would do what you want, without impacting users who do want the interactive setup.

-Peff

Pugh, Logan· Jun 25, 2019, 23:09 UTC · re: Jeff King · lore

Re: [2.22.0] difftool no longer passes through to git diff if diff.tool is unset

Show 6 quoted lines

> Well, it _is_ true that you can use it the same way. It's just that you
> need to configure it to use whatever 3rd-party tool you want (and if you
> do not want to configure a tool, then you are better off just using
> git-diff directly). It was only due to a bug/historical accident that it
> behaved just like git-diff in the no-index case (but not in the regular
> case -- AFAICT, that would have been broken for your script always).

Yes, it would seem that I had only stumbled upon the broken behavior because of my --no-index use case, but at least the inconsistency is fixed.

> That does make some sense to me for your use case. But I'm worried it
> would be a worse experience for people new to difftool (they run it and
> scratch their heads why it does not do anything different, whereas now
> they get walked through an interactive configuration).

That is a fair point. UX matters even for CLI programs. Prior to it being fixed, I myself was confused as to why I was getting a git-diff output when trying to use an external tool with git-difftool but hadn't configured it correctly. At least now there is some feedback when the configuration is invalid.

> In the meantime, I think you can probably switch behavior in your script
> by checking if the diff.tool config is set. It might be nice if difftool
> had a better way to query that without you having to know if it's
> configured.

What I ended up doing in my app was just requiring the user to be explicit (via separate arguments) about whether they wanted to use git-diff or git-difftool. My app also accepts additional arguments that get passed through to git-diff/difftool and I didn't want to have to check those in addition to git-config. I think that would have been significantly more complex to implement.

> Or in your case I suppose even better would just be an
> option like "--if-not-configured-just-use-regular-diff". Then it would
> do what you want, without impacting users who do want the interactive
> setup.

If such an option was considered I would be in favor of it. Maybe call it "--no-tutorial" or perhaps "--diff-fallback".

But having fixed my app, I'm content with the status quo too, now.

Thanks,

Logan

Jeff King· Jun 26, 2019, 18:08 UTC · re: Pugh, Logan · lore

Re: [2.22.0] difftool no longer passes through to git diff if diff.tool is unset

On Tue, Jun 25, 2019 at 11:09:08PM +0000, Pugh, Logan wrote:

Show 9 quoted lines

> > Or in your case I suppose even better would just be an
> > option like "--if-not-configured-just-use-regular-diff". Then it would
> > do what you want, without impacting users who do want the interactive
> > setup.
> 
> If such an option was considered I would be in favor of it. Maybe call 
> it "--no-tutorial" or perhaps "--diff-fallback".
> 
> But having fixed my app, I'm content with the status quo too, now.

Yeah, those are definitely better names. :)

I think we're on the same page about a good path forward, then. I don't plan to work on this myself, but maybe it would be a good #leftoverbits candidate for somebody wanting to get started on modifying Git.

-Peff