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

3 messages, 2 authors, 2016-06-15 · open the first message on its own page

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

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:43:27

Steven Grimm [off-list ref] writes:

Signed-off-by: Steven Grimm <redacted>
---
	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

From: Steven Grimm <hidden>
Date: 2016-06-15 22:43:27

Signed-off-by: Steven Grimm <redacted>
---
	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

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:43:28

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.

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help