[PATCH 0/5] "diff --stat" counting fixes

[PATCH 0/5] "diff --stat" counting fixes

[Junio C Hamano]

It turns out that there are at least two bugs in the diffstat
counting code.  This series comes on top of the earlier 74faaa1 (Fix
"git diff --stat" for interesting - but empty - file changes,
2012-10-17) to fix them.

Junio C Hamano (5):
  test: add failing tests for "diff --stat" to t4049
  diff --stat: status of unmodified pair in diff-q is not zero
  diff --stat: use "file" temporary variable to refer to data->files[i]
  diff --stat: move the "total count" logic to the last loop
  diff --stat: do not count "unmerged" entries

 diff.c                     | 49 +++++++++++++++++++++++++---------------------
 t/t4049-diff-stat-count.sh | 46 ++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 72 insertions(+), 23 deletions(-)

-- 
1.8.0.1.331.g808d2af

[PATCH 1/5] test: add failing tests for "diff --stat" to t4049

[Junio C Hamano]

There are a few problems in diff.c around --stat area, partially
caused by the recent 74faaa1 (Fix "git diff --stat" for interesting
- but empty - file changes, 2012-10-17), and largely caused by the
earlier change that introduced when --stat-count was added.

Add a few test pieces to t4049 to expose the issues.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t4049-diff-stat-count.sh | 46 +++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 45 insertions(+), 1 deletion(-)

diff --git a/t/t4049-diff-stat-count.sh b/t/t4049-diff-stat-count.sh
index 7b3ef00..e212b11 100755
--- a/t/t4049-diff-stat-count.sh
+++ b/t/t4049-diff-stat-count.sh
@@ -4,12 +4,17 @@
 test_description='diff --stat-count'
 . ./test-lib.sh
 
-test_expect_success setup '
+test_expect_success 'setup' '
 	>a &&
 	>b &&
 	>c &&
 	>d &&
 	git add a b c d &&
+	git commit -m initial
+'
+
+test_expect_success 'limit output to 2 (simple)' '
+	git reset --hard &&
 	chmod +x c d &&
 	echo a >a &&
 	echo b >b &&
@@ -23,4 +28,43 @@ test_expect_success setup '
 	test_i18ncmp expect actual
 '
 
+test_expect_failure 'binary changes do not count in lines' '
+	git reset --hard &&
+	chmod +x c d &&
+	echo a >a &&
+	echo b >b &&
+	cat "$TEST_DIRECTORY"/test-binary-1.png >d &&
+	cat >expect <<-\EOF
+	 a | 1 +
+	 b | 1 +
+	 ...
+	 4 files changed, 2 insertions(+)
+	EOF
+	git diff --stat --stat-count=2 >actual &&
+	test_i18ncmp expect actual
+'
+
+test_expect_failure 'exclude unmerged entries from total file count' '
+	git reset --hard &&
+	echo a >a &&
+	echo b >b &&
+	git ls-files -s a >x &&
+	git rm -f d &&
+	for stage in 1 2 3
+	do
+		sed -e "s/ 0	a/ $stage	d/" x
+	done |
+	git update-index --index-info &&
+	echo d >d &&
+	chmod +x c d &&
+	cat >expect <<-\EOF
+	 a | 1 +
+	 b | 1 +
+	 ...
+	 4 files changed, 3 insertions(+)
+	EOF
+	git diff --stat --stat-count=2 >actual &&
+	test_i18ncmp expect actual
+'
+
 test_done
-- 
1.8.0.1.331.g808d2af

[PATCH 2/5] diff --stat: status of unmodified pair in diff-q is not zero

[Junio C Hamano]

It is spelled DIFF_STATUS_UNKNOWN these days, and is different from zero.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 diff.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/diff.c b/diff.c
index 95bbad6..ce6baa4 100644
--- a/diff.c
+++ b/diff.c
@@ -2411,7 +2411,7 @@ static void builtin_diffstat(const char *name_a, const char *name_b,
 	}
 
 	data = diffstat_add(diffstat, name_a, name_b);
-	data->is_interesting = p->status != 0;
+	data->is_interesting = p->status != DIFF_STATUS_UNKNOWN;
 
 	if (!one || !two) {
 		data->is_unmerged = 1;
-- 
1.8.0.1.331.g808d2af

[PATCH 3/5] diff --stat: use "file" temporary variable to refer to data->files[i]

[Junio C Hamano]

The generated code shouldn't change but it is easier to read.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 diff.c | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/diff.c b/diff.c
index ce6baa4..e4e70e5 100644
--- a/diff.c
+++ b/diff.c
@@ -1470,8 +1470,8 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)
 	for (i = 0; (i < count) && (i < data->nr); i++) {
 		struct diffstat_file *file = data->files[i];
 		uintmax_t change = file->added + file->deleted;
-		if (!data->files[i]->is_interesting &&
-			 (change == 0)) {
+
+		if (!file->is_interesting && (change == 0)) {
 			count++; /* not shown == room for one more */
 			continue;
 		}
@@ -1586,13 +1586,13 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)
 	 */
 	for (i = 0; i < count; i++) {
 		const char *prefix = "";
-		char *name = data->files[i]->print_name;
-		uintmax_t added = data->files[i]->added;
-		uintmax_t deleted = data->files[i]->deleted;
+		struct diffstat_file *file = data->files[i];
+		char *name = file->print_name;
+		uintmax_t added = file->added;
+		uintmax_t deleted = file->deleted;
 		int name_len;
 
-		if (!data->files[i]->is_interesting &&
-			 (added + deleted == 0)) {
+		if (!file->is_interesting && (added + deleted == 0)) {
 			total_files--;
 			continue;
 		}
@@ -1611,7 +1611,7 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)
 				name = slash;
 		}
 
-		if (data->files[i]->is_binary) {
+		if (file->is_binary) {
 			fprintf(options->file, "%s", line_prefix);
 			show_name(options->file, prefix, name, len);
 			fprintf(options->file, " %*s", number_width, "Bin");
@@ -1628,7 +1628,7 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)
 			fprintf(options->file, "\n");
 			continue;
 		}
-		else if (data->files[i]->is_unmerged) {
+		else if (file->is_unmerged) {
 			fprintf(options->file, "%s", line_prefix);
 			show_name(options->file, prefix, name, len);
 			fprintf(options->file, " Unmerged\n");
@@ -1668,10 +1668,10 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)
 		fprintf(options->file, "\n");
 	}
 	for (i = count; i < data->nr; i++) {
-		uintmax_t added = data->files[i]->added;
-		uintmax_t deleted = data->files[i]->deleted;
-		if (!data->files[i]->is_interesting &&
-			 (added + deleted == 0)) {
+		struct diffstat_file *file = data->files[i];
+		uintmax_t added = file->added;
+		uintmax_t deleted = file->deleted;
+		if (!file->is_interesting && (added + deleted == 0)) {
 			total_files--;
 			continue;
 		}
-- 
1.8.0.1.331.g808d2af

[PATCH 4/5] diff --stat: move the "total count" logic to the last loop

[Junio C Hamano]

The diffstat generation logic, with --stat-count limit, is
implemented as three loops.

 - The first counts the width necessary to show stats up to
   specified number of entries, and notes up to how many entries in
   the data we need to iterate to show the graph;

 - The second iterates that many times to draw the graph, adjusts
   the number of "total modified files", and counts the total
   added/deleted lines for the part that was shown in the graph;

 - The third iterates over the remainder and only does the part to
   count "total added/deleted lines" and to adjust "total modified
   files" without drawing anything.

Move the logic to count added/deleted lines and modified files from
the second loop to the third loop.

This incidentally fixes a bug.  The third loop was not filtering
binary changes (counted in bytes) from the total added/deleted as it
should.  The second loop implemented this correctly, so if a binary
change appeared earlier than the --stat-count cutoff, the code
counted number of added/deleted lines correctly, but if it appeared
beyond the cutoff, the number of lines would have mixed with the
byte count in the buggy third loop.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 diff.c                     | 21 ++++++++++++---------
 t/t4049-diff-stat-count.sh |  2 +-
 2 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/diff.c b/diff.c
index e4e70e5..4105260 100644
--- a/diff.c
+++ b/diff.c
@@ -1498,7 +1498,7 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)
 		if (max_change < change)
 			max_change = change;
 	}
-	count = i; /* min(count, data->nr) */
+	count = i; /* where we can stop scanning in data->files[] */
 
 	/*
 	 * We have width = stat_width or term_columns() columns total.
@@ -1592,10 +1592,9 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)
 		uintmax_t deleted = file->deleted;
 		int name_len;
 
-		if (!file->is_interesting && (added + deleted == 0)) {
-			total_files--;
+		if (!file->is_interesting && (added + deleted == 0))
 			continue;
-		}
+
 		/*
 		 * "scale" the filename
 		 */
@@ -1640,8 +1639,6 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)
 		 */
 		add = added;
 		del = deleted;
-		adds += add;
-		dels += del;
 
 		if (graph_width <= max_change) {
 			int total = add + del;
@@ -1667,7 +1664,8 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)
 		show_graph(options->file, '-', del, del_c, reset);
 		fprintf(options->file, "\n");
 	}
-	for (i = count; i < data->nr; i++) {
+
+	for (i = 0; i < data->nr; i++) {
 		struct diffstat_file *file = data->files[i];
 		uintmax_t added = file->added;
 		uintmax_t deleted = file->deleted;
@@ -1675,8 +1673,13 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)
 			total_files--;
 			continue;
 		}
-		adds += added;
-		dels += deleted;
+
+		if (!file->is_binary && !file->is_unmerged) {
+			adds += added;
+			dels += deleted;
+		}
+		if (i < count)
+			continue;
 		if (!extra_shown)
 			fprintf(options->file, "%s ...\n", line_prefix);
 		extra_shown = 1;
diff --git a/t/t4049-diff-stat-count.sh b/t/t4049-diff-stat-count.sh
index e212b11..70ee073 100755
--- a/t/t4049-diff-stat-count.sh
+++ b/t/t4049-diff-stat-count.sh
@@ -28,7 +28,7 @@ test_expect_success 'limit output to 2 (simple)' '
 	test_i18ncmp expect actual
 '
 
-test_expect_failure 'binary changes do not count in lines' '
+test_expect_success 'binary changes do not count in lines' '
 	git reset --hard &&
 	chmod +x c d &&
 	echo a >a &&
-- 
1.8.0.1.331.g808d2af

[PATCH 5/5] diff --stat: do not count "unmerged" entries

[Junio C Hamano]

Even though we show a separate *UNMERGED* entry in the patch and
diffstat output (or in the --raw format, for that matter) in
addition to and separately from the diff against the specified stage
(defaulting to #2) for unmerged paths, they should not be counted in
the total number of files affected---that would lead to counting the
same path twice.

The separation done by the previous step makes this fix simple and
straightforward.  Among the filepairs in diff_queue, paths that
weren't modified, and the extra "unmerged" entries do not count as
total number of files.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 diff.c                     | 6 ++++--
 t/t4049-diff-stat-count.sh | 2 +-
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/diff.c b/diff.c
index 4105260..26ede82 100644
--- a/diff.c
+++ b/diff.c
@@ -1669,12 +1669,14 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)
 		struct diffstat_file *file = data->files[i];
 		uintmax_t added = file->added;
 		uintmax_t deleted = file->deleted;
-		if (!file->is_interesting && (added + deleted == 0)) {
+
+		if (file->is_unmerged ||
+		    (!file->is_interesting && (added + deleted == 0))) {
 			total_files--;
 			continue;
 		}
 
-		if (!file->is_binary && !file->is_unmerged) {
+		if (!file->is_binary) {
 			adds += added;
 			dels += deleted;
 		}
diff --git a/t/t4049-diff-stat-count.sh b/t/t4049-diff-stat-count.sh
index 70ee073..37f50cd 100755
--- a/t/t4049-diff-stat-count.sh
+++ b/t/t4049-diff-stat-count.sh
@@ -44,7 +44,7 @@ test_expect_success 'binary changes do not count in lines' '
 	test_i18ncmp expect actual
 '
 
-test_expect_failure 'exclude unmerged entries from total file count' '
+test_expect_success 'exclude unmerged entries from total file count' '
 	git reset --hard &&
 	echo a >a &&
 	echo b >b &&
-- 
1.8.0.1.331.g808d2af

[PATCH 6/5] diff --shortstat: do not count "unmerged" entries

[Junio C Hamano]

Fix the same issue as the previous one for "git diff --stat";
unmerged entries was doubly-counted.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 diff.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/diff.c b/diff.c
index 26ede82..374b235 100644
--- a/diff.c
+++ b/diff.c
@@ -1701,9 +1701,8 @@ static void show_shortstats(struct diffstat_t *data, struct diff_options *option
 		int added = data->files[i]->added;
 		int deleted= data->files[i]->deleted;
 
-		if (data->files[i]->is_unmerged)
-			continue;
-		if (!data->files[i]->is_interesting && (added + deleted == 0)) {
+		if (data->files[i]->is_unmerged ||
+		    (!data->files[i]->is_interesting && (added + deleted == 0))) {
 			total_files--;
 		} else if (!data->files[i]->is_binary) { /* don't count bytes */
 			adds += added;
-- 
1.8.0.1.337.gd2c5e06

[PATCH] t4049: avoid test failures on filemode challenged file systems (Windows)

[Johannes Sixt]

From: Johannes Sixt <j6t@kdbg.org>

The earlier change 74faaa16 (Fix "git diff --stat" for interesting - but
empty - file changes) needed to change the count of differing files
because the executable-bit changes of two empty files are now counted.

On file systems that do not record the executable bit, however, the old
file count was actually correct (and the updated tests fail) because the
mode change cannot be diagnosed by looking at the file system alone.

Change the mode not only on the file system, but also in the index;
compare the new state against the commit, so that the tests do not depend
on the file system's ability to record the executable bit, when possible.

The exception is the test for unmerged entries, which does depend on the
file system; we have to skip it.

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
Am 11/27/2012 22:21, schrieb Junio C Hamano:
> It turns out that there are at least two bugs in the diffstat
> counting code.  This series comes on top of the earlier 74faaa1 (Fix
> "git diff --stat" for interesting - but empty - file changes,
> 2012-10-17) to fix them.

The tests still fail on Windows. I am not sure whether there is a
difference in comparing the file system against the index or a commit.
If there is, then the updated tests might not test the same thing.

-- Hannes

 t/t4049-diff-stat-count.sh | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/t/t4049-diff-stat-count.sh b/t/t4049-diff-stat-count.sh
index 37f50cd..9e29e71 100755
--- a/t/t4049-diff-stat-count.sh
+++ b/t/t4049-diff-stat-count.sh
@@ -15,7 +15,7 @@ test_expect_success 'setup' '
 
 test_expect_success 'limit output to 2 (simple)' '
 	git reset --hard &&
-	chmod +x c d &&
+	test_chmod +x c d &&
 	echo a >a &&
 	echo b >b &&
 	cat >expect <<-\EOF
@@ -24,13 +24,13 @@ test_expect_success 'limit output to 2 (simple)' '
 	 ...
 	 4 files changed, 2 insertions(+)
 	EOF
-	git diff --stat --stat-count=2 >actual &&
+	git diff --stat --stat-count=2 HEAD >actual &&
 	test_i18ncmp expect actual
 '
 
 test_expect_success 'binary changes do not count in lines' '
 	git reset --hard &&
-	chmod +x c d &&
+	test_chmod +x c d &&
 	echo a >a &&
 	echo b >b &&
 	cat "$TEST_DIRECTORY"/test-binary-1.png >d &&
@@ -40,11 +40,11 @@ test_expect_success 'binary changes do not count in lines' '
 	 ...
 	 4 files changed, 2 insertions(+)
 	EOF
-	git diff --stat --stat-count=2 >actual &&
+	git diff --stat --stat-count=2 HEAD >actual &&
 	test_i18ncmp expect actual
 '
 
-test_expect_success 'exclude unmerged entries from total file count' '
+test_expect_success FILEMODE 'exclude unmerged entries from total file count' '
 	git reset --hard &&
 	echo a >a &&
 	echo b >b &&
-- 
1.8.0.1.1524.gaf6675c

Re: [PATCH] t4049: avoid test failures on filemode challenged file systems (Windows)

[Junio C Hamano]

Johannes Sixt <j.sixt@viscovery.net> writes:

>> It turns out that there are at least two bugs in the diffstat
>> counting code.  This series comes on top of the earlier 74faaa1 (Fix
>> "git diff --stat" for interesting - but empty - file changes,
>> 2012-10-17) to fix them.
>
> The tests still fail on Windows. I am not sure whether there is a
> difference in comparing the file system against the index or a commit.
> If there is, then the updated tests might not test the same thing.

The hunks in the patch look fine.  The last one that tests unmerged
entries do not have to have "chmod" if it gives you trouble (you
would need to reduce number of files from 4 to 3 if you go that
route, I think).

Re: [PATCH] t4049: avoid test failures on filemode challenged file systems (Windows)

[Junio C Hamano]

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

> Johannes Sixt <j.sixt@viscovery.net> writes:
>
>>> It turns out that there are at least two bugs in the diffstat
>>> counting code.  This series comes on top of the earlier 74faaa1 (Fix
>>> "git diff --stat" for interesting - but empty - file changes,
>>> 2012-10-17) to fix them.
>>
>> The tests still fail on Windows. I am not sure whether there is a
>> difference in comparing the file system against the index or a commit.
>> If there is, then the updated tests might not test the same thing.
>
> The hunks in the patch look fine.  The last one that tests unmerged
> entries do not have to have "chmod" if it gives you trouble (you
> would need to reduce number of files from 4 to 3 if you go that
> route, I think).

That is, something like this.

-- >8 --
Subject: [PATCH] t4049: refocus tests

The primary thing Linus's patch wanted to change was to make sure
that 0-line change appears for a mode-only change.  Update the
first test to chmod a file that we can see in the output (limited
by --stat-count) to demonstrate it.  Also make sure to use test_chmod
and compare the index and the tree, so that we can run this test
even on a filesystem without permission bits.

Later two tests are about fixes to separate issues that were
introduced and/or uncovered by Linus's patch as a side effect, but
the issues are not related to mode-only changes.  Remove chmod from
the tests.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t4049-diff-stat-count.sh | 20 +++++++++-----------
 1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/t/t4049-diff-stat-count.sh b/t/t4049-diff-stat-count.sh
index 37f50cd..5b594e8 100755
--- a/t/t4049-diff-stat-count.sh
+++ b/t/t4049-diff-stat-count.sh
@@ -13,32 +13,31 @@ test_expect_success 'setup' '
 	git commit -m initial
 '
 
-test_expect_success 'limit output to 2 (simple)' '
+test_expect_success 'mode-only change show as a 0-line change' '
 	git reset --hard &&
-	chmod +x c d &&
+	test_chmod +x b d &&
 	echo a >a &&
-	echo b >b &&
+	echo c >c &&
 	cat >expect <<-\EOF
 	 a | 1 +
-	 b | 1 +
+	 b | 0
 	 ...
 	 4 files changed, 2 insertions(+)
 	EOF
-	git diff --stat --stat-count=2 >actual &&
+	git diff --stat --stat-count=2 HEAD >actual &&
 	test_i18ncmp expect actual
 '
 
 test_expect_success 'binary changes do not count in lines' '
 	git reset --hard &&
-	chmod +x c d &&
 	echo a >a &&
-	echo b >b &&
+	echo c >c &&
 	cat "$TEST_DIRECTORY"/test-binary-1.png >d &&
 	cat >expect <<-\EOF
 	 a | 1 +
-	 b | 1 +
+	 c | 1 +
 	 ...
-	 4 files changed, 2 insertions(+)
+	 3 files changed, 2 insertions(+)
 	EOF
 	git diff --stat --stat-count=2 >actual &&
 	test_i18ncmp expect actual
@@ -56,12 +55,11 @@ test_expect_success 'exclude unmerged entries from total file count' '
 	done |
 	git update-index --index-info &&
 	echo d >d &&
-	chmod +x c d &&
 	cat >expect <<-\EOF
 	 a | 1 +
 	 b | 1 +
 	 ...
-	 4 files changed, 3 insertions(+)
+	 3 files changed, 3 insertions(+)
 	EOF
 	git diff --stat --stat-count=2 >actual &&
 	test_i18ncmp expect actual
-- 
1.8.0.1.407.g3c58eb7

Re: [PATCH] t4049: avoid test failures on filemode challenged file systems (Windows)

[Junio C Hamano]

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

> Junio C Hamano <gitster@pobox.com> writes:
>> ...
>> The hunks in the patch look fine.  The last one that tests unmerged
>> entries do not have to have "chmod" if it gives you trouble (you
>> would need to reduce number of files from 4 to 3 if you go that
>> route, I think).
>
> That is, something like this.

I've tested this with the testpen set on vfat mounted on my Linux
box, i.e.

    $ cd t
    $ sh t4049-diff-stat-count.sh --root=/media/5599-553B/test -v
        
and it seems to work OK, so I'll be merging the topic with this
patch to 'master' later today.

Thanks for noticing.

> -- >8 --
> Subject: [PATCH] t4049: refocus tests
>
> The primary thing Linus's patch wanted to change was to make sure
> that 0-line change appears for a mode-only change.  Update the
> first test to chmod a file that we can see in the output (limited
> by --stat-count) to demonstrate it.  Also make sure to use test_chmod
> and compare the index and the tree, so that we can run this test
> even on a filesystem without permission bits.
>
> Later two tests are about fixes to separate issues that were
> introduced and/or uncovered by Linus's patch as a side effect, but
> the issues are not related to mode-only changes.  Remove chmod from
> the tests.
>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  t/t4049-diff-stat-count.sh | 20 +++++++++-----------
>  1 file changed, 9 insertions(+), 11 deletions(-)
>
> diff --git a/t/t4049-diff-stat-count.sh b/t/t4049-diff-stat-count.sh
> index 37f50cd..5b594e8 100755
> --- a/t/t4049-diff-stat-count.sh
> +++ b/t/t4049-diff-stat-count.sh
> @@ -13,32 +13,31 @@ test_expect_success 'setup' '
>  	git commit -m initial
>  '
>  
> -test_expect_success 'limit output to 2 (simple)' '
> +test_expect_success 'mode-only change show as a 0-line change' '
>  	git reset --hard &&
> -	chmod +x c d &&
> +	test_chmod +x b d &&
>  	echo a >a &&
> -	echo b >b &&
> +	echo c >c &&
>  	cat >expect <<-\EOF
>  	 a | 1 +
> -	 b | 1 +
> +	 b | 0
>  	 ...
>  	 4 files changed, 2 insertions(+)
>  	EOF
> -	git diff --stat --stat-count=2 >actual &&
> +	git diff --stat --stat-count=2 HEAD >actual &&
>  	test_i18ncmp expect actual
>  '
>  
>  test_expect_success 'binary changes do not count in lines' '
>  	git reset --hard &&
> -	chmod +x c d &&
>  	echo a >a &&
> -	echo b >b &&
> +	echo c >c &&
>  	cat "$TEST_DIRECTORY"/test-binary-1.png >d &&
>  	cat >expect <<-\EOF
>  	 a | 1 +
> -	 b | 1 +
> +	 c | 1 +
>  	 ...
> -	 4 files changed, 2 insertions(+)
> +	 3 files changed, 2 insertions(+)
>  	EOF
>  	git diff --stat --stat-count=2 >actual &&
>  	test_i18ncmp expect actual
> @@ -56,12 +55,11 @@ test_expect_success 'exclude unmerged entries from total file count' '
>  	done |
>  	git update-index --index-info &&
>  	echo d >d &&
> -	chmod +x c d &&
>  	cat >expect <<-\EOF
>  	 a | 1 +
>  	 b | 1 +
>  	 ...
> -	 4 files changed, 3 insertions(+)
> +	 3 files changed, 3 insertions(+)
>  	EOF
>  	git diff --stat --stat-count=2 >actual &&
>  	test_i18ncmp expect actual

Re: [PATCH] t4049: avoid test failures on filemode challenged file systems (Windows)

[Johannes Sixt]

Am 11/29/2012 21:48, schrieb Junio C Hamano:
> I've tested this with the testpen set on vfat mounted on my Linux
> box, ...
> and it seems to work OK,

Works well here on Windows, too.

Thanks,
-- Hannes

Re: [PATCH 0/5] "diff --stat" counting fixes

[Antoine Pelisse]

Hi Junio,

That does make a lot of sense and I would have indeed missed a couple
of things here.

I've been thinking about that "Unmerged" line quite a lot, and I can't
get myself any good reason to keep it.
Would you mind taking a couple of minutes to make it clear ?

I feel like (but I can obviously be wrong):
1. The info is redundant. When performing a merge, all diffs (without
--staged flag) are unmerged
2. While status shows the line once, while diff shows the diff for the file
once, while diff --shortstat counts the file once, diff --stat shows two
lines for the file.
3. diff --numstat shows two lines for the same file. As a script
writer (I guess that's what it's meant for), I would definitely expect
uniqueness in third column/filenames.

Cheers,
Antoine

On Tue, Nov 27, 2012 at 10:21 PM, Junio C Hamano <gitster@pobox.com> wrote:
>
> It turns out that there are at least two bugs in the diffstat
> counting code.  This series comes on top of the earlier 74faaa1 (Fix
> "git diff --stat" for interesting - but empty - file changes,
> 2012-10-17) to fix them.
>
> Junio C Hamano (5):
>   test: add failing tests for "diff --stat" to t4049
>   diff --stat: status of unmodified pair in diff-q is not zero
>   diff --stat: use "file" temporary variable to refer to data->files[i]
>   diff --stat: move the "total count" logic to the last loop
>   diff --stat: do not count "unmerged" entries
>
>  diff.c                     | 49 +++++++++++++++++++++++++---------------------
>  t/t4049-diff-stat-count.sh | 46 ++++++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 72 insertions(+), 23 deletions(-)
>
> --
> 1.8.0.1.331.g808d2af
>

Re: [PATCH 0/5] "diff --stat" counting fixes

[Junio C Hamano]

Antoine Pelisse <apelisse@gmail.com> writes:

> I feel like (but I can obviously be wrong):
> 1. The info is redundant. When performing a merge, all diffs (without
> --staged flag) are unmerged

Yes, it is redundant.  They are primarily meant as a warning to
anybody who runs "git diff --stat" while their index is not fully
merged so that they do not mistakenly think they are looking at
meaningful numbers.  The number of added lines likely includes the
conflict markers you haven't dealt with ;-)

> 2. While status shows the line once, while diff shows the diff for the file
> once, while diff --shortstat counts the file once, diff --stat shows two
> lines for the file.

Yeah, don't use shortstat while your index is unmerged.

> 3. diff --numstat shows two lines for the same file. As a script
> writer (I guess that's what it's meant for), I would definitely expect
> uniqueness in third column/filenames.

Then your script wouldn't receive any hint in the output that you
are reading from a nonsense input.  When you see the same filename
twice, you will know there is something strange (not just "I seem to
have more added lines than I thought I added -- Ah, I see added
files from both sides because I still have conflicts unresolved")
that gives you a chance to notice.

Re: [PATCH] t4049: avoid test failures on filemode challenged file systems (Windows)

[Junio C Hamano]

Johannes Sixt <j.sixt@viscovery.net> writes:

> Am 11/29/2012 21:48, schrieb Junio C Hamano:
>> I've tested this with the testpen set on vfat mounted on my Linux
>> box, ...
>> and it seems to work OK,
>
> Works well here on Windows, too.

Thanks for checking.