# [PATCH 0/5] "diff --stat" counting fixes 15 messages from 2012-11-27 to 2012-12-02. Participants: Junio C Hamano, Johannes Sixt, Antoine Pelisse. Thread: https://gitlist.dev/t/32215 ## Junio C Hamano, 2012-11-27 21:21 Subject: [PATCH 0/5] "diff --stat" counting fixes Message-ID: <1354051310-29093-1-git-send-email-gitster@pobox.com> URL: https://gitlist.dev/e/1354051310-29093-1-git-send-email-gitster%40pobox.com ``` 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 ``` ## Junio C Hamano, 2012-11-27 21:21 Subject: [PATCH 1/5] test: add failing tests for "diff --stat" to t4049 Message-ID: <1354051310-29093-2-git-send-email-gitster@pobox.com> URL: https://gitlist.dev/e/1354051310-29093-2-git-send-email-gitster%40pobox.com In-Reply-To: <1354051310-29093-1-git-send-email-gitster@pobox.com> ``` 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 --- 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 ``` ## Junio C Hamano, 2012-11-27 21:21 Subject: [PATCH 2/5] diff --stat: status of unmodified pair in diff-q is not zero Message-ID: <1354051310-29093-3-git-send-email-gitster@pobox.com> URL: https://gitlist.dev/e/1354051310-29093-3-git-send-email-gitster%40pobox.com In-Reply-To: <1354051310-29093-1-git-send-email-gitster@pobox.com> ``` It is spelled DIFF_STATUS_UNKNOWN these days, and is different from zero. Signed-off-by: Junio C Hamano --- 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 ``` ## Junio C Hamano, 2012-11-27 21:21 Subject: [PATCH 3/5] diff --stat: use "file" temporary variable to refer to data->files[i] Message-ID: <1354051310-29093-4-git-send-email-gitster@pobox.com> URL: https://gitlist.dev/e/1354051310-29093-4-git-send-email-gitster%40pobox.com In-Reply-To: <1354051310-29093-1-git-send-email-gitster@pobox.com> ``` The generated code shouldn't change but it is easier to read. Signed-off-by: Junio C Hamano --- 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 ``` ## Junio C Hamano, 2012-11-27 21:21 Subject: [PATCH 4/5] diff --stat: move the "total count" logic to the last loop Message-ID: <1354051310-29093-5-git-send-email-gitster@pobox.com> URL: https://gitlist.dev/e/1354051310-29093-5-git-send-email-gitster%40pobox.com In-Reply-To: <1354051310-29093-1-git-send-email-gitster@pobox.com> ``` 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 --- 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 ``` ## Junio C Hamano, 2012-11-27 21:21 Subject: [PATCH 5/5] diff --stat: do not count "unmerged" entries Message-ID: <1354051310-29093-6-git-send-email-gitster@pobox.com> URL: https://gitlist.dev/e/1354051310-29093-6-git-send-email-gitster%40pobox.com In-Reply-To: <1354051310-29093-1-git-send-email-gitster@pobox.com> ``` 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 --- 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 ``` ## Junio C Hamano, 2012-11-27 22:21 Subject: [PATCH 6/5] diff --shortstat: do not count "unmerged" entries Message-ID: <7vsj7ulmad.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vsj7ulmad.fsf%40alter.siamese.dyndns.org In-Reply-To: <1354051310-29093-1-git-send-email-gitster@pobox.com> ``` Fix the same issue as the previous one for "git diff --stat"; unmerged entries was doubly-counted. Signed-off-by: Junio C Hamano --- 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 ``` ## Johannes Sixt, 2012-11-29 08:22 Subject: [PATCH] t4049: avoid test failures on filemode challenged file systems (Windows) Message-ID: <50B71B33.1090000@viscovery.net> URL: https://gitlist.dev/e/50B71B33.1090000%40viscovery.net In-Reply-To: <1354051310-29093-1-git-send-email-gitster@pobox.com> ``` From: Johannes Sixt 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 --- 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 ``` ## Junio C Hamano, 2012-11-29 16:13 Subject: Re: [PATCH] t4049: avoid test failures on filemode challenged file systems (Windows) Message-ID: <7v38zss7zb.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7v38zss7zb.fsf%40alter.siamese.dyndns.org In-Reply-To: <50B71B33.1090000@viscovery.net> ``` Johannes Sixt 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). ``` ## Junio C Hamano, 2012-11-29 17:51 Subject: Re: [PATCH] t4049: avoid test failures on filemode challenged file systems (Windows) Message-ID: <7vfw3sqoup.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vfw3sqoup.fsf%40alter.siamese.dyndns.org In-Reply-To: <7v38zss7zb.fsf@alter.siamese.dyndns.org> ``` Junio C Hamano writes: > Johannes Sixt 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 --- 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 ``` ## Junio C Hamano, 2012-11-29 20:48 Subject: Re: [PATCH] t4049: avoid test failures on filemode challenged file systems (Windows) Message-ID: <7vfw3sp232.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vfw3sp232.fsf%40alter.siamese.dyndns.org In-Reply-To: <7vfw3sqoup.fsf@alter.siamese.dyndns.org> ``` Junio C Hamano writes: > Junio C Hamano 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 > --- > 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 ``` ## Johannes Sixt, 2012-11-30 07:46 Subject: Re: [PATCH] t4049: avoid test failures on filemode challenged file systems (Windows) Message-ID: <50B86438.3010806@viscovery.net> URL: https://gitlist.dev/e/50B86438.3010806%40viscovery.net In-Reply-To: <7vfw3sp232.fsf@alter.siamese.dyndns.org> ``` 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 ``` ## Antoine Pelisse, 2012-12-01 10:29 Subject: Re: [PATCH 0/5] "diff --stat" counting fixes Message-ID: URL: https://gitlist.dev/e/CALWbr2xYrWkfSCUc7gocVGLQP_RHjWrMmFspPe5O_ekjisNqxw%40mail.gmail.com In-Reply-To: <1354051310-29093-1-git-send-email-gitster@pobox.com> ``` 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 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 > ``` ## Junio C Hamano, 2012-12-02 02:23 Subject: Re: [PATCH 0/5] "diff --stat" counting fixes Message-ID: <7vhao5mbtx.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vhao5mbtx.fsf%40alter.siamese.dyndns.org In-Reply-To: ``` Antoine Pelisse 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. ``` ## Junio C Hamano, 2012-12-02 02:24 Subject: Re: [PATCH] t4049: avoid test failures on filemode challenged file systems (Windows) Message-ID: <7vd2ytmbrs.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vd2ytmbrs.fsf%40alter.siamese.dyndns.org In-Reply-To: <50B86438.3010806@viscovery.net> ``` Johannes Sixt 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. ```