{"thread":{"id":"25452","subject":"[PATCH v6 3/5] [RFC] Handle the simpler case of a subdir invalidating bulk move.","startedAt":"2010-10-14T23:29:54Z","lastAt":"2010-10-17T19:24:03Z","messageCount":17,"participants":["Yann Dirson","Jonathan Nieder","Ævar Arnfjörð Bjarmason","Johannes Sixt","Erik Faye-Lund","Ludvig Strigeus","Junio C Hamano"],"isPatch":true,"patchVersion":6,"patchTotal":5},"messages":[{"id":"153546","messageId":"1287098999-9244-1-git-send-email-ydirson@altern.org","threadId":"25452","inReplyTo":null,"subject":"[PATCH v6 0/5] Detection of directory renames","fromName":"Yann Dirson","fromEmail":"ydirson@altern.org","sentAt":"2010-10-14T23:29:54Z","receivedAt":"2010-10-14T23:29:54Z","isPatch":true,"sender":{"key":"ydirson@altern.org","avatar":"https://avatars.githubusercontent.com/u/1190950?v=4"},"body":"Changes since v5:\n\n* added more tests (one OK, one for suboptimal work only, one for a\n  bug fixed)\n* adjusted expected output for 'diff-index --detect-bulk-moves on a\n  move including a subdir.', it is OK\n* include a better copy_dirname based on Johnathan's code\n* dropped the \"Transfer special display of toplevel dir to display-time.\"\n  patch posted earlier, I really think that was a bad idea.\n* include 2 more patches that deal with problems with moves of deep trees,\n  separated for easier review, but which should be squashed into the main\n  one ultimately\n\nOnly relatively easy things are still on my immediate TODO list for\nthis series:\n\n* unified diff informational output\n* memrchr() implementation for portability\n* cleanups (free allocated structures, split long funcs a bit more, better\n  comments, etc)\n* audit remaining FIXME tags\n\nFinishing the \"--hide\" feature will be next.\n\nYann Dirson (5):\n  Introduce bulk-move detection in diffcore.\n  Add testcases for the --detect-bulk-moves diffcore flag.\n  [RFC] Handle the simpler case of a subdir invalidating bulk move.\n  [RFC] Consider all parents of a file as candidates for bulk rename.\n  [WIP] Allow hiding renames of individual files involved in a\n    directory rename.\n\n diff-lib.c                       |    6 +-\n diff.c                           |   21 ++-\n diff.h                           |    6 +\n diffcore-rename.c                |  376 +++++++++++++++++++++++++++++++++++++-\n diffcore.h                       |    2 +\n t/t4046-diff-rename-factorize.sh |  301 ++++++++++++++++++++++++++++++\n tree-diff.c                      |    4 +-\n 7 files changed, 703 insertions(+), 13 deletions(-)\n create mode 100755 t/t4046-diff-rename-factorize.sh\n\n-- \n1.7.2.3\n"},{"id":"153549","messageId":"1287098999-9244-2-git-send-email-ydirson@altern.org","threadId":"25452","inReplyTo":"1287098999-9244-1-git-send-email-ydirson@altern.org","subject":"[PATCH v6 1/5] Introduce bulk-move detection in diffcore.","fromName":"Yann Dirson","fromEmail":"ydirson@altern.org","sentAt":"2010-10-14T23:29:55Z","receivedAt":"2010-10-14T23:29:55Z","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/\".  Those cannot be\nconfused with renames of files named \"whatever/*\" with a literal star\ncharacter, from the full-zero SHA1's.\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\nSigned-off-by: Yann Dirson <ydirson@free.fr>\n---\n diff-lib.c        |    6 +-\n diff.c            |   14 +++-\n diff.h            |    3 +\n diffcore-rename.c |  274 +++++++++++++++++++++++++++++++++++++++++++++++++++--\n diffcore.h        |    1 +\n tree-diff.c       |    4 +-\n 6 files changed, 290 insertions(+), 12 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..4de43d6 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\"))\n@@ -3466,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/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..f252da7 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,196 @@ 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+/*\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+ * Supports in-place modification of src by passing dst == src.\n+ */\n+static const char* copy_dirname(char* dst, const char* src)\n+{\n+\tsize_t len = strlen(src);\n+\tconst char *slash;\n+\n+\tif (len > 0 && src[len - 1] == '/')\n+\t\t/* Trailing slash.  Ignore it. */\n+\t\tlen--;\n+\n+\tslash = memrchr(src, '/', len);\n+\tif (!slash) {\n+\t\t*dst = '\\0';\n+\t\treturn dst;\n+\t}\n+\n+\tchar* end = mempcpy(dst, src, slash - src + 1);\n+\t*end = '\\0';\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: do we want to replace linked list with sorted array ?\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) { /* 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, S_IFDIR);\n+\t\tseen->two = alloc_filespec(two_parent_path);\n+\t\tfill_filespec(seen->two, null_sha1, 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 bulk move 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 bulk moves (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 +675,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 +743,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 +805,29 @@ 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 */\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\tpair->is_bulkmove = 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/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/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":"153547","messageId":"1287098999-9244-3-git-send-email-ydirson@altern.org","threadId":"25452","inReplyTo":"1287098999-9244-2-git-send-email-ydirson@altern.org","subject":"[PATCH v6 2/5] Add testcases for the --detect-bulk-moves diffcore flag.","fromName":"Yann Dirson","fromEmail":"ydirson@altern.org","sentAt":"2010-10-14T23:29:56Z","receivedAt":"2010-10-14T23:29:56Z","isPatch":true,"sender":{"key":"ydirson@altern.org","avatar":"https://avatars.githubusercontent.com/u/1190950?v=4"},"body":"This notably includes a couple of tests for cases known not to be\nworking correctly yet.\n\nSigned-off-by: Yann Dirson <ydirson@free.fr>\n---\n t/t4046-diff-rename-factorize.sh |  301 ++++++++++++++++++++++++++++++++++++++\n 1 files changed, 301 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..dca7cb7\n--- /dev/null\n+++ b/t/t4046-diff-rename-factorize.sh\n@@ -0,0 +1,301 @@\n+#!/bin/sh\n+#\n+# Copyright (c) 2008,2010 Yann Dirson\n+# Copyright (c) 2005 Junio C Hamano\n+#\n+\n+# TODO for dir renames:\n+# * two dirs or more moving all their files to a single dir\n+# * simultaneous bulkmove and rename\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+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+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+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+\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 with new file in source dir' '\n+\techo > c/anotherpath \"How much wood?\" &&\n+\tgit update-index --add c/another*\n+'\n+test_expect_success 'diff-index --detect-bulk-moves with new file in source dir.' '\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+\t:000000 100644 X X A#\tc/anotherpath\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 with interfering copy' '\n+\trm c/anotherpath &&\n+\tgit update-index --remove c/anotherpath &&\n+\tmkdir b &&\n+\tcp a/apath0 b/apath9 &&\n+\techo >> a/apath0 \"more\" &&\n+\tgit update-index --add a/apath0 b/apath9\n+'\n+# scores select the \"wrong\" one as \"moved\" (only a suboptimal detection)\n+test_expect_failure 'diff-index --detect-bulk-moves with interfering copy.' '\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+\t:100644 100644 X X C#\tc/apath0\tb/apath9\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+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+\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+test_expect_success '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/c/*\tb/c/\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+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+test_expect_success '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+# the same with different ordering\n+test_expect_success 'setup move of files and subdirs to different places' '\n+\tmv d 0 &&\n+\tgit update-index --add --remove d/path0 d/path1 d/path2 d/path3 0/path*\n+'\n+test_expect_success '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/path0\t0/path0\n+\t:100644 100644 X X R#\ta/path1\t0/path1\n+\t:100644 100644 X X R#\ta/path2\t0/path2\n+\t:100644 100644 X X R#\ta/path3\t0/path3\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+\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+# TODO: only a suboptimal non-detection\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: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+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+\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":"153545","messageId":"1287098999-9244-4-git-send-email-ydirson@altern.org","threadId":"25452","inReplyTo":"1287098999-9244-3-git-send-email-ydirson@altern.org","subject":"[PATCH v6 3/5] [RFC] Handle the simpler case of a subdir invalidating bulk move.","fromName":"Yann Dirson","fromEmail":"ydirson@altern.org","sentAt":"2010-10-14T23:29:57Z","receivedAt":"2010-10-14T23:29:57Z","isPatch":true,"sender":{"key":"ydirson@altern.org","avatar":"https://avatars.githubusercontent.com/u/1190950?v=4"},"body":"Doing it so simply depends on the subdir moves being noticed later,\nthis fixes only one of the remaining failing testcases.  Doing better\nrequires real handling of deep trees, which is addressed by next\npatch, but add much complexity.\n\nSigned-off-by: Yann Dirson <ydirson@free.fr>\n---\n diffcore-rename.c |   59 ++++++++++++++++++++++++++++++++++++++++------------\n 1 files changed, 45 insertions(+), 14 deletions(-)\n\ndiff --git a/diffcore-rename.c b/diffcore-rename.c\nindex f252da7..1be1af1 100644\n--- a/diffcore-rename.c\n+++ b/diffcore-rename.c\n@@ -596,10 +596,52 @@ 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+\tfprintf(stderr, \"[] %s -> %s ?\\n\",dstpair->one->path, dstpair->two->path);\n+\n+\t// FIXME: loop over successive prefixes\n+\tunsigned needs_adding = 1;\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) { /* record potential dir rename */\n+\tfor (seen=bulkmove_candidates; seen; seen = seen->next) {\n+\t\tif (seen->discarded) {\n+\t\t\t/* already seen a rename from seen->one to some than ->two */\n+\t\t\tneeds_adding = 0;\n+\t\t\tcontinue;\n+\t\t}\n+\t\t/* check exact dir */\n+\t\tif (!strcmp(seen->one->path, one_parent_path)) {\n+\t\t\t/* already added */\n+\t\t\tneeds_adding = 0;\n+\t\t\t/* check that seen entry matches this rename */\n+\t\t\tif (strcmp(two_parent_path, seen->two->path)) {\n+\t\t\t\tfprintf(stderr, \"[DBG] discarding dir %s from bulk moves (split into %s and %s)\\n\",\n+\t\t\t\t\tseen->one->path, two_parent_path, seen->two->path);\n+\t\t\t\t// FIXME: may be worth to free it instead\n+\t\t\t\tseen->discarded = 1;\n+\t\t\t}\n+\t\t\tcontinue;\n+\t\t}\n+\t\tif (!prefixcmp(one_parent_path, seen->one->path)) {\n+\t\t\tif (prefixcmp(two_parent_path, seen->two->path)) {\n+\t\t\t\tfprintf(stderr, \"[DBG] discarding dir %s from bulk moves (split into %s and %s)\\n\",\n+\t\t\t\t\tseen->one->path, two_parent_path, seen->two->path);\n+\t\t\t\t// FIXME: may be worth to free it instead\n+\t\t\t\tseen->discarded = 1;\n+\t\t\t\tcontinue;\n+\t\t\t}\n+\t\t} else {\n+\t\t\tfprintf(stderr, \"[DBG]  %s considered irrelevant for %s -> %s\\n\",\n+\t\t\t\tdstpair->one->path, seen->one->path, seen->two->path);\n+\t\t\tcontinue;\n+\t\t}\n+\n+\t\t/* dstpair confirms seen */\n+\t\tfprintf(stderr, \"[DBG] %s -> %s DOES NOT cause discard of %s -> %s\\n\",\n+\t\t\tdstpair->one->path, dstpair->two->path,\n+\t\t\tseen->one->path, seen->two->path);\n+\t}\n+\tif (needs_adding) { /* record potential dir rename */\n+\t\t/* all checks ok, we keep that entry */\n \t\tseen = xmalloc(sizeof(*seen));\n \t\tseen->one = alloc_filespec(one_parent_path);\n \t\tfill_filespec(seen->one, null_sha1, S_IFDIR);\n@@ -614,17 +656,6 @@ static void check_one_bulk_move(struct diff_filepair *dstpair)\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 bulk moves (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-- \n1.7.2.3\n"},{"id":"153550","messageId":"1287098999-9244-5-git-send-email-ydirson@altern.org","threadId":"25452","inReplyTo":"1287098999-9244-4-git-send-email-ydirson@altern.org","subject":"[PATCH v6 4/5] [RFC] Consider all parents of a file as candidates for bulk rename.","fromName":"Yann Dirson","fromEmail":"ydirson@altern.org","sentAt":"2010-10-14T23:29:58Z","receivedAt":"2010-10-14T23:29:58Z","isPatch":true,"sender":{"key":"ydirson@altern.org","avatar":"https://avatars.githubusercontent.com/u/1190950?v=4"},"body":"We previously only looked at the immediate parents of a file involved\nin a move.  Now consider upper ones as candidates too.\n\nThe way it is done here is somewhat inefficient, but at least it\nhandles all previously-known cases of incorrectness.\n\nNote this patch largely puts existing code in a look, and is a bit\neasier to read with -w.\n\nSigned-off-by: Yann Dirson <ydirson@free.fr>\n---\n diffcore-rename.c |  118 ++++++++++++++++++++++++++++-------------------------\n 1 files changed, 63 insertions(+), 55 deletions(-)\n\ndiff --git a/diffcore-rename.c b/diffcore-rename.c\nindex 1be1af1..ff69201 100644\n--- a/diffcore-rename.c\n+++ b/diffcore-rename.c\n@@ -542,7 +542,7 @@ static int maybe_disqualify_bulkmove(const char* one_parent_path,\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\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@@ -596,66 +596,74 @@ 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-\tfprintf(stderr, \"[] %s -> %s ?\\n\",dstpair->one->path, dstpair->two->path);\n+\tfprintf(stderr, \"[] %s -> %s ?\\n\", dstpair->one->path, dstpair->two->path);\n \n-\t// FIXME: loop over successive prefixes\n-\tunsigned needs_adding = 1;\n+\t// loop over successive prefixes\n+\t// FIXME: also loop over two_parent_path prefixes ?\n+\tdo {\n+\t\tunsigned needs_adding = 1;\n \n-\t/* already considered ? */\n-\tfor (seen=bulkmove_candidates; seen; seen = seen->next) {\n-\t\tif (seen->discarded) {\n-\t\t\t/* already seen a rename from seen->one to some than ->two */\n-\t\t\tneeds_adding = 0;\n-\t\t\tcontinue;\n-\t\t}\n-\t\t/* check exact dir */\n-\t\tif (!strcmp(seen->one->path, one_parent_path)) {\n-\t\t\t/* already added */\n-\t\t\tneeds_adding = 0;\n-\t\t\t/* check that seen entry matches this rename */\n-\t\t\tif (strcmp(two_parent_path, seen->two->path)) {\n-\t\t\t\tfprintf(stderr, \"[DBG] discarding dir %s from bulk moves (split into %s and %s)\\n\",\n-\t\t\t\t\tseen->one->path, two_parent_path, seen->two->path);\n-\t\t\t\t// FIXME: may be worth to free it instead\n-\t\t\t\tseen->discarded = 1;\n-\t\t\t}\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif (!prefixcmp(one_parent_path, seen->one->path)) {\n-\t\t\tif (prefixcmp(two_parent_path, seen->two->path)) {\n-\t\t\t\tfprintf(stderr, \"[DBG] discarding dir %s from bulk moves (split into %s and %s)\\n\",\n-\t\t\t\t\tseen->one->path, two_parent_path, seen->two->path);\n-\t\t\t\t// FIXME: may be worth to free it instead\n-\t\t\t\tseen->discarded = 1;\n+\t\tfprintf(stderr, \"[[]] %s ...\\n\", one_parent_path);\n+\n+\t\t/* already considered ? */\n+\t\tfor (seen=bulkmove_candidates; seen; seen = seen->next) {\n+\t\t\tif (seen->discarded) {\n+\t\t\t\t/* already seen a rename from seen->one to some than ->two */\n+\t\t\t\tneeds_adding = 0;\n \t\t\t\tcontinue;\n \t\t\t}\n-\t\t} else {\n-\t\t\tfprintf(stderr, \"[DBG]  %s considered irrelevant for %s -> %s\\n\",\n-\t\t\t\tdstpair->one->path, seen->one->path, seen->two->path);\n-\t\t\tcontinue;\n+\t\t\tfprintf(stderr, \"[DBG]  ? %s -> %s\\n\", seen->one->path, seen->two->path);\n+\t\t\t/* subdir of \"seen\" source dir ? */\n+\t\t\tif (!prefixcmp(one_parent_path, seen->one->path)) {\n+\t\t\t\t/* subdir of \"seen\" dest dir ? */\n+\t\t\t\tif (!prefixcmp(two_parent_path, seen->two->path)) {\n+\t\t\t\t\tif (!strcmp(seen->one->path, one_parent_path) &&\n+\t\t\t\t\t    !strcmp(seen->two->path, two_parent_path)) {\n+\t\t\t\t\t\t/* already added */\n+\t\t\t\t\t\tfprintf(stderr, \"[DBG] already added\\n\");\n+\t\t\t\t\t\tneeds_adding = 0;\n+\t\t\t\t\t} else // confirms\n+\t\t\t\t\t\tfprintf(stderr, \"[DBG]  'dstpair' conforts 'seen'\\n\");\n+\t\t\t\t} else {\n+\t\t\t\t\tfprintf(stderr, \"[DBG] discarding %s -> %s from bulk moves (split into %s and %s)\\n\",\n+\t\t\t\t\t\tseen->one->path, seen->two->path,\n+\t\t\t\t\t\ttwo_parent_path, seen->two->path);\n+\t\t\t\t\t// FIXME: may be worth to free it instead\n+\t\t\t\t\tseen->discarded = 1;\n+\t\t\t\t\tif (!strcmp(seen->one->path, one_parent_path) &&\n+\t\t\t\t\t    prefixcmp(seen->two->path, two_parent_path)) {\n+\t\t\t\t\t\tfprintf(stderr, \"[DBG] ... and not adding self\\n\");\n+\t\t\t\t\t\tneeds_adding = 0;\n+\t\t\t\t\t}\n+\t\t\t\t\tcontinue;\n+\t\t\t\t}\n+\t\t\t}\n+\t\t\telse fprintf(stderr, \"[DBG]  'dstpair' unrelated to 'seen'\\n\");\n+\n+\t\t\t/* dstpair confirms seen, or does not infirm */\n+\t\t\tfprintf(stderr, \"[DBG] %s -> %s DOES NOT cause discard of %s -> %s\\n\",\n+\t\t\t\tdstpair->one->path, dstpair->two->path,\n+\t\t\t\tseen->one->path, seen->two->path);\n+\t\t}\n+\t\tif (needs_adding) { /* record potential dir rename */\n+\t\t\t/* all checks ok, we keep that entry */\n+\t\t\tseen = xmalloc(sizeof(*seen));\n+\t\t\tseen->one = alloc_filespec(one_parent_path);\n+\t\t\tfill_filespec(seen->one, null_sha1, S_IFDIR);\n+\t\t\tseen->two = alloc_filespec(two_parent_path);\n+\t\t\tfill_filespec(seen->two, null_sha1, S_IFDIR);\n+\t\t\tseen->discarded = 0;\n+\t\t\tseen->next = bulkmove_candidates;\n+\t\t\tbulkmove_candidates = seen;\n+\t\t\tfprintf(stderr, \"[DBG] %s -> %s suggests possible bulk move from %s to %s\\n\",\n+\t\t\t\tdstpair->one->path,\n+\t\t\t\tdstpair->two->path,\n+\t\t\t\tone_parent_path, two_parent_path);\n \t\t}\n \n-\t\t/* dstpair confirms seen */\n-\t\tfprintf(stderr, \"[DBG] %s -> %s DOES NOT cause discard of %s -> %s\\n\",\n-\t\t\tdstpair->one->path, dstpair->two->path,\n-\t\t\tseen->one->path, seen->two->path);\n-\t}\n-\tif (needs_adding) { /* record potential dir rename */\n-\t\t/* all checks ok, we keep that entry */\n-\t\tseen = xmalloc(sizeof(*seen));\n-\t\tseen->one = alloc_filespec(one_parent_path);\n-\t\tfill_filespec(seen->one, null_sha1, S_IFDIR);\n-\t\tseen->two = alloc_filespec(two_parent_path);\n-\t\tfill_filespec(seen->two, null_sha1, 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 bulk move 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+\t\t/* next parent if any */\n+\t\tcopy_dirname(one_parent_path, one_parent_path);\n+\t} while (*one_parent_path);\n }\n \n /*\n-- \n1.7.2.3\n"},{"id":"153548","messageId":"1287098999-9244-6-git-send-email-ydirson@altern.org","threadId":"25452","inReplyTo":"1287098999-9244-5-git-send-email-ydirson@altern.org","subject":"[PATCH v6 5/5] [WIP] Allow hiding renames of individual files involved in a directory rename.","fromName":"Yann Dirson","fromEmail":"ydirson@altern.org","sentAt":"2010-10-14T23:29:59Z","receivedAt":"2010-10-14T23:29:59Z","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\nSigned-off-by: Yann Dirson <ydirson@free.fr>\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 4de43d6..81282bf 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 ff69201..b419953 100644\n--- a/diffcore-rename.c\n+++ b/diffcore-rename.c\n@@ -456,6 +456,34 @@ struct diff_dir_rename {\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+/*\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@@ -670,11 +698,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@@ -847,7 +907,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@@ -881,7 +941,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 6dab95b..a4eb8e1 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 \tunsigned is_bulkmove : 1;\n };\n #define DIFF_PAIR_UNMERGED(p) ((p)->is_unmerged)\n-- \n1.7.2.3\n"},{"id":"153558","messageId":"20101015051750.GA21830@burratino","threadId":"25452","inReplyTo":"1287098999-9244-1-git-send-email-ydirson@altern.org","subject":"[PATCH] compat: add memrchr()","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-10-15T05:17:50Z","receivedAt":"2010-10-15T05:17:50Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Reimplement another handy convenience function from glibc.  memrchr()\nsearches from the end of a memory area for a particular character.  It\nis similar to strrchr() but takes a length argument and is\nbinary-safe.\n\nThe whole-directory rename detection patch could use this to find the\nlast directory separator in a (possibly truncated) pathname.\n\nSigned-off-by: Jonathan Nieder <jrnieder@gmail.com>\n---\nYann Dirson wrote:\n\n> * memrchr() implementation for portability\n\nSomething like this?  Untested.\n\n git-compat-util.h |   16 ++++++++++++++++\n 1 files changed, 16 insertions(+), 0 deletions(-)\n\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex 2af8d3e..6f1020e 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -366,6 +366,9 @@ extern int git_vsnprintf(char *str, size_t maxsize,\n #define HAVE_STRCHRNUL\n #define HAVE_MEMPCPY\n #endif\n+#if __GLIBC_PREREQ(2, 2)\n+#define HAVE_MEMRCHR\n+#endif\n #endif\n \n #ifndef HAVE_STRCHRNUL\n@@ -386,6 +389,19 @@ static inline void *gitmempcpy(void *dest, const void *src, size_t n)\n }\n #endif\n \n+#ifndef HAVE_MEMRCHR\n+#define memrchr gitmemrchr\n+static inline void *gitmemrchr(const void *s, int c, size_t n)\n+{\n+\tconst char *p = s;\n+\tp += n;\n+\twhile (p != s)\n+\t\tif (*--p == c)\n+\t\t\treturn p;\n+\treturn NULL;\n+}\n+#endif\n+\n extern void release_pack_memory(size_t, int);\n \n typedef void (*try_to_free_t)(size_t);\n-- \n1.7.2.3\n"},{"id":"153561","messageId":"AANLkTinBuAhLLUMLd6ZWtVFWCjnFAxYwVSo1Pkja7bT4@mail.gmail.com","threadId":"25452","inReplyTo":"20101015051750.GA21830@burratino","subject":"Re: [PATCH] compat: add memrchr()","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-10-15T05:31:41Z","receivedAt":"2010-10-15T05:31:41Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Fri, Oct 15, 2010 at 05:17, Jonathan Nieder <jrnieder@gmail.com> wrote:\n> Reimplement another handy convenience function from glibc.  memrchr()\n> searches from the end of a memory area for a particular character.  It\n> is similar to strrchr() but takes a length argument and is\n> binary-safe.\n>\n> The whole-directory rename detection patch could use this to find the\n> last directory separator in a (possibly truncated) pathname.\n\nMaybe something like this for configure.ac:\n\n    AC_CHECK_LIB([c], [memchr],\n    [HAVE_MEMRCHR=YesPlease],\n    [HAVE_MEMRCHR=])\n    AC_SUBST(HAVE_MEMRCHR)\n\nAnd documentation with the other HAVE_* variables at the top of the\nMakefile?\n"},{"id":"153564","messageId":"20101015060654.GD21830@burratino","threadId":"25452","inReplyTo":"AANLkTinBuAhLLUMLd6ZWtVFWCjnFAxYwVSo1Pkja7bT4@mail.gmail.com","subject":"Re: [PATCH] compat: add memrchr()","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-10-15T06:06:54Z","receivedAt":"2010-10-15T06:06:54Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ævar Arnfjörð Bjarmason wrote:\n\n> Maybe something like this for configure.ac:\n> \n>     AC_CHECK_LIB([c], [memchr],\n>     [HAVE_MEMRCHR=YesPlease],\n>     [HAVE_MEMRCHR=])\n>     AC_SUBST(HAVE_MEMRCHR)\n> \n> And documentation with the other HAVE_* variables at the top of the\n> Makefile?\n\nHmm, the BSDs and plan 9 have an memrchr() apparently.   Any idea for\ntaking advantage of that (the makefile support part) that's less ugly\nthan this?\n\nIf we miss a platform, that's no big deal.  The 1-char-at-a-time\nloop is not so slow, and the \"#define memrchr gitmemrchr\" ensures\nthat it would not conflict with the libc version.\n---\ndiff --git a/Makefile b/Makefile\nindex 1f1ce04..fa91530 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -211,6 +211,8 @@ all::\n #\n # Define NO_REGEX if you have no or inferior regex support in your C library.\n #\n+# Define HAVE_MEMRCHR if you have memrchr() in your C library.\n+#\n # Define JSMIN to point to JavaScript minifier that functions as\n # a filter to have gitweb.js minified.\n #\n@@ -1388,6 +1390,9 @@ endif\n ifdef NO_UINTMAX_T\n \tBASIC_CFLAGS += -Duintmax_t=uint32_t\n endif\n+ifdef HAVE_MEMRCHR\n+\tBASIC_CFLAGS += -DHAVE_MEMRCHR\n+endif\n ifdef NO_SOCKADDR_STORAGE\n ifdef NO_IPV6\n \tBASIC_CFLAGS += -Dsockaddr_storage=sockaddr_in\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex 6f1020e..45aaebc 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -366,7 +366,7 @@ extern int git_vsnprintf(char *str, size_t maxsize,\n #define HAVE_STRCHRNUL\n #define HAVE_MEMPCPY\n #endif\n-#if __GLIBC_PREREQ(2, 2)\n+#if __GLIBC_PREREQ(2, 2) && !defined(HAVE_MEMRCHR)\n #define HAVE_MEMRCHR\n #endif\n #endif\n"},{"id":"153566","messageId":"4CB7FB53.8020802@viscovery.net","threadId":"25452","inReplyTo":"AANLkTinBuAhLLUMLd6ZWtVFWCjnFAxYwVSo1Pkja7bT4@mail.gmail.com","subject":"Re: [PATCH] compat: add memrchr()","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2010-10-15T06:57:23Z","receivedAt":"2010-10-15T06:57:23Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 10/15/2010 7:31, schrieb Ævar Arnfjörð Bjarmason:\n> Maybe something like this for configure.ac:\n> \n>     AC_CHECK_LIB([c], [memchr],\n\nDon't forget to change this to [memrchr] before you submit a patch.\n\n>     [HAVE_MEMRCHR=YesPlease],\n>     [HAVE_MEMRCHR=])\n>     AC_SUBST(HAVE_MEMRCHR)\n\n-- Hannes\n"},{"id":"153572","messageId":"loom.20101015T105350-205@post.gmane.org","threadId":"25452","inReplyTo":"20101015051750.GA21830@burratino","subject":"Re: [PATCH] compat: add memrchr()","fromName":"Ludvig Strigeus","fromEmail":"ludde@strigeus.com","sentAt":"2010-10-15T08:56:08Z","receivedAt":"2010-10-15T08:56:08Z","isPatch":true,"sender":{"key":"ludde@strigeus.com","avatar":null},"body":"Jonathan Nieder <jrnieder <at> gmail.com> writes:\n\n> \n> Reimplement another handy convenience function from glibc.  memrchr()\n> searches from the end of a memory area for a particular character.  It\n> is similar to strrchr() but takes a length argument and is\n> binary-safe.\n\nSystem memrchr uses a byte comparison rather than an int comparison.\n\n\"The memchr() function scans the first n bytes of the memory area pointed to by\ns for the character c. The first byte to match c (interpreted as an unsigned\ncharacter) stops the operation. \"\n\nSee: http://linux.die.net/man/3/memrchr\n\nIt's a good idea to do the same thing for compatibility reasons.\n\n/Ludde\n"},{"id":"153571","messageId":"AANLkTi=MbSeeUcPpPt_yLyeTv10pNzHqGUu3ihXf8jb+@mail.gmail.com","threadId":"25452","inReplyTo":"20101015051750.GA21830@burratino","subject":"Re: [PATCH] compat: add memrchr()","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2010-10-15T08:56:57Z","receivedAt":"2010-10-15T08:56:57Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Fri, Oct 15, 2010 at 7:17 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:\n> Reimplement another handy convenience function from glibc.  memrchr()\n> searches from the end of a memory area for a particular character.  It\n> is similar to strrchr() but takes a length argument and is\n> binary-safe.\n>\n> The whole-directory rename detection patch could use this to find the\n> last directory separator in a (possibly truncated) pathname.\n>\n\nAre you sure this would work on Windows where both '/' and '\\' are\nvalid directory separators?\n"},{"id":"153574","messageId":"4CB82079.4030402@viscovery.net","threadId":"25452","inReplyTo":"AANLkTi=MbSeeUcPpPt_yLyeTv10pNzHqGUu3ihXf8jb+@mail.gmail.com","subject":"Re: [PATCH] compat: add memrchr()","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2010-10-15T09:35:53Z","receivedAt":"2010-10-15T09:35:53Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 10/15/2010 10:56, schrieb Erik Faye-Lund:\n> On Fri, Oct 15, 2010 at 7:17 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:\n>> Reimplement another handy convenience function from glibc.  memrchr()\n>> searches from the end of a memory area for a particular character.  It\n>> is similar to strrchr() but takes a length argument and is\n>> binary-safe.\n>>\n>> The whole-directory rename detection patch could use this to find the\n>> last directory separator in a (possibly truncated) pathname.\n>>\n> \n> Are you sure this would work on Windows where both '/' and '\\' are\n> valid directory separators?\n\nI'm sure this would be used only on paths that were constructed from index\nor repository contents; there, the directory separator is '/' by definition.\n\n-- Hannes\n"},{"id":"153575","messageId":"AANLkTi=YN41n-BnVNo3HsnxNxQNBX=Ev-upmM0N49uOZ@mail.gmail.com","threadId":"25452","inReplyTo":"20101015060654.GD21830@burratino","subject":"Re: [PATCH] compat: add memrchr()","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-10-15T10:49:00Z","receivedAt":"2010-10-15T10:49:00Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Fri, Oct 15, 2010 at 06:06, Jonathan Nieder <jrnieder@gmail.com> wrote:\n> Ævar Arnfjörð Bjarmason wrote:\n>\n>> Maybe something like this for configure.ac:\n>>\n>>     AC_CHECK_LIB([c], [memchr],\n>>     [HAVE_MEMRCHR=YesPlease],\n>>     [HAVE_MEMRCHR=])\n>>     AC_SUBST(HAVE_MEMRCHR)\n>>\n>> And documentation with the other HAVE_* variables at the top of the\n>> Makefile?\n>\n> Hmm, the BSDs and plan 9 have an memrchr() apparently.   Any idea for\n> taking advantage of that (the makefile support part) that's less ugly\n> than this?\n\nCheck out my \"Makefile & configure: add a NO_FNMATCH flag\" and\n\"Makefile & configure: add a NO_FNMATCH_CASEFOLD flag\" patches already\nin pu.\n\nMaybe it should be a NO_* flag since that's what we use when we expect\nsanity, e.g. we have NO_REGEXP=, NO_CURL and NO_GETTEXT.\n\nThen you just need NO_MEMRCHR=UnfortunatelyYes entries in the big\nif/else block for those platforms that don't have it.\n\nBut memrchr() is a GNU extension so it should probably be a HAVE_*. I\ndon't know.\n\n> If we miss a platform, that's no big deal.  The 1-char-at-a-time\n> loop is not so slow, and the \"#define memrchr gitmemrchr\" ensures\n> that it would not conflict with the libc version.\n\nYup, probably best as a HAVE_*. In any case the autoconf probe I added\nshould work when memrchr is spelled properly.\n"},{"id":"153586","messageId":"20101015152608.GA9450@burratino","threadId":"25452","inReplyTo":"loom.20101015T105350-205@post.gmane.org","subject":"[PATCH v2] compat: add memrchr()","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-10-15T15:26:08Z","receivedAt":"2010-10-15T15:26:08Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Reimplement another handy convenience function from glibc.  memrchr()\nsearches from the end of a memory area for a particular character.  It\nis similar to strrchr() but takes a length argument and is\nbinary-safe.\n\nThe whole-directory rename detection patch could use this to find the\nlast '/' in a (possibly truncated) pathname.\n\nThe system memrchr() is used on glibc systems to provide a sanity\ncheck that our code works with a non-custom implementation.  Yes, the\nvarious BSDs have their own highly optimized memrchr(), too.  The\nplanned use of memrchr in git is for clarity, not speed, so it is not\nobvious that the makefile+autoconf magic to use libc's implementation\non a wide variety of operating systems would be worth the time.\n\nSigned-off-by: Jonathan Nieder <jrnieder@gmail.com>\n---\nLudvig Strigeus wrote:\n\n> System memrchr uses a byte comparison rather than an int comparison.\n> \n> \"The memchr() function scans the first n bytes of the memory area pointed to by\n> s for the character c. The first byte to match c (interpreted as an unsigned\n> character) stops the operation. \"\n\nOh, right.  (strchr() uses char, memchr() uses unsigned char.)\n\n git-compat-util.h |   16 ++++++++++++++++\n 1 files changed, 16 insertions(+), 0 deletions(-)\n\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex 2af8d3e..6de9dee 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -366,6 +366,9 @@ extern int git_vsnprintf(char *str, size_t maxsize,\n #define HAVE_STRCHRNUL\n #define HAVE_MEMPCPY\n #endif\n+#if __GLIBC_PREREQ(2, 2)\n+#define HAVE_MEMRCHR\n+#endif\n #endif\n \n #ifndef HAVE_STRCHRNUL\n@@ -386,6 +389,19 @@ static inline void *gitmempcpy(void *dest, const void *src, size_t n)\n }\n #endif\n \n+#ifndef HAVE_MEMRCHR\n+#define memrchr gitmemrchr\n+static inline void *gitmemrchr(const void *s, int c, size_t n)\n+{\n+\tconst unsigned char *p = s;\n+\tp += n;\n+\twhile (p != s)\n+\t\tif (*--p == (unsigned char) c)\n+\t\t\treturn p;\n+\treturn NULL;\n+}\n+#endif\n+\n extern void release_pack_memory(size_t, int);\n \n typedef void (*try_to_free_t)(size_t);\n-- \n1.7.2.3\n"},{"id":"153604","messageId":"7vy69zay7b.fsf@alter.siamese.dyndns.org","threadId":"25452","inReplyTo":"AANLkTi=YN41n-BnVNo3HsnxNxQNBX=Ev-upmM0N49uOZ@mail.gmail.com","subject":"Re: [PATCH] compat: add memrchr()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-10-15T22:27:36Z","receivedAt":"2010-10-15T22:27:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:\n\n> Maybe it should be a NO_* flag since that's what we use when we expect\n> sanity, e.g. we have NO_REGEXP=, NO_CURL and NO_GETTEXT.\n>\n> Then you just need NO_MEMRCHR=UnfortunatelyYes entries in the big\n> if/else block for those platforms that don't have it.\n\nHmmm, perhaps our naming convention have deteriorated over time.\n\nI think one of the oldest one is NO_CURL, which started out as a way to\nsay \"I do not have to go over http and dumb http transport is of no use to\nme.  I _might_ have curl library installed on this system, but it does not\nmatter.  No thanks, please do not build git with curl support\".  For some\npeople, it also meant \"Unfortunately I do not have a working curl library\nhere, so I'll live with a git without http support\".\n\nOn the other hand, NO_REGEX means \"regrettably the platform implementation\nis not good enough for our codebase, and we do need to use a replacement\nimplementation from compat/\".\n\nThe difference is that it is not an option to build a less capable git\nthat lacks features that depend on REGEX.  But somehow we ended up using\nthe same NO_ prefix, conflating the two.  In that sense NO_GETTEXT is\nnamed correctly, I think, as we fall back on implementation (i.e. less\ncapable git) that does not do i18n/l10n.\n\n> But memrchr() is a GNU extension so it should probably be a HAVE_*. I\n> don't know.\n\nYes, if we want to express that we positively rely on the existence of an\nextension, HAVE_* does sound more correct.  It also is how the world with\nautoconf/configure works.\n\nWe might want to clean up the names of variables at some point, but from a\nquick glance things do not look very good.\n\nMost NO_FOO are misnamed and should be !HAVE_FOO, but because our codebase\ntend to use more common features, switching from the way we currently do,\ni.e. assuming we have FOO and defining NO_FOO for oddball platforms, to\nthe other way around of defining HAVE_FOO for platforms where we do have FOO\nand omit it for the ones that lack, may make the Makefile more cluttered.\n\nBut as I already said, switching to HAVE_FOO has one definite advantage;\nit is more conventional and would mesh a lot better with the way\nautoconf/configure would do things.\n\nHere is a quick break-down of the current set I found from the 'master'\nbranch.\n\n* You decline to build, or are unable to build, git with...\n\n  NO_CURL\t\ttransport based on libcurl e.g. HTTP.\n  NO_ICONV\t\tcharset re-encoding support.\n  NO_IPV6\t\tIPV6\n  NO_NSEC\t\tnanosecond resolution file timestamps\n  NO_POSIX_ONLY_PROGRAMS\n  NO_PYTHON\t\tanything that depends on Python\n  NO_SYMLINK_HEAD\tsupport for original layout where .git/HEAD was a symlink\n  NO_SVN_TESTS\t\ttest that spends too much time on git-svn\n\n\n* We work around the lack of the platform feature FOO without loss of\n  features from git (i.e. should be !HAVE_FOO):\n\n  NO_C99_FORMAT\t\tSZ_FMT and friends\n  NO_D_INO_IN_DIRENT\t(struct dirent)d.d_ino\n  NO_D_TYPE_IN_DIRENT\t(struct dirent)d.d_type\n  NO_HSTRERROR\n  NO_INET_NTOP\n  NO_INET_PTON\n  NO_LIBGEN_H\n  NO_MEMMEM\n  NO_MKDTEMP\n  NO_MKSTEMPS\n  NO_MMAP\n  HAVE_PATHS_H\t\t#include <paths.h>\n  NO_PERL_MAKEMAKER\n  NO_PREAD\n  NO_REGEX\n  NO_R_TO_GCC_LINKER\n  NO_SETENV\n  NO_SOCKADDR_STORAGE\n  NO_ST_BLOCKS_IN_STRUCT_STAT\n  NO_STRCASESTR\n  NO_STRLCPY\n  NO_STRTOK_R\n  NO_STRTOULL\n  NO_STRTOUMAX\n  NO_SYS_SELECT_H\n  NO_TRUSTABLE_FILEMODE\n  NO_UINTMAX_T\n  NO_UNSETENV\n\nUnfortunately the world is not so black-and-white.  There is another class\n\n* Depending on platform features, we use different implementation with\n  almost no observable difference in behaviour (perhaps except for\n  performance)\n\n  NO_PTHREADS\n\tUse of threads for better machine utilization\n\n  NO_FAST_WORKING_DIRECTORY\n\tReading from object database is faster than opening a file in the\n\tworking directory and reading from it on this platform, so we do\n\tthe former when we know the file in the working directory is clean.\n        Perhaps !HAVE_FAST_WORKING_DIRECTORY.\n\nAnd yet another that is completely unrelated:\n\n  NO_SUBDIR\n"},{"id":"153675","messageId":"1287343443-23098-1-git-send-email-ydirson@altern.org","threadId":"25452","inReplyTo":"1287098999-9244-5-git-send-email-ydirson@altern.org","subject":"[PATCH v6.1] [RFC] Consider all parents of a file as candidates for bulk rename.","fromName":"Yann Dirson","fromEmail":"ydirson@altern.org","sentAt":"2010-10-17T19:24:03Z","receivedAt":"2010-10-17T19:24:03Z","isPatch":true,"sender":{"key":"ydirson@altern.org","avatar":"https://avatars.githubusercontent.com/u/1190950?v=4"},"body":"We previously only looked at the immediate parents of a file involved\nin a move.  Now consider upper ones as candidates too.\n\nThe way it is done here is somewhat inefficient, but at least it\nhandles all previously-known cases of incorrectness.\n\nSigned-off-by: Yann Dirson <ydirson@free.fr>\n---\n diffcore-rename.c |  120 ++++++++++++++++++++++++++++-------------------------\n 1 files changed, 63 insertions(+), 57 deletions(-)\n\nThere was a major flaw in the first iteration of this patch (never\nlooking again at discarded renames).  This iteration also adds more\nexplicit comments about why this piece of code should work, despite\nhaving been written late at night and having already shown one problem\nwhich demonstrated that a testsuite is no replacement for a good night\nof sleep :)\n\nI'm still not happy with the thing, though - the loops can surely be\nmade shorter by using the discarded flags previously set, and more\nimportantly we need to consider moves to parents of the file's\ndestination too, which will add a loop of its own which will have to\nbe made as short as possible.  All of this seems to require to sort\nbulkmove_candidates so we can binary-search into it and rapidly access\nrelated paths.\n\nOther opinions on the spirit of this patch still gladly welcome :)\n\ndiff --git a/diffcore-rename.c b/diffcore-rename.c\nindex 1be1af1..a8f3a8b 100644\n--- a/diffcore-rename.c\n+++ b/diffcore-rename.c\n@@ -542,7 +542,7 @@ static int maybe_disqualify_bulkmove(const char* one_parent_path,\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\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@@ -596,66 +596,72 @@ 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-\tfprintf(stderr, \"[] %s -> %s ?\\n\",dstpair->one->path, dstpair->two->path);\n-\n-\t// FIXME: loop over successive prefixes\n-\tunsigned needs_adding = 1;\n+\tfprintf(stderr, \"[] %s -> %s ?\\n\", dstpair->one->path, dstpair->two->path);\n \n-\t/* already considered ? */\n-\tfor (seen=bulkmove_candidates; seen; seen = seen->next) {\n-\t\tif (seen->discarded) {\n-\t\t\t/* already seen a rename from seen->one to some than ->two */\n-\t\t\tneeds_adding = 0;\n-\t\t\tcontinue;\n-\t\t}\n-\t\t/* check exact dir */\n-\t\tif (!strcmp(seen->one->path, one_parent_path)) {\n-\t\t\t/* already added */\n-\t\t\tneeds_adding = 0;\n-\t\t\t/* check that seen entry matches this rename */\n-\t\t\tif (strcmp(two_parent_path, seen->two->path)) {\n-\t\t\t\tfprintf(stderr, \"[DBG] discarding dir %s from bulk moves (split into %s and %s)\\n\",\n-\t\t\t\t\tseen->one->path, two_parent_path, seen->two->path);\n-\t\t\t\t// FIXME: may be worth to free it instead\n-\t\t\t\tseen->discarded = 1;\n+\t/* loop up one_parent_path over successive parents */\n+\t// FIXME: also loop over two_parent_path prefixes ?\n+\t// FIXME: use a more informative prefixcmp to avoid strcmp calls\n+\tdo {\n+\t\tunsigned needs_adding = 1;\n+\n+\t\tfprintf(stderr, \"[[]] %s ...\\n\", one_parent_path);\n+\n+\t\t/* already considered ? */\n+\t\tfor (seen=bulkmove_candidates; seen; seen = seen->next) {\n+\t\t\tfprintf(stderr, \"[DBG]  ? %s -> %s\\n\", seen->one->path, seen->two->path);\n+\t\t\t/* subdir of \"seen\" source dir ? */\n+\t\t\tif (!prefixcmp(one_parent_path, seen->one->path)) {\n+\t\t\t\t/* subdir of \"seen\" dest dir ? */\n+\t\t\t\tif (!prefixcmp(two_parent_path, seen->two->path)) {\n+\t\t\t\t\tif (!strcmp(seen->one->path, one_parent_path) &&\n+\t\t\t\t\t    !strcmp(seen->two->path, two_parent_path)) {\n+\t\t\t\t\t\t/* already added */\n+\t\t\t\t\t\tfprintf(stderr, \"[DBG]  already added\\n\");\n+\t\t\t\t\t\tneeds_adding = 0;\n+\t\t\t\t\t} else /* is in a subdir: confirms but still\n+\t\t\t\t\t\t* may need adding */\n+\t\t\t\t\t\tfprintf(stderr, \"[DBG]  'dstpair' conforts 'seen'\\n\");\n+\t\t\t\t} else {\n+\t\t\t\t\tfprintf(stderr, \"[DBG] discarding %s -> %s from bulk moves (split into %s and %s)\\n\",\n+\t\t\t\t\t\tseen->one->path, seen->two->path,\n+\t\t\t\t\t\ttwo_parent_path, seen->two->path);\n+\t\t\t\t\tseen->discarded = 1;\n+\t\t\t\t\t/* Need to discard dstpair as well, unless moving\n+\t\t\t\t\t * from a strict subdir of seen->one or to a\n+\t\t\t\t\t * strict subdir of seen->two */\n+\t\t\t\t\tif (!strcmp(seen->one->path, one_parent_path) &&\n+\t\t\t\t\t    prefixcmp(seen->two->path, two_parent_path)) {\n+\t\t\t\t\t\tfprintf(stderr, \"[DBG] ... and not adding self\\n\");\n+\t\t\t\t\t\tneeds_adding = 0;\n+\t\t\t\t\t}\n+\t\t\t\t\tcontinue;\n+\t\t\t\t}\n \t\t\t}\n-\t\t\tcontinue;\n+\t\t\telse fprintf(stderr, \"[DBG]  %s outside of %s\\n\",\n+\t\t\t\t     one_parent_path, seen->one->path);\n+\n+\t\t\t/* dstpair confirms seen, or does not infirm */\n+\t\t\tfprintf(stderr, \"[DBG] %s -> %s DOES NOT cause discard of %s -> %s\\n\",\n+\t\t\t\tdstpair->one->path, dstpair->two->path,\n+\t\t\t\tseen->one->path, seen->two->path);\n \t\t}\n-\t\tif (!prefixcmp(one_parent_path, seen->one->path)) {\n-\t\t\tif (prefixcmp(two_parent_path, seen->two->path)) {\n-\t\t\t\tfprintf(stderr, \"[DBG] discarding dir %s from bulk moves (split into %s and %s)\\n\",\n-\t\t\t\t\tseen->one->path, two_parent_path, seen->two->path);\n-\t\t\t\t// FIXME: may be worth to free it instead\n-\t\t\t\tseen->discarded = 1;\n-\t\t\t\tcontinue;\n-\t\t\t}\n-\t\t} else {\n-\t\t\tfprintf(stderr, \"[DBG]  %s considered irrelevant for %s -> %s\\n\",\n-\t\t\t\tdstpair->one->path, seen->one->path, seen->two->path);\n-\t\t\tcontinue;\n+\t\tif (needs_adding) { /* record potential dir rename */\n+\t\t\tseen = xmalloc(sizeof(*seen));\n+\t\t\tseen->one = alloc_filespec(one_parent_path);\n+\t\t\tfill_filespec(seen->one, null_sha1, S_IFDIR);\n+\t\t\tseen->two = alloc_filespec(two_parent_path);\n+\t\t\tfill_filespec(seen->two, null_sha1, S_IFDIR);\n+\t\t\tseen->discarded = 0;\n+\t\t\tseen->next = bulkmove_candidates;\n+\t\t\tbulkmove_candidates = seen;\n+\t\t\tfprintf(stderr, \"[DBG] %s -> %s suggests possible bulk move from %s to %s\\n\",\n+\t\t\t\tdstpair->one->path,\n+\t\t\t\tdstpair->two->path,\n+\t\t\t\tone_parent_path, two_parent_path);\n \t\t}\n-\n-\t\t/* dstpair confirms seen */\n-\t\tfprintf(stderr, \"[DBG] %s -> %s DOES NOT cause discard of %s -> %s\\n\",\n-\t\t\tdstpair->one->path, dstpair->two->path,\n-\t\t\tseen->one->path, seen->two->path);\n-\t}\n-\tif (needs_adding) { /* record potential dir rename */\n-\t\t/* all checks ok, we keep that entry */\n-\t\tseen = xmalloc(sizeof(*seen));\n-\t\tseen->one = alloc_filespec(one_parent_path);\n-\t\tfill_filespec(seen->one, null_sha1, S_IFDIR);\n-\t\tseen->two = alloc_filespec(two_parent_path);\n-\t\tfill_filespec(seen->two, null_sha1, 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 bulk move 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+\t\t/* next parent if any */\n+\t\tcopy_dirname(one_parent_path, one_parent_path);\n+\t} while (*one_parent_path);\n }\n \n /*\n-- \n1.7.2.3\n"}]}