{"thread":{"id":"32215","subject":"[PATCH 0/5] \"diff --stat\" counting fixes","startedAt":"2012-11-27T21:21:45Z","lastAt":"2012-12-02T02:24:39Z","messageCount":15,"participants":["Junio C Hamano","Johannes Sixt","Antoine Pelisse"],"isPatch":true,"patchVersion":1,"patchTotal":5},"messages":[{"id":"204029","messageId":"1354051310-29093-1-git-send-email-gitster@pobox.com","threadId":"32215","inReplyTo":null,"subject":"[PATCH 0/5] \"diff --stat\" counting fixes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-11-27T21:21:45Z","receivedAt":"2012-11-27T21:21:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"It turns out that there are at least two bugs in the diffstat\ncounting code. This series comes on top of the earlier 74faaa1 (Fix\n\"git diff --stat\" for interesting - but empty - file changes,\n2012-10-17) to fix them.\n\nJunio C Hamano (5):\n test: add failing tests for \"diff --stat\" to t4049\n diff --stat: status of unmodified pair in diff-q is not zero\n diff --stat: use \"file\" temporary variable to refer to data->files[i]\n diff --stat: move the \"total count\" logic to the last loop\n diff --stat: do not count \"unmerged\" entries\n\n diff.c | 49 +++++++++++++++++++++++++---------------------\n t/t4049-diff-stat-count.sh | 46 ++++++++++++++++++++++++++++++++++++++++++-\n 2 files changed, 72 insertions(+), 23 deletions(-)\n\n-- \n1.8.0.1.331.g808d2af\n"},{"id":"204030","messageId":"1354051310-29093-2-git-send-email-gitster@pobox.com","threadId":"32215","inReplyTo":"1354051310-29093-1-git-send-email-gitster@pobox.com","subject":"[PATCH 1/5] test: add failing tests for \"diff --stat\" to t4049","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-11-27T21:21:46Z","receivedAt":"2012-11-27T21:21:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"There are a few problems in diff.c around --stat area, partially\ncaused by the recent 74faaa1 (Fix \"git diff --stat\" for interesting\n- but empty - file changes, 2012-10-17), and largely caused by the\nearlier change that introduced when --stat-count was added.\n\nAdd a few test pieces to t4049 to expose the issues.\n\nSigned-off-by: Junio C Hamano \n---\n t/t4049-diff-stat-count.sh | 46 +++++++++++++++++++++++++++++++++++++++++++++-\n 1 file changed, 45 insertions(+), 1 deletion(-)\n\ndiff --git a/t/t4049-diff-stat-count.sh b/t/t4049-diff-stat-count.sh\nindex 7b3ef00..e212b11 100755\n--- a/t/t4049-diff-stat-count.sh\n+++ b/t/t4049-diff-stat-count.sh\n@@ -4,12 +4,17 @@\n test_description='diff --stat-count'\n . ./test-lib.sh\n \n-test_expect_success setup '\n+test_expect_success 'setup' '\n \t>a &&\n \t>b &&\n \t>c &&\n \t>d &&\n \tgit add a b c d &&\n+\tgit commit -m initial\n+'\n+\n+test_expect_success 'limit output to 2 (simple)' '\n+\tgit reset --hard &&\n \tchmod +x c d &&\n \techo a >a &&\n \techo b >b &&\n@@ -23,4 +28,43 @@ test_expect_success setup '\n \ttest_i18ncmp expect actual\n '\n \n+test_expect_failure 'binary changes do not count in lines' '\n+\tgit reset --hard &&\n+\tchmod +x c d &&\n+\techo a >a &&\n+\techo b >b &&\n+\tcat \"$TEST_DIRECTORY\"/test-binary-1.png >d &&\n+\tcat >expect <<-\\EOF\n+\t a | 1 +\n+\t b | 1 +\n+\t ...\n+\t 4 files changed, 2 insertions(+)\n+\tEOF\n+\tgit diff --stat --stat-count=2 >actual &&\n+\ttest_i18ncmp expect actual\n+'\n+\n+test_expect_failure 'exclude unmerged entries from total file count' '\n+\tgit reset --hard &&\n+\techo a >a &&\n+\techo b >b &&\n+\tgit ls-files -s a >x &&\n+\tgit rm -f d &&\n+\tfor stage in 1 2 3\n+\tdo\n+\t\tsed -e \"s/ 0\ta/ $stage\td/\" x\n+\tdone |\n+\tgit update-index --index-info &&\n+\techo d >d &&\n+\tchmod +x c d &&\n+\tcat >expect <<-\\EOF\n+\t a | 1 +\n+\t b | 1 +\n+\t ...\n+\t 4 files changed, 3 insertions(+)\n+\tEOF\n+\tgit diff --stat --stat-count=2 >actual &&\n+\ttest_i18ncmp expect actual\n+'\n+\n test_done\n-- \n1.8.0.1.331.g808d2af\n"},{"id":"204031","messageId":"1354051310-29093-3-git-send-email-gitster@pobox.com","threadId":"32215","inReplyTo":"1354051310-29093-1-git-send-email-gitster@pobox.com","subject":"[PATCH 2/5] diff --stat: status of unmodified pair in diff-q is not zero","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-11-27T21:21:47Z","receivedAt":"2012-11-27T21:21:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"It is spelled DIFF_STATUS_UNKNOWN these days, and is different from zero.\n\nSigned-off-by: Junio C Hamano \n---\n diff.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/diff.c b/diff.c\nindex 95bbad6..ce6baa4 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -2411,7 +2411,7 @@ static void builtin_diffstat(const char *name_a, const char *name_b,\n \t}\n \n \tdata = diffstat_add(diffstat, name_a, name_b);\n-\tdata->is_interesting = p->status != 0;\n+\tdata->is_interesting = p->status != DIFF_STATUS_UNKNOWN;\n \n \tif (!one || !two) {\n \t\tdata->is_unmerged = 1;\n-- \n1.8.0.1.331.g808d2af\n"},{"id":"204033","messageId":"1354051310-29093-4-git-send-email-gitster@pobox.com","threadId":"32215","inReplyTo":"1354051310-29093-1-git-send-email-gitster@pobox.com","subject":"[PATCH 3/5] diff --stat: use \"file\" temporary variable to refer to data->files[i]","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-11-27T21:21:48Z","receivedAt":"2012-11-27T21:21:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The generated code shouldn't change but it is easier to read.\n\nSigned-off-by: Junio C Hamano \n---\n diff.c | 26 +++++++++++++-------------\n 1 file changed, 13 insertions(+), 13 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex ce6baa4..e4e70e5 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -1470,8 +1470,8 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n \tfor (i = 0; (i < count) && (i < data->nr); i++) {\n \t\tstruct diffstat_file *file = data->files[i];\n \t\tuintmax_t change = file->added + file->deleted;\n-\t\tif (!data->files[i]->is_interesting &&\n-\t\t\t (change == 0)) {\n+\n+\t\tif (!file->is_interesting && (change == 0)) {\n \t\t\tcount++; /* not shown == room for one more */\n \t\t\tcontinue;\n \t\t}\n@@ -1586,13 +1586,13 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n \t */\n \tfor (i = 0; i < count; i++) {\n \t\tconst char *prefix = \"\";\n-\t\tchar *name = data->files[i]->print_name;\n-\t\tuintmax_t added = data->files[i]->added;\n-\t\tuintmax_t deleted = data->files[i]->deleted;\n+\t\tstruct diffstat_file *file = data->files[i];\n+\t\tchar *name = file->print_name;\n+\t\tuintmax_t added = file->added;\n+\t\tuintmax_t deleted = file->deleted;\n \t\tint name_len;\n \n-\t\tif (!data->files[i]->is_interesting &&\n-\t\t\t (added + deleted == 0)) {\n+\t\tif (!file->is_interesting && (added + deleted == 0)) {\n \t\t\ttotal_files--;\n \t\t\tcontinue;\n \t\t}\n@@ -1611,7 +1611,7 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n \t\t\t\tname = slash;\n \t\t}\n \n-\t\tif (data->files[i]->is_binary) {\n+\t\tif (file->is_binary) {\n \t\t\tfprintf(options->file, \"%s\", line_prefix);\n \t\t\tshow_name(options->file, prefix, name, len);\n \t\t\tfprintf(options->file, \" %*s\", number_width, \"Bin\");\n@@ -1628,7 +1628,7 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n \t\t\tfprintf(options->file, \"\\n\");\n \t\t\tcontinue;\n \t\t}\n-\t\telse if (data->files[i]->is_unmerged) {\n+\t\telse if (file->is_unmerged) {\n \t\t\tfprintf(options->file, \"%s\", line_prefix);\n \t\t\tshow_name(options->file, prefix, name, len);\n \t\t\tfprintf(options->file, \" Unmerged\\n\");\n@@ -1668,10 +1668,10 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n \t\tfprintf(options->file, \"\\n\");\n \t}\n \tfor (i = count; i < data->nr; i++) {\n-\t\tuintmax_t added = data->files[i]->added;\n-\t\tuintmax_t deleted = data->files[i]->deleted;\n-\t\tif (!data->files[i]->is_interesting &&\n-\t\t\t (added + deleted == 0)) {\n+\t\tstruct diffstat_file *file = data->files[i];\n+\t\tuintmax_t added = file->added;\n+\t\tuintmax_t deleted = file->deleted;\n+\t\tif (!file->is_interesting && (added + deleted == 0)) {\n \t\t\ttotal_files--;\n \t\t\tcontinue;\n \t\t}\n-- \n1.8.0.1.331.g808d2af\n"},{"id":"204034","messageId":"1354051310-29093-5-git-send-email-gitster@pobox.com","threadId":"32215","inReplyTo":"1354051310-29093-1-git-send-email-gitster@pobox.com","subject":"[PATCH 4/5] diff --stat: move the \"total count\" logic to the last loop","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-11-27T21:21:49Z","receivedAt":"2012-11-27T21:21:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The diffstat generation logic, with --stat-count limit, is\nimplemented as three loops.\n\n - The first counts the width necessary to show stats up to\n specified number of entries, and notes up to how many entries in\n the data we need to iterate to show the graph;\n\n - The second iterates that many times to draw the graph, adjusts\n the number of \"total modified files\", and counts the total\n added/deleted lines for the part that was shown in the graph;\n\n - The third iterates over the remainder and only does the part to\n count \"total added/deleted lines\" and to adjust \"total modified\n files\" without drawing anything.\n\nMove the logic to count added/deleted lines and modified files from\nthe second loop to the third loop.\n\nThis incidentally fixes a bug. The third loop was not filtering\nbinary changes (counted in bytes) from the total added/deleted as it\nshould. The second loop implemented this correctly, so if a binary\nchange appeared earlier than the --stat-count cutoff, the code\ncounted number of added/deleted lines correctly, but if it appeared\nbeyond the cutoff, the number of lines would have mixed with the\nbyte count in the buggy third loop.\n\nSigned-off-by: Junio C Hamano \n---\n diff.c | 21 ++++++++++++---------\n t/t4049-diff-stat-count.sh | 2 +-\n 2 files changed, 13 insertions(+), 10 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex e4e70e5..4105260 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -1498,7 +1498,7 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n \t\tif (max_change < change)\n \t\t\tmax_change = change;\n \t}\n-\tcount = i; /* min(count, data->nr) */\n+\tcount = i; /* where we can stop scanning in data->files[] */\n \n \t/*\n \t * We have width = stat_width or term_columns() columns total.\n@@ -1592,10 +1592,9 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n \t\tuintmax_t deleted = file->deleted;\n \t\tint name_len;\n \n-\t\tif (!file->is_interesting && (added + deleted == 0)) {\n-\t\t\ttotal_files--;\n+\t\tif (!file->is_interesting && (added + deleted == 0))\n \t\t\tcontinue;\n-\t\t}\n+\n \t\t/*\n \t\t * \"scale\" the filename\n \t\t */\n@@ -1640,8 +1639,6 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n \t\t */\n \t\tadd = added;\n \t\tdel = deleted;\n-\t\tadds += add;\n-\t\tdels += del;\n \n \t\tif (graph_width <= max_change) {\n \t\t\tint total = add + del;\n@@ -1667,7 +1664,8 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n \t\tshow_graph(options->file, '-', del, del_c, reset);\n \t\tfprintf(options->file, \"\\n\");\n \t}\n-\tfor (i = count; i < data->nr; i++) {\n+\n+\tfor (i = 0; i < data->nr; i++) {\n \t\tstruct diffstat_file *file = data->files[i];\n \t\tuintmax_t added = file->added;\n \t\tuintmax_t deleted = file->deleted;\n@@ -1675,8 +1673,13 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n \t\t\ttotal_files--;\n \t\t\tcontinue;\n \t\t}\n-\t\tadds += added;\n-\t\tdels += deleted;\n+\n+\t\tif (!file->is_binary && !file->is_unmerged) {\n+\t\t\tadds += added;\n+\t\t\tdels += deleted;\n+\t\t}\n+\t\tif (i < count)\n+\t\t\tcontinue;\n \t\tif (!extra_shown)\n \t\t\tfprintf(options->file, \"%s ...\\n\", line_prefix);\n \t\textra_shown = 1;\ndiff --git a/t/t4049-diff-stat-count.sh b/t/t4049-diff-stat-count.sh\nindex e212b11..70ee073 100755\n--- a/t/t4049-diff-stat-count.sh\n+++ b/t/t4049-diff-stat-count.sh\n@@ -28,7 +28,7 @@ test_expect_success 'limit output to 2 (simple)' '\n \ttest_i18ncmp expect actual\n '\n \n-test_expect_failure 'binary changes do not count in lines' '\n+test_expect_success 'binary changes do not count in lines' '\n \tgit reset --hard &&\n \tchmod +x c d &&\n \techo a >a &&\n-- \n1.8.0.1.331.g808d2af\n"},{"id":"204032","messageId":"1354051310-29093-6-git-send-email-gitster@pobox.com","threadId":"32215","inReplyTo":"1354051310-29093-1-git-send-email-gitster@pobox.com","subject":"[PATCH 5/5] diff --stat: do not count \"unmerged\" entries","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-11-27T21:21:50Z","receivedAt":"2012-11-27T21:21:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Even though we show a separate *UNMERGED* entry in the patch and\ndiffstat output (or in the --raw format, for that matter) in\naddition to and separately from the diff against the specified stage\n(defaulting to #2) for unmerged paths, they should not be counted in\nthe total number of files affected---that would lead to counting the\nsame path twice.\n\nThe separation done by the previous step makes this fix simple and\nstraightforward. Among the filepairs in diff_queue, paths that\nweren't modified, and the extra \"unmerged\" entries do not count as\ntotal number of files.\n\nSigned-off-by: Junio C Hamano \n---\n diff.c | 6 ++++--\n t/t4049-diff-stat-count.sh | 2 +-\n 2 files changed, 5 insertions(+), 3 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex 4105260..26ede82 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -1669,12 +1669,14 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n \t\tstruct diffstat_file *file = data->files[i];\n \t\tuintmax_t added = file->added;\n \t\tuintmax_t deleted = file->deleted;\n-\t\tif (!file->is_interesting && (added + deleted == 0)) {\n+\n+\t\tif (file->is_unmerged ||\n+\t\t (!file->is_interesting && (added + deleted == 0))) {\n \t\t\ttotal_files--;\n \t\t\tcontinue;\n \t\t}\n \n-\t\tif (!file->is_binary && !file->is_unmerged) {\n+\t\tif (!file->is_binary) {\n \t\t\tadds += added;\n \t\t\tdels += deleted;\n \t\t}\ndiff --git a/t/t4049-diff-stat-count.sh b/t/t4049-diff-stat-count.sh\nindex 70ee073..37f50cd 100755\n--- a/t/t4049-diff-stat-count.sh\n+++ b/t/t4049-diff-stat-count.sh\n@@ -44,7 +44,7 @@ test_expect_success 'binary changes do not count in lines' '\n \ttest_i18ncmp expect actual\n '\n \n-test_expect_failure 'exclude unmerged entries from total file count' '\n+test_expect_success 'exclude unmerged entries from total file count' '\n \tgit reset --hard &&\n \techo a >a &&\n \techo b >b &&\n-- \n1.8.0.1.331.g808d2af\n"},{"id":"204041","messageId":"7vsj7ulmad.fsf@alter.siamese.dyndns.org","threadId":"32215","inReplyTo":"1354051310-29093-1-git-send-email-gitster@pobox.com","subject":"[PATCH 6/5] diff --shortstat: do not count \"unmerged\" entries","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-11-27T22:21:30Z","receivedAt":"2012-11-27T22:21:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Fix the same issue as the previous one for \"git diff --stat\";\nunmerged entries was doubly-counted.\n\nSigned-off-by: Junio C Hamano \n---\n diff.c | 5 ++---\n 1 file changed, 2 insertions(+), 3 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex 26ede82..374b235 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -1701,9 +1701,8 @@ static void show_shortstats(struct diffstat_t *data, struct diff_options *option\n \t\tint added = data->files[i]->added;\n \t\tint deleted= data->files[i]->deleted;\n \n-\t\tif (data->files[i]->is_unmerged)\n-\t\t\tcontinue;\n-\t\tif (!data->files[i]->is_interesting && (added + deleted == 0)) {\n+\t\tif (data->files[i]->is_unmerged ||\n+\t\t (!data->files[i]->is_interesting && (added + deleted == 0))) {\n \t\t\ttotal_files--;\n \t\t} else if (!data->files[i]->is_binary) { /* don't count bytes */\n \t\t\tadds += added;\n-- \n1.8.0.1.337.gd2c5e06\n"},{"id":"204242","messageId":"50B71B33.1090000@viscovery.net","threadId":"32215","inReplyTo":"1354051310-29093-1-git-send-email-gitster@pobox.com","subject":"[PATCH] t4049: avoid test failures on filemode challenged file systems (Windows)","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2012-11-29T08:22:11Z","receivedAt":"2012-11-29T08:22:11Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"From: Johannes Sixt \n\nThe earlier change 74faaa16 (Fix \"git diff --stat\" for interesting - but\nempty - file changes) needed to change the count of differing files\nbecause the executable-bit changes of two empty files are now counted.\n\nOn file systems that do not record the executable bit, however, the old\nfile count was actually correct (and the updated tests fail) because the\nmode change cannot be diagnosed by looking at the file system alone.\n\nChange the mode not only on the file system, but also in the index;\ncompare the new state against the commit, so that the tests do not depend\non the file system's ability to record the executable bit, when possible.\n\nThe exception is the test for unmerged entries, which does depend on the\nfile system; we have to skip it.\n\nSigned-off-by: Johannes Sixt \n---\nAm 11/27/2012 22:21, schrieb Junio C Hamano:\n> It turns out that there are at least two bugs in the diffstat\n> counting code. This series comes on top of the earlier 74faaa1 (Fix\n> \"git diff --stat\" for interesting - but empty - file changes,\n> 2012-10-17) to fix them.\n\nThe tests still fail on Windows. I am not sure whether there is a\ndifference in comparing the file system against the index or a commit.\nIf there is, then the updated tests might not test the same thing.\n\n-- Hannes\n\n t/t4049-diff-stat-count.sh | 10 +++++-----\n 1 file changed, 5 insertions(+), 5 deletions(-)\n\ndiff --git a/t/t4049-diff-stat-count.sh b/t/t4049-diff-stat-count.sh\nindex 37f50cd..9e29e71 100755\n--- a/t/t4049-diff-stat-count.sh\n+++ b/t/t4049-diff-stat-count.sh\n@@ -15,7 +15,7 @@ test_expect_success 'setup' '\n \n test_expect_success 'limit output to 2 (simple)' '\n \tgit reset --hard &&\n-\tchmod +x c d &&\n+\ttest_chmod +x c d &&\n \techo a >a &&\n \techo b >b &&\n \tcat >expect <<-\\EOF\n@@ -24,13 +24,13 @@ test_expect_success 'limit output to 2 (simple)' '\n \t ...\n \t 4 files changed, 2 insertions(+)\n \tEOF\n-\tgit diff --stat --stat-count=2 >actual &&\n+\tgit diff --stat --stat-count=2 HEAD >actual &&\n \ttest_i18ncmp expect actual\n '\n \n test_expect_success 'binary changes do not count in lines' '\n \tgit reset --hard &&\n-\tchmod +x c d &&\n+\ttest_chmod +x c d &&\n \techo a >a &&\n \techo b >b &&\n \tcat \"$TEST_DIRECTORY\"/test-binary-1.png >d &&\n@@ -40,11 +40,11 @@ test_expect_success 'binary changes do not count in lines' '\n \t ...\n \t 4 files changed, 2 insertions(+)\n \tEOF\n-\tgit diff --stat --stat-count=2 >actual &&\n+\tgit diff --stat --stat-count=2 HEAD >actual &&\n \ttest_i18ncmp expect actual\n '\n \n-test_expect_success 'exclude unmerged entries from total file count' '\n+test_expect_success FILEMODE 'exclude unmerged entries from total file count' '\n \tgit reset --hard &&\n \techo a >a &&\n \techo b >b &&\n-- \n1.8.0.1.1524.gaf6675c\n"},{"id":"204259","messageId":"7v38zss7zb.fsf@alter.siamese.dyndns.org","threadId":"32215","inReplyTo":"50B71B33.1090000@viscovery.net","subject":"Re: [PATCH] t4049: avoid test failures on filemode challenged file systems (Windows)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-11-29T16:13:12Z","receivedAt":"2012-11-29T16:13:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Sixt writes:\n\n>> It turns out that there are at least two bugs in the diffstat\n>> counting code. This series comes on top of the earlier 74faaa1 (Fix\n>> \"git diff --stat\" for interesting - but empty - file changes,\n>> 2012-10-17) to fix them.\n>\n> The tests still fail on Windows. I am not sure whether there is a\n> difference in comparing the file system against the index or a commit.\n> If there is, then the updated tests might not test the same thing.\n\nThe hunks in the patch look fine. The last one that tests unmerged\nentries do not have to have \"chmod\" if it gives you trouble (you\nwould need to reduce number of files from 4 to 3 if you go that\nroute, I think).\n"},{"id":"204270","messageId":"7vfw3sqoup.fsf@alter.siamese.dyndns.org","threadId":"32215","inReplyTo":"7v38zss7zb.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] t4049: avoid test failures on filemode challenged file systems (Windows)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-11-29T17:51:42Z","receivedAt":"2012-11-29T17:51:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Johannes Sixt writes:\n>\n>>> It turns out that there are at least two bugs in the diffstat\n>>> counting code. This series comes on top of the earlier 74faaa1 (Fix\n>>> \"git diff --stat\" for interesting - but empty - file changes,\n>>> 2012-10-17) to fix them.\n>>\n>> The tests still fail on Windows. I am not sure whether there is a\n>> difference in comparing the file system against the index or a commit.\n>> If there is, then the updated tests might not test the same thing.\n>\n> The hunks in the patch look fine. The last one that tests unmerged\n> entries do not have to have \"chmod\" if it gives you trouble (you\n> would need to reduce number of files from 4 to 3 if you go that\n> route, I think).\n\nThat is, something like this.\n\n-- >8 --\nSubject: [PATCH] t4049: refocus tests\n\nThe primary thing Linus's patch wanted to change was to make sure\nthat 0-line change appears for a mode-only change. Update the\nfirst test to chmod a file that we can see in the output (limited\nby --stat-count) to demonstrate it. Also make sure to use test_chmod\nand compare the index and the tree, so that we can run this test\neven on a filesystem without permission bits.\n\nLater two tests are about fixes to separate issues that were\nintroduced and/or uncovered by Linus's patch as a side effect, but\nthe issues are not related to mode-only changes. Remove chmod from\nthe tests.\n\nSigned-off-by: Junio C Hamano \n---\n t/t4049-diff-stat-count.sh | 20 +++++++++-----------\n 1 file changed, 9 insertions(+), 11 deletions(-)\n\ndiff --git a/t/t4049-diff-stat-count.sh b/t/t4049-diff-stat-count.sh\nindex 37f50cd..5b594e8 100755\n--- a/t/t4049-diff-stat-count.sh\n+++ b/t/t4049-diff-stat-count.sh\n@@ -13,32 +13,31 @@ test_expect_success 'setup' '\n \tgit commit -m initial\n '\n \n-test_expect_success 'limit output to 2 (simple)' '\n+test_expect_success 'mode-only change show as a 0-line change' '\n \tgit reset --hard &&\n-\tchmod +x c d &&\n+\ttest_chmod +x b d &&\n \techo a >a &&\n-\techo b >b &&\n+\techo c >c &&\n \tcat >expect <<-\\EOF\n \t a | 1 +\n-\t b | 1 +\n+\t b | 0\n \t ...\n \t 4 files changed, 2 insertions(+)\n \tEOF\n-\tgit diff --stat --stat-count=2 >actual &&\n+\tgit diff --stat --stat-count=2 HEAD >actual &&\n \ttest_i18ncmp expect actual\n '\n \n test_expect_success 'binary changes do not count in lines' '\n \tgit reset --hard &&\n-\tchmod +x c d &&\n \techo a >a &&\n-\techo b >b &&\n+\techo c >c &&\n \tcat \"$TEST_DIRECTORY\"/test-binary-1.png >d &&\n \tcat >expect <<-\\EOF\n \t a | 1 +\n-\t b | 1 +\n+\t c | 1 +\n \t ...\n-\t 4 files changed, 2 insertions(+)\n+\t 3 files changed, 2 insertions(+)\n \tEOF\n \tgit diff --stat --stat-count=2 >actual &&\n \ttest_i18ncmp expect actual\n@@ -56,12 +55,11 @@ test_expect_success 'exclude unmerged entries from total file count' '\n \tdone |\n \tgit update-index --index-info &&\n \techo d >d &&\n-\tchmod +x c d &&\n \tcat >expect <<-\\EOF\n \t a | 1 +\n \t b | 1 +\n \t ...\n-\t 4 files changed, 3 insertions(+)\n+\t 3 files changed, 3 insertions(+)\n \tEOF\n \tgit diff --stat --stat-count=2 >actual &&\n \ttest_i18ncmp expect actual\n-- \n1.8.0.1.407.g3c58eb7\n"},{"id":"204294","messageId":"7vfw3sp232.fsf@alter.siamese.dyndns.org","threadId":"32215","inReplyTo":"7vfw3sqoup.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] t4049: avoid test failures on filemode challenged file systems (Windows)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-11-29T20:48:49Z","receivedAt":"2012-11-29T20:48:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Junio C Hamano writes:\n>> ...\n>> The hunks in the patch look fine. The last one that tests unmerged\n>> entries do not have to have \"chmod\" if it gives you trouble (you\n>> would need to reduce number of files from 4 to 3 if you go that\n>> route, I think).\n>\n> That is, something like this.\n\nI've tested this with the testpen set on vfat mounted on my Linux\nbox, i.e.\n\n $ cd t\n $ sh t4049-diff-stat-count.sh --root=/media/5599-553B/test -v\n \nand it seems to work OK, so I'll be merging the topic with this\npatch to 'master' later today.\n\nThanks for noticing.\n\n> -- >8 --\n> Subject: [PATCH] t4049: refocus tests\n>\n> The primary thing Linus's patch wanted to change was to make sure\n> that 0-line change appears for a mode-only change. Update the\n> first test to chmod a file that we can see in the output (limited\n> by --stat-count) to demonstrate it. Also make sure to use test_chmod\n> and compare the index and the tree, so that we can run this test\n> even on a filesystem without permission bits.\n>\n> Later two tests are about fixes to separate issues that were\n> introduced and/or uncovered by Linus's patch as a side effect, but\n> the issues are not related to mode-only changes. Remove chmod from\n> the tests.\n>\n> Signed-off-by: Junio C Hamano \n> ---\n> t/t4049-diff-stat-count.sh | 20 +++++++++-----------\n> 1 file changed, 9 insertions(+), 11 deletions(-)\n>\n> diff --git a/t/t4049-diff-stat-count.sh b/t/t4049-diff-stat-count.sh\n> index 37f50cd..5b594e8 100755\n> --- a/t/t4049-diff-stat-count.sh\n> +++ b/t/t4049-diff-stat-count.sh\n> @@ -13,32 +13,31 @@ test_expect_success 'setup' '\n> \tgit commit -m initial\n> '\n> \n> -test_expect_success 'limit output to 2 (simple)' '\n> +test_expect_success 'mode-only change show as a 0-line change' '\n> \tgit reset --hard &&\n> -\tchmod +x c d &&\n> +\ttest_chmod +x b d &&\n> \techo a >a &&\n> -\techo b >b &&\n> +\techo c >c &&\n> \tcat >expect <<-\\EOF\n> \t a | 1 +\n> -\t b | 1 +\n> +\t b | 0\n> \t ...\n> \t 4 files changed, 2 insertions(+)\n> \tEOF\n> -\tgit diff --stat --stat-count=2 >actual &&\n> +\tgit diff --stat --stat-count=2 HEAD >actual &&\n> \ttest_i18ncmp expect actual\n> '\n> \n> test_expect_success 'binary changes do not count in lines' '\n> \tgit reset --hard &&\n> -\tchmod +x c d &&\n> \techo a >a &&\n> -\techo b >b &&\n> +\techo c >c &&\n> \tcat \"$TEST_DIRECTORY\"/test-binary-1.png >d &&\n> \tcat >expect <<-\\EOF\n> \t a | 1 +\n> -\t b | 1 +\n> +\t c | 1 +\n> \t ...\n> -\t 4 files changed, 2 insertions(+)\n> +\t 3 files changed, 2 insertions(+)\n> \tEOF\n> \tgit diff --stat --stat-count=2 >actual &&\n> \ttest_i18ncmp expect actual\n> @@ -56,12 +55,11 @@ test_expect_success 'exclude unmerged entries from total file count' '\n> \tdone |\n> \tgit update-index --index-info &&\n> \techo d >d &&\n> -\tchmod +x c d &&\n> \tcat >expect <<-\\EOF\n> \t a | 1 +\n> \t b | 1 +\n> \t ...\n> -\t 4 files changed, 3 insertions(+)\n> +\t 3 files changed, 3 insertions(+)\n> \tEOF\n> \tgit diff --stat --stat-count=2 >actual &&\n> \ttest_i18ncmp expect actual\n"},{"id":"204321","messageId":"50B86438.3010806@viscovery.net","threadId":"32215","inReplyTo":"7vfw3sp232.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] t4049: avoid test failures on filemode challenged file systems (Windows)","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2012-11-30T07:46:00Z","receivedAt":"2012-11-30T07:46:00Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 11/29/2012 21:48, schrieb Junio C Hamano:\n> I've tested this with the testpen set on vfat mounted on my Linux\n> box, ...\n> and it seems to work OK,\n\nWorks well here on Windows, too.\n\nThanks,\n-- Hannes\n"},{"id":"204372","messageId":"CALWbr2xYrWkfSCUc7gocVGLQP_RHjWrMmFspPe5O_ekjisNqxw@mail.gmail.com","threadId":"32215","inReplyTo":"1354051310-29093-1-git-send-email-gitster@pobox.com","subject":"Re: [PATCH 0/5] \"diff --stat\" counting fixes","fromName":"Antoine Pelisse","fromEmail":"apelisse@gmail.com","sentAt":"2012-12-01T10:29:53Z","receivedAt":"2012-12-01T10:29:53Z","isPatch":true,"sender":{"key":"apelisse@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1929644?v=4"},"body":"Hi Junio,\n\nThat does make a lot of sense and I would have indeed missed a couple\nof things here.\n\nI've been thinking about that \"Unmerged\" line quite a lot, and I can't\nget myself any good reason to keep it.\nWould you mind taking a couple of minutes to make it clear ?\n\nI feel like (but I can obviously be wrong):\n1. The info is redundant. When performing a merge, all diffs (without\n--staged flag) are unmerged\n2. While status shows the line once, while diff shows the diff for the file\nonce, while diff --shortstat counts the file once, diff --stat shows two\nlines for the file.\n3. diff --numstat shows two lines for the same file. As a script\nwriter (I guess that's what it's meant for), I would definitely expect\nuniqueness in third column/filenames.\n\nCheers,\nAntoine\n\nOn Tue, Nov 27, 2012 at 10:21 PM, Junio C Hamano wrote:\n>\n> It turns out that there are at least two bugs in the diffstat\n> counting code. This series comes on top of the earlier 74faaa1 (Fix\n> \"git diff --stat\" for interesting - but empty - file changes,\n> 2012-10-17) to fix them.\n>\n> Junio C Hamano (5):\n> test: add failing tests for \"diff --stat\" to t4049\n> diff --stat: status of unmodified pair in diff-q is not zero\n> diff --stat: use \"file\" temporary variable to refer to data->files[i]\n> diff --stat: move the \"total count\" logic to the last loop\n> diff --stat: do not count \"unmerged\" entries\n>\n> diff.c | 49 +++++++++++++++++++++++++---------------------\n> t/t4049-diff-stat-count.sh | 46 ++++++++++++++++++++++++++++++++++++++++++-\n> 2 files changed, 72 insertions(+), 23 deletions(-)\n>\n> --\n> 1.8.0.1.331.g808d2af\n>\n"},{"id":"204404","messageId":"7vhao5mbtx.fsf@alter.siamese.dyndns.org","threadId":"32215","inReplyTo":"CALWbr2xYrWkfSCUc7gocVGLQP_RHjWrMmFspPe5O_ekjisNqxw@mail.gmail.com","subject":"Re: [PATCH 0/5] \"diff --stat\" counting fixes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-12-02T02:23:22Z","receivedAt":"2012-12-02T02:23:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Antoine Pelisse writes:\n\n> I feel like (but I can obviously be wrong):\n> 1. The info is redundant. When performing a merge, all diffs (without\n> --staged flag) are unmerged\n\nYes, it is redundant. They are primarily meant as a warning to\nanybody who runs \"git diff --stat\" while their index is not fully\nmerged so that they do not mistakenly think they are looking at\nmeaningful numbers. The number of added lines likely includes the\nconflict markers you haven't dealt with ;-)\n\n> 2. While status shows the line once, while diff shows the diff for the file\n> once, while diff --shortstat counts the file once, diff --stat shows two\n> lines for the file.\n\nYeah, don't use shortstat while your index is unmerged.\n\n> 3. diff --numstat shows two lines for the same file. As a script\n> writer (I guess that's what it's meant for), I would definitely expect\n> uniqueness in third column/filenames.\n\nThen your script wouldn't receive any hint in the output that you\nare reading from a nonsense input. When you see the same filename\ntwice, you will know there is something strange (not just \"I seem to\nhave more added lines than I thought I added -- Ah, I see added\nfiles from both sides because I still have conflicts unresolved\")\nthat gives you a chance to notice.\n"},{"id":"204405","messageId":"7vd2ytmbrs.fsf@alter.siamese.dyndns.org","threadId":"32215","inReplyTo":"50B86438.3010806@viscovery.net","subject":"Re: [PATCH] t4049: avoid test failures on filemode challenged file systems (Windows)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-12-02T02:24:39Z","receivedAt":"2012-12-02T02:24:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Sixt writes:\n\n> Am 11/29/2012 21:48, schrieb Junio C Hamano:\n>> I've tested this with the testpen set on vfat mounted on my Linux\n>> box, ...\n>> and it seems to work OK,\n>\n> Works well here on Windows, too.\n\nThanks for checking.\n"}]}