[PATCH 0/1] difftool: --no-index is incompatible with --dir-diff

[PATCH 0/1] difftool: --no-index is incompatible with --dir-diff

[Johannes Schindelin via GitGitGadget]

While debugging why a straight-forward backport of the js/difftool-no-index 
branch onto Git for Windows' master does not work as expected (spoiler: it's
due to git diff --no-index now implying --ext-diff, a change that is not yet
in Git for Windows' master), I ran into quite a few segmentation faults
while trying to make --no-index work nicely with --dir-diff.

And this was a much bigger project than I thought it would be: the worktree 
is NULL and causes a segmentation fault, make_transient_cache_entry() 
returns NULL instead of a cache entry, while looking up replacement objects
there is a crash because git_replace_ref_base is NULL, during 
checkout_entry()'s life time, the prepare_alt_odb() function tries to
dereference the_repository.objects->odb (which is NULL, of course). I'll
stop here with all the segmentation faults.

Even worse, when trying to compare ../1 to ../2 (as we do in the regression
test for difftool --no-index outside any repository), we run afoul of 
verify_path() complaining that this is not a valid path for an index entry.

After figuring all of that out, I took a step back and guess what: it turns
out that it does not even make sense to combine those two options. The 
--dir-diff option exists to enable diff'ing subdirectories of a worktree
while pretending that untracked and ignored files in them do not even exist.
And without a worktree, there simply are no untracked or ignored files, so 
--dir-diff is not necessary.

Still, we should address those segmentation faults. And I think we address
them best by explicitly telling the user that they cannot use those two
options together.

Johannes Schindelin (1):
  difftool --no-index: error out on --dir-diff (and don't crash)

 builtin/difftool.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)


base-commit: 20de316e33446f37200e51aa333ba7d824dfd478
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-186%2Fdscho%2Fdifftool-no-index-extra-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-186/dscho/difftool-no-index-extra-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/186
-- 
gitgitgadget

[PATCH 1/1] difftool --no-index: error out on --dir-diff (and don't crash)

[Johannes Schindelin via GitGitGadget]

From: Johannes Schindelin <johannes.schindelin@gmx.de>

In `--no-index` mode, we now no longer require a worktree nor a
repository. But some code paths in `difftool` expect those to be
present.

The most notable such code path is the `--dir-diff` one: we use the
existing checkout machinery to copy the files, and that machinery looks
up replacement refs, looks at alternate ODBs, wants to use the worktree
path, etc.

Rather than running into segmentation faults, let's die with an
informative error message.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 builtin/difftool.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/builtin/difftool.c b/builtin/difftool.c
index 4fff1e83f9..5704a76088 100644
--- a/builtin/difftool.c
+++ b/builtin/difftool.c
@@ -735,7 +735,8 @@ int cmd_difftool(int argc, const char **argv, const char *prefix)
 		setup_work_tree();
 		setenv(GIT_DIR_ENVIRONMENT, absolute_path(get_git_dir()), 1);
 		setenv(GIT_WORK_TREE_ENVIRONMENT, absolute_path(get_git_work_tree()), 1);
-	}
+	} else if (dir_diff)
+		die(_("--dir-diff is incompatible with --no-index"));
 
 	if (use_gui_tool && diff_gui_tool && *diff_gui_tool)
 		setenv("GIT_DIFF_TOOL", diff_gui_tool, 1);
-- 
gitgitgadget

Re: [PATCH 0/1] difftool: --no-index is incompatible with --dir-diff

[Junio C Hamano]

"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
writes:

> ... it does not even make sense to combine those two options. The 
> --dir-diff option exists to enable diff'ing subdirectories of a worktree
> while pretending that untracked and ignored files in them do not even exist.

Hmph.  So there is no point in using the dir-diff mode when you have
say two extracts of pristine tarballs?

I guess that's true.

The difftool will launch an external "directory diff tool" on two
directories populated with the two "things" (be it "the contents
from a tree-ish", "the working tree files", "the contents in the
index") in the end, but you already have two directories to be
compared when you run "diff --no-index", so there is no point in
going through the "difftool" middleman.  You can just feed the two
directories you have directly to that external directory diff tool.

> base-commit: 20de316e33446f37200e51aa333ba7d824dfd478
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-186%2Fdscho%2Fdifftool-no-index-extra-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-186/dscho/difftool-no-index-extra-v1
> Pull-Request: https://github.com/gitgitgadget/git/pull/186