Re: [PATCH] git diff/diff-index/diff-files: call setup_work_tree()

[Junio C Hamano <gitster@pobox.com>]

"Nguyen Thai Ngoc Duy" <pclouds@gmail.com> writes:

> On 8/25/08, Miklos Vajna <vmiklos@frugalware.org> wrote:
>>  diff --git a/builtin-diff.c b/builtin-diff.c
>>
>> index 7ffea97..57da6ed 100644
>>
>> --- a/builtin-diff.c
>>  +++ b/builtin-diff.c
>>
>> @@ -279,6 +279,7 @@ int cmd_diff(int argc, const char **argv, const char *prefix)
>>         diff_no_index(&rev, argc, argv, nongit, prefix);
>>
>>         /* Otherwise, we are doing the usual "git" diff */
>>  +       setup_work_tree();
>>         rev.diffopt.skip_stat_unmatch = !!diff_auto_refresh_index;
>>
>>         if (nongit)
>
> At least builtin_diff_blobs() and builtin_diff_tree() won't need
> worktree, so NACK again. Anyway I'm not familiar with diff*. Junio
> should know these better.

How about doing it this way then?

 * diff-files is about comparing with work tree, so it obviously needs a
   work tree;

 * diff-index also does;

 * no-index is about random files outside git context, so it obviously
   doesn't need any work tree;

 * comparing two (or more) trees doesn't;

 * comparing two blobs doesn't;

 * comparing a blob with a random file doesn't;

What could be problematic is "git diff --cached".  Strictly speaking, it
compares the index and a tree so it shouldn't need any work tree.  The
same obviously applies to "git diff-index --cached".

While it is theoretically possible to have an index in a bare repository
and build your history using it without using any worktree, I do not think
it is a use case worth worrying about.  As long as setup_work_tree() does
not complain and die in such a setup, "diff --cached" itself won't look at
the work tree (whereever random place setup_work_tree() sets it) at all,
so probably it is a non issue.  I dunno.

I do not have a test environment that uses a separate worktree settings,
so this is obviously untested.

Perhaps people who are interested in keeping core.worktree alive can add
test scripts in t/ somewhere to help salvaging the feature?

---

 builtin-diff.c |    3 +++
 git.c          |    4 ++--
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git i/builtin-diff.c w/builtin-diff.c
index 7ffea97..06c85da 100644
--- i/builtin-diff.c
+++ w/builtin-diff.c
@@ -114,6 +114,8 @@ static int builtin_diff_index(struct rev_info *revs,
 			      int argc, const char **argv)
 {
 	int cached = 0;
+
+	setup_work_tree();
 	while (1 < argc) {
 		const char *arg = argv[1];
 		if (!strcmp(arg, "--cached"))
@@ -207,6 +209,7 @@ static int builtin_diff_files(struct rev_info *revs, int argc, const char **argv
 	int result;
 	unsigned int options = 0;
 
+	setup_work_tree();
 	while (1 < argc && argv[1][0] == '-') {
 		if (!strcmp(argv[1], "--base"))
 			revs->max_count = 1;
diff --git i/git.c w/git.c
index 37b1d76..a8e730d 100644
--- i/git.c
+++ w/git.c
@@ -286,8 +286,8 @@ static void handle_internal_command(int argc, const char **argv)
 		{ "count-objects", cmd_count_objects, RUN_SETUP },
 		{ "describe", cmd_describe, RUN_SETUP },
 		{ "diff", cmd_diff },
-		{ "diff-files", cmd_diff_files, RUN_SETUP },
-		{ "diff-index", cmd_diff_index, RUN_SETUP },
+		{ "diff-files", cmd_diff_files, RUN_SETUP | NEED_WORK_TREE },
+		{ "diff-index", cmd_diff_index, RUN_SETUP | NEED_WORK_TREE },
 		{ "diff-tree", cmd_diff_tree, RUN_SETUP },
 		{ "fast-export", cmd_fast_export, RUN_SETUP },
 		{ "fetch", cmd_fetch, RUN_SETUP },