{"thread":{"id":"7993","subject":"[PATCH 3/3] Add test for blame corner cases.","startedAt":"2007-05-06T06:02:41Z","lastAt":"2007-05-06T06:02:43Z","messageCount":3,"participants":["Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"41173","messageId":"11784313631165-git-send-email-junkio@cox.net","threadId":"7993","inReplyTo":null,"subject":"[PATCH 1/3] blame: handle the tail-match correctly in -C/-M","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-05-06T06:02:41Z","receivedAt":"2007-05-06T06:02:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The -C/-M option to blame tries to find a section of a preimage\nfile by running diff against the lines whose origin is still\nunknown, and excluding the different parts.  The code however\ndid not cover the case where the tail part of the section\nmatched, which we handle for the normal non-move/copy codepath.\n\nThis breakage was most visible when preimage file matches in its\nentirety and failed to pass blame in such a case.\n\nSigned-off-by: Junio C Hamano <junkio@cox.net>\n---\n builtin-blame.c |   52 ++++++++++++++++++++++++++++++++++++++++------------\n 1 files changed, 40 insertions(+), 12 deletions(-)\n\ndiff --git a/builtin-blame.c b/builtin-blame.c\nindex 8919b02..f8843e6 100644\n--- a/builtin-blame.c\n+++ b/builtin-blame.c\n@@ -891,6 +891,39 @@ static void copy_split_if_better(struct scoreboard *sb,\n }\n \n /*\n+ * We are looking at a part of the final image represented by\n+ * ent (tlno and same are offset by ent->s_lno).\n+ * tlno is where we are looking at in the final image.\n+ * up to (but not including) same match preimage.\n+ * plno is where we are looking at in the preimage.\n+ *\n+ * <-------------- final image ---------------------->\n+ *       <------ent------>\n+ *         ^tlno ^same\n+ *    <---------preimage----->\n+ *         ^plno\n+ *\n+ * All line numbers are 0-based.\n+ */\n+static void handle_split(struct scoreboard *sb,\n+\t\t\t struct blame_entry *ent,\n+\t\t\t int tlno, int plno, int same,\n+\t\t\t struct origin *parent,\n+\t\t\t struct blame_entry *split)\n+{\n+\tif (ent->num_lines <= tlno)\n+\t\treturn;\n+\tif (tlno < same) {\n+\t\tstruct blame_entry this[3];\n+\t\ttlno += ent->s_lno;\n+\t\tsame += ent->s_lno;\n+\t\tsplit_overlap(this, ent, tlno, plno, same, parent);\n+\t\tcopy_split_if_better(sb, split, this);\n+\t\tdecref_split(this);\n+\t}\n+}\n+\n+/*\n  * Find the lines from parent that are the same as ent so that\n  * we can pass blames to it.  file_p has the blob contents for\n  * the parent.\n@@ -922,26 +955,21 @@ static void find_copy_in_blob(struct scoreboard *sb,\n \n \tpatch = compare_buffer(file_p, &file_o, 1);\n \n+\t/*\n+\t * file_o is a part of final image we are annotating.\n+\t * file_p partially may match that image.\n+\t */\n \tmemset(split, 0, sizeof(struct blame_entry [3]));\n \tplno = tlno = 0;\n \tfor (i = 0; i < patch->num; i++) {\n \t\tstruct chunk *chunk = &patch->chunks[i];\n \n-\t\t/* tlno to chunk->same are the same as ent */\n-\t\tif (ent->num_lines <= tlno)\n-\t\t\tbreak;\n-\t\tif (tlno < chunk->same) {\n-\t\t\tstruct blame_entry this[3];\n-\t\t\tsplit_overlap(this, ent,\n-\t\t\t\t      tlno + ent->s_lno, plno,\n-\t\t\t\t      chunk->same + ent->s_lno,\n-\t\t\t\t      parent);\n-\t\t\tcopy_split_if_better(sb, split, this);\n-\t\t\tdecref_split(this);\n-\t\t}\n+\t\thandle_split(sb, ent, tlno, plno, chunk->same, parent, split);\n \t\tplno = chunk->p_next;\n \t\ttlno = chunk->t_next;\n \t}\n+\t/* remainder, if any, all match the preimage */\n+\thandle_split(sb, ent, tlno, plno, ent->num_lines, parent, split);\n \tfree_patch(patch);\n }\n \n-- \n1.5.2.rc1.709.g9462\n"},{"id":"41174","messageId":"11784313642492-git-send-email-junkio@cox.net","threadId":"7993","inReplyTo":"11784313631165-git-send-email-junkio@cox.net","subject":"[PATCH 2/3] blame: -C -C -C","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-05-06T06:02:42Z","receivedAt":"2007-05-06T06:02:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Existing \"blame -C -C\" would not find that the latter half of\nthe file2 came from the existing file1:\n\n\t... both file1 and file2 are tracked ...\n\t$ cat file1 >>file2\n\t$ git add file1 file2\n\t$ git commit\n\nThis is because we avoid the expensive find-copies-harder code\nthat makes unchanged file (in this case, file1) as a candidate\nfor copy & paste source when annotating an existing file\n(file2).  The third -C now allows it.  However, this obviously\nmakes the process very expensive.  We've actually seen this\npatch before, but I dismissed it because it covers such a narrow\n(and arguably stupid) corner case.\n\nSigned-off-by: Junio C Hamano <junkio@cox.net>\n---\n builtin-blame.c |   15 +++++++++++++--\n 1 files changed, 13 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin-blame.c b/builtin-blame.c\nindex f8843e6..65d029a 100644\n--- a/builtin-blame.c\n+++ b/builtin-blame.c\n@@ -55,6 +55,7 @@ static int num_commits;\n #define PICKAXE_BLAME_MOVE\t\t01\n #define PICKAXE_BLAME_COPY\t\t02\n #define PICKAXE_BLAME_COPY_HARDER\t04\n+#define PICKAXE_BLAME_COPY_HARDEST\t010\n \n /*\n  * blame for a blame_entry with score lower than these thresholds\n@@ -1079,8 +1080,9 @@ static int find_copy_in_parent(struct scoreboard *sb,\n \t * and this code needs to be after diff_setup_done(), which\n \t * usually makes find-copies-harder imply copy detection.\n \t */\n-\tif ((opt & PICKAXE_BLAME_COPY_HARDER) &&\n-\t    (!porigin || strcmp(target->path, porigin->path)))\n+\tif ((opt & PICKAXE_BLAME_COPY_HARDEST)\n+\t    || ((opt & PICKAXE_BLAME_COPY_HARDER)\n+\t\t&& (!porigin || strcmp(target->path, porigin->path))))\n \t\tdiff_opts.find_copies_harder = 1;\n \n \tif (is_null_sha1(target->commit->object.sha1))\n@@ -2127,6 +2129,15 @@ int cmd_blame(int argc, const char **argv, const char *prefix)\n \t\t\tblame_move_score = parse_score(arg+2);\n \t\t}\n \t\telse if (!prefixcmp(arg, \"-C\")) {\n+\t\t\t/*\n+\t\t\t * -C enables copy from removed files;\n+\t\t\t * -C -C enables copy from existing files, but only\n+\t\t\t *       when blaming a new file;\n+\t\t\t * -C -C -C enables copy from existing files for\n+\t\t\t *          everybody\n+\t\t\t */\n+\t\t\tif (opt & PICKAXE_BLAME_COPY_HARDER)\n+\t\t\t\topt |= PICKAXE_BLAME_COPY_HARDEST;\n \t\t\tif (opt & PICKAXE_BLAME_COPY)\n \t\t\t\topt |= PICKAXE_BLAME_COPY_HARDER;\n \t\t\topt |= PICKAXE_BLAME_COPY | PICKAXE_BLAME_MOVE;\n-- \n1.5.2.rc1.709.g9462\n"},{"id":"41172","messageId":"11784313642623-git-send-email-junkio@cox.net","threadId":"7993","inReplyTo":"11784313631165-git-send-email-junkio@cox.net","subject":"[PATCH 3/3] Add test for blame corner cases.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-05-06T06:02:43Z","receivedAt":"2007-05-06T06:02:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Signed-off-by: Junio C Hamano <junkio@cox.net>\n---\n t/t8003-blame.sh |  132 ++++++++++++++++++++++++++++++++++++++++++++++++++++++\n 1 files changed, 132 insertions(+), 0 deletions(-)\n create mode 100755 t/t8003-blame.sh\n\ndiff --git a/t/t8003-blame.sh b/t/t8003-blame.sh\nnew file mode 100755\nindex 0000000..db51b3a\n--- /dev/null\n+++ b/t/t8003-blame.sh\n@@ -0,0 +1,132 @@\n+#!/bin/sh\n+\n+test_description='git blame corner cases'\n+. ./test-lib.sh\n+\n+pick_fc='s/^[0-9a-f^]* *\\([^ ]*\\) *(\\([^ ]*\\) .*/\\1-\\2/'\n+\n+test_expect_success setup '\n+\n+\techo A A A A A >one &&\n+\techo B B B B B >two &&\n+\techo C C C C C >tres &&\n+\techo ABC >mouse &&\n+\tgit add one two tres mouse &&\n+\ttest_tick &&\n+\tGIT_AUTHOR_NAME=Initial git commit -m Initial &&\n+\n+\tcat one >uno &&\n+\tmv two dos &&\n+\tcat one >>tres &&\n+\techo DEF >>mouse\n+\tgit add uno dos tres mouse &&\n+\ttest_tick &&\n+\tGIT_AUTHOR_NAME=Second git commit -a -m Second &&\n+\n+\techo GHIJK >>mouse &&\n+\tgit add mouse &&\n+\ttest_tick &&\n+\tGIT_AUTHOR_NAME=Third git commit -m Third &&\n+\n+\tcat mouse >cow &&\n+\tgit add cow &&\n+\ttest_tick &&\n+\tGIT_AUTHOR_NAME=Fourth git commit -m Fourth &&\n+\n+\t{\n+\t\techo ABC\n+\t\techo DEF\n+\t\techo XXXX\n+\t\techo GHIJK\n+\t} >cow &&\n+\tgit add cow &&\n+\ttest_tick &&\n+\tGIT_AUTHOR_NAME=Fifth git commit -m Fifth\n+'\n+\n+test_expect_success 'straight copy without -C' '\n+\n+\tgit blame uno | grep Second\n+\n+'\n+\n+test_expect_success 'straight move without -C' '\n+\n+\tgit blame dos | grep Initial\n+\n+'\n+\n+test_expect_success 'straight copy with -C' '\n+\n+\tgit blame -C1 uno | grep Second\n+\n+'\n+\n+test_expect_success 'straight move with -C' '\n+\n+\tgit blame -C1 dos | grep Initial\n+\n+'\n+\n+test_expect_success 'straight copy with -C -C' '\n+\n+\tgit blame -C -C1 uno | grep Initial\n+\n+'\n+\n+test_expect_success 'straight move with -C -C' '\n+\n+\tgit blame -C -C1 dos | grep Initial\n+\n+'\n+\n+test_expect_success 'append without -C' '\n+\n+\tgit blame -L2 tres | grep Second\n+\n+'\n+\n+test_expect_success 'append with -C' '\n+\n+\tgit blame -L2 -C1 tres | grep Second\n+\n+'\n+\n+test_expect_success 'append with -C -C' '\n+\n+\tgit blame -L2 -C -C1 tres | grep Second\n+\n+'\n+\n+test_expect_success 'append with -C -C -C' '\n+\n+\tgit blame -L2 -C -C -C1 tres | grep Initial\n+\n+'\n+\n+test_expect_success 'blame wholesale copy' '\n+\n+\tgit blame -f -C -C1 HEAD^ -- cow | sed -e \"$pick_fc\" >current &&\n+\t{\n+\t\techo mouse-Initial\n+\t\techo mouse-Second\n+\t\techo mouse-Third\n+\t} >expected &&\n+\tdiff -u expected current\n+\n+'\n+\n+test_expect_success 'blame wholesale copy and more' '\n+\n+\tgit blame -f -C -C1 HEAD -- cow | sed -e \"$pick_fc\" >current &&\n+\t{\n+\t\techo mouse-Initial\n+\t\techo mouse-Second\n+\t\techo cow-Fifth\n+\t\techo mouse-Third\n+\t} >expected &&\n+\tdiff -u expected current\n+\n+'\n+\n+test_done\n-- \n1.5.2.rc1.709.g9462\n"}]}