{"thread":{"id":"25405","subject":"[PATCH v5 0/4] Detection of directory renames","startedAt":"2010-10-09T21:31:31Z","lastAt":"2010-10-10T19:59:55Z","messageCount":9,"participants":["Yann Dirson"],"isPatch":true,"patchVersion":5,"patchTotal":4},"messages":[{"id":"153095","messageId":"1286659895-1813-1-git-send-email-ydirson@altern.org","threadId":"25405","inReplyTo":null,"subject":"[PATCH v5 0/4] Detection of directory renames","fromName":"Yann Dirson","fromEmail":"ydirson@altern.org","sentAt":"2010-10-09T21:31:31Z","receivedAt":"2010-10-09T21:31:31Z","isPatch":true,"sender":{"key":"ydirson@altern.org","avatar":"https://avatars.githubusercontent.com/u/1190950?v=4"},"body":"Evolutions from v4 to v5:\n\n(Jonathan Nieder) use memcpy, not strncpy overkill\n(Jonathan Nieder) split 2 funcs out of diffcore_bulk_renames\n(Jonathan Nieder) more comments\n(Jonathan Nieder) avoid allocating often-useless filespec too early\n(Jonathan Nieder) split into new func\n(Ævar Arnfjörð Bjarmason) convert C99 comments intended to stay to C89 ones\n\n(Jonathan Nieder, Ævar Arnfjörð Bjarmason) reworked style of test script\n(Jonathan Nieder, Sverre Rabbelier) use compare_diff_raw, not compare_diff_patch;\n\tanonymize hashes and scores\n(Jonathan Nieder) use \"git commit\" in test instead of only plumbing, and use test_tick\n\nlisted a number of additional FIXME's\nchanged wording \"factorization\" -> \"bulk move\"\nchanged --detect-dir-renames to --detect-bulk-moves and\n\t--hide-dir-rename-details to --hide-bulk-move-details\nfirst attempt at showing the \"/*\" suffix in bulk moves, for comment\nstop maintaining testcases for --hide-bulk-move-details, I'm just keeping\n\tthe patch adding the flag in sync with other code changes anyway.\n\n\nYann Dirson (4):\n  Introduce bulk-move detection in diffcore.\n  Add testcases for the --detect-bulk-moves diffcore flag.\n  [RFC] Only show bulkmoves in output.\n  [RFC] Allow hiding renames of individual files involved in a\n    directory rename.\n\n diff-lib.c                       |    6 +-\n diff.c                           |   12 ++\n diff.h                           |    6 +\n diffcore-rename.c                |  349 +++++++++++++++++++++++++++++++++++++-\n diffcore.h                       |    1 +\n t/t4046-diff-rename-factorize.sh |  251 +++++++++++++++++++++++++++\n tree-diff.c                      |    4 +-\n 7 files changed, 617 insertions(+), 12 deletions(-)\n create mode 100755 t/t4046-diff-rename-factorize.sh\n\n-- \n1.7.2.3\n"},{"id":"153098","messageId":"1286659895-1813-2-git-send-email-ydirson@altern.org","threadId":"25405","inReplyTo":"1286659895-1813-1-git-send-email-ydirson@altern.org","subject":"[PATCH v5 1/4] Introduce bulk-move detection in diffcore.","fromName":"Yann Dirson","fromEmail":"ydirson@altern.org","sentAt":"2010-10-09T21:31:32Z","receivedAt":"2010-10-09T21:31:32Z","isPatch":true,"sender":{"key":"ydirson@altern.org","avatar":"https://avatars.githubusercontent.com/u/1190950?v=4"},"body":"This feature tries to group together files moving from and to\nidentical directories - the most common case being directory renames.\n\nIt is activated by the new --detect-bulk-moves diffcore\nflag.\n\nThis is only the first step, adding the basic functionnality and\nadding support to raw diff output (and it is not supported in\nunified-diff output yet)\n\nThe output of raw diff is displayed as \"Rnnn a/* b/\".\n\nOther future developements to be made on top of this include:\n* extension of unified-diff format to express this\n* display as such the special case of directory move/rename\n* application of such new diffs: issue a conflict, or just a warning ?\n* teach git-svn (and others ?) to make use of that flag\n* handle new conflict type \"bulk-move/add\"\n* detect \"directory splits\" as well\n* use inexact dir renames to bump score of below-threshold renames\n  from/to same locations\n* add yours here\n---\n diff-lib.c        |    6 +-\n diff.c            |    5 +\n diff.h            |    3 +\n diffcore-rename.c |  278 +++++++++++++++++++++++++++++++++++++++++++++++++++--\n tree-diff.c       |    4 +-\n 5 files changed, 285 insertions(+), 11 deletions(-)\n\ndiff --git a/diff-lib.c b/diff-lib.c\nindex 392ce2b..5ec3ddc 100644\n--- a/diff-lib.c\n+++ b/diff-lib.c\n@@ -208,7 +208,8 @@ int run_diff_files(struct rev_info *revs, unsigned int option)\n \t\t\t\t\t\t    ce_option, &dirty_submodule);\n \t\tif (!changed && !dirty_submodule) {\n \t\t\tce_mark_uptodate(ce);\n-\t\t\tif (!DIFF_OPT_TST(&revs->diffopt, FIND_COPIES_HARDER))\n+\t\t\tif (!DIFF_OPT_TST(&revs->diffopt, FIND_COPIES_HARDER) &&\n+\t\t\t    !DIFF_OPT_TST(&revs->diffopt, DETECT_BULK_MOVES))\n \t\t\t\tcontinue;\n \t\t}\n \t\toldmode = ce->ce_mode;\n@@ -338,7 +339,8 @@ static int show_modified(struct rev_info *revs,\n \n \toldmode = old->ce_mode;\n \tif (mode == oldmode && !hashcmp(sha1, old->sha1) && !dirty_submodule &&\n-\t    !DIFF_OPT_TST(&revs->diffopt, FIND_COPIES_HARDER))\n+\t    !DIFF_OPT_TST(&revs->diffopt, FIND_COPIES_HARDER) &&\n+\t    !DIFF_OPT_TST(&revs->diffopt, DETECT_BULK_MOVES))\n \t\treturn 0;\n \n \tdiff_change(&revs->diffopt, oldmode, mode,\ndiff --git a/diff.c b/diff.c\nindex 71efa8e..64e01b3 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -3188,6 +3188,11 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)\n \t\tDIFF_OPT_SET(options, REVERSE_DIFF);\n \telse if (!strcmp(arg, \"--find-copies-harder\"))\n \t\tDIFF_OPT_SET(options, FIND_COPIES_HARDER);\n+\telse if (!strcmp(arg, \"--detect-bulk-moves\")) {\n+\t\tDIFF_OPT_SET(options, DETECT_BULK_MOVES);\n+\t\tif (!options->detect_rename)\n+\t\t\toptions->detect_rename = DIFF_DETECT_RENAME;\n+\t}\n \telse if (!strcmp(arg, \"--follow\"))\n \t\tDIFF_OPT_SET(options, FOLLOW_RENAMES);\n \telse if (!strcmp(arg, \"--color\"))\ndiff --git a/diff.h b/diff.h\nindex 1fd44f5..b0d6fa6 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -78,6 +78,7 @@ typedef struct strbuf *(*diff_prefix_fn_t)(struct diff_options *opt, void *data)\n #define DIFF_OPT_IGNORE_UNTRACKED_IN_SUBMODULES (1 << 25)\n #define DIFF_OPT_IGNORE_DIRTY_SUBMODULES (1 << 26)\n #define DIFF_OPT_OVERRIDE_SUBMODULE_CONFIG (1 << 27)\n+#define DIFF_OPT_DETECT_BULK_MOVES  (1 << 28)\n \n #define DIFF_OPT_TST(opts, flag)    ((opts)->flags & DIFF_OPT_##flag)\n #define DIFF_OPT_SET(opts, flag)    ((opts)->flags |= DIFF_OPT_##flag)\n@@ -265,6 +266,8 @@ extern void diffcore_fix_diff_index(struct diff_options *);\n \"  -C            detect copies.\\n\" \\\n \"  --find-copies-harder\\n\" \\\n \"                try unchanged files as candidate for copy detection.\\n\" \\\n+\"  --detect-bulk-moves\\n\" \\\n+\"                detect wholesale directory renames.\\n\" \\\n \"  -l<n>         limit rename attempts up to <n> paths.\\n\" \\\n \"  -O<file>      reorder diffs according to the <file>.\\n\" \\\n \"  -S<string>    find filepair whose only one side contains the string.\\n\" \\\ndiff --git a/diffcore-rename.c b/diffcore-rename.c\nindex df41be5..18a0605 100644\n--- a/diffcore-rename.c\n+++ b/diffcore-rename.c\n@@ -11,9 +11,13 @@\n static struct diff_rename_dst {\n \tstruct diff_filespec *two;\n \tstruct diff_filepair *pair;\n+\tint i_am_not_single:1; /* does not look for a match, only here to be looked at */\n } *rename_dst;\n static int rename_dst_nr, rename_dst_alloc;\n \n+/*\n+ * Do a binary search of \"two\" in \"rename_dst\", inserting it if not found.\n+ */\n static struct diff_rename_dst *locate_rename_dst(struct diff_filespec *two,\n \t\t\t\t\t\t int insert_ok)\n {\n@@ -49,9 +53,36 @@ static struct diff_rename_dst *locate_rename_dst(struct diff_filespec *two,\n \trename_dst[first].two = alloc_filespec(two->path);\n \tfill_filespec(rename_dst[first].two, two->sha1, two->mode);\n \trename_dst[first].pair = NULL;\n+\trename_dst[first].i_am_not_single = 0;\n \treturn &(rename_dst[first]);\n }\n \n+/*\n+ * Do a binary search in \"rename_dst\" of any entry under \"dirname\".\n+ */\n+static struct diff_rename_dst *locate_rename_dst_dir(const char *dirname)\n+{\n+\tint first, last;\n+\tint prefixlength = strlen(dirname);\n+\n+\tfirst = 0;\n+\tlast = rename_dst_nr;\n+\twhile (last > first) {\n+\t\tint next = (last + first) >> 1;\n+\t\tstruct diff_rename_dst *dst = &(rename_dst[next]);\n+\t\tint cmp = strncmp(dirname, dst->two->path, prefixlength);\n+\t\tif (!cmp)\n+\t\t\treturn dst;\n+\t\tif (cmp < 0) {\n+\t\t\tlast = next;\n+\t\t\tcontinue;\n+\t\t}\n+\t\tfirst = next+1;\n+\t}\n+\t/* not found */\n+\treturn NULL;\n+}\n+\n /* Table of rename/copy src files */\n static struct diff_rename_src {\n \tstruct diff_filespec *one;\n@@ -386,8 +417,11 @@ static int find_exact_renames(void)\n \tfor (i = 0; i < rename_src_nr; i++)\n \t\tinsert_file_table(&file_table, -1, i, rename_src[i].one);\n \n-\tfor (i = 0; i < rename_dst_nr; i++)\n+\tfor (i = 0; i < rename_dst_nr; i++) {\n+\t\tif (rename_dst[i].i_am_not_single)\n+\t\t\tcontinue;\n \t\tinsert_file_table(&file_table, 1, i, rename_dst[i].two);\n+\t}\n \n \t/* Find the renames */\n \ti = for_each_hash(&file_table, find_same_files);\n@@ -414,6 +448,201 @@ static void record_if_better(struct diff_score m[], struct diff_score *o)\n \t\tm[worst] = *o;\n }\n \n+struct diff_dir_rename {\n+\tstruct diff_filespec *one;\n+\tstruct diff_filespec *two;\n+\tint discarded;\n+\tstruct diff_dir_rename* next;\n+};\n+\n+// FIXME: replace with JN's version\n+/*\n+ * Copy dirname of src into dst, suitable to append a filename without\n+ * an additional \"/\".\n+ * Only handles relative paths since there is no absolute path in a git repo.\n+ * Writes \"\" when there is no \"/\" in src.\n+ * May overwrite more chars than really needed, if src ends with a \"/\".\n+ */\n+static const char* copy_dirname(char* dst, const char* src)\n+{\n+\tchar* lastslash = strrchr(src, '/');\n+\tif (!lastslash) {\n+\t\t*dst = '\\0';\n+\t\treturn dst;\n+\t}\n+\tmemcpy(dst, src, lastslash - src + 1);\n+\tdst[lastslash - src + 1] = '\\0';\n+\n+\t/* if src ends with a \"/\" strip the last component */\n+\tif (lastslash[1] == '\\0') {\n+\t\tlastslash = strrchr(dst, '/');\n+\t\tif (!lastslash)\n+\t\t\treturn strcpy(dst, \".\");\n+\t\tlastslash[1] = '\\0';\n+\t}\n+\n+\treturn dst;\n+}\n+\n+// FIXME: investigate avoiding i_am_not_single by using a separate list\n+// FIXME: we could optimize the 100%-rename case by preventing\n+//        recursion to unfold what we know we would refold here.\n+// FIXME: check what happens when bulk move interferes with copy+move detection\n+//        (which may have selected the wrong target for move)\n+// FIXME: do we want to replace linked list with sorted array ?\n+// FIXME: this prototype only properly handles renaming of dirs without a subdir.\n+// FIXME: leaks like hell.\n+// FIXME: ideas to evaluate a similarity score, anyone ?\n+//        10% * tree similarity + 90% * moved files similarity ?\n+// FIXME: ideas to consider under-threshold moves as part of bulk move ?\n+\n+// FIXME: handle as sorted list to allow for binary search ?\n+static struct diff_dir_rename* bulkmove_candidates = NULL;\n+\n+/* See if the fact that one_leftover exists under one_parent_path in\n+ * dst tree should disqualify one_parent_path from bulkmove eligibility.\n+ * Return 1 if it disqualifies, 0 if it is OK.\n+ */\n+static int maybe_disqualify_bulkmove(const char* one_parent_path,\n+\t\t\t\t       struct diff_rename_dst* one_leftover)\n+{\n+\t// FIXME: should only be run if !seen\n+\t/* this might be a dir split, or files added\n+\t * after the bulk move, or just an isolated\n+\t * rename */\n+\t/* FIXME: should we cache this information to avoid\n+\t * recomputing that result when many files trigger it ?\n+\t * See eg. git.git:761e742d692.\n+\t */\n+\tint two_idx, j, onep_len, maybe_dir_rename;\n+\n+\t/* try to see if it is a file added after the bulk move */\n+\ttwo_idx = one_leftover - rename_dst;\n+\tonep_len = strlen(one_parent_path);\n+\tmaybe_dir_rename = 1;\n+\n+\t/* check no leftover file was already here before */\n+\tfor (j = two_idx; j < rename_dst_nr; j++) {\n+\t\tif (strncmp(rename_dst[j].two->path,\n+\t\t\t    one_parent_path, onep_len))\n+\t\t\tbreak; /* exhausted directory in this direction */\n+\t\tfprintf(stderr, \"[DBG] leftover file %s in %s\\n\",\n+\t\t\trename_dst[j].two->path, one_parent_path);\n+\t\tif (rename_dst[j].i_am_not_single || /* those were already here */\n+\t\t    (rename_dst[j].pair &&\n+\t\t     !strncmp(rename_dst[j].pair->one->path,\n+\t\t\t      one_parent_path, onep_len) && /* renamed from here */\n+\t\t     strncmp(rename_dst[j].two->path,\n+\t\t\t     one_parent_path, onep_len))) { /* not to a subdir */\n+\t\t\tmaybe_dir_rename = 0;\n+\t\t\tfprintf(stderr, \"[DBG] ... tells not a bulk move\\n\");\n+\t\t\tbreak;\n+\t\t}\n+\t\tfprintf(stderr, \"[DBG] ... not believed to prevent bulk move\\n\");\n+\t}\n+\tif (!maybe_dir_rename) return 1;\n+\t/* try the other direction (dup code) */\n+\tfor (j = two_idx-1; j >= 0; j--) {\n+\t\tif (strncmp(rename_dst[j].two->path,\n+\t\t\t    one_parent_path, onep_len))\n+\t\t\tbreak; /* exhausted directory in this direction */\n+\t\tfprintf(stderr, \"[DBG] leftover file %s in %s\\n\",\n+\t\t\trename_dst[j].two->path, one_parent_path);\n+\t\tif (rename_dst[j].i_am_not_single || /* those were already here */\n+\t\t    (rename_dst[j].pair &&\n+\t\t     !strncmp(rename_dst[j].pair->one->path,\n+\t\t\t      one_parent_path, onep_len) && /* renamed from here */\n+\t\t     strncmp(rename_dst[j].two->path,\n+\t\t\t     one_parent_path, onep_len))) { /* not to a subdir */\n+\t\t\tmaybe_dir_rename = 0;\n+\t\t\tfprintf (stderr, \"[DBG] ... tells not a bulk move\\n\");\n+\t\t\tbreak;\n+\t\t}\n+\t\tfprintf(stderr, \"[DBG] ... not believed to prevent bulk move\\n\");\n+\t}\n+\tif (!maybe_dir_rename) return 1;\n+\n+\t/* Here we are in the case where a directory\n+\t * content was completely moved, but files\n+\t * were added to it afterwards.  Proceed as\n+\t * for a simple bulk move. */\n+\treturn 0;\n+}\n+\n+/*\n+ * Check if the rename specified by \"dstpair\" could cause a\n+ * bulk move to be detected, record it in bulkmove_candidates if yes.\n+ */\n+static void check_one_bulk_move(struct diff_filepair *dstpair)\n+{\n+\tchar one_parent_path[PATH_MAX], two_parent_path[PATH_MAX];\n+\tstruct diff_dir_rename* seen;\n+\n+\t/* genuine new files (or believed to be so) */\n+\tif (!dstpair)\n+\t\treturn;\n+\t/* dummy renames used by copy detection */\n+\tif (!strcmp(dstpair->one->path, dstpair->two->path))\n+\t\treturn;\n+\n+\tcopy_dirname(one_parent_path, dstpair->one->path);\n+\tcopy_dirname(two_parent_path, dstpair->two->path);\n+\n+\t/* simple rename with no directory change */\n+\tif (!strcmp(one_parent_path, two_parent_path))\n+\t\treturn;\n+\n+\t/* After this commit, are there any files still under one_parent_path ?\n+\t * Any file left would disqualifies this dir for bulk move.\n+\t */\n+\tstruct diff_rename_dst* one_leftover = locate_rename_dst_dir(one_parent_path);\n+\tif (one_leftover &&\n+\t    maybe_disqualify_bulkmove(one_parent_path, one_leftover))\n+\t\treturn;\n+\n+\t/* already considered ? */\n+\tfor (seen=bulkmove_candidates; seen; seen = seen->next)\n+\t\tif (!strcmp(seen->one->path, one_parent_path)) break;\n+\tif (!seen) {\n+\t\t/* record potential dir rename */\n+\t\tseen = xmalloc(sizeof(*seen));\n+\t\tseen->one = alloc_filespec(one_parent_path);\n+\t\tfill_filespec(seen->one, null_sha1 /*FIXME*/, S_IFDIR);\n+\t\tseen->two = alloc_filespec(two_parent_path);\n+\t\tfill_filespec(seen->two, null_sha1 /*FIXME*/, S_IFDIR);\n+\t\tseen->discarded = 0;\n+\t\tseen->next = bulkmove_candidates;\n+\t\tbulkmove_candidates = seen;\n+\t\tfprintf(stderr, \"[DBG] %s -> %s suggests possible rename from %s to %s\\n\",\n+\t\t\tdstpair->one->path,\n+\t\t\tdstpair->two->path,\n+\t\t\tone_parent_path, two_parent_path);\n+\t\treturn;\n+\t}\n+\tif (seen->discarded)\n+\t\t/* already seen a rename from seen->one to some than ->two */\n+\t\treturn;\n+\t/* check that seen entry matches this rename */\n+\tif (strcmp(two_parent_path, seen->two->path)) {\n+\t\tfprintf(stderr, \"[DBG] discarding dir %s from renames (split into %s and %s)\\n\",\n+\t\t\tone_parent_path, two_parent_path, seen->two->path);\n+\t\tseen->discarded = 1;\n+\t}\n+\n+\t/* all checks ok, we keep that entry */\n+}\n+\n+/*\n+ * Take all file renames recorded so far and check if they could cause\n+ * a bulk move to be detected.\n+ */\n+static void diffcore_bulk_moves(void)\n+{\n+\tint i;\n+\tfor (i = 0; i < rename_dst_nr; i++)\n+\t\tcheck_one_bulk_move(rename_dst[i].pair);\n+}\n+\n void diffcore_rename(struct diff_options *options)\n {\n \tint detect_rename = options->detect_rename;\n@@ -451,13 +680,23 @@ void diffcore_rename(struct diff_options *options)\n \t\t\t\tp->one->rename_used++;\n \t\t\tregister_rename_src(p->one, p->score);\n \t\t}\n-\t\telse if (detect_rename == DIFF_DETECT_COPY) {\n-\t\t\t/*\n-\t\t\t * Increment the \"rename_used\" score by\n-\t\t\t * one, to indicate ourselves as a user.\n-\t\t\t */\n-\t\t\tp->one->rename_used++;\n-\t\t\tregister_rename_src(p->one, p->score);\n+\t\telse {\n+\t\t\tif (detect_rename == DIFF_DETECT_COPY) {\n+\t\t\t\t/*\n+\t\t\t\t * Increment the \"rename_used\" score by\n+\t\t\t\t * one, to indicate ourselves as a user.\n+\t\t\t\t */\n+\t\t\t\tp->one->rename_used++;\n+\t\t\t\tregister_rename_src(p->one, p->score);\n+\t\t\t}\n+\t\t\tif (DIFF_OPT_TST(options, DETECT_BULK_MOVES)) {\n+\t\t\t\t/* similarly, bulk move detection needs to\n+\t\t\t\t * see all files from second tree, but we don't\n+\t\t\t\t * want them to be matched against single sources.\n+\t\t\t\t */\n+\t\t\t\t// FIXME: check interaction with --find-copies-harder\n+\t\t\t\tlocate_rename_dst(p->two, 1)->i_am_not_single = 1;\n+\t\t\t}\n \t\t}\n \t}\n \tif (rename_dst_nr == 0 || rename_src_nr == 0)\n@@ -509,6 +748,8 @@ void diffcore_rename(struct diff_options *options)\n \n \t\tif (rename_dst[i].pair)\n \t\t\tcontinue; /* dealt with exact match already. */\n+\t\tif (rename_dst[i].i_am_not_single)\n+\t\t\tcontinue; /* not looking for a match. */\n \n \t\tm = &mx[dst_cnt * NUM_CANDIDATE_PER_DST];\n \t\tfor (j = 0; j < NUM_CANDIDATE_PER_DST; j++)\n@@ -569,7 +810,28 @@ void diffcore_rename(struct diff_options *options)\n \t/* At this point, we have found some renames and copies and they\n \t * are recorded in rename_dst.  The original list is still in *q.\n \t */\n+\n+\t/* Now possibly factorize those renames and copies. */\n+\tif (DIFF_OPT_TST(options, DETECT_BULK_MOVES))\n+\t\tdiffcore_bulk_moves();\n+\n \tDIFF_QUEUE_CLEAR(&outq);\n+\n+\t/* Now turn non-discarded bulkmove_candidates into real renames */\n+\tstruct diff_dir_rename* candidate;\n+\tfor (candidate=bulkmove_candidates; candidate; candidate = candidate->next) {\n+\t\tstruct diff_filepair* pair;\n+\t\tif (candidate->discarded) continue;\n+\t\t/* visualize toplevel dir if needed */ //FIXME: wrong place for this ?\n+\t\tif (!*candidate->one->path)\n+\t\t\tcandidate->one->path = \"./\";\n+\t\tif (!*candidate->two->path)\n+\t\t\tcandidate->two->path = \"./\";\n+\t\tpair = diff_queue(&outq, candidate->one, candidate->two);\n+\t\tpair->score = MAX_SCORE;\n+\t\tpair->renamed_pair = 1;\n+\t}\n+\n \tfor (i = 0; i < q->nr; i++) {\n \t\tstruct diff_filepair *p = q->queue[i];\n \t\tstruct diff_filepair *pair_to_free = NULL;\ndiff --git a/tree-diff.c b/tree-diff.c\nindex cd659c6..5d9f123 100644\n--- a/tree-diff.c\n+++ b/tree-diff.c\n@@ -49,7 +49,9 @@ static int compare_tree_entry(struct tree_desc *t1, struct tree_desc *t2, const\n \t\tshow_entry(opt, \"+\", t2, base, baselen);\n \t\treturn 1;\n \t}\n-\tif (!DIFF_OPT_TST(opt, FIND_COPIES_HARDER) && !hashcmp(sha1, sha2) && mode1 == mode2)\n+\tif (!DIFF_OPT_TST(opt, FIND_COPIES_HARDER) &&\n+\t    !DIFF_OPT_TST(opt, DETECT_BULK_MOVES) &&\n+\t    !hashcmp(sha1, sha2) && mode1 == mode2)\n \t\treturn 0;\n \n \t/*\n-- \n1.7.2.3\n"},{"id":"153096","messageId":"1286659895-1813-3-git-send-email-ydirson@altern.org","threadId":"25405","inReplyTo":"1286659895-1813-2-git-send-email-ydirson@altern.org","subject":"[PATCH v5 2/4] Add testcases for the --detect-bulk-moves diffcore flag.","fromName":"Yann Dirson","fromEmail":"ydirson@altern.org","sentAt":"2010-10-09T21:31:33Z","receivedAt":"2010-10-09T21:31:33Z","isPatch":true,"sender":{"key":"ydirson@altern.org","avatar":"https://avatars.githubusercontent.com/u/1190950?v=4"},"body":"From: Yann Dirson <ydirson@free.fr>\n\nThis notably includes a couple of tests for cases known not to be\nworking correctly yet.\n---\n t/t4046-diff-rename-factorize.sh |  251 ++++++++++++++++++++++++++++++++++++++\n 1 files changed, 251 insertions(+), 0 deletions(-)\n create mode 100755 t/t4046-diff-rename-factorize.sh\n\ndiff --git a/t/t4046-diff-rename-factorize.sh b/t/t4046-diff-rename-factorize.sh\nnew file mode 100755\nindex 0000000..d063e25\n--- /dev/null\n+++ b/t/t4046-diff-rename-factorize.sh\n@@ -0,0 +1,251 @@\n+#!/bin/sh\n+#\n+# Copyright (c) 2008,2010 Yann Dirson\n+# Copyright (c) 2005 Junio C Hamano\n+#\n+\n+# TODO - missing tests:\n+# * two dirs or more moving all their files to a single dir\n+# * simultaneous bulkmove and rename\n+# * add a new file under a dir that was moved in the same commit\n+# * copy of a bulkmoved file, when scores would otherwise select the\n+#   wrong one as \"moved\"\n+\n+test_description='Test rename factorization in diff engine.\n+\n+'\n+. ./test-lib.sh\n+. \"$TEST_DIRECTORY\"/diff-lib.sh\n+\n+test_expect_success 'setup' '\n+\tgit commit --allow-empty -m \"original empty commit\"\n+\n+\tmkdir a &&\n+\tprintf \"Line %s\\n\" 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 >a/path0 &&\n+\tsed <a/path0 >a/path1 s/Line/Record/ &&\n+\tsed <a/path0 >a/path2 s/Line/Stuff/ &&\n+\tsed <a/path0 >a/path3 s/Line/Blurb/ &&\n+\n+\tgit update-index --add a/path* &&\n+\ttest_tick &&\n+\tgit commit -m \"original set of files\"\n+\n+\t: rename the directory &&\n+\tmv a b &&\n+\tgit update-index --add --remove a/path0 a/path1 a/path2 a/path3 b/path*\n+'\n+\n+test_expect_success 'diff-index --detect-bulk-moves after directory move.' '\n+\tcat >expected <<-EOF &&\n+\t:040000 040000 X X R#\ta/\tb/\n+\t:100644 100644 X X R#\ta/path0\tb/path0\n+\t:100644 100644 X X R#\ta/path1\tb/path1\n+\t:100644 100644 X X R#\ta/path2\tb/path2\n+\t:100644 100644 X X R#\ta/path3\tb/path3\n+\tEOF\n+\tgit diff-index --detect-bulk-moves HEAD >current &&\n+\tgrep -v \"^\\[DBG\\] \" <current >current.filtered &&\n+\tcompare_diff_raw expected current.filtered\n+'\n+\n+test_expect_success 'setup non-100% rename' '\n+\techo \"Line 16\" >>b/path0 &&\n+\tmv b/path2 b/2path &&\n+\trm b/path3 &&\n+\techo anything >b/path100 &&\n+\tgit update-index --add --remove b/* b/path2 b/path3\n+'\n+\n+test_expect_success 'diff-index --detect-bulk-moves after content changes.' '\n+\tcat >expected <<-EOF &&\n+\t:040000 040000 X X R#\ta/\tb/\n+\t:100644 000000 X X D#\ta/path3\n+\t:100644 100644 X X R#\ta/path2\tb/2path\n+\t:100644 100644 X X R#\ta/path0\tb/path0\n+\t:100644 100644 X X R#\ta/path1\tb/path1\n+\t:000000 100644 X X A#\tb/path100\n+\tEOF\n+\tgit diff-index --detect-bulk-moves HEAD >current &&\n+\tgrep -v \"^\\[DBG\\] \" <current >current.filtered &&\n+\tcompare_diff_raw expected current.filtered\n+'\n+\n+test_expect_success 'setup bulk move that is not directory move' '\n+\tgit reset -q --hard &&\n+\n+\tmkdir c &&\n+\t(\n+\t\tfor i in 0 1 2; do\n+\t\t\tcp a/path$i c/apath$i || exit\n+\t\tdone\n+\t) &&\n+\tgit update-index --add c/apath* &&\n+\ttest_tick &&\n+\tgit commit -m \"first set of changes\" &&\n+\n+\tmv c/* a/ &&\n+\tgit update-index --add --remove a/* c/apath0 c/apath1 c/apath2\n+'\n+\n+test_expect_failure 'diff-index --detect-bulk-moves without full-dir rename.' '\n+\tcat >expected <<-EOF &&\n+\t:040000 040000 X X R#\tc/*\ta/\n+\t:100644 100644 X X R#\tc/apath0\ta/apath0\n+\t:100644 100644 X X R#\tc/apath1\ta/apath1\n+\t:100644 100644 X X R#\tc/apath2\ta/apath2\n+\tEOF\n+\tgit diff-index --detect-bulk-moves HEAD >current\n+\tgrep -v \"^\\[DBG\\] \" <current >current.filtered &&\n+\tcompare_diff_raw expected current.filtered\n+'\n+\n+test_expect_success 'setup bulk move to toplevel' '\n+\tgit reset -q --hard &&\n+\tmv c/* . &&\n+\tgit update-index --add --remove apath* c/apath0 c/apath1 c/apath2\n+'\n+\n+test_expect_failure 'diff-index --detect-bulk-moves bulk move to toplevel.' '\n+\tcat >expected <<-EOF &&\n+\t:040000 040000 X X R#\tc/*\t./\n+\t:100644 100644 X X R#\tc/apath0\tapath0\n+\t:100644 100644 X X R#\tc/apath1\tapath1\n+\t:100644 100644 X X R#\tc/apath2\tapath2\n+\tEOF\n+\tgit diff-index --detect-bulk-moves HEAD >current &&\n+\tgrep -v \"^\\[DBG\\] \" <current >current.filtered &&\n+\tcompare_diff_raw expected current.filtered\n+'\n+\n+test_expect_success 'setup move including a subdir, with some content changes' '\n+\tgit reset -q --hard &&\n+\tmv c a/ &&\n+\tgit update-index --add --remove a/c/* c/apath0 c/apath1 c/apath2 &&\n+\ttest_tick &&\n+\tgit commit -m \"move as subdir\" &&\n+\n+\tmv a b &&\n+\techo foo >>b/c/apath0 &&\n+\tgit update-index --add --remove a/path0 a/path1 a/path2 a/path3 \\\n+\t\t\ta/c/apath0 a/c/apath1 a/c/apath2 b/path* b/c/apath*\n+'\n+\n+test_expect_failure 'diff-index --detect-bulk-moves on a move including a subdir.' '\n+\tcat >expected <<-EOF &&\n+\t:040000 040000 X X R#\ta/\tb/\n+\t:100644 100644 X X R#\ta/c/apath0\tb/c/apath0\n+\t:100644 100644 X X R#\ta/c/apath1\tb/c/apath1\n+\t:100644 100644 X X R#\ta/c/apath2\tb/c/apath2\n+\t:100644 100644 X X R#\ta/path0\tb/path0\n+\t:100644 100644 X X R#\ta/path1\tb/path1\n+\t:100644 100644 X X R#\ta/path2\tb/path2\n+\t:100644 100644 X X R#\ta/path3\tb/path3\n+\tEOF\n+\tgit diff-index --detect-bulk-moves HEAD >current &&\n+\tgrep -v \"^\\[DBG\\] \" <current >current.filtered &&\n+\tcompare_diff_raw expected current.filtered\n+'\n+\n+test_expect_success 'setup move without a subdir' '\n+\tgit reset -q --hard &&\n+\tmkdir b &&\n+\tmv a/path* b/ &&\n+\t: rename files in the directory but not subdir. &&\n+\tgit update-index --add --remove a/path0 a/path1 a/path2 a/path3 b/path*\n+'\n+\n+test_expect_success 'moving files but not subdirs is not mistaken for dir move' '\n+\tcat >expected <<-EOF &&\n+\t:100644 100644 X X R#\ta/path0\tb/path0\n+\t:100644 100644 X X R#\ta/path1\tb/path1\n+\t:100644 100644 X X R#\ta/path2\tb/path2\n+\t:100644 100644 X X R#\ta/path3\tb/path3\n+\tEOF\n+\tgit diff-index --detect-bulk-moves HEAD >current &&\n+\tgrep -v \"^\\[DBG\\] \" <current >current.filtered &&\n+\tcompare_diff_raw expected current.filtered\n+'\n+\n+test_expect_success 'setup move of files and subdirs to different places' '\n+\tgit reset -q --hard &&\n+\tmv a/c b &&\n+\tmv a d &&\n+\tgit update-index --add --remove a/path0 a/path1 a/path2 a/path3 \\\n+\t\ta/c/apath0 a/c/apath1 a/c/apath2 d/path* b/apath*\n+'\n+\n+test_expect_failure 'moving subdirs into one dir and files into another is not mistaken for dir move' '\n+\tcat >expected <<-EOF &&\n+\t:040000 040000 X X R#\ta/c/\tb/\n+\t:100644 100644 X X R#\ta/c/apath0\tb/apath0\n+\t:100644 100644 X X R#\ta/c/apath1\tb/apath1\n+\t:100644 100644 X X R#\ta/c/apath2\tb/apath2\n+\t:100644 100644 X X R#\ta/path0\td/path0\n+\t:100644 100644 X X R#\ta/path1\td/path1\n+\t:100644 100644 X X R#\ta/path2\td/path2\n+\t:100644 100644 X X R#\ta/path3\td/path3\n+\tEOF\n+\tgit diff-index --detect-bulk-moves HEAD >current &&\n+\tgrep -v \"^\\[DBG\\] \" <current >current.filtered &&\n+\tcompare_diff_raw expected current.filtered\n+'\n+\n+test_expect_success 'setup move of dir with only subdirs' '\n+\tgit reset -q --hard &&\n+\tmkdir a/b &&\n+\tmv a/path* a/b/ &&\n+\tgit update-index --add --remove a/path0 a/path1 a/path2 a/path3 a/b/path* &&\n+\ttest_tick &&\n+\tgit commit -m \"move all toplevel files down one level\" &&\n+\n+\tgit mv a z\n+'\n+\n+test_expect_failure 'moving a dir with no files' '\n+\tcat >expected <<-EOF &&\n+\t:040000 040000 X X R#\ta/\tz/\n+\t:040000 040000 X X R#\ta/b/\tz/b/\n+\t:040000 040000 X X R#\ta/c/\tz/c/\n+\t:100644 100644 X X R#\ta/b/path0\tz/b/path0\n+\t:100644 100644 X X R#\ta/b/path1\tz/b/path1\n+\t:100644 100644 X X R#\ta/b/path2\tz/b/path2\n+\t:100644 100644 X X R#\ta/b/path3\tz/b/path3\n+\t:100644 100644 X X R#\ta/c/apath0\tz/c/apath0\n+\t:100644 100644 X X R#\ta/c/apath1\tz/c/apath1\n+\t:100644 100644 X X R#\ta/c/apath2\tz/c/apath2\n+\tEOF\n+\tgit diff-index --detect-bulk-moves HEAD >current &&\n+\tgrep -v \"^\\[DBG\\] \" <current >current.filtered &&\n+\tcompare_diff_raw expected current.filtered\n+'\n+# now test moving all files from toplevel into subdir (does not hides file moves) (needs consensus on syntax)\n+# Note: this is a special case of move of a dir into one of its own subdirs, which in\n+# turn is a variant of new files/dirs being added into a dir after all its contents\n+# are moved away\n+\n+test_expect_success 'setup move from toplevel to subdir' '\n+\tgit reset -q --hard HEAD~3 &&\n+\tmv a/* . &&\n+\tgit update-index --add --remove a/path0 a/path1 a/path2 a/path3 path* &&\n+\ttest_tick &&\n+\tgit commit -m \"move all files to toplevel\" &&\n+\n+\tmkdir z &&\n+\tmv path* z/ &&\n+\tgit update-index --add --remove path0 path1 path2 path3 z/path*\n+'\n+\n+test_expect_failure '--detect-bulk-moves everything from toplevel.' '\n+\tcat >expected <<-EOF &&\n+\t:040000 040000 X X R#\t./*\tz/\n+\t:100644 100644 X X R#\tpath0\tz/path0\n+\t:100644 100644 X X R#\tpath1\tz/path1\n+\t:100644 100644 X X R#\tpath2\tz/path2\n+\t:100644 100644 X X R#\tpath3\tz/path3\n+\tEOF\n+\tgit diff-index --detect-bulk-moves HEAD >current &&\n+\tgrep -v \"^\\[DBG\\] \" <current >current.filtered &&\n+\tcompare_diff_raw expected current.filtered\n+'\n+\n+test_done\n-- \n1.7.2.3\n"},{"id":"153099","messageId":"1286659895-1813-4-git-send-email-ydirson@altern.org","threadId":"25405","inReplyTo":"1286659895-1813-3-git-send-email-ydirson@altern.org","subject":"[PATCH v5 3/4] [RFC] Only show bulkmoves in output.","fromName":"Yann Dirson","fromEmail":"ydirson@altern.org","sentAt":"2010-10-09T21:31:34Z","receivedAt":"2010-10-09T21:31:34Z","isPatch":true,"sender":{"key":"ydirson@altern.org","avatar":"https://avatars.githubusercontent.com/u/1190950?v=4"},"body":"In theory we could just append those at display time (possibly diluting\nthe code too much ?), or before queing the diff.  In practice we cannot\ndo the latter easily in either case (strcat to a constant string (\"./\"),\nnor to the paths tighly-allocated from alloc_filespec).\n\nSo I went the way of including the trailing \"*\" from the beginning.\nSince I'm still unsure whether keeping it that way, I leave it as a\nseparated patch for now.\n\nAfter this patch only one of the expected failures can be considered\ninnocuous.\n---\n diffcore-rename.c                |   22 +++++++++++++++-------\n t/t4046-diff-rename-factorize.sh |   22 +++++++++++-----------\n 2 files changed, 26 insertions(+), 18 deletions(-)\n\ndiff --git a/diffcore-rename.c b/diffcore-rename.c\nindex 18a0605..6d21792 100644\n--- a/diffcore-rename.c\n+++ b/diffcore-rename.c\n@@ -588,6 +588,13 @@ static void check_one_bulk_move(struct diff_filepair *dstpair)\n \tcopy_dirname(one_parent_path, dstpair->one->path);\n \tcopy_dirname(two_parent_path, dstpair->two->path);\n \n+\t/* Visualize toplevel dir if needed.  Need it here because of\n+\t * \"*\" handling below. */\n+\tif (!*one_parent_path)\n+\t\tstrcpy(one_parent_path, \"./\");\n+\tif (!*two_parent_path)\n+\t\tstrcpy(two_parent_path, \"./\");\n+\n \t/* simple rename with no directory change */\n \tif (!strcmp(one_parent_path, two_parent_path))\n \t\treturn;\n@@ -600,11 +607,16 @@ static void check_one_bulk_move(struct diff_filepair *dstpair)\n \t    maybe_disqualify_bulkmove(one_parent_path, one_leftover))\n \t\treturn;\n \n+\t/* For output format reason we want this \"*\" for the general\n+\t * case of bulk moves.  Most be here for alloc_filespec to\n+\t * reserve enough space, and to allow for proper comparison\n+\t * with those previously recorded in bulkmove_candidates. */\n+\tstrcat(one_parent_path, \"*\");\n+\n \t/* already considered ? */\n \tfor (seen=bulkmove_candidates; seen; seen = seen->next)\n \t\tif (!strcmp(seen->one->path, one_parent_path)) break;\n-\tif (!seen) {\n-\t\t/* record potential dir rename */\n+\tif (!seen) { /* record potential dir rename */\n \t\tseen = xmalloc(sizeof(*seen));\n \t\tseen->one = alloc_filespec(one_parent_path);\n \t\tfill_filespec(seen->one, null_sha1 /*FIXME*/, S_IFDIR);\n@@ -822,11 +834,7 @@ void diffcore_rename(struct diff_options *options)\n \tfor (candidate=bulkmove_candidates; candidate; candidate = candidate->next) {\n \t\tstruct diff_filepair* pair;\n \t\tif (candidate->discarded) continue;\n-\t\t/* visualize toplevel dir if needed */ //FIXME: wrong place for this ?\n-\t\tif (!*candidate->one->path)\n-\t\t\tcandidate->one->path = \"./\";\n-\t\tif (!*candidate->two->path)\n-\t\t\tcandidate->two->path = \"./\";\n+\n \t\tpair = diff_queue(&outq, candidate->one, candidate->two);\n \t\tpair->score = MAX_SCORE;\n \t\tpair->renamed_pair = 1;\ndiff --git a/t/t4046-diff-rename-factorize.sh b/t/t4046-diff-rename-factorize.sh\nindex d063e25..9353929 100755\n--- a/t/t4046-diff-rename-factorize.sh\n+++ b/t/t4046-diff-rename-factorize.sh\n@@ -37,7 +37,7 @@ test_expect_success 'setup' '\n \n test_expect_success 'diff-index --detect-bulk-moves after directory move.' '\n \tcat >expected <<-EOF &&\n-\t:040000 040000 X X R#\ta/\tb/\n+\t:040000 040000 X X R#\ta/*\tb/\n \t:100644 100644 X X R#\ta/path0\tb/path0\n \t:100644 100644 X X R#\ta/path1\tb/path1\n \t:100644 100644 X X R#\ta/path2\tb/path2\n@@ -58,7 +58,7 @@ test_expect_success 'setup non-100% rename' '\n \n test_expect_success 'diff-index --detect-bulk-moves after content changes.' '\n \tcat >expected <<-EOF &&\n-\t:040000 040000 X X R#\ta/\tb/\n+\t:040000 040000 X X R#\ta/*\tb/\n \t:100644 000000 X X D#\ta/path3\n \t:100644 100644 X X R#\ta/path2\tb/2path\n \t:100644 100644 X X R#\ta/path0\tb/path0\n@@ -87,7 +87,7 @@ test_expect_success 'setup bulk move that is not directory move' '\n \tgit update-index --add --remove a/* c/apath0 c/apath1 c/apath2\n '\n \n-test_expect_failure 'diff-index --detect-bulk-moves without full-dir rename.' '\n+test_expect_success 'diff-index --detect-bulk-moves without full-dir rename.' '\n \tcat >expected <<-EOF &&\n \t:040000 040000 X X R#\tc/*\ta/\n \t:100644 100644 X X R#\tc/apath0\ta/apath0\n@@ -105,7 +105,7 @@ test_expect_success 'setup bulk move to toplevel' '\n \tgit update-index --add --remove apath* c/apath0 c/apath1 c/apath2\n '\n \n-test_expect_failure 'diff-index --detect-bulk-moves bulk move to toplevel.' '\n+test_expect_success 'diff-index --detect-bulk-moves bulk move to toplevel.' '\n \tcat >expected <<-EOF &&\n \t:040000 040000 X X R#\tc/*\t./\n \t:100644 100644 X X R#\tc/apath0\tapath0\n@@ -132,7 +132,7 @@ test_expect_success 'setup move including a subdir, with some content changes' '\n \n test_expect_failure 'diff-index --detect-bulk-moves on a move including a subdir.' '\n \tcat >expected <<-EOF &&\n-\t:040000 040000 X X R#\ta/\tb/\n+\t:040000 040000 X X R#\ta/*\tb/\n \t:100644 100644 X X R#\ta/c/apath0\tb/c/apath0\n \t:100644 100644 X X R#\ta/c/apath1\tb/c/apath1\n \t:100644 100644 X X R#\ta/c/apath2\tb/c/apath2\n@@ -176,7 +176,7 @@ test_expect_success 'setup move of files and subdirs to different places' '\n \n test_expect_failure 'moving subdirs into one dir and files into another is not mistaken for dir move' '\n \tcat >expected <<-EOF &&\n-\t:040000 040000 X X R#\ta/c/\tb/\n+\t:040000 040000 X X R#\ta/c/*\tb/\n \t:100644 100644 X X R#\ta/c/apath0\tb/apath0\n \t:100644 100644 X X R#\ta/c/apath1\tb/apath1\n \t:100644 100644 X X R#\ta/c/apath2\tb/apath2\n@@ -201,11 +201,11 @@ test_expect_success 'setup move of dir with only subdirs' '\n \tgit mv a z\n '\n \n-test_expect_failure 'moving a dir with no files' '\n+test_expect_failure 'moving a dir with no direct children files' '\n \tcat >expected <<-EOF &&\n-\t:040000 040000 X X R#\ta/\tz/\n-\t:040000 040000 X X R#\ta/b/\tz/b/\n-\t:040000 040000 X X R#\ta/c/\tz/c/\n+\t:040000 040000 X X R#\ta/*\tz/\n+\t:040000 040000 X X R#\ta/b/*\tz/b/\n+\t:040000 040000 X X R#\ta/c/*\tz/c/\n \t:100644 100644 X X R#\ta/b/path0\tz/b/path0\n \t:100644 100644 X X R#\ta/b/path1\tz/b/path1\n \t:100644 100644 X X R#\ta/b/path2\tz/b/path2\n@@ -235,7 +235,7 @@ test_expect_success 'setup move from toplevel to subdir' '\n \tgit update-index --add --remove path0 path1 path2 path3 z/path*\n '\n \n-test_expect_failure '--detect-bulk-moves everything from toplevel.' '\n+test_expect_success '--detect-bulk-moves everything from toplevel.' '\n \tcat >expected <<-EOF &&\n \t:040000 040000 X X R#\t./*\tz/\n \t:100644 100644 X X R#\tpath0\tz/path0\n-- \n1.7.2.3\n"},{"id":"153097","messageId":"1286659895-1813-5-git-send-email-ydirson@altern.org","threadId":"25405","inReplyTo":"1286659895-1813-4-git-send-email-ydirson@altern.org","subject":"[PATCH v5 4/4] [RFC] Allow hiding renames of individual files involved in a directory rename.","fromName":"Yann Dirson","fromEmail":"ydirson@altern.org","sentAt":"2010-10-09T21:31:35Z","receivedAt":"2010-10-09T21:31:35Z","isPatch":true,"sender":{"key":"ydirson@altern.org","avatar":"https://avatars.githubusercontent.com/u/1190950?v=4"},"body":"Once has identified groups of bulk-moved files, and then\nthe --hide-bulk-move-details flag hides those of the individual renames\nwhich carry no other information (further name change, or content changes).\n\nThis makes it much easier to a human reader to spot content changes\nin a commit that also moves a whole subtree.\n\nImportant note: unified diff output is not currently useful, since the \"bulk move\"\nheaders are not yet added by --detect-bulk-moves, but the redundant renames are\nreally removed.\n---\n diff.c            |    7 +++++\n diff.h            |    3 ++\n diffcore-rename.c |   67 ++++++++++++++++++++++++++++++++++++++++++++++++++--\n diffcore.h        |    1 +\n 4 files changed, 75 insertions(+), 3 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex 64e01b3..28f5be3 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -3193,6 +3193,13 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)\n \t\tif (!options->detect_rename)\n \t\t\toptions->detect_rename = DIFF_DETECT_RENAME;\n \t}\n+\telse if (!strcmp(arg, \"--hide-bulk-move-details\")) {\n+\t\tDIFF_OPT_SET(options, HIDE_DIR_RENAME_DETAILS);\n+\t\tif (!DIFF_OPT_TST(options, DETECT_BULK_MOVES))\n+\t\t\tDIFF_OPT_SET(options, DETECT_BULK_MOVES);\n+\t\tif (!options->detect_rename)\n+\t\t\toptions->detect_rename = DIFF_DETECT_RENAME;\n+\t}\n \telse if (!strcmp(arg, \"--follow\"))\n \t\tDIFF_OPT_SET(options, FOLLOW_RENAMES);\n \telse if (!strcmp(arg, \"--color\"))\ndiff --git a/diff.h b/diff.h\nindex b0d6fa6..7f132d0 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -79,6 +79,7 @@ typedef struct strbuf *(*diff_prefix_fn_t)(struct diff_options *opt, void *data)\n #define DIFF_OPT_IGNORE_DIRTY_SUBMODULES (1 << 26)\n #define DIFF_OPT_OVERRIDE_SUBMODULE_CONFIG (1 << 27)\n #define DIFF_OPT_DETECT_BULK_MOVES  (1 << 28)\n+#define DIFF_OPT_HIDE_DIR_RENAME_DETAILS (1 << 29)\n \n #define DIFF_OPT_TST(opts, flag)    ((opts)->flags & DIFF_OPT_##flag)\n #define DIFF_OPT_SET(opts, flag)    ((opts)->flags |= DIFF_OPT_##flag)\n@@ -268,6 +269,8 @@ extern void diffcore_fix_diff_index(struct diff_options *);\n \"                try unchanged files as candidate for copy detection.\\n\" \\\n \"  --detect-bulk-moves\\n\" \\\n \"                detect wholesale directory renames.\\n\" \\\n+\"  --hide-bulk-move-details\\n\" \\\n+\"                hide renames of individual files in a directory rename.\\n\" \\\n \"  -l<n>         limit rename attempts up to <n> paths.\\n\" \\\n \"  -O<file>      reorder diffs according to the <file>.\\n\" \\\n \"  -S<string>    find filepair whose only one side contains the string.\\n\" \\\ndiff --git a/diffcore-rename.c b/diffcore-rename.c\nindex 6d21792..cf15fc8 100644\n--- a/diffcore-rename.c\n+++ b/diffcore-rename.c\n@@ -455,6 +455,34 @@ struct diff_dir_rename {\n \tstruct diff_dir_rename* next;\n };\n \n+/*\n+ * Marks as such file_rename if it is made uninteresting by dir_rename.\n+ * Returns -1 if the file_rename is outside of the range in which given\n+ * renames concerned by dir_rename are to be found (ie. end of loop),\n+ * 0 otherwise.\n+ */\n+static int maybe_mark_uninteresting(struct diff_rename_dst* file_rename,\n+\t\t\t\t    struct diff_dir_rename* dir_rename,\n+\t\t\t\t    int one_len, int two_len)\n+{\n+\tif (!file_rename->pair) /* file add */\n+\t\treturn 0;\n+\tif (strncmp(file_rename->two->path,\n+\t\t    dir_rename->two->path, two_len))\n+\t\treturn -1;\n+\tif (strncmp(file_rename->pair->one->path,\n+\t\t    dir_rename->one->path, one_len) ||\n+\t    !basename_same(file_rename->pair->one, file_rename->two) ||\n+\t    file_rename->pair->score != MAX_SCORE)\n+\t\treturn 0;\n+\n+\tfile_rename->pair->uninteresting_rename = 1;\n+\tfprintf (stderr, \"[DBG] %s* -> %s* makes %s -> %s uninteresting\\n\",\n+\t\tdir_rename->one->path, dir_rename->two->path,\n+\t\tfile_rename->pair->one->path, file_rename->two->path);\n+\treturn 0;\n+}\n+\n // FIXME: replace with JN's version\n /*\n  * Copy dirname of src into dst, suitable to append a filename without\n@@ -648,11 +676,43 @@ static void check_one_bulk_move(struct diff_filepair *dstpair)\n  * Take all file renames recorded so far and check if they could cause\n  * a bulk move to be detected.\n  */\n-static void diffcore_bulk_moves(void)\n+static void diffcore_bulk_moves(int opt_hide_renames)\n {\n \tint i;\n \tfor (i = 0; i < rename_dst_nr; i++)\n \t\tcheck_one_bulk_move(rename_dst[i].pair);\n+\n+\tif (opt_hide_renames) {\n+\t\t// flag as \"uninteresting\" those candidates hidden by dir move\n+\t\tstruct diff_dir_rename* candidate;\n+\t\tfor (candidate=bulkmove_candidates;\n+\t\t     candidate; candidate = candidate->next) {\n+\t\t\tint two_idx, i, one_len, two_len;\n+\t\t\tif (candidate->discarded)\n+\t\t\t\tcontinue;\n+\n+\t\t\t// bisect to any entry within candidate dst dir\n+\t\t\tstruct diff_rename_dst* two_sample =\n+\t\t\t\tlocate_rename_dst_dir(candidate->two->path);\n+\t\t\tif (!two_sample) {\n+\t\t\t\tdie (\"PANIC: %s candidate of rename not in target tree (from %s)\\n\",\n+\t\t\t\t     candidate->two->path, candidate->one->path);\n+\t\t\t}\n+\t\t\ttwo_idx = two_sample - rename_dst;\n+\n+\t\t\t// now remove extraneous 100% files inside.\n+\t\t\tone_len = strlen(candidate->one->path);\n+\t\t\ttwo_len = strlen(candidate->two->path);\n+\t\t\tfor (i = two_idx; i < rename_dst_nr; i++)\n+\t\t\t\tif (maybe_mark_uninteresting (rename_dst+i, candidate,\n+\t\t\t\t\t\t\t      one_len, two_len) < 0)\n+\t\t\t\t\tbreak;\n+\t\t\tfor (i = two_idx-1; i >= 0; i--)\n+\t\t\t\tif (maybe_mark_uninteresting (rename_dst+i, candidate,\n+\t\t\t\t\t\t\t      one_len, two_len) < 0)\n+\t\t\t\t\tbreak;\n+\t\t}\n+\t}\n }\n \n void diffcore_rename(struct diff_options *options)\n@@ -825,7 +885,7 @@ void diffcore_rename(struct diff_options *options)\n \n \t/* Now possibly factorize those renames and copies. */\n \tif (DIFF_OPT_TST(options, DETECT_BULK_MOVES))\n-\t\tdiffcore_bulk_moves();\n+\t\tdiffcore_bulk_moves(DIFF_OPT_TST(options, HIDE_DIR_RENAME_DETAILS));\n \n \tDIFF_QUEUE_CLEAR(&outq);\n \n@@ -854,7 +914,8 @@ void diffcore_rename(struct diff_options *options)\n \t\t\tstruct diff_rename_dst *dst =\n \t\t\t\tlocate_rename_dst(p->two, 0);\n \t\t\tif (dst && dst->pair) {\n-\t\t\t\tdiff_q(&outq, dst->pair);\n+\t\t\t\tif (!dst->pair->uninteresting_rename)\n+\t\t\t\t\tdiff_q(&outq, dst->pair);\n \t\t\t\tpair_to_free = p;\n \t\t\t}\n \t\t\telse\ndiff --git a/diffcore.h b/diffcore.h\nindex b8f1fde..beee596 100644\n--- a/diffcore.h\n+++ b/diffcore.h\n@@ -69,6 +69,7 @@ struct diff_filepair {\n \tunsigned broken_pair : 1;\n \tunsigned renamed_pair : 1;\n \tunsigned is_unmerged : 1;\n+\tunsigned uninteresting_rename : 1;\n };\n #define DIFF_PAIR_UNMERGED(p) ((p)->is_unmerged)\n \n-- \n1.7.2.3\n"},{"id":"153128","messageId":"20101010123954.GB4983@home.lan","threadId":"25405","inReplyTo":"1286659895-1813-4-git-send-email-ydirson@altern.org","subject":"Re: [PATCH v5 3/4] [RFC] Only show bulkmoves in output.","fromName":"Yann Dirson","fromEmail":"ydirson@free.fr","sentAt":"2010-10-10T12:39:54Z","receivedAt":"2010-10-10T12:39:54Z","isPatch":true,"sender":{"key":"ydirson@free.fr","avatar":null},"body":"On Sat, Oct 09, 2010 at 11:31:34PM +0200, Yann Dirson wrote:\n> In theory we could just append those at display time (possibly diluting\n> the code too much ?), or before queing the diff.  In practice we cannot\n> do the latter easily in either case (strcat to a constant string (\"./\"),\n> nor to the paths tighly-allocated from alloc_filespec).\n> \n> So I went the way of including the trailing \"*\" from the beginning.\n> Since I'm still unsure whether keeping it that way, I leave it as a\n> separated patch for now.\n\nThinking twice, that approach has a major problem: it will break when\na file is named \"*\".  Finally looks like differing this to display\ntime will be the most sensible solution.\n"},{"id":"153169","messageId":"1286740795-7929-1-git-send-email-ydirson@altern.org","threadId":"25405","inReplyTo":"20101010123954.GB4983@home.lan","subject":"[PATCH 0/2] Re: [PATCH v5 3/4] [RFC] Only show bulkmoves in output.","fromName":"Yann Dirson","fromEmail":"ydirson@altern.org","sentAt":"2010-10-10T19:59:53Z","receivedAt":"2010-10-10T19:59:53Z","isPatch":true,"sender":{"key":"ydirson@altern.org","avatar":"https://avatars.githubusercontent.com/u/1190950?v=4"},"body":">Thinking twice, that approach has a major problem: it will break when\n>a file is named \"*\".  Finally looks like differing this to display\n>time will be the most sensible solution.\n\nSo here is a matching implementation (patch 1/2).  At first I also\nthought of moving the responsibility of the toplevel special case\n(displaying toplevel dir az \"./\") to the formatting routines (patch\n2/2), but unless there are compelling reasons to proceed that way, I\nguess that approach should be dropped - correct ?\n\nYann Dirson (2):\n  [RFC] Only show bulkmoves in output.\n  [RFC] Transfer special display of toplevel dir to display-time.\n\n diff.c                           |   15 +++++++++++++++\n diffcore-rename.c                |    9 ++-------\n diffcore.h                       |    1 +\n t/t4046-diff-rename-factorize.sh |   22 +++++++++++-----------\n 4 files changed, 29 insertions(+), 18 deletions(-)\n\n-- \n1.7.2.3\n"},{"id":"153170","messageId":"1286740795-7929-2-git-send-email-ydirson@altern.org","threadId":"25405","inReplyTo":"1286740795-7929-1-git-send-email-ydirson@altern.org","subject":"[PATCH 1/2] [RFC] Only show bulkmoves in output.","fromName":"Yann Dirson","fromEmail":"ydirson@altern.org","sentAt":"2010-10-10T19:59:54Z","receivedAt":"2010-10-10T19:59:54Z","isPatch":true,"sender":{"key":"ydirson@altern.org","avatar":"https://avatars.githubusercontent.com/u/1190950?v=4"},"body":"At display time, append a \"*\" suffix to bulk-move sources.  Later\nwe will be more selective and not display it for bulk moves that are\nindeed a directory rename.\n\nOpen question: \"foo/*\" is a valid name for a file, so if such a file\nis part of a bulk move, we will see one \"foo/*\" entry for the bulk\nmove (with zeroed sha1's), and one \"foo/*\" for the individual file\n(with its own sha1).  Is that seen as a problem ?  After all, the\nzeroed sha1's are probably here to stay (or do we want to add tree\nsha1's where possible ?), so it is pretty easy to tell which is which.\nAn alternative would be to have another way of telling it is a\nbulk-move, such as a flag to M or C (eg. M100B), but there is no\nprecedent for this, so I can't tell for myself.\n\nAfter this patch only one of the expected failures can be considered\ninnocuous.\n---\n diff.c                           |    9 ++++++++-\n diffcore-rename.c                |    4 ++--\n diffcore.h                       |    1 +\n t/t4046-diff-rename-factorize.sh |   22 +++++++++++-----------\n 4 files changed, 22 insertions(+), 14 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex 64e01b3..4de43d6 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -3471,7 +3471,14 @@ static void diff_flush_raw(struct diff_filepair *p, struct diff_options *opt)\n \tif (p->status == DIFF_STATUS_COPIED ||\n \t    p->status == DIFF_STATUS_RENAMED) {\n \t\tconst char *name_a, *name_b;\n-\t\tname_a = p->one->path;\n+\t\tif (p->is_bulkmove) {\n+\t\t\t/* append \"*\" to the first dirname */\n+\t\t\tchar buf[PATH_MAX];\n+\t\t\tchar* next = memccpy(buf, p->one->path, '\\0', PATH_MAX);\n+\t\t\tnext[-1] = '*'; *next = '\\0';\n+\t\t\tname_a = buf;\n+\t\t} else\n+\t\t\tname_a = p->one->path;\n \t\tname_b = p->two->path;\n \t\tstrip_prefix(opt->prefix_length, &name_a, &name_b);\n \t\twrite_name_quoted(name_a, opt->file, inter_name_termination);\ndiff --git a/diffcore-rename.c b/diffcore-rename.c\nindex 18a0605..8de0d57 100644\n--- a/diffcore-rename.c\n+++ b/diffcore-rename.c\n@@ -603,8 +603,7 @@ static void check_one_bulk_move(struct diff_filepair *dstpair)\n \t/* already considered ? */\n \tfor (seen=bulkmove_candidates; seen; seen = seen->next)\n \t\tif (!strcmp(seen->one->path, one_parent_path)) break;\n-\tif (!seen) {\n-\t\t/* record potential dir rename */\n+\tif (!seen) { /* record potential dir rename */\n \t\tseen = xmalloc(sizeof(*seen));\n \t\tseen->one = alloc_filespec(one_parent_path);\n \t\tfill_filespec(seen->one, null_sha1 /*FIXME*/, S_IFDIR);\n@@ -830,6 +829,7 @@ void diffcore_rename(struct diff_options *options)\n \t\tpair = diff_queue(&outq, candidate->one, candidate->two);\n \t\tpair->score = MAX_SCORE;\n \t\tpair->renamed_pair = 1;\n+\t\tpair->is_bulkmove = 1;\n \t}\n \n \tfor (i = 0; i < q->nr; i++) {\ndiff --git a/diffcore.h b/diffcore.h\nindex b8f1fde..6dab95b 100644\n--- a/diffcore.h\n+++ b/diffcore.h\n@@ -69,6 +69,7 @@ struct diff_filepair {\n \tunsigned broken_pair : 1;\n \tunsigned renamed_pair : 1;\n \tunsigned is_unmerged : 1;\n+\tunsigned is_bulkmove : 1;\n };\n #define DIFF_PAIR_UNMERGED(p) ((p)->is_unmerged)\n \ndiff --git a/t/t4046-diff-rename-factorize.sh b/t/t4046-diff-rename-factorize.sh\nindex d063e25..9353929 100755\n--- a/t/t4046-diff-rename-factorize.sh\n+++ b/t/t4046-diff-rename-factorize.sh\n@@ -37,7 +37,7 @@ test_expect_success 'setup' '\n \n test_expect_success 'diff-index --detect-bulk-moves after directory move.' '\n \tcat >expected <<-EOF &&\n-\t:040000 040000 X X R#\ta/\tb/\n+\t:040000 040000 X X R#\ta/*\tb/\n \t:100644 100644 X X R#\ta/path0\tb/path0\n \t:100644 100644 X X R#\ta/path1\tb/path1\n \t:100644 100644 X X R#\ta/path2\tb/path2\n@@ -58,7 +58,7 @@ test_expect_success 'setup non-100% rename' '\n \n test_expect_success 'diff-index --detect-bulk-moves after content changes.' '\n \tcat >expected <<-EOF &&\n-\t:040000 040000 X X R#\ta/\tb/\n+\t:040000 040000 X X R#\ta/*\tb/\n \t:100644 000000 X X D#\ta/path3\n \t:100644 100644 X X R#\ta/path2\tb/2path\n \t:100644 100644 X X R#\ta/path0\tb/path0\n@@ -87,7 +87,7 @@ test_expect_success 'setup bulk move that is not directory move' '\n \tgit update-index --add --remove a/* c/apath0 c/apath1 c/apath2\n '\n \n-test_expect_failure 'diff-index --detect-bulk-moves without full-dir rename.' '\n+test_expect_success 'diff-index --detect-bulk-moves without full-dir rename.' '\n \tcat >expected <<-EOF &&\n \t:040000 040000 X X R#\tc/*\ta/\n \t:100644 100644 X X R#\tc/apath0\ta/apath0\n@@ -105,7 +105,7 @@ test_expect_success 'setup bulk move to toplevel' '\n \tgit update-index --add --remove apath* c/apath0 c/apath1 c/apath2\n '\n \n-test_expect_failure 'diff-index --detect-bulk-moves bulk move to toplevel.' '\n+test_expect_success 'diff-index --detect-bulk-moves bulk move to toplevel.' '\n \tcat >expected <<-EOF &&\n \t:040000 040000 X X R#\tc/*\t./\n \t:100644 100644 X X R#\tc/apath0\tapath0\n@@ -132,7 +132,7 @@ test_expect_success 'setup move including a subdir, with some content changes' '\n \n test_expect_failure 'diff-index --detect-bulk-moves on a move including a subdir.' '\n \tcat >expected <<-EOF &&\n-\t:040000 040000 X X R#\ta/\tb/\n+\t:040000 040000 X X R#\ta/*\tb/\n \t:100644 100644 X X R#\ta/c/apath0\tb/c/apath0\n \t:100644 100644 X X R#\ta/c/apath1\tb/c/apath1\n \t:100644 100644 X X R#\ta/c/apath2\tb/c/apath2\n@@ -176,7 +176,7 @@ test_expect_success 'setup move of files and subdirs to different places' '\n \n test_expect_failure 'moving subdirs into one dir and files into another is not mistaken for dir move' '\n \tcat >expected <<-EOF &&\n-\t:040000 040000 X X R#\ta/c/\tb/\n+\t:040000 040000 X X R#\ta/c/*\tb/\n \t:100644 100644 X X R#\ta/c/apath0\tb/apath0\n \t:100644 100644 X X R#\ta/c/apath1\tb/apath1\n \t:100644 100644 X X R#\ta/c/apath2\tb/apath2\n@@ -201,11 +201,11 @@ test_expect_success 'setup move of dir with only subdirs' '\n \tgit mv a z\n '\n \n-test_expect_failure 'moving a dir with no files' '\n+test_expect_failure 'moving a dir with no direct children files' '\n \tcat >expected <<-EOF &&\n-\t:040000 040000 X X R#\ta/\tz/\n-\t:040000 040000 X X R#\ta/b/\tz/b/\n-\t:040000 040000 X X R#\ta/c/\tz/c/\n+\t:040000 040000 X X R#\ta/*\tz/\n+\t:040000 040000 X X R#\ta/b/*\tz/b/\n+\t:040000 040000 X X R#\ta/c/*\tz/c/\n \t:100644 100644 X X R#\ta/b/path0\tz/b/path0\n \t:100644 100644 X X R#\ta/b/path1\tz/b/path1\n \t:100644 100644 X X R#\ta/b/path2\tz/b/path2\n@@ -235,7 +235,7 @@ test_expect_success 'setup move from toplevel to subdir' '\n \tgit update-index --add --remove path0 path1 path2 path3 z/path*\n '\n \n-test_expect_failure '--detect-bulk-moves everything from toplevel.' '\n+test_expect_success '--detect-bulk-moves everything from toplevel.' '\n \tcat >expected <<-EOF &&\n \t:040000 040000 X X R#\t./*\tz/\n \t:100644 100644 X X R#\tpath0\tz/path0\n-- \n1.7.2.3\n"},{"id":"153171","messageId":"1286740795-7929-3-git-send-email-ydirson@altern.org","threadId":"25405","inReplyTo":"1286740795-7929-2-git-send-email-ydirson@altern.org","subject":"[PATCH 2/2] [RFC] Transfer special display of toplevel dir to display-time.","fromName":"Yann Dirson","fromEmail":"ydirson@altern.org","sentAt":"2010-10-10T19:59:55Z","receivedAt":"2010-10-10T19:59:55Z","isPatch":true,"sender":{"key":"ydirson@altern.org","avatar":"https://avatars.githubusercontent.com/u/1190950?v=4"},"body":"I have mixed feelings about this one: it makes the display code much\nmore messy than the rename code was, and it will need to be duplicated\nfor each output format.\n\nSo I suggest to finally just keep the \"./\" assignation in diffcore_rename.\nAny better idea ?\n---\n diff.c            |   22 +++++++++++++++-------\n diffcore-rename.c |    5 -----\n 2 files changed, 15 insertions(+), 12 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex 4de43d6..6fe47c2 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -3471,15 +3471,23 @@ static void diff_flush_raw(struct diff_filepair *p, struct diff_options *opt)\n \tif (p->status == DIFF_STATUS_COPIED ||\n \t    p->status == DIFF_STATUS_RENAMED) {\n \t\tconst char *name_a, *name_b;\n+\t\tchar buf[PATH_MAX];\n+\t\tname_a = p->one->path;\n+\t\tname_b = p->two->path;\n \t\tif (p->is_bulkmove) {\n-\t\t\t/* append \"*\" to the first dirname */\n-\t\t\tchar buf[PATH_MAX];\n-\t\t\tchar* next = memccpy(buf, p->one->path, '\\0', PATH_MAX);\n-\t\t\tnext[-1] = '*'; *next = '\\0';\n-\t\t\tname_a = buf;\n+\t\t\tif (!*name_a)\n+\t\t\t\tname_a = \"./*\"; /* visualize toplevel dir */\n+\t\t\telse {\n+\t\t\t\t/* append \"*\" to the first dirname */\n+\t\t\t\tchar* next = memccpy(buf, name_a, '\\0', PATH_MAX);\n+\t\t\t\tnext[-1] = '*'; *next = '\\0';\n+\t\t\t\tname_a = buf;\n+\t\t\t}\n \t\t} else\n-\t\t\tname_a = p->one->path;\n-\t\tname_b = p->two->path;\n+\t\t\tif (!*name_a)\n+\t\t\t\tname_a = \"./\"; /* visualize toplevel dir */\n+\t\tif (!*name_b)\n+\t\t\tname_b = \"./\"; /* visualize toplevel dir */\n \t\tstrip_prefix(opt->prefix_length, &name_a, &name_b);\n \t\twrite_name_quoted(name_a, opt->file, inter_name_termination);\n \t\twrite_name_quoted(name_b, opt->file, line_termination);\ndiff --git a/diffcore-rename.c b/diffcore-rename.c\nindex 8de0d57..a075a25 100644\n--- a/diffcore-rename.c\n+++ b/diffcore-rename.c\n@@ -821,11 +821,6 @@ void diffcore_rename(struct diff_options *options)\n \tfor (candidate=bulkmove_candidates; candidate; candidate = candidate->next) {\n \t\tstruct diff_filepair* pair;\n \t\tif (candidate->discarded) continue;\n-\t\t/* visualize toplevel dir if needed */ //FIXME: wrong place for this ?\n-\t\tif (!*candidate->one->path)\n-\t\t\tcandidate->one->path = \"./\";\n-\t\tif (!*candidate->two->path)\n-\t\t\tcandidate->two->path = \"./\";\n \t\tpair = diff_queue(&outq, candidate->one, candidate->two);\n \t\tpair->score = MAX_SCORE;\n \t\tpair->renamed_pair = 1;\n-- \n1.7.2.3\n"}]}