git-diff on touched files: bug or feature?

git-diff on touched files: bug or feature?

[Matthieu Moy]

Hi,

When a file is "touched" (ie. stat information not matching the index,
but the content still matching), git-status doesn't report the file as
modified (as expected), but git-diff does (with an empty diff):

$ git st
# On branch master
nothing to commit (working directory clean)
$ ls       
bar
$ touch bar
$ git diff
diff --git a/bar b/bar         <--- here ---<
$ git status
# On branch master
nothing to commit (working directory clean)
$ git diff                     <--- status updated
                                    the stat in the index.

Is this intended, or just that the code that reconciles the file and
the index has been written for status, but not used in diff?

Thanks,

-- 
Matthieu

Re: git-diff on touched files: bug or feature?

[Junio C Hamano]

Matthieu Moy <Matthieu.Moy@imag.fr> writes:

> $ touch bar
> $ git diff
> diff --git a/bar b/bar         <--- here ---<
> $ git status
> # On branch master
> nothing to commit (working directory clean)
> $ git diff                     <--- status updated
>                                     the stat in the index.
>
> Is this intended,

Yes.  Very much so, intentionally, from very early days of git.
This serves as a reminder to the user that he started editing
but changed his mind to end up with the same contents as the
original, until the next "update-index --refresh" (which is
internally invoked from "status").

If the feature still makes sense in the modern world is a
different story, but I do find it useful.

Not the made-up "touch" example, but often in real life, I
explore a solution by first making changes to one part of the
system, realizing a better way is to change the caller of what I
initially thought should be changed, edit the file back and
modify the caller which is in another file.  The former file
will show that empty "header-only" diff as the reminder of what
I did.

After that, when I reach the point to run "git status", because
I have been reminded, I already know about these "tried but
discarded" changes, and I find the fact that the they are forgot
by that operation very convenient.

Re: git-diff on touched files: bug or feature?

[Alexandre Julliard]

Junio C Hamano <gitster@pobox.com> writes:

> Yes.  Very much so, intentionally, from very early days of git.
> This serves as a reminder to the user that he started editing
> but changed his mind to end up with the same contents as the
> original, until the next "update-index --refresh" (which is
> internally invoked from "status").

It would be nice to have a way to refresh a single file though. For
instance in vc-git.el the workfile-unchanged-p function currently has
to rehash the file every time to see if it really changed, because we
can't afford to refresh the whole project at that point.

-- 
Alexandre Julliard
julliard@winehq.org

Re: git-diff on touched files: bug or feature?

[Junio C Hamano]

Alexandre Julliard <julliard@winehq.org> writes:

> .... For
> instance in vc-git.el the workfile-unchanged-p function currently has
> to rehash the file every time to see if it really changed, because we
> can't afford to refresh the whole project at that point.

Maybe I am missing something.  Why can't you "afford to"?

"update-index --refresh" looks at only the files whose cached
stat information does indicate there might be chanegs.  It does
not rehash already up-to-date ones.

Re: git-diff on touched files: bug or feature?

[Alexandre Julliard]

Junio C Hamano <gitster@pobox.com> writes:

> Alexandre Julliard <julliard@winehq.org> writes:
>
>> .... For
>> instance in vc-git.el the workfile-unchanged-p function currently has
>> to rehash the file every time to see if it really changed, because we
>> can't afford to refresh the whole project at that point.
>
> Maybe I am missing something.  Why can't you "afford to"?
>
> "update-index --refresh" looks at only the files whose cached
> stat information does indicate there might be chanegs.  It does
> not rehash already up-to-date ones.

No, but it goes through the tree and stats every single file. On a
large project that can be slow, especially if you don't have enough
RAM to keep it all in cache, so it's not something we can do while the
user is interacting with a file.

-- 
Alexandre Julliard
julliard@winehq.org

Re: git-diff on touched files: bug or feature?

[Matthieu Moy]

Junio C Hamano <gitster@pobox.com> writes:

> Matthieu Moy <Matthieu.Moy@imag.fr> writes:
>
>> $ touch bar
>> $ git diff
>> diff --git a/bar b/bar         <--- here ---<
>> $ git status
>> # On branch master
>> nothing to commit (working directory clean)
>> $ git diff                     <--- status updated
>>                                     the stat in the index.
>>
>> Is this intended,
>
> Yes.  Very much so, intentionally, from very early days of git.
> This serves as a reminder to the user that he started editing
> but changed his mind to end up with the same contents as the
> original, until the next "update-index --refresh" (which is
> internally invoked from "status").
>
> If the feature still makes sense in the modern world is a
> different story, but I do find it useful.

I understand that it can be usefull, but I really don't like having it
by default (is there a way to deactivate it BTW?):

I've hit this while working on a project, doing a lot of modifications
through scripting (some regexp substitutions and such kinds of
things). Then, git-diff shows me pages of "diff --git ...", and a few
relevant entries in the middle of it. That's very bad from the
usability point of view (I actually had some ~20 lines diff surrounded
by 100+ irrelevant lines), and also kills performance: if a script
touches a lot of files, I expect the next "diff" or "status" to be
slow, but not the second next. Here, diff will be slow until I run
git-status again.

And I find the "reminder" feature very fragile. That means git-status
is no longer a read-only operation for the user. As a user, I expect
to be able to run git-status without changing the behavior of
subsequent git commands, which is not the case here. That means for
example that someone used to running git-diff /before/ git-status will
get the reminder, while someone used to running git-diff /after/
git-status (which I find sensible, get an overview before getting the
details of what you did) won't get it. Note also that this makes a
difference between git-status (which updates the stat in the index)
and git-status -a (which doesn't). That's an implementation detail
that shouldn't be exposed to the user.

Since I don't see any mention of this in the man pages for git-diff or
git-status (I might have missed it), I wonder how many user actually
ever used this as a feature.

I'd be in favor of disabling this by default, and providing a
configuration option and/or a command line option to diff to enable
it. I can try writting a patch for this if people agree on the
specification.

-- 
Matthieu

Re: git-diff on touched files: bug or feature?

[Johannes Schindelin]

Hi,

On Thu, 2 Aug 2007, Matthieu Moy wrote:

> Junio C Hamano <gitster@pobox.com> writes:
> 
> > Matthieu Moy <Matthieu.Moy@imag.fr> writes:
> >
> >> $ touch bar
> >> $ git diff
> >> diff --git a/bar b/bar         <--- here ---<
> >> $ git status
> >> # On branch master
> >> nothing to commit (working directory clean)
> >> $ git diff                     <--- status updated
> >>                                     the stat in the index.
> >>
> >> Is this intended,
> >
> > Yes.  Very much so, intentionally, from very early days of git.
> > This serves as a reminder to the user that he started editing
> > but changed his mind to end up with the same contents as the
> > original, until the next "update-index --refresh" (which is
> > internally invoked from "status").
> >
> > If the feature still makes sense in the modern world is a
> > different story, but I do find it useful.
> 
> I understand that it can be usefull, but I really don't like having it
> by default (is there a way to deactivate it BTW?).

Yes.  Just call "git status" and be done with it.

Ciao,
Dscho

Re: git-diff on touched files: bug or feature?

[Matthieu Moy]

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

> On Thu, 2 Aug 2007, Matthieu Moy wrote:
>
>> > If the feature still makes sense in the modern world is a
>> > different story, but I do find it useful.
>> 
>> I understand that it can be usefull, but I really don't like having it
>> by default (is there a way to deactivate it BTW?).
>
> Yes.  Just call "git status" and be done with it.

That's not what I mean (my original message mentionned that already
BTW). By "deactivate", I mean "make git-diff never show empty diffs".
I don't want to run two commands where I need only one.

-- 
Matthieu

Re: git-diff on touched files: bug or feature?

[Johannes Schindelin]

Hi,

On Thu, 2 Aug 2007, Matthieu Moy wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > On Thu, 2 Aug 2007, Matthieu Moy wrote:
> >
> >> > If the feature still makes sense in the modern world is a
> >> > different story, but I do find it useful.
> >> 
> >> I understand that it can be usefull, but I really don't like having it
> >> by default (is there a way to deactivate it BTW?).
> >
> > Yes.  Just call "git status" and be done with it.
> 
> That's not what I mean (my original message mentionned that already
> BTW). By "deactivate", I mean "make git-diff never show empty diffs".
> I don't want to run two commands where I need only one.

Then don't touch the files you do not want to touch!  Or if you want to 
have it convenient, and have a script that touches everything, even if it 
does not change the contents, just add "git add -u" at the end of the 
script".  Not that difficult.

Ciao,
Dscho

Re: git-diff on touched files: bug or feature?

[Jean-François Veillette]

I find comments like this to be counter productive.
Admin it, git porcelain still has some work to be done.  We can't  
expect new users to know the git internals workflow before they can  
use git effectively.  We can expect new users to read the man pages,  
but not necessarely expect them to understand all the plumbing  
implied by what they read.  Here I think M.Moy understand the  
plumbing and could silently deal with it.  But instead he decided to  
help improve git and decided to raise a flag about inconsistencies he  
faced.  We should never answer request for improvement with ' Just do  
X and be done with it'.  This is a 'geek' answer to a legitime comment.

I know the goal of git is not to reign over the world of vcs, but  
it's not a reason to refuse to improve it when constructive comments  
are made about it.

- jfv

Le 07-08-02 à 06:48, Johannes Schindelin a écrit :

> Hi,
>
> On Thu, 2 Aug 2007, Matthieu Moy wrote:
>
>> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>>
>>> On Thu, 2 Aug 2007, Matthieu Moy wrote:
>>>
>>>>> If the feature still makes sense in the modern world is a
>>>>> different story, but I do find it useful.
>>>>
>>>> I understand that it can be usefull, but I really don't like  
>>>> having it
>>>> by default (is there a way to deactivate it BTW?).
>>>
>>> Yes.  Just call "git status" and be done with it.
>>
>> That's not what I mean (my original message mentionned that already
>> BTW). By "deactivate", I mean "make git-diff never show empty diffs".
>> I don't want to run two commands where I need only one.
>
> Then don't touch the files you do not want to touch!  Or if you  
> want to
> have it convenient, and have a script that touches everything, even  
> if it
> does not change the contents, just add "git add -u" at the end of the
> script".  Not that difficult.
>
> Ciao,
> Dscho

Re: git-diff on touched files: bug or feature?

[Johannes Schindelin]

Hi,

[please do not top-post.  Either comment on what you quote, or delete it.]

On Thu, 2 Aug 2007, Jean-Fran?ois Veillette wrote:

> Admin it, git porcelain still has some work to be done.

No need to argue there, I admit it.

> We can't expect new users to know the git internals workflow before they 
> can use git effectively.

This use case has not much to do with new users.  A new user _has_ to know 
that updating all files, even if their content does not change, is not 
right.

At least we do not commit empty changes like CVS did all too happily.

Ciao,
Dscho

Re: git-diff on touched files: bug or feature?

[Steven Grimm]

Johannes Schindelin wrote:
> This use case has not much to do with new users.  A new user _has_ to know 
> that updating all files, even if their content does not change, is not 
> right.
>   

Someone who has used, say, Subversion might have a perfectly reasonable 
expectation that "git diff" will show differences in content, and when 
there are no differences in content, will not mention a file at all. 
Other version control systems have "diff" commands that ignore touched 
files.

I admit I also thought the empty diffs were a bug (albeit a minor one 
not worth making noise about) until this thread. Now I understand why it 
happens, though I still think we'd be better off just not displaying the 
filename in git-diff until we know there's an actual diff to display.

I certainly don't think the "it's a feature: it reminds you when you've 
edited a file without changing it" argument holds any water at all. If 
that were truly the intent, if we truly considered that to be useful 
information a developer would want to get at after the fact, then why 
would git-status throw away that information? If I check to see what 
files I've modified/added (for which I run git-status) why does that 
automatically imply I am no longer interested in being reminded that I 
have saved a file without making changes, especially given that such 
files *don't* show up in the git-status output? git-status is silently 
losing information here; it gives you no indication that it has 
refreshed the index for those touched-but-not-edited files.

Now, I happen to think throwing away that information is just fine, 
because I don't think I have ever once cared to know that I touched a 
file but didn't change it. But fundamentally it's either a piece of 
information we care about (in which case we shouldn't go silently 
discarding it) or not (in which case it is just clutter in git-diff).

In the meantime, though, it's trivial enough to put a wrapper around 
git-diff to filter out the diffless files. I haven't cared enough to 
bother, but if I did it'd be just a few lines of Perl, no big deal.

-Steve

Re: git-diff on touched files: bug or feature?

[Johannes Schindelin]

Hi,

On Thu, 2 Aug 2007, Steven Grimm wrote:

> Johannes Schindelin wrote:
> > This use case has not much to do with new users.  A new user _has_ to know
> > that updating all files, even if their content does not change, is not
> > right.
> >   
> 
> Someone who has used, say, Subversion might have a perfectly reasonable
> expectation that "git diff" will show differences in content, and when there
> are no differences in content, will not mention a file at all. Other version
> control systems have "diff" commands that ignore touched files.
> 
> I admit I also thought the empty diffs were a bug (albeit a minor one not
> worth making noise about) until this thread. Now I understand why it happens,
> though I still think we'd be better off just not displaying the filename in
> git-diff until we know there's an actual diff to display.
> 
> I certainly don't think the "it's a feature: it reminds you when you've edited
> a file without changing it" argument holds any water at all. If that were
> truly the intent, if we truly considered that to be useful information a
> developer would want to get at after the fact, then why would git-status throw
> away that information?

Okay, I'll answer just this one, instead of pointing you to the thread 
that I've been pointing to twice now (because your ideas about how 
git should work are usually similar to mine, and by way of saying thanks 
for your contributions):

When is the time to say "git status"?

It is just before committing.  I.e when you really think that you're done 
editing, and want to have the end picture.  "git status" only gives you 
names, and therefore it _has_ to update the index if it got out of sync, 
to show meaningful results.

When is the time to say "git diff"?

Much more often.  In the middle of your work.  And there it would be 
_disruptive_ if it updated the index all the time, especially if you have 
a quite large working tree.

But then, normal users do not touch all the files.  They don't.

So I doubt that in the common case the subject we are discussing matters 
at all.

Yes, "perl -pi" is something I used myself.  Yes, I think it is a bug that 
it writes new files when it does not really change anything.  And yes, I 
had a script lying somewhere on my backup hard disk which uses some evil 
"git diff --name-only | xargs bla" mantra (it does not even use the 
--quiet option, since that was not invented back then) to actually _undo_ 
the effects by setting the timestamps back, since the full compilation 
time in that project _hurt_.

But it is hardly an operation that I use daily.  Hardly even twice a 
year.

Ciao,
Dscho

Re: git-diff on touched files: bug or feature?

[Matthieu Moy]

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

> Okay, I'll answer just this one, instead of pointing you to the thread 
> that I've been pointing to twice now

The link was probably not the right one since I saw only two [PATCH]
message, but yes, I remember a thread pointing out why git-status had
to update the index. I'm not arguing against that, I'm happy with the
current git-status behavior, but I find the git-diff one inconsistant
with git-status, and still don't understand any reason why a normal
user would want a difference.

> When is the time to say "git status"?
>
> It is just before committing.  I.e when you really think that you're done 
> editing, and want to have the end picture.  "git status" only gives you 
> names, and therefore it _has_ to update the index if it got out of sync, 
> to show meaningful results.
>
> When is the time to say "git diff"?
>
> Much more often.  In the middle of your work.  And there it would be 
> _disruptive_ if it updated the index all the time, especially if you have 
> a quite large working tree.

That's your point of view, but I do not share it. Depending on the
kind of things I'm doing, I usually run status regularly, because it's
short to read, and it shows me both staged and unstaged changes.
git-status tells me if I did something obviously totally wrong
(changing a file on which I was not working, deleting something
important ...), and after that, git-diff gives me a finer-grained
vision of what I did.

Does this really sound so much irreasonable to you?

-- 
Matthieu

Re: git-diff on touched files: bug or feature?

[J. Bruce Fields]

On Thu, Aug 02, 2007 at 05:45:20PM +0200, Matthieu Moy wrote:
> Depending on the kind of things I'm doing, I usually run status
> regularly, because it's short to read, and it shows me both staged and
> unstaged changes.  git-status tells me if I did something obviously
> totally wrong (changing a file on which I was not working, deleting
> something important ...), and after that, git-diff gives me a
> finer-grained vision of what I did.

Yeah, ditto for me.  When I return to a project after having been away a
few minutes, the first things I do are

	git branch	# remind me which topic I was working on
	git status	# remind me if I was in the middle of something.

So I end up running it a lot.  I only do a git-diff if I need some
details.

--b.

[PATCH] Add --show-touched option to show "diff --git" line when contents are unchanged

[Steven Grimm]

The default is now to not show the diff --git header line if the file's
timestamp has changed but the contents and/or file mode haven't.

Signed-off-by: Steven Grimm <koreth@midwinter.com>
---

	Okay, enough arguing about whether the empty diff lines are
	useful or not -- here's a patch to get rid of them.

	This passes all the existing "diff" tests, with one minor tweak
	to the symlink test (since it expected the old behavior.)
	If someone can find a case where this will spit out an actual
	diff but not the "diff --git" line, please tell me how to make
	that happen. The code *looks* like it has such a path, but I was
	unable to make it happen in my ad-hoc testing and it doesn't
	happen in any of the existing diff test cases.

	Personally I'm in favor of doing away with the option altogether
	and having the code always work the way it works by default with
	this patch, but if some people find the old behavior useful they
	can still get at it with the new option.

	My xmalloc() call allocates a few more bytes than strictly
	needed, but I found it was less readable to subtract out the
	space taken by the "%s" tokens in the format string.

 Documentation/diff-options.txt |    4 ++
 diff.c                         |   46 ++++++++++++++++++++++++++----
 diff.h                         |    3 +-
 t/t4011-diff-symlink.sh        |    2 +-
 t/t4021-diff-untouched.sh      |   61 ++++++++++++++++++++++++++++++++++++++++
 5 files changed, 108 insertions(+), 8 deletions(-)
 create mode 100755 t/t4021-diff-untouched.sh

diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index 228ccaf..12ad048 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -185,5 +185,9 @@
 --no-ext-diff::
 	Disallow external diff drivers.
 
+--show-touched::
+	Display the "diff --git" message for files whose modification
+	timestamps have changed, even if the contents don't differ.
+
 For more detailed explanation on these common options, see also
 link:diffcore.html[diffcore documentation].
diff --git a/diff.c b/diff.c
index a5fc56b..e1112e5 100644
--- a/diff.c
+++ b/diff.c
@@ -1260,6 +1260,9 @@ static const char *diff_funcname_pattern(struct diff_filespec *one)
 	return NULL;
 }
 
+/* The message that gets printed at the top of a file's diffs */
+#define DIFF_MESSAGE_FORMAT_STRING "%sdiff --git %s %s%s\n"
+
 static void builtin_diff(const char *name_a,
 			 const char *name_b,
 			 struct diff_filespec *one,
@@ -1268,6 +1271,7 @@ static void builtin_diff(const char *name_a,
 			 struct diff_options *o,
 			 int complete_rewrite)
 {
+	char *diff_message;
 	mmfile_t mf1, mf2;
 	const char *lbl[2];
 	char *a_one, *b_two;
@@ -1278,25 +1282,50 @@ static void builtin_diff(const char *name_a,
 	b_two = quote_two("b/", name_b + (*name_b == '/'));
 	lbl[0] = DIFF_FILE_VALID(one) ? a_one : "/dev/null";
 	lbl[1] = DIFF_FILE_VALID(two) ? b_two : "/dev/null";
-	printf("%sdiff --git %s %s%s\n", set, a_one, b_two, reset);
+
+	/*
+	 * Generate the "diff --git" status message. By default we only
+	 * show it if we have a difference to display, but the user can
+	 * optionally choose to show it for all files that we examine for
+	 * content differences (e.g. because their timestamps have changed.)
+	 */
+	diff_message = xmalloc(strlen(set) + strlen(reset) +
+			       strlen(a_one) + strlen(b_two) +
+			       sizeof(DIFF_MESSAGE_FORMAT_STRING));
+	sprintf(diff_message, DIFF_MESSAGE_FORMAT_STRING,
+	        set, a_one, b_two, reset);
+	if (o->show_touched) {
+		fputs(diff_message, stdout);
+		*diff_message = '\0';
+	}
+
 	if (lbl[0][0] == '/') {
 		/* /dev/null */
-		printf("%snew file mode %06o%s\n", set, two->mode, reset);
+		printf("%s%snew file mode %06o%s\n",
+		       diff_message, set, two->mode, reset);
+		*diff_message = '\0';
 		if (xfrm_msg && xfrm_msg[0])
 			printf("%s%s%s\n", set, xfrm_msg, reset);
 	}
 	else if (lbl[1][0] == '/') {
-		printf("%sdeleted file mode %06o%s\n", set, one->mode, reset);
+		printf("%s%sdeleted file mode %06o%s\n",
+		       diff_message, set, one->mode, reset);
+		*diff_message = '\0';
 		if (xfrm_msg && xfrm_msg[0])
 			printf("%s%s%s\n", set, xfrm_msg, reset);
 	}
 	else {
 		if (one->mode != two->mode) {
-			printf("%sold mode %06o%s\n", set, one->mode, reset);
+			printf("%s%sold mode %06o%s\n",
+			       diff_message, set, one->mode, reset);
 			printf("%snew mode %06o%s\n", set, two->mode, reset);
+			*diff_message = '\0';
+		}
+		if (xfrm_msg && xfrm_msg[0]) {
+			printf("%s%s%s%s\n",
+			       diff_message, set, xfrm_msg, reset);
+			*diff_message = '\0';
 		}
-		if (xfrm_msg && xfrm_msg[0])
-			printf("%s%s%s\n", set, xfrm_msg, reset);
 		/*
 		 * we do not run diff between different kind
 		 * of objects.
@@ -1304,6 +1333,8 @@ static void builtin_diff(const char *name_a,
 		if ((one->mode ^ two->mode) & S_IFMT)
 			goto free_ab_and_return;
 		if (complete_rewrite) {
+			fputs(diff_message, stdout);
+			*diff_message = '\0';
 			emit_rewrite_diff(name_a, name_b, one, two,
 					o->color_diff);
 			o->found_changes = 1;
@@ -1372,6 +1403,7 @@ static void builtin_diff(const char *name_a,
 	diff_free_filespec_data(two);
 	free(a_one);
 	free(b_two);
+	free(diff_message);
 	return;
 }
 
@@ -2381,6 +2413,8 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)
 		options->allow_external = 1;
 	else if (!strcmp(arg, "--no-ext-diff"))
 		options->allow_external = 0;
+	else if (!strcmp(arg, "--show-touched"))
+		options->show_touched = 1;
 	else
 		return 0;
 	return 1;
diff --git a/diff.h b/diff.h
index 9fd6d44..e172ecf 100644
--- a/diff.h
+++ b/diff.h
@@ -61,7 +61,8 @@ struct diff_options {
 		 has_changes:1,
 		 quiet:1,
 		 allow_external:1,
-		 exit_with_status:1;
+		 exit_with_status:1,
+		 show_touched:1;
 	int context;
 	int break_opt;
 	int detect_rename;
diff --git a/t/t4011-diff-symlink.sh b/t/t4011-diff-symlink.sh
index c6d1369..910c6cc 100755
--- a/t/t4011-diff-symlink.sh
+++ b/t/t4011-diff-symlink.sh
@@ -60,7 +60,7 @@ test_expect_success \
     'diff identical, but newly created symlink' \
     'sleep 3 &&
     ln -s xyzzy frotz &&
-    git diff-index -M -p $tree > current &&
+    git diff-index --show-touched -M -p $tree > current &&
     compare_diff_patch current expected'
 
 cat > expected << EOF
diff --git a/t/t4021-diff-untouched.sh b/t/t4021-diff-untouched.sh
new file mode 100755
index 0000000..a8153e0
--- /dev/null
+++ b/t/t4021-diff-untouched.sh
@@ -0,0 +1,61 @@
+#!/bin/sh
+#
+# Copyright (c) 2005 Johannes Schindelin
+#
+
+test_description='Test display and suppression of unmodified files.
+
+'
+. ./test-lib.sh
+. ../diff-lib.sh
+
+touch empty
+
+test_expect_success 'no output when no changes' '
+
+	echo foobar > file1 &&
+	chmod 644 file1 &&
+	git add file1 &&
+	git commit -m "initial commit" &&
+	git diff > current &&
+	compare_diff_patch current empty
+'
+
+test_expect_success 'no output when file touched' '
+
+	sleep 1 &&
+	touch file1 &&
+	git diff > current &&
+	compare_diff_patch current empty
+'
+
+cat > expected << EOF
+diff --git a/file1 b/file1
+EOF
+
+test_expect_success 'output when --show-touched is used' '
+
+	git diff --show-touched > current &&
+	compare_diff_patch current expected
+'
+
+test_expect_success 'no output when index updated with touched file' '
+
+	git add file1 &&
+	git diff --cached > current &&
+	compare_diff_patch current empty
+'
+
+cat > expected << EOF
+diff --git a/file1 b/file1
+old mode 100644
+new mode 100755
+EOF
+
+test_expect_success 'output when mode is changed' '
+
+	chmod 755 file1 &&
+	git diff > current &&
+	compare_diff_patch current expected
+'
+test_done
-- 
1.5.3.rc2.4.g726f9

Re: [PATCH] Add --show-touched option to show "diff --git" line when contents are unchanged

[Junio C Hamano]

Steven Grimm <koreth@midwinter.com> writes:

> 	Okay, enough arguing about whether the empty diff lines are
> 	useful or not -- here's a patch to get rid of them.

I do not think this addresses anything but -p (i.e. textual
diff) output.  If we _were_ to really do this, I think the patch
I sent earlier today, with possible improvements I suggested,
would be a better direction to go.

Re: [PATCH] Add --show-touched option to show "diff --git" line when contents are unchanged

[Johannes Schindelin]

Hi,

On Thu, 2 Aug 2007, Junio C Hamano wrote:

> Steven Grimm <koreth@midwinter.com> writes:
> 
> > 	Okay, enough arguing about whether the empty diff lines are
> > 	useful or not -- here's a patch to get rid of them.
> 
> I do not think this addresses anything but -p (i.e. textual
> diff) output.  If we _were_ to really do this, I think the patch
> I sent earlier today, with possible improvements I suggested,
> would be a better direction to go.

But I'd really think that what should be done (if anything has to be done 
at all) is to introduce a config variable which triggers the same logic in 
git-diff as was introduced in 2b5f9a8c0cff511f2bb0833b1ee02645b79323f4.

Ciao,
Dscho

Re: [PATCH] Add --show-touched option to show "diff --git" line when contents are unchanged

[Junio C Hamano]

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

> But I'd really think that what should be done (if anything has to be done 
> at all) is to introduce a config variable which triggers the same logic in 
> git-diff as was introduced in 2b5f9a8c0cff511f2bb0833b1ee02645b79323f4.

Sorry, I don't follow at all.  The diff toolchain works all
inside core without having to write a temporary index out, which
was the issue the commit you are quoting was about.

In any case, enough discussion.  Here is an updated patch, which
I _could_ be pursuaded to consider for inclusion after v1.5.3
happens, if there are enough agreements and Acks.

-- >8 --
git-diff: --stat-unmatch

Traditionally, git-diff with the working tree files showed files
whose lstat(2) information did not match with what were recorded
in the index, even if the actual contents did not have any
differences.  This squelches such output from git-diff by
default.

A new option, --stat-unmatch, is introduced to restore the
traditional behaviour.  This is useful to see if you want to
know you have too many files you only touch(1)ed without
modifying.  Having many such paths hurts performance, and you
can run "git-update-index --refresh" to update the lstat(2)
information recorded in the index in such a case.

The low level git-diff-files command is not affected.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 Documentation/git-diff.txt |   13 ++++++++++
 builtin-diff.c             |   13 ++++++++++
 diff.c                     |   54 ++++++++++++++++++++++++++++++++++++++++++++
 diff.h                     |    1 +
 4 files changed, 81 insertions(+), 0 deletions(-)

diff --git a/Documentation/git-diff.txt b/Documentation/git-diff.txt
index b36e705..efdc65b 100644
--- a/Documentation/git-diff.txt
+++ b/Documentation/git-diff.txt
@@ -59,6 +59,19 @@ OPTIONS
 -------
 include::diff-options.txt[]
 
+--stat-unmatch::
+	Traditionally, `git-diff` with the working tree files
+	showed files whose `lstat(2)` information did not match
+	with what were recorded in the index, even if the actual
+	contents did not have any differences, but recent git
+	squelches such output.  This option can be used to
+	restore the traditional behaviour.  This is useful to
+	see if you want to know you have too many files you only
+	`touch(1)`ed without modifying.  Having many such paths
+	hurts performance, and you can run `git-update-index
+	--refresh` to update the `lstat(2)` information recorded
+	in the index in such a case.
+
 <path>...::
 	The <paths> parameters, when given, are used to limit
 	the diff to the named paths (you can give directory
diff --git a/builtin-diff.c b/builtin-diff.c
index b48121e..c8e137d 100644
--- a/builtin-diff.c
+++ b/builtin-diff.c
@@ -222,6 +222,7 @@ int cmd_diff(int argc, const char **argv, const char *prefix)
 	prefix = setup_git_directory_gently(&nongit);
 	git_config(git_diff_ui_config);
 	init_revisions(&rev, prefix);
+	rev.diffopt.skip_stat_unmatch = 1;
 
 	if (!setup_diff_no_index(&rev, argc, argv, nongit, prefix))
 		argc = 0;
@@ -338,5 +339,17 @@ int cmd_diff(int argc, const char **argv, const char *prefix)
 					     ent, ents);
 	if (rev.diffopt.exit_with_status)
 		result = rev.diffopt.has_changes;
+
+	/*
+	 * We do not actually do this here because 99% of the time
+	 * the pager is in effect and this will not be shown, but
+	 * you could if you wanted to.
+	 *
+	 * if (1 < rev.diffopt.skip_stat_unmatch)
+	 *	fprintf(stderr,
+	 *		"Squelched %d stat-only differences.\n",
+	 *		rev.diffopt.skip_stat_unmatch - 1);
+	 *
+	 */
 	return result;
 }
diff --git a/diff.c b/diff.c
index a5fc56b..8c26e4d 100644
--- a/diff.c
+++ b/diff.c
@@ -2261,6 +2261,8 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)
 	else if (!strcmp(arg, "--shortstat")) {
 		options->output_format |= DIFF_FORMAT_SHORTSTAT;
 	}
+	else if (!strcmp(arg, "--stat-unmatch"))
+		options->skip_stat_unmatch = 0;
 	else if (!prefixcmp(arg, "--stat")) {
 		char *end;
 		int width = options->stat_width;
@@ -3143,11 +3145,63 @@ static void diffcore_apply_filter(const char *filter)
 	*q = outq;
 }
 
+static void diffcore_skip_stat_unmatch(struct diff_options *diffopt)
+{
+	int i;
+	struct diff_queue_struct *q = &diff_queued_diff;
+	struct diff_queue_struct outq;
+	outq.queue = NULL;
+	outq.nr = outq.alloc = 0;
+
+	for (i = 0; i < q->nr; i++) {
+		struct diff_filepair *p = q->queue[i];
+
+		/*
+		 * 1. Entries that come from stat info dirtyness
+		 *    always have both sides (iow, not create/delete),
+		 *    one side of the object name is unknown, with
+		 *    the same mode and size.  Keep the ones that
+		 *    do not match these criteria.  They have real
+		 *    differences.
+		 *
+		 * 2. At this point, the file is known to be modified,
+		 *    with the same mode and size, and the object
+		 *    name of one side is unknown.  Need to inspect
+		 *    the identical contents.
+		 */
+		if (!DIFF_FILE_VALID(p->one) || /* (1) */
+		    !DIFF_FILE_VALID(p->two) ||
+		    (p->one->sha1_valid && p->two->sha1_valid) ||
+		    (p->one->mode != p->two->mode) ||
+		    diff_populate_filespec(p->one, 1) ||
+		    diff_populate_filespec(p->two, 1) ||
+		    (p->one->size != p->two->size) ||
+
+		    diff_populate_filespec(p->one, 0) || /* (2) */
+		    diff_populate_filespec(p->two, 0) ||
+		    memcmp(p->one->data, p->two->data, p->one->size))
+			diff_q(&outq, p);
+		else {
+			/*
+			 * The caller can subtract 1 from skip_stat_unmatch
+			 * to determine how many paths were dirty only
+			 * due to stat info mismatch.
+			 */
+			diffopt->skip_stat_unmatch++;
+			diff_free_filepair(p);
+		}
+	}
+	free(q->queue);
+	*q = outq;
+}
+
 void diffcore_std(struct diff_options *options)
 {
 	if (options->quiet)
 		return;
 
+	if (options->skip_stat_unmatch && !options->find_copies_harder)
+		diffcore_skip_stat_unmatch(options);
 	if (options->break_opt != -1)
 		diffcore_break(options->break_opt);
 	if (options->detect_rename)
diff --git a/diff.h b/diff.h
index 9fd6d44..de21f8e 100644
--- a/diff.h
+++ b/diff.h
@@ -65,6 +65,7 @@ struct diff_options {
 	int context;
 	int break_opt;
 	int detect_rename;
+	int skip_stat_unmatch;
 	int line_termination;
 	int output_format;
 	int pickaxe_opts;

Re: [PATCH] Add --show-touched option to show "diff --git" line when contents are unchanged

[Matthieu Moy]

Junio C Hamano <gitster@pobox.com> writes:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
>> But I'd really think that what should be done (if anything has to be done 
>> at all) is to introduce a config variable which triggers the same logic in 
>> git-diff as was introduced in 2b5f9a8c0cff511f2bb0833b1ee02645b79323f4.
>
> Sorry, I don't follow at all.  The diff toolchain works all
> inside core without having to write a temporary index out, which
> was the issue the commit you are quoting was about.
>
> In any case, enough discussion.  Here is an updated patch, which
> I _could_ be pursuaded to consider for inclusion after v1.5.3
> happens, if there are enough agreements and Acks.

To me, that patch makes sense, yes.

That said, a configuration option would probably be better than a
command-line option. The expected behavior seems to depend on user,
but not much on use-cases. So, people who like the old behavior could
set the option and forget about it, and other would not be distracted
about it.

Also, is there any particular reason not to update the index stat
information when files are found to be identical? Well, we've
discussed that quite much in this thread, but this is what status is
doing, and I still fail to see why diff shouldn't. (I can update the
patch myself if you agree it should be done. I don't know the git
codebase well, so it takes time for me, but the exercise is fun ;-) )

Side performance note: if I understand your patch correctly, you're
comparing the file content twice for actually modified changes. But
that probably doesn't matter much since the expansive thing is to load
the files and to compute the diff.

Thanks,

-- 
Matthieu

Re: [PATCH] Add --show-touched option to show "diff --git" line when contents are unchanged

[Junio C Hamano]

Matthieu Moy <Matthieu.Moy@imag.fr> writes:

> Also, is there any particular reason not to update the index stat
> information when files are found to be identical?

Very much --- diff is a read-only operation.

"git-status $args" on the other hand is a preview of "what would
happen if I say 'git-commit $args'", and in order to compute
that, you would fundamentally need to be able to write into the
object store.  In a special case of giving empty $args it can be
read-only.  The commit Dscho quoted earlier was to hack that
around so that "git-status" can pretend to be a read-only
operation in a repository you do not have write permission to.

Re: [PATCH] Add --show-touched option to show "diff --git" line when contents are unchanged

[Matthieu Moy]

Junio C Hamano <gitster@pobox.com> writes:

> Matthieu Moy <Matthieu.Moy@imag.fr> writes:
>
>> Also, is there any particular reason not to update the index stat
>> information when files are found to be identical?
>
> Very much --- diff is a read-only operation.
>
> "git-status $args" on the other hand is a preview of "what would
> happen if I say 'git-commit $args'", and in order to compute
> that, you would fundamentally need to be able to write into the
> object store.  In a special case of giving empty $args it can be
> read-only.

Can you give an example where it _could_ not be read-only?

> The commit Dscho quoted earlier was to hack that around so that
> "git-status" can pretend to be a read-only operation in a repository
> you do not have write permission to.

I really, really, really, don't understand your argument.

There are two different concepts:

* Should git-diff be "read-only for the user"?
  (i.e. external specification)

* Should git-diff be actually read-only for the filesystem?
  (i.e. implementation)


"read-only for the user" is a user-interface thing. It just means that
running

$ git-diff >& /dev/null; whatever

will give the same result as

$ whatever

In another context, "cat", for example, is a read-only operation for
the user. I can run "cat whatever-file" without influencing the
behavior of subsequent operations. Now, somewhere in the kernel of my
OS, I do hope that reading this file will have side-effects (putting
the file in the cache, and why not decide to physically move the file
on disk).


Here, obviously, git-diff is a read-only operation for the user. I
don't expect git-diff to modify the behavior of subsequent commands,
but I appreciate if git-diff can improve the speed of subsequent
commands.

Now, both of us agree that git-status should not be read-only for the
filesystem, both of us agree that git-diff should be read-only for the
user, but we disagree on the two other cases.

In the same way, I expect git-status to be read-only for the user. You
say "what _would_ happen _if_ I say commit $args". But you don't
commit, the sentence is conditionnal. I don't expect any tool to have
visible side-effects when I say "what would happen if ...".

-- 
Matthieu

Re: [PATCH] Add --show-touched option to show "diff --git" line when contents are unchanged

[Junio C Hamano]

Matthieu Moy <Matthieu.Moy@imag.fr> writes:

Matthieu Moy <Matthieu.Moy@imag.fr> writes:

>> "git-status $args" on the other hand is a preview of "what would
>> happen if I say 'git-commit $args'", and in order to compute
>> that, you would fundamentally need to be able to write into the
>> object store.  In a special case of giving empty $args it can be
>> read-only.
>
> Can you give an example where it _could_ not be read-only?

Think of what "git commit -a" would have to do.  It needs to
hash and deposit a new object for blobs that have been
modified.  Where do those new blob object go?

> In the same way, I expect git-status to be read-only for the user. You
> say "what _would_ happen _if_ I say commit $args". But you don't
> commit, the sentence is conditionnal. I don't expect any tool to have
> visible side-effects when I say "what would happen if ...".

By running git-status the user is asking to get the overall
picture, discarding the "touched but not modified" information.
Once you _HAVE TO_ refresh the index for whatever reason, it is
better to keep the result of the effort and cycles spent for
that refresh operation for obvious performance reasons in
practice, and that is what we have now in git-status.  IOW, we
are practical bunch.

Maybe in a theoretical ideal world, you might prefer to
reverting back to the stat-dirty original index to make
git-status appear a read-only operation, with continued degraded
performance.  You are welcome to reimplement it that way, and
the patch should be trivial (while git-commit.sh is still a
script, at least) but that is not what we did.

Re: [PATCH] Add --show-touched option to show "diff --git" line when contents are unchanged

[Matthieu Moy]

Junio C Hamano <gitster@pobox.com> writes:

> Matthieu Moy <Matthieu.Moy@imag.fr> writes:
>
>>> "git-status $args" on the other hand is a preview of "what would
>>> happen if I say 'git-commit $args'", and in order to compute
>>> that, you would fundamentally need to be able to write into the
>>> object store.  In a special case of giving empty $args it can be
>>> read-only.
>>
>> Can you give an example where it _could_ not be read-only?
>
> Think of what "git commit -a" would have to do.

I don't know whether it was a typo, but we're not talking about
"commit", but "status".

> It needs to hash and deposit a new object for blobs that have been
> modified. Where do those new blob object go?

git-status _does_ hash and deposit new objects, but it doesn't _need_
to. It can very well show you what "commit -a" would do without
actually doing it.

A trivial (and very stupid, yes) way to do this would be

cp -r . /tmp/git/
cd /tmp/git
git-status -a

There's no visible side-effects for the user.

IIRC, git-status -a does actually "git-add" the modified objects, but
does so in a temporary index, so I believe the objects you leave in
the objects database are not pointed to by anyone (indeed, I just
checked, git-fsck --unreachable shows the dangling blob), and are not
really useful (but will probably be used later when you run commit or
add).

> Maybe in a theoretical ideal world, you might prefer to
> reverting back to the stat-dirty original index to make
> git-status appear a read-only operation, with continued degraded
> performance.  You are welcome to reimplement it that way, and
> the patch should be trivial (while git-commit.sh is still a
> script, at least) but that is not what we did.

You still didn't understand my point about the difference between
user-specification and internal behavior. I'm very happy with
git-status updating the stat information in the index, since it is not
suppose to have user-visible side effects (it has with the current
empty-diff-for-touched-files behavior of git-diff).

Now, at that point, if I still didn't manage to show you the
difference between user-visible behavior and implementation, I believe
I have no better thing to do than giving up.

-- 
Matthieu

Re: [PATCH] Add --show-touched option to show "diff --git" line when contents are unchanged

[Johannes Schindelin]

Hi,

On Sun, 5 Aug 2007, Matthieu Moy wrote:

> Junio C Hamano <gitster@pobox.com> writes:
> 
> > Matthieu Moy <Matthieu.Moy@imag.fr> writes:
> >
> >>> "git-status $args" on the other hand is a preview of "what would
> >>> happen if I say 'git-commit $args'", and in order to compute
> >>> that, you would fundamentally need to be able to write into the
> >>> object store.  In a special case of giving empty $args it can be
> >>> read-only.
> >>
> >> Can you give an example where it _could_ not be read-only?
> >
> > Think of what "git commit -a" would have to do.
> 
> I don't know whether it was a typo, but we're not talking about
> "commit", but "status".

No typo.  "git status" is literally "git commit --dry-run".  Why?  Because 
people expected it to be called "git status".

And even if I think about it over and over again, it makes sense.

Ciao,
Dscho

Re: [PATCH] Add --show-touched option to show "diff --git" line when contents are unchanged

[Matthieu Moy]

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

>> I don't know whether it was a typo, but we're not talking about
>> "commit", but "status".
>
> No typo.  "git status" is literally "git commit --dry-run".  Why?  Because 
> people expected it to be called "git status".
>
> And even if I think about it over and over again, it makes sense.

I was surprised of that when looking at the source, but yes, it makes
sense.

But the message I was replying to claimed that 
"git status-or-commit -a" _needed_ to put objects in the object
database. That's true of "git commit -a" but not of "git status -a",
even if you call it "git commit --dry-run -a", precisely because of
the --dry-run thing.

-- 
Matthieu

Re: [PATCH] Add --show-touched option to show "diff --git" line when contents are unchanged

[Matthias Lederhofer]

Junio C Hamano <gitster@pobox.com> wrote:
> In any case, enough discussion.  Here is an updated patch, which
> I _could_ be pursuaded to consider for inclusion after v1.5.3
> happens, if there are enough agreements and Acks.

I like this new behaviour but I don't see the old one too often
either.

Re: [PATCH] Add --show-touched option to show "diff --git" line when contents are unchanged

[David Kastrup]

Junio C Hamano <gitster@pobox.com> writes:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
>> But I'd really think that what should be done (if anything has to be done 
>> at all) is to introduce a config variable which triggers the same logic in 
>> git-diff as was introduced in 2b5f9a8c0cff511f2bb0833b1ee02645b79323f4.
>
> Sorry, I don't follow at all.  The diff toolchain works all
> inside core without having to write a temporary index out, which
> was the issue the commit you are quoting was about.
>
> In any case, enough discussion.  Here is an updated patch, which
> I _could_ be pursuaded to consider for inclusion after v1.5.3
> happens, if there are enough agreements and Acks.

Ack, ack, ack.  The current default behavior is plainly unusable.  For
example, I've rsynced -a a tree including .git, and suddenly git-diff
goes out of kilter.  And stops doing so when running git-status once.

This is the worst kind of "unpredictable", and I don't care one bit
that there are conceivable use cases.

I don't even think it prudent to _offer_ the --show-touched option in
a porcelain such as git-diff as long as purportedly read-only
porcelain commands like git-status can trash the state: what is
reported is not actually "touched" but something internal to the
operation of git.

At least not without a notice in the manual that this option might or
might not work, depending on what one did previously.

-- 
David Kastrup

Re: [PATCH] Add --show-touched option to show "diff --git" line when contents are unchanged

[David Kastrup]

David Kastrup <dak@gnu.org> writes:

> I don't even think it prudent to _offer_ the --show-touched option
> in a porcelain such as git-diff as long as purportedly read-only
> porcelain commands like git-status can trash the state: what is
> reported is not actually "touched" but something internal to the
> operation of git.
>
> At least not without a notice in the manual that this option might
> or might not work, depending on what one did previously.

Proposal: if this option is to stay, call it rather --show-stale since
that corresponds better with what the option actually does: show
whether git's inode cache went stale.  It does _not_ show whether the
file has been touched (git-status does not touch files, for example).

-- 
David Kastrup

Re: [PATCH] Add --show-touched option to show "diff --git" line when contents are unchanged

[Matthieu Moy]

David Kastrup <dak@gnu.org> writes:

> Ack, ack, ack.  The current default behavior is plainly unusable.  For
> example, I've rsynced -a a tree including .git, and suddenly git-diff
> goes out of kilter.  And stops doing so when running git-status
> once.

Unfortunately, the patch solves the "large and irrelevant output" of
git-diff, but not the performance problem (see the rest of the thread,
I failed to convince Junio that updating the index was a performance
improvement while keeping the same user semantics).

-- 
Matthieu

Re: [PATCH] Add --show-touched option to show "diff --git" line when contents are unchanged

[Junio C Hamano]

Matthieu Moy <Matthieu.Moy@imag.fr> writes:

> Unfortunately, the patch solves the "large and irrelevant output" of
> git-diff, but not the performance problem (see the rest of the thread,
> I failed to convince Junio that updating the index was a performance
> improvement while keeping the same user semantics).

That's what update-index --refresh (or status if you insist) are
for, and the coalmine canary you are so dead set to kill are
helping you realize the need for running.

Re: [PATCH] Add --show-touched option to show "diff --git" line when contents are unchanged

[David Kastrup]

Junio C Hamano <gitster@pobox.com> writes:

> Matthieu Moy <Matthieu.Moy@imag.fr> writes:
>
>> Unfortunately, the patch solves the "large and irrelevant output"
>> of git-diff, but not the performance problem (see the rest of the
>> thread, I failed to convince Junio that updating the index was a
>> performance improvement while keeping the same user semantics).
>
> That's what update-index --refresh (or status if you insist) are
> for, and the coalmine canary you are so dead set to kill are helping
> you realize the need for running.

That does not convince me.  Cache staleness should be a problem of
git, not of the user.  In particular if the user is just using
porcelain.  If letting the cache get stale impacts performance, then
git should clean up its act on its own without barfing when using
unrelated commands.  If it notices this during diff (presumably by
overstepping some staleness ratio), then it can set a "regenerate on
next opportunity" flag on the index, and then the next command wanting
to process the index from the start can rewrite a refreshed version.

-- 
David Kastrup

Re: [PATCH] Add --show-touched option to show "diff --git" line when contents are unchanged

[J. Bruce Fields]

On Tue, Aug 07, 2007 at 08:32:55AM +0200, David Kastrup wrote:
> Junio C Hamano <gitster@pobox.com> writes:
> 
> > Matthieu Moy <Matthieu.Moy@imag.fr> writes:
> >
> >> Unfortunately, the patch solves the "large and irrelevant output"
> >> of git-diff, but not the performance problem (see the rest of the
> >> thread, I failed to convince Junio that updating the index was a
> >> performance improvement while keeping the same user semantics).
> >
> > That's what update-index --refresh (or status if you insist) are
> > for, and the coalmine canary you are so dead set to kill are helping
> > you realize the need for running.
> 
> That does not convince me.  Cache staleness should be a problem of
> git, not of the user.  In particular if the user is just using
> porcelain.  If letting the cache get stale impacts performance, then
> git should clean up its act on its own without barfing when using
> unrelated commands.  If it notices this during diff (presumably by
> overstepping some staleness ratio), then it can set a "regenerate on
> next opportunity" flag on the index, and then the next command wanting
> to process the index from the start can rewrite a refreshed version.

The last time I had a serious problem with "cache staleness", it was
with Beagle, which modifies the files it indexes (by writing some
extended attributes).  I figured out what was happening when I noticed
that the list of touched files was growing each time I did a diff
(implying the something was working on them right then), so I ran top,
noticed beagled, eventually thought to query the extended attributes,
and finally turned off beagled's indexing to solve the problem.

So, in this case:

	- If git had fixed up the problem silently, I probably would
	  have just assumed git was slow and not found the problem.

	- Seeing the actual list of files for which the index was dirty
	  helped me identify the problem.  I probably would have
	  eventually figured it out even if all I'd had was a single
	  "index is stale" message, but I suspect it would have taken
	  longer.

Draw whatever moral you'd like....

--b.

Re: [PATCH] Add --show-touched option to show "diff --git" line when contents are unchanged

[Linus Torvalds]

On Thu, 2 Aug 2007, Steven Grimm wrote:
>
> The default is now to not show the diff --git header line if the file's
> timestamp has changed but the contents and/or file mode haven't.

I don't mind this per se, but I'd *really* want some kind of warning that 
the index is not up-to-date.

Otherwise, git usage can be horrendously slow, and you're never even told 
why. The diffs just take lots of time (because it reads each file), but 
the output is empty.

> 	Personally I'm in favor of doing away with the option altogether
> 	and having the code always work the way it works by default with
> 	this patch, but if some people find the old behavior useful they
> 	can still get at it with the new option.

It's not that the old output is "useful" in itself, but it's important for 
people to know that the index is clean. So I'd suggest just setting a flag 
when the header isn't printed, and then printing out a single line at the 
end about "git index not up-to-date" or something.

Doing a "git diff" cannot actually update the index (since it very much 
has to work on a read-only setup too), which is why the index _stays_ 
stale unless something is done (eg "git status") to refresh it. And it's 
that stale index that continues to make for bad performance without any 
indication of why that is a problem.

			Linus

Re: [PATCH] Add --show-touched option to show "diff --git" line when contents are unchanged

[Junio C Hamano]

Linus Torvalds <torvalds@linux-foundation.org> writes:

> It's not that the old output is "useful" in itself, but it's important for 
> people to know that the index is clean. So I'd suggest just setting a flag 
> when the header isn't printed, and then printing out a single line at the 
> end about "git index not up-to-date" or something.

That's essentially the patch I sent out in another thread allows
you to do, and some of these people even Acked them, but there
is one minor issue.  "git diff" output is paged, and that "not
up to date" warning, if it is given to stderr, would not be
usually seen.

> Doing a "git diff" cannot actually update the index (since it very much 
> has to work on a read-only setup too), which is why the index _stays_ 
> stale unless something is done (eg "git status") to refresh it. And it's 
> that stale index that continues to make for bad performance without any 
> indication of why that is a problem.

Indeed.

At least, I am now glad to know that somebody else is of the
same opinion as I am.

Re: [PATCH] Add --show-touched option to show "diff --git" line when contents are unchanged

[David Kastrup]

Junio C Hamano <gitster@pobox.com> writes:

> Linus Torvalds <torvalds@linux-foundation.org> writes:
>
>> Doing a "git diff" cannot actually update the index (since it very
>> much has to work on a read-only setup too), which is why the index
>> _stays_ stale unless something is done (eg "git status") to refresh
>> it. And it's that stale index that continues to make for bad
>> performance without any indication of why that is a problem.
>
> Indeed.
>
> At least, I am now glad to know that somebody else is of the same
> opinion as I am.

I don't want a system to tell me when it is shooting itself in the
foot.  It should not be doing this in the first place.

File systems have automatic fsck procedures enforced regularly, too,
to keep them operative. If git finds that it is getting inefficient,
it should just mark the index as "regenerate at next access".  And
then do it.

-- 
David Kastrup

Re: [PATCH] Add --show-touched option to show "diff --git" line when contents are unchanged

[Linus Torvalds]

[ Slow at responding to email, sorry ]

On Mon, 6 Aug 2007, Junio C Hamano wrote:
> 
> That's essentially the patch I sent out in another thread allows
> you to do, and some of these people even Acked them, but there
> is one minor issue.  "git diff" output is paged, and that "not
> up to date" warning, if it is given to stderr, would not be
> usually seen.

I agree. I wouldn't send it to stderr, I'd literally make it part of the 
output.

Everybody who takes patches will accept crud afterwards, since the normal 
thing is to email them around, so there's no real downside to adding some 
status output at the end. It shouldn't screw anything up, but people will 
hopefully notice (sure, if you exit the pager without looking at it all 
you wouldn't notice, but that's _already_ true, so..)

		Linus

Re: [PATCH] Add --show-touched option to show "diff --git" line when contents are unchanged

[Junio C Hamano]

Linus Torvalds <torvalds@linux-foundation.org> writes:

> Everybody who takes patches will accept crud afterwards, since the normal 
> thing is to email them around, so there's no real downside to adding some 
> status output at the end. It shouldn't screw anything up, but people will 
> hopefully notice (sure, if you exit the pager without looking at it all 
> you wouldn't notice, but that's _already_ true, so..)

Well, at least "hundreds of empty diff" has a value of getting
attention for even such a use case, so you _could_ argue this
patch is a regression ;-)

In any case, this will go in as part of the first batch after
1.5.3, I would guess.

Re: [PATCH] Add --show-touched option to show "diff --git" line when contents are unchanged

[Johannes Schindelin]

Hi,

On Tue, 7 Aug 2007, Linus Torvalds wrote:

> Everybody who takes patches will accept crud afterwards, since the 
> normal thing is to email them around, so there's no real downside to 
> adding some status output at the end. It shouldn't screw anything up, 
> but people will hopefully notice (sure, if you exit the pager without 
> looking at it all you wouldn't notice, but that's _already_ true, so..)

But then, you could output the message _twice_: first possibly in-between 
patches ("WARNING: ...") and then at the end.

However, I have the slight suspicion that people will not even notice.  I 
mean, we had bug reports on merge-recursive, where the reporter failed to 
even acknowledge the fact that merge-recursive said that there were 
conflicts, and even listed them.

So I have the slight suspicion that all this will accomplish is "shut the 
darn thing up", and old-timers will have a harder time, since they no 
longer spot easily when they did a Dumb Thing and left the index out of 
sync.

Slightly negative.

Ciao,
Dscho

Re: [PATCH] Add --show-touched option to show "diff --git" line when contents are unchanged

[Junio C Hamano]

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

> So I have the slight suspicion that all this will accomplish is "shut the 
> darn thing up", and old-timers will have a harder time, since they no 
> longer spot easily when they did a Dumb Thing and left the index out of 
> sync.

The hardest hit would be old-timers who try to be friendly by
trying to help new people, who has much less chance to notice
and report these much less prominent warnings, over e-mail or
irc.

Re: [PATCH] Add --show-touched option to show "diff --git" line when contents are unchanged

[David Kastrup]

Junio C Hamano <gitster@pobox.com> writes:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
>> So I have the slight suspicion that all this will accomplish is "shut the 
>> darn thing up", and old-timers will have a harder time, since they no 
>> longer spot easily when they did a Dumb Thing and left the index out of 
>> sync.
>
> The hardest hit would be old-timers who try to be friendly by
> trying to help new people, who has much less chance to notice
> and report these much less prominent warnings, over e-mail or
> irc.

"Dude, you got a stale index hanging out of your trousers."

I find it ridiculous to parade local problems in patches sent out to
the world rather than fixing them, so that old-timers have a chance to
get karma points.

Really: if stale indexes are considered a problem, git should silently
mark the staleness in the file, and the next time (or after three more
times or whatever) the index is used, it is silently regenerated.

Old-timers can get an option disabling this so that they can proud
themselves on cleaning up after themselves consciously, but for the
normal user, this is a bother he can do without.

-- 
David Kastrup

Re: [PATCH] Add --show-touched option to show "diff --git" line when contents are unchanged

[Johannes Schindelin]

Hi,

On Wed, 8 Aug 2007, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > So I have the slight suspicion that all this will accomplish is "shut the 
> > darn thing up", and old-timers will have a harder time, since they no 
> > longer spot easily when they did a Dumb Thing and left the index out of 
> > sync.
> 
> The hardest hit would be old-timers who try to be friendly by
> trying to help new people, who has much less chance to notice
> and report these much less prominent warnings, over e-mail or
> irc.

True.  It is even bigger than that annoyance to people who know how git 
works.

Ciao,
Dscho

Re: [PATCH] Add --show-touched option to show "diff --git" line when contents are unchanged

[Jakub Narebski]

Johannes Schindelin wrote:

> On Wed, 8 Aug 2007, Junio C Hamano wrote:
> 
>> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>> 
>>> So I have the slight suspicion that all this will accomplish is "shut the 
>>> darn thing up", and old-timers will have a harder time, since they no 
>>> longer spot easily when they did a Dumb Thing and left the index out of 
>>> sync.
>> 
>> The hardest hit would be old-timers who try to be friendly by
>> trying to help new people, who has much less chance to notice
>> and report these much less prominent warnings, over e-mail or
>> irc.
> 
> True.  It is even bigger than that annoyance to people who know how git 
> works.

Perhaps config variable, by default old behaviour if not set?

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

Re: [PATCH] Add --show-touched option to show "diff --git" line when contents are unchanged

[Steven Grimm]

Linus Torvalds wrote:
> It's not that the old output is "useful" in itself, but it's important for 
> people to know that the index is clean. So I'd suggest just setting a flag 
> when the header isn't printed, and then printing out a single line at the 
> end about "git index not up-to-date" or something.
>   

Or even a count of the number of files whose index data is unclean. I'd 
be fine with that as a suffix to the diff output.

> Doing a "git diff" cannot actually update the index (since it very much 
> has to work on a read-only setup too), which is why the index _stays_ 
> stale unless something is done (eg "git status") to refresh it. And it's 
> that stale index that continues to make for bad performance without any 
> indication of why that is a problem.
>   

I totally agree that there needs to be a way to tell if the index is 
clean or not. I do wonder if the default output of "git diff" is the 
right place for that information, but if the notification can be 
collapsed to a line or two (rather than the unbounded number of lines 
that it potentially outputs now) then that's probably good enough.

Actually, though this will probably make people roll their eyes, before 
this discussion I would have guessed that "git status" would be the 
command that would tell you the index was out of date, and that there'd 
be a separate command (say, "git update-index"?) that you could then use 
to sync things up again. The fact that "git status" is really "git 
update index a little bit then show status" was not something I 
expected; it presents itself as a query utility, not an update utility, 
so I would have expected it to be read-only. Its index-modifying 
behavior is not even hinted at in the documentation (a patch for which 
follows.)

-Steve

[PATCH] Add a note about the index being updated by git-status in some cases

[Steven Grimm]

Signed-off-by: Steven Grimm <koreth@midwinter.com>
---
 Documentation/git-status.txt |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/Documentation/git-status.txt b/Documentation/git-status.txt
index 6f16eb0..8fd0fc6 100644
--- a/Documentation/git-status.txt
+++ b/Documentation/git-status.txt
@@ -27,6 +27,13 @@ The command takes the same set of options as `git-commit`; it
 shows what would be committed if the same options are given to
 `git-commit`.
 
+If any paths have been touched in the working tree (that is,
+their modification times have changed) but their contents and
+permissions are identical to those in the index file, the command
+updates the index file. Running `git-status` can thus speed up
+subsequent operations such as `git-diff` if the working tree
+contains many paths that have been touched but not modified.
+
 
 OUTPUT
 ------
-- 
1.5.3.rc2.4.g726f9

[PATCH] git-diff: Output a warning about stale files in the index

[Steven Grimm]

Signed-off-by: Steven Grimm <koreth@midwinter.com>
---
	This is based on (and includes) Junio's patch. This should
	hopefully address the "I want to know when my index is very
	stale" problem with both his original patch and mine.

	If we are running a pager, I output the warning to standard
	output so it doesn't get immediately scrolled off the screen by
	the paged diff output. Otherwise I output to standard error
	which is really the more appropriate place for the warning.
	Obviously that is no good if the user is running his own pager,
	but I'm not sure how to detect that and not cause problems for
	diffs that are piped into other programs.

 diff.c     |   59 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++---
 diffcore.h |    1 +
 2 files changed, 57 insertions(+), 3 deletions(-)

diff --git a/diff.c b/diff.c
index a5fc56b..7b11195 100644
--- a/diff.c
+++ b/diff.c
@@ -2979,7 +2979,7 @@ int diff_flush_patch_id(struct diff_options *options, unsigned char *sha1)
 
 	free(q->queue);
 	q->queue = NULL;
-	q->nr = q->alloc = 0;
+	q->nr = q->alloc = q->removed = 0;
 
 	return result;
 }
@@ -3015,6 +3015,17 @@ void diff_flush(struct diff_options *options)
 	int i, output_format = options->output_format;
 	int separator = 0;
 
+	if (q->removed > 0 && ! (output_format & DIFF_FORMAT_NO_OUTPUT)) {
+		char *format = "Warning: %d %s touched but not modified. "
+			       "Consider running git-status.\n";
+		char *plural = q->removed == 1 ? "path" : "paths";
+
+		if (pager_in_use)
+			printf(format, q->removed, plural);
+		else
+			fprintf(stderr, format, q->removed, plural);
+	}
+
 	/*
 	 * Order: raw, stat, summary, patch
 	 * or:    name/name-status/checkdiff (other bits clear)
@@ -3084,7 +3095,7 @@ void diff_flush(struct diff_options *options)
 free_queue:
 	free(q->queue);
 	q->queue = NULL;
-	q->nr = q->alloc = 0;
+	q->nr = q->alloc = q->removed = 0;
 }
 
 static void diffcore_apply_filter(const char *filter)
@@ -3093,7 +3104,7 @@ static void diffcore_apply_filter(const char *filter)
 	struct diff_queue_struct *q = &diff_queued_diff;
 	struct diff_queue_struct outq;
 	outq.queue = NULL;
-	outq.nr = outq.alloc = 0;
+	outq.nr = outq.alloc = outq.removed = 0;
 
 	if (!filter)
 		return;
@@ -3143,6 +3154,47 @@ static void diffcore_apply_filter(const char *filter)
 	*q = outq;
 }
 
+static void diffcore_remove_empty(void)
+{
+	int i;
+	struct diff_queue_struct *q = &diff_queued_diff;
+	struct diff_queue_struct outq;
+	outq.queue = NULL;
+	outq.nr = outq.alloc = outq.removed = 0;
+
+	for (i = 0; i < q->nr; i++) {
+		struct diff_filepair *p = q->queue[i];
+
+		/*
+		 * 1. Keep the ones that cannot be diff-files
+		 *    "false" match that are only queued due to
+		 *    cache dirtyness.
+		 *
+		 * 2. Modified, same size and mode, and the object
+		 *    name of one side is unknown.  If they do not
+		 *    have identical contents, keep them.
+		 *    They are different.
+		 */
+		if ((p->status != DIFF_STATUS_MODIFIED) || /* (1) */
+		    (p->one->sha1_valid && p->two->sha1_valid) ||
+		    (p->one->mode != p->two->mode) ||
+
+		    diff_populate_filespec(p->one, 1) || /* (2) */
+		    diff_populate_filespec(p->two, 1) ||
+		    (p->one->size != p->two->size) ||
+		    diff_populate_filespec(p->one, 0) ||
+		    diff_populate_filespec(p->two, 0) ||
+		    memcmp(p->one->data, p->two->data, p->one->size))
+			diff_q(&outq, p);
+		else {
+			diff_free_filepair(p);
+			outq.removed++;
+		}
+	}
+	free(q->queue);
+	*q = outq;
+}
+
 void diffcore_std(struct diff_options *options)
 {
 	if (options->quiet)
@@ -3160,6 +3212,7 @@ void diffcore_std(struct diff_options *options)
 		diffcore_order(options->orderfile);
 	diff_resolve_rename_copy();
 	diffcore_apply_filter(options->filter);
+	diffcore_remove_empty();
 
 	options->has_changes = !!diff_queued_diff.nr;
 }
diff --git a/diffcore.h b/diffcore.h
index eef17c4..e5a9244 100644
--- a/diffcore.h
+++ b/diffcore.h
@@ -81,6 +81,7 @@ struct diff_queue_struct {
 	struct diff_filepair **queue;
 	int alloc;
 	int nr;
+	int removed;
 };
 
 extern struct diff_queue_struct diff_queued_diff;
-- 
1.5.3.rc2.4.g726f9

Re: [PATCH] git-diff: Output a warning about stale files in the index

[Junio C Hamano]

Steven Grimm <koreth@midwinter.com> writes:

> Signed-off-by: Steven Grimm <koreth@midwinter.com>
> ---
> 	This is based on (and includes) Junio's patch. This should
> 	hopefully address the "I want to know when my index is very
> 	stale" problem with both his original patch and mine.
>
> 	If we are running a pager, I output the warning to standard
> 	output so it doesn't get immediately scrolled off the screen by
> 	the paged diff output. Otherwise I output to standard error
> 	which is really the more appropriate place for the warning.
> 	Obviously that is no good if the user is running his own pager,
> 	but I'm not sure how to detect that and not cause problems for
> 	diffs that are piped into other programs.

Hmph.  One way to avoid causing problems for diffs that are
piped into other programs and still give the "index of sync"
warning is to emit "diff --git" line and no patch body fot
textual diffs, or 0{40} SHA-1 on the right hand side for --raw
format diffs.

Jokes aside...

For textual diffs, I think we can always spit out the warning
message at the beginning of at the end on the standard output
without harming any of the patch based toolchain.

So how about...

 - If and only if the output format asks for textual diff
   (DIFF_FORMAT_PATCH), we do this "stat-dirty-removal";
   otherwise we do not spend extra cycles and keep the current
   behaviour.

 - At the end of patch text, show "stat-dirty-removal" warning
   on stdout.

[PATCH v2] git-diff: Output a warning about stale files in the index

[Steven Grimm]

Signed-off-by: Steven Grimm <koreth@midwinter.com>
---
	Modified as suggested by Junio.

 diff.c     |   55 ++++++++++++++++++++++++++++++++++++++++++++++++++++---
 diffcore.h |    1 +
 2 files changed, 53 insertions(+), 3 deletions(-)

diff --git a/diff.c b/diff.c
index a5fc56b..5f2e1fe 100644
--- a/diff.c
+++ b/diff.c
@@ -2979,7 +2979,7 @@ int diff_flush_patch_id(struct diff_options *options, unsigned char *sha1)
 
 	free(q->queue);
 	q->queue = NULL;
-	q->nr = q->alloc = 0;
+	q->nr = q->alloc = q->removed = 0;
 
 	return result;
 }
@@ -3074,6 +3074,12 @@ void diff_flush(struct diff_options *options)
 			if (check_pair_status(p))
 				diff_flush_patch(p, options);
 		}
+
+		if (q->removed > 0) {
+			printf("Warning: %d %s touched but not modified. "
+			       "Consider running git-status.\n",
+			       q->removed, q->removed == 1 ? "path" : "paths");
+		}
 	}
 
 	if (output_format & DIFF_FORMAT_CALLBACK)
@@ -3084,7 +3090,7 @@ void diff_flush(struct diff_options *options)
 free_queue:
 	free(q->queue);
 	q->queue = NULL;
-	q->nr = q->alloc = 0;
+	q->nr = q->alloc = q->removed = 0;
 }
 
 static void diffcore_apply_filter(const char *filter)
@@ -3093,7 +3099,7 @@ static void diffcore_apply_filter(const char *filter)
 	struct diff_queue_struct *q = &diff_queued_diff;
 	struct diff_queue_struct outq;
 	outq.queue = NULL;
-	outq.nr = outq.alloc = 0;
+	outq.nr = outq.alloc = outq.removed = 0;
 
 	if (!filter)
 		return;
@@ -3143,6 +3149,47 @@ static void diffcore_apply_filter(const char *filter)
 	*q = outq;
 }
 
+static void diffcore_remove_empty(void)
+{
+	int i;
+	struct diff_queue_struct *q = &diff_queued_diff;
+	struct diff_queue_struct outq;
+	outq.queue = NULL;
+	outq.nr = outq.alloc = outq.removed = 0;
+
+	for (i = 0; i < q->nr; i++) {
+		struct diff_filepair *p = q->queue[i];
+
+		/*
+		 * 1. Keep the ones that cannot be diff-files
+		 *    "false" match that are only queued due to
+		 *    cache dirtyness.
+		 *
+		 * 2. Modified, same size and mode, and the object
+		 *    name of one side is unknown.  If they do not
+		 *    have identical contents, keep them.
+		 *    They are different.
+		 */
+		if ((p->status != DIFF_STATUS_MODIFIED) || /* (1) */
+		    (p->one->sha1_valid && p->two->sha1_valid) ||
+		    (p->one->mode != p->two->mode) ||
+
+		    diff_populate_filespec(p->one, 1) || /* (2) */
+		    diff_populate_filespec(p->two, 1) ||
+		    (p->one->size != p->two->size) ||
+		    diff_populate_filespec(p->one, 0) ||
+		    diff_populate_filespec(p->two, 0) ||
+		    memcmp(p->one->data, p->two->data, p->one->size))
+			diff_q(&outq, p);
+		else {
+			diff_free_filepair(p);
+			outq.removed++;
+		}
+	}
+	free(q->queue);
+	*q = outq;
+}
+
 void diffcore_std(struct diff_options *options)
 {
 	if (options->quiet)
@@ -3160,6 +3207,8 @@ void diffcore_std(struct diff_options *options)
 		diffcore_order(options->orderfile);
 	diff_resolve_rename_copy();
 	diffcore_apply_filter(options->filter);
+	if (options->output_format & DIFF_FORMAT_PATCH)
+		diffcore_remove_empty();
 
 	options->has_changes = !!diff_queued_diff.nr;
 }
diff --git a/diffcore.h b/diffcore.h
index eef17c4..e5a9244 100644
--- a/diffcore.h
+++ b/diffcore.h
@@ -81,6 +81,7 @@ struct diff_queue_struct {
 	struct diff_filepair **queue;
 	int alloc;
 	int nr;
+	int removed;
 };
 
 extern struct diff_queue_struct diff_queued_diff;
-- 
1.5.3.rc2.4.g726f9

Re: [PATCH v2] git-diff: Output a warning about stale files in the index

[Junio C Hamano]

Steven Grimm <koreth@midwinter.com> writes:

> Signed-off-by: Steven Grimm <koreth@midwinter.com>
> ---
> 	Modified as suggested by Junio.

Okay.

Assuming that other people are happy with this version (I have
to warn you that I haven't even attempted to apply this patch,
let alone compiling yet), I'd prefer to keep our combined
thought process in the commit log, so that we do not have to
rehash this later, over and over again.

Something along the following lines, perhaps...?

  After starting to edit a working tree file but later when your
  edit ends up identical to the original (this can also happen
  when you ran a wholesale regexp replace with something like
  "perl -i" that does not touch many of the paths), "git diff"
  between the index and the working tree outputs many "empty"
  diffs that show "diff --git" header and nothing else, because
  these paths are stat dirty.  While it was _a_ way to warn the
  user that the earlier action of the user made the index
  ineffective as an optimization mechanism, it was felt too loud
  for the purpose of warning even to experienced users, and also
  resulted in confusing people new to git.

  This replaces the "empty" diffs with a single warning message
  at the end.  When you see such a message, you know you did
  something suboptimal to your index; you can optimize the index
  again by running "git-update-index --refresh".

  The change affects only "git diff" that outputs patch text,
  because that is where the annoyance of too many "empty" diff
  is most strongly felt, and because the warning message can be
  safely ignored by downstream tools without getting mistaken as
  part of the patch.  For the low-level "git diff-files", the
  traditional behaviour is retained.

Re: [PATCH v2] git-diff: Output a warning about stale files in the index

[Steven Grimm]

Junio C Hamano wrote:
> Something along the following lines, perhaps...?
>   

That seems like a fine commit message to me. My one-liner definitely 
assumed too much context, coming in the middle of this discussion as it 
did -- much better to be explicit about the reasoning for posterity's sake.

-Steve

Re: [PATCH v2] git-diff: Output a warning about stale files in the index

[Jakub Narebski]

Junio C Hamano wrote:

>   After starting to edit a working tree file but later when your
>   edit ends up identical to the original (this can also happen
>   when you ran a wholesale regexp replace with something like
>   "perl -i" that does not touch many of the paths),

Does touch the file (makes file stat-dirty, changes file mtime),
but doesn't change it.

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

Re: [PATCH v2] git-diff: Output a warning about stale files in the index

[Junio C Hamano]

Actually this is wrong in two points:

 * The filtering should be done upfront at the beginning of the
   diffcore_std(), not before the end.  Otherwise, unchanged but
   cache dirty file could be subject to copy detection.

 * I do not think it should affect the low-level git-diff-* (or
   you should update the tests, documentations and perhaps
   whatever people can find from google).

By the way, I had an updated version of my patch to fix the
first point on 'pu' for a while.

Re: [PATCH] Add --show-touched option to show "diff --git" line when contents are unchanged

[Linus Torvalds]

On Tue, 7 Aug 2007, Steven Grimm wrote:
> 
> Actually, though this will probably make people roll their eyes, before this
> discussion I would have guessed that "git status" would be the command that
> would tell you the index was out of date, and that there'd be a separate
> command (say, "git update-index"?) that you could then use to sync things up
> again.

Well, historically, you literally would just do

	git update-index --refresh

to do that.

"git status" is fairly newfangled, and is purely because users from other 
SCM's expected that kind of command to exist. The fact that as part of it 
running it does that update-index is really just a side effect.

		Linus

Re: [PATCH] Add --show-touched option to show "diff --git" line when contents are unchanged

[Steven Grimm]

Linus Torvalds wrote:
> Doing a "git diff" cannot actually update the index (since it very much 
> has to work on a read-only setup too), which is why the index _stays_ 
> stale unless something is done (eg "git status") to refresh it. 

Another thought: How about if git-diff *tries* to update the index if 
needed, but failure to do so is not treated as an error condition? That 
seems like the best of both worlds to me: git would self-correct a 
potential performance problem without user intervention, while still 
working properly in a read-only environment.

-Steve

Re: [PATCH] Add --show-touched option to show "diff --git" line when contents are unchanged

[Matthieu Moy]

Linus Torvalds <torvalds@linux-foundation.org> writes:

> It's not that the old output is "useful" in itself, but it's important for 
> people to know that the index is clean. So I'd suggest just setting a flag 
> when the header isn't printed, and then printing out a single line at the 
> end about "git index not up-to-date" or something.

Yes. Junio's patch has this as a comment, it's probably good to
uncomment it, and perhaps print it directly on stdout so that you see
it even with a pager.

> Doing a "git diff" cannot actually update the index (since it very much 
> has to work on a read-only setup too),

Err, what's the relationship between the two parts of your sentence?
You can't be sure that git-diff will update the index (because you may
be working on a read-only setup, yes), but git-diff can at least _try_
to, and fall-back to the read-only behavior if updating the index
fails.

That's not a highly original idea since this is what git already does
with "status".

Once more, I'm willing to write the code for that if it has a chance
to be accepted.

-- 
Matthieu

Re: git-diff on touched files: bug or feature?

[Junio C Hamano]

Jean-François Veillette <jean_francois_veillette@yahoo.ca>
writes:

> I find comments like this to be counter productive.
> Admin it, git porcelain still has some work to be done.

Yes, but this certainly is not an area that needs changing.

Re: git-diff on touched files: bug or feature?

[Junio C Hamano]

Matthieu Moy <Matthieu.Moy@imag.fr> writes:

> I understand that it can be usefull, but I really don't like having it
> by default (is there a way to deactivate it BTW?):

You said it yourself below --- run git-status (or update-index --refresh)
first.

> I've hit this while working on a project, doing a lot of modifications
> through scripting (some regexp substitutions and such kinds of
> things).

I have to say that you are quite mistaken.

Scripted style bulk modification that indiscriminately touch
everbody but actually only modifies some, e.g. "perl -p -i", is
a fine component of people's workflow, but that is *NOT* the
norm.  If it were, then you are not programming nor editing --
your script is doing the work.  But as you know, after such a
bulk operation, you can always...

> ... until I run git-status again.

... refresh away the cache-dirtiness.

The default should be tuned for users who perform manual editing
with status checks.  And power users like yourself who run "bulk
touch indiscriminately but modify only some" scripts should
learn to run git-status (or "update-index --refresh") after such
operation.  Swapping the defaults to optimize for the abnormal
case is madness.

Re: git-diff on touched files: bug or feature?

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Matthieu Moy <Matthieu.Moy@imag.fr> writes:
> ...
>> I've hit this while working on a project, doing a lot of modifications
>> through scripting (some regexp substitutions and such kinds of
>> things).
>
> I have to say that you are quite mistaken.
>
> Scripted style bulk modification that indiscriminately touch
> everbody but actually only modifies some, e.g. "perl -p -i", is
> a fine component of people's workflow, but that is *NOT* the
> norm.

Having said that, there is another lesson to take home from
this.

Quite honestly, a script that indiscriminately touches everybody
but only modifies a few is simply broken.  Think about "make".
"git diff" reporting many cache-dirty files is simply reminding
you the brokenness of such a script.

Re: git-diff on touched files: bug or feature?

[Matthieu Moy]

Junio C Hamano <gitster@pobox.com> writes:

> Quite honestly, a script that indiscriminately touches everybody
> but only modifies a few is simply broken.  Think about "make".
> "git diff" reporting many cache-dirty files is simply reminding
> you the brokenness of such a script.

I wouldn't call this "broken", but clearly suboptimal, yes. But for an
occasionnal one-liner (perl -pi -e ... or so), I lose less time
recompiling extra-files than I would writting a cleaner script. "make"
has no way to detect the absence of modification, while git has.

-- 
Matthieu

Re: git-diff on touched files: bug or feature?

[Johannes Schindelin]

Hi,

On Thu, 2 Aug 2007, Matthieu Moy wrote:

> Junio C Hamano <gitster@pobox.com> writes:
> 
> > Quite honestly, a script that indiscriminately touches everybody
> > but only modifies a few is simply broken.  Think about "make".
> > "git diff" reporting many cache-dirty files is simply reminding
> > you the brokenness of such a script.
> 
> I wouldn't call this "broken", but clearly suboptimal, yes. But for an
> occasionnal one-liner (perl -pi -e ... or so), I lose less time
> recompiling extra-files than I would writting a cleaner script. "make"
> has no way to detect the absence of modification, while git has.

_You_ can afford compiling them extra-files.

That is _exactly_ what Junio meant by "corner case".

Ciao,
Dscho

Re: git-diff on touched files: bug or feature?

[Junio C Hamano]

Matthieu Moy <Matthieu.Moy@imag.fr> writes:

> Junio C Hamano <gitster@pobox.com> writes:
>
>> Quite honestly, a script that indiscriminately touches everybody
>> but only modifies a few is simply broken.  Think about "make".
>> "git diff" reporting many cache-dirty files is simply reminding
>> you the brokenness of such a script.
>
> I wouldn't call this "broken", but clearly suboptimal, yes. But for an
> occasionnal one-liner (perl -pi -e ... or so), I lose less time
> recompiling extra-files than I would writting a cleaner script. "make"
> has no way to detect the absence of modification, while git has.

The first part of your sentence makes sense.  You are trading
"make" time with the reduction in effort to write a throw-away
script.  That makes sense --- and you pay with a more expensive
make, but that is a valid tradeoff you choose to make.  You can
just choose to make the same tradeoff by running an extra
refresh.

You could do something like the trivial patch attached below to
squelch diff-files "cache-dirty" output, if you wanted to.

After trying to work with this modified git for some time,
however, it has become very clear that doing this and nothing
else is a stupid thing to do (I'll suggest potential
improvements at the end).  For one thing, it obviously loses the
"quick git-diff to see what I touched" benefit.  And that issue
is real, as I first touched diff-lib.c to come up with this
until I realized that doing it in here is much cleaner and
simpler --- this patch is discarding that information, and makes
writing a changelog for this patch, if it were to be included,
more expensive for the user -- that is me.

Another thing is that it loses the coal-mine canary value the
"cache-dirty" output has.  After running a loosely written bulk
regexp script, like this:

	perl -p -i -e 's/foo/bar/' *.c

the cached stat data are destroyed for all paths, and it makes
all the subsequent git worktree operations needlessly more
expensive; existing output makes me realize this situation.

It is not a stupid thing to run such a script [*1*]; in this
case, I am choosing the convenience of using such a suboptimal
(as you said) script.  But after running such a script, I DO
WANT git to tell me that I made the index suboptimal, so that I
can and should refresh it to gain the lost performance back.

Personally, I almost never run "git status".  The command is
there primarily because other systems had a command called
"status", and migrant wondered why we didn't.  We do not need
it, and we do not have to use it.

 * For getting the status so-far, I use "git diff" (and "git
   diff ." when in a subdirectory).  It is "how have I changed
   things?", and that is when it is very useful to know "ah, I
   originally went in that direction but decided against it and
   did it differently" by the cache-dirtiness output.  The
   coal-mine canary value is also felt here;

 * For a quick final status, "git diff --stat" is much simpler
   to read than "git status", and that is what I use.  It is
   "what have I changed overall?".  As this actually counts the
   real changes, you would not see those null changes that come
   from the cache dirtiness you are complaining about;

 * When I am ready to commit, "git status" output comes in the
   commit log editor.  At that point, I already have been
   reminded by the previous "git diff" (which is usually run
   often unless the patch is very small like this), and I do not
   need cache-dirtiness output anymore after making my commit.

You do not have to say, to the above paragraph, that it is
different from your workflow.  I am showing what the opmimum
workflow would be, and it is up to you not to listen to me.

Even if we were willing to lose the "quick git-diff to see what
I touched" benefit and want to go the route of the attached
patch suggests (which I personally do not think we are), there
should be a way to either (1) tell the user that many paths are
found to be cache-dirty and it is a good idea to refresh the
stat information, after squelching diff-files output this way,
or (2) update the stat information automatically when diff-files
finds too many paths are cache-dirty (perhaps without even
telling the user).  The latter requires you to declare that
git-diff is not a read-only operation anymore, though, so you
would need some thought before going in that direction.


[Footnote]

*1* Some people might argue that "perl -i" should have a mode to
leave the original if the script does not modify the contents.
Maybe they are right, but that is an orthogonal issue.

---

 diff.c |   40 ++++++++++++++++++++++++++++++++++++++++
 1 files changed, 40 insertions(+), 0 deletions(-)

diff --git a/diff.c b/diff.c
index a5fc56b..ea1239d 100644
--- a/diff.c
+++ b/diff.c
@@ -3143,6 +3143,45 @@ static void diffcore_apply_filter(const char *filter)
 	*q = outq;
 }
 
+static void diffcore_remove_empty(void)
+{
+	int i;
+	struct diff_queue_struct *q = &diff_queued_diff;
+	struct diff_queue_struct outq;
+	outq.queue = NULL;
+	outq.nr = outq.alloc = 0;
+
+	for (i = 0; i < q->nr; i++) {
+		struct diff_filepair *p = q->queue[i];
+
+		/*
+		 * 1. Keep the ones that cannot be diff-files
+		 *    "false" match that are only queued due to
+		 *    cache dirtyness.
+		 *
+		 * 2. Modified, same size and mode, and the object
+		 *    name of one side is unknown.  If they do not
+		 *    have identical contents, keep them.
+		 *    They are different.
+		 */
+		if ((p->status != DIFF_STATUS_MODIFIED) || /* (1) */
+		    (p->one->sha1_valid && p->two->sha1_valid) ||
+		    (p->one->mode != p->two->mode) ||
+
+		    diff_populate_filespec(p->one, 1) || /* (2) */
+		    diff_populate_filespec(p->two, 1) ||
+		    (p->one->size != p->two->size) ||
+		    diff_populate_filespec(p->one, 0) ||
+		    diff_populate_filespec(p->two, 0) ||
+		    memcmp(p->one->data, p->two->data, p->one->size))
+			diff_q(&outq, p);
+		else
+			diff_free_filepair(p);
+	}
+	free(q->queue);
+	*q = outq;
+}
+
 void diffcore_std(struct diff_options *options)
 {
 	if (options->quiet)
@@ -3160,6 +3199,7 @@ void diffcore_std(struct diff_options *options)
 		diffcore_order(options->orderfile);
 	diff_resolve_rename_copy();
 	diffcore_apply_filter(options->filter);
+	diffcore_remove_empty();
 
 	options->has_changes = !!diff_queued_diff.nr;
 }

Re: git-diff on touched files: bug or feature?

[Jeff King]

On Thu, Aug 02, 2007 at 12:56:19PM -0700, Junio C Hamano wrote:

> Personally, I almost never run "git status".  The command is
> there primarily because other systems had a command called
> "status", and migrant wondered why we didn't.  We do not need
> it, and we do not have to use it.

So what is the recommended command to summarize which files have been
modified, which files have been marked for commit, and which remain
untracked?

I find "git-diff --stat" totally insufficient for seeing the progress of
my work. How will it remind me that I have also previously git-added
some changes? They won't be mentioned at all, unless you are really
advocating "git-diff --stat HEAD". How will I be reminded that some
files from my work need to be git-added? git-diff won't mention
untracked files at all.

> You do not have to say, to the above paragraph, that it is
> different from your workflow.  I am showing what the opmimum
> workflow would be, and it is up to you not to listen to me.

You are throwing the word "optimum" out here, but I have no idea what
you mean in this context. Optimum with respect to what criteria?

I know you are just trying to show your workflow, and that you
understand that others might have a different workflow. But you seem to
be implying that workflows using "git-status" are lesser for some
reason, and I really think it is a matter of taste.

-Peff

Re: git-diff on touched files: bug or feature?

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Thu, Aug 02, 2007 at 12:56:19PM -0700, Junio C Hamano wrote:
>
>> Personally, I almost never run "git status".  The command is
>> there primarily because other systems had a command called
>> "status", and migrant wondered why we didn't.  We do not need
>> it, and we do not have to use it.
>
> So what is the recommended command to summarize which files have been
> modified, which files have been marked for commit, and which remain
> untracked?

Ok, you got me.  If I need such a summary, git-status would
obviously be the choice.  Although I do admit that I added the
interactive commit to support people who want to keep 30 hunks
across 10 different files in the working tree, and make a commit
using only 3 of them, I do not make partial commits myself, so
distinction between staged and unstaged are not something I am
usually interested in.  If your workflow care about that
distinction, and that is a very valid and natural workflow in
git, you would find git-status and git-diff --cached more useful
than they are to me.  I should not used words such as optimum.
It is just "different".

When you think about it, in such a workflow whose work tree that
does not match commits created from it, it is not very useful to
know the "touched but ended up unmodified", because (1) the
worktree changes are full of not-yet-ready changes (to the
immediate commit you are going to create) anyway, and (2) the
"touched but not modified" files may further be modified and
become modified before their changes hit a (later) commit.  The
side effect that "git-status" loses that information suddenly
becomes a useful feature for such a workflow.

On the other hand, if your workflow is "work on one thing at a
time, and never make partial commits", then your diff tends to
be small and more focused to begin with, and you can afford to
care about "touched but ended up unmodified".  Interestingly, it
happens to be a useful correlation that "git status", which
clears such information, is less useful command for such a
workflow.

Re: git-diff on touched files: bug or feature?

[Jeff King]

On Fri, Aug 03, 2007 at 12:59:43AM -0700, Junio C Hamano wrote:

> On the other hand, if your workflow is "work on one thing at a
> time, and never make partial commits", then your diff tends to
> be small and more focused to begin with, and you can afford to
> care about "touched but ended up unmodified".  Interestingly, it

In an ideal world, I would work that way. But often you uncover a bug in
existing code while writing new code, and you want to make that bugfix a
separate commit. I generally make a partial commit to stash the bugfix
and test it individually. Without making a partial commit, how would you
split the bugfix changes from the working changes?  Or do you manually
pull the bugfix into another branch or working tree?

There is one point you didn't address from my original mail which I
would be curious to hear your take on. In your workflow, how do you
remind yourself that there are untracked files that need to be added? Do
you just wait until you see the commit template at the end?

-Peff

Re: git-diff on touched files: bug or feature?

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> Without making a partial commit, how would you split the
> bugfix changes from the working changes?  Or do you manually
> pull the bugfix into another branch or working tree?

I typically stash the WIP away with "git diff HEAD >P.diff &&
git reset --hard" (I should learn to use "git stash" these
days), and switch to an appropriate branch for bugfix (if it is
generally applicable) or stay on the branch (if it is a fix-up
for an earlier patch for the topic) to work on the fix.  Then
unstash to continue where I left off.

> In your workflow, how do you
> remind yourself that there are untracked files that need to be added? Do
> you just wait until you see the commit template at the end?

I do not leave files that need to be added untracked for a long
time.  Also, I tend to be picky about making sure that (1)
things build from scratch, and that (2) "make clean" removes all
crufts.  Because of this, I run "make clean" followed by "git
clean -n" more often than other people.  The latter picks them
up if I forget to add them when I created them.

Re: git-diff on touched files: bug or feature?

[Shawn O. Pearce]

Junio C Hamano <gitster@pobox.com> wrote:
> > On Thu, Aug 02, 2007 at 12:56:19PM -0700, Junio C Hamano wrote:
> >
> >> Personally, I almost never run "git status".  The command is
> >> there primarily because other systems had a command called
> >> "status", and migrant wondered why we didn't.  We do not need
> >> it, and we do not have to use it.
> >
> > So what is the recommended command to summarize which files have been
> > modified, which files have been marked for commit, and which remain
> > untracked?

git-gui?  ;-)

I also use the following two aliases:

  [alias]
    dw = diff --stat --summary
    di = diff --stat --summary --cached

...
> I do not make partial commits myself, so
> distinction between staged and unstaged are not something I am
> usually interested in.

I never used to either.  Then git-gui got really useful at showing
the distinction and I started using the index for a staging ground.
I almost never make partial commits, unless it is completely trivial,
e.g. a comment fixup that isn't related to what I'm really doing
but that was too darn obvious to not fix _right now_.

But I always toss things into the index when I've read through the
diff a few times and am very happy with it.  I may not be done with
the overall commit, but I park the hunks into the index so I don't
have to look at them again.  I use a trackball so "tossing into the
index" is really just a flick of the wrist to select the menu item
from the pop-up menu on that hunk.  Quite like a toss.  ;-)

I tend to test only once I have everything staged into the index and
my working directory is clean (nothing changed that isn't staged).
Its at that point that I think my change is done and I'm happy with
how the diff looks.  Usually the code is correct at this point too;
but if its not I'll fix it, then commit.


So where does that leave me regarding the touched but not changed
files?  Usually they just get in my way in the end.  I don't much
care that I've undone the file back to what I had in the index.
It just doesn't provide any value to my workflow.  It is actually
incredible rare that I cause it to happen too.  Usually I won't
write the file back to disk if I'm just going to undo it.

If I do write it to disk I'm likely to stage it or at least some
hunks of it.  If I later change my mind and undo those changes I'm
going to effectively stage the reverse difference.  This is a very
nice hint showing me that yes in fact the older way was better.

Personally?  The index is a killer feature for me.  Totally.
I can't work without it anymore, it has become a total crutch to me.
You would have to pry the index from my cold dead fingers to get
me to stop using it.

Yea, that is a total about-face for me.  I used to think the index
was only useful for merges.  Boy was I wrong!

-- 
Shawn.

Re: git-diff on touched files: bug or feature?

[Junio C Hamano]

"Shawn O. Pearce" <spearce@spearce.org> writes:

>> I do not make partial commits myself, so
>> distinction between staged and unstaged are not something I am
>> usually interested in.
>
> I never used to either.  Then git-gui got really useful at showing
> the distinction and I started using the index for a staging ground.

I guess we are saying the same thing.  I do use index for
staging large-ish changes.  I simply do not care about the
staged/not-staged distinction in the sense that "I still haven't
staged these, that's good because these do not belong to the
commit I am going to make next".

Output from "git diff" being truly empty is the cue that such a
large-ish worktree change is ready to be committed, and the
final "git diff --cached" would give me the full picture.  A
small-ish change won't make "git diff" in the middle empty, but
checking the final "git diff --cached" before committing is the
same.

Re: git-diff on touched files: bug or feature?

[Matthieu Moy]

Junio C Hamano <gitster@pobox.com> writes:

> The default should be tuned for users who perform manual editing
> with status checks.  And power users like yourself who run "bulk
> touch indiscriminately but modify only some" scripts should
> learn to run git-status (or "update-index --refresh") after such
> operation.  Swapping the defaults to optimize for the abnormal
> case is madness.

I fully agree that git should be optimized for the common case. But
even for the common case, I also find the feature strange. You didn't
answer that part of my message, but I still fail to see a rationale
for making "git-diff; git-status" different from "git-status; git-diff".

IOW, why should people running git-status before git-diff not get a
reminder if you think people running git-diff without or before
git-status should get it?

-- 
Matthieu

Re: git-diff on touched files: bug or feature?

[Johannes Schindelin]

Hi,

On Thu, 2 Aug 2007, Matthieu Moy wrote:

> Junio C Hamano <gitster@pobox.com> writes:
> 
> > The default should be tuned for users who perform manual editing
> > with status checks.  And power users like yourself who run "bulk
> > touch indiscriminately but modify only some" scripts should
> > learn to run git-status (or "update-index --refresh") after such
> > operation.  Swapping the defaults to optimize for the abnormal
> > case is madness.
> 
> I fully agree that git should be optimized for the common case. But
> even for the common case, I also find the feature strange. You didn't
> answer that part of my message, but I still fail to see a rationale
> for making "git-diff; git-status" different from "git-status; git-diff".

For performance reasons, git always compares the files' stat information 
with that stored in the index.

By updating the file, you make that check fail always.

Without updating the index (which is not a read-only operation, and 
therefore must not be done when doing a read-only operation like diff), 
you will therefore _destroy_ the main reason of git's kick-ass 
performance.

So when you do "git diff" and it tells you all those diff lines, while no 
file was really changed, it tells you "get your act together!  You just 
_willfully_ slowed down git's performance".

Okay?

Ciao,
Dscho

P.S.: I wish we had something similar for the cases that you did not "git 
gc", so that people, posting to their blogs all over the world that they 
became benchmark experts overnight and tested git and it sucks, would know 
that it is not git that sucks.

Re: git-diff on touched files: bug or feature?

[Matthieu Moy]

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

>> I fully agree that git should be optimized for the common case. But
>> even for the common case, I also find the feature strange. You didn't
>> answer that part of my message, but I still fail to see a rationale
>> for making "git-diff; git-status" different from "git-status; git-diff".
>
> For performance reasons, git always compares the files' stat information 
> with that stored in the index.

I know that, but how does it answer the part of my message that you
are citing?

> So when you do "git diff" and it tells you all those diff lines, while no 
> file was really changed, it tells you "get your act together!  You just 
> _willfully_ slowed down git's performance".

The question remains: why should someone running git-diff get this,
and someone running git-status not get this?

(I mean, from the user point of view, not the implementation point of
view)

-- 
Matthieu

Re: git-diff on touched files: bug or feature?

[Johannes Schindelin]

Hi,

On Thu, 2 Aug 2007, Matthieu Moy wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> >> I fully agree that git should be optimized for the common case. But
> >> even for the common case, I also find the feature strange. You didn't
> >> answer that part of my message, but I still fail to see a rationale
> >> for making "git-diff; git-status" different from "git-status; git-diff".
> >
> > For performance reasons, git always compares the files' stat information 
> > with that stored in the index.
> 
> I know that, but how does it answer the part of my message that you
> are citing?

You _acknowledge_ that git is optimized for performance!  And therefore 
you should also acknowledge that you _throw that away_ if you let your 
index go out of sync.

> > So when you do "git diff" and it tells you all those diff lines, while no 
> > file was really changed, it tells you "get your act together!  You just 
> > _willfully_ slowed down git's performance".
> 
> The question remains: why should someone running git-diff get this,
> and someone running git-status not get this?

Because git-status is an index-updating operation.  That's why.

Ciao,
Dscho

Re: git-diff on touched files: bug or feature?

[Matthieu Moy]

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

>> The question remains: why should someone running git-diff get this,
>> and someone running git-status not get this?
>
> Because git-status is an index-updating operation.  That's why.

That sounds like "it is this way because it is not the other way
around".

So, yes, git-status updates the index because it's an index-updating
operation, while git-diff does not update the index because it's a
non-index-updating operation.

Then, I'll rephrase my sentence as "*why* is git-status an
index-updating operation while git-diff is not". But you'll probably
find another way to avoid answering.

-- 
Matthieu

Re: git-diff on touched files: bug or feature?

[Johannes Schindelin]

Hi,

On Thu, 2 Aug 2007, Matthieu Moy wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> >> The question remains: why should someone running git-diff get this,
> >> and someone running git-status not get this?
> >
> > Because git-status is an index-updating operation.  That's why.
> 
> That sounds like "it is this way because it is not the other way
> around".
> 
> So, yes, git-status updates the index because it's an index-updating
> operation, while git-diff does not update the index because it's a
> non-index-updating operation.
> 
> Then, I'll rephrase my sentence as "*why* is git-status an
> index-updating operation while git-diff is not". But you'll probably
> find another way to avoid answering.

Yes.  I do.  The issue whether or not git status should be a read-only 
operation has been discussed -- in _length_ -- here:

http://thread.gmane.org/gmane.comp.version-control.git/40205/focus=40339

Ciao,
Dscho

Re: git-diff on touched files: bug or feature?

[Joel Reed]

On Thu, Aug 02, 2007 at 01:19:55PM +0100, Johannes Schindelin wrote:

<snip>

> For performance reasons, git always compares the files' stat information 
> with that stored in the index.
> 
> By updating the file, you make that check fail always.
> 
> Without updating the index (which is not a read-only operation, and 
> therefore must not be done when doing a read-only operation like diff), 
> you will therefore _destroy_ the main reason of git's kick-ass 
> performance.

The idea that read-only operation like diff shouldn't update the
index makes a lot of sense.

But, as a user of git and not a git developer, I certainly _thought_
that git-status was a read-only operation as well. Now I know it
isn't, but this doesn't seem very consistent.

jr

Re: git-diff on touched files: bug or feature?

[Johannes Schindelin]

Hi,

On Thu, 2 Aug 2007, Joel Reed wrote:

> On Thu, Aug 02, 2007 at 01:19:55PM +0100, Johannes Schindelin wrote:
> 
> <snip>
> 
> > For performance reasons, git always compares the files' stat information 
> > with that stored in the index.
> > 
> > By updating the file, you make that check fail always.
> > 
> > Without updating the index (which is not a read-only operation, and 
> > therefore must not be done when doing a read-only operation like diff), 
> > you will therefore _destroy_ the main reason of git's kick-ass 
> > performance.
> 
> The idea that read-only operation like diff shouldn't update the
> index makes a lot of sense.
> 
> But, as a user of git and not a git developer, I certainly _thought_
> that git-status was a read-only operation as well. Now I know it
> isn't, but this doesn't seem very consistent.

I'll not go into details again.  See 
http://thread.gmane.org/gmane.comp.version-control.git/40205/focus=40339

Ciao,
Dscho