{"thread":{"id":"39756","subject":"[PATCH v2 00/13] \"rerere\" minor clean-up","startedAt":"2015-07-01T06:04:46Z","lastAt":"2015-07-24T20:42:38Z","messageCount":39,"participants":["Junio C Hamano","Philip Oakley","Eric Sunshine"],"isPatch":true,"patchVersion":2,"patchTotal":13},"messages":[{"id":"265285","messageId":"1435730699-9124-1-git-send-email-gitster@pobox.com","threadId":"39756","inReplyTo":null,"subject":"[PATCH v2 00/13] \"rerere\" minor clean-up","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-07-01T06:04:46Z","receivedAt":"2015-07-01T06:04:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Here is an collection of various minor clean-ups in the\nimplementation of one of my most favourite feature, rerere.\n\nThis still hasn't reached the step to make the right refactoring to\nallow me to fix a bug I wanted to fix, which prompted me to look at\nthis code, but it should give me a good starting point.\n\nJunio C Hamano (13):\n  rerere: fix an off-by-one non-bug\n  rerere: plug conflict ID leaks\n  rerere: lift PATH_MAX limitation\n  rerere: write out each record of MERGE_RR in one go\n  rerere: report autoupdated paths only after actually updating them\n  rerere: drop want_sp parameter from is_cmarker()\n  rerere: stop looping unnecessarily\n  rerere: explain the rerere I/O abstraction\n  rerere: explain MERGE_RR management helpers\n  rerere: explain the primary codepath\n  rerere: explain \"rerere forget\" codepath\n  rerere: explain the remainder\n  rerere: refactor \"replay\" part of do_plain_rerere()\n\n rerere.c | 389 +++++++++++++++++++++++++++++++++++++++++++++++----------------\n 1 file changed, 293 insertions(+), 96 deletions(-)\n\n-- \n2.5.0-rc0-209-g5e1f148\n"},{"id":"265288","messageId":"1435730699-9124-2-git-send-email-gitster@pobox.com","threadId":"39756","inReplyTo":"1435730699-9124-1-git-send-email-gitster@pobox.com","subject":"[PATCH v2 01/13] rerere: fix an off-by-one non-bug","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-07-01T06:04:47Z","receivedAt":"2015-07-01T06:04:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"When ac49f5ca (rerere \"remaining\", 2011-02-16) split out a new\nhelper function check_one_conflict() out of find_conflict()\nfunction, so that the latter will use the returned value from the\nnew helper to update the loop control variable that is an index into\nactive_cache[], the new variable incremented the index by one too\nmany when it found a path with only stage #1 entry at the very end\nof active_cache[].\n\nThis \"strange\" return value does not have any effect on the loop\ncontrol of two callers of this function, as they all notice that\nactive_nr+2 is larger than active_nr just like active_nr+1 is, but\nnevertheless it puzzles the readers when they are trying to figure\nout what the function is trying to do.\n\nIn fact, there is no need to do an early return.  The code that\nfollows after skipping the stage #1 entry is fully prepared to\nhandle a case where the entry is at the very end of active_cache[].\n\nHelp future readers from unnecessary confusion by dropping an early\nreturn.  We skip the stage #1 entry, and if there are stage #2 and\nstage #3 entries for the same path, we diagnose the path as\nTHREE_STAGED (otherwise we say PUNTED), and then we skip all entries\nfor the same path.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n rerere.c | 6 ++----\n 1 file changed, 2 insertions(+), 4 deletions(-)\n\ndiff --git a/rerere.c b/rerere.c\nindex 31644de..e307711 100644\n--- a/rerere.c\n+++ b/rerere.c\n@@ -369,10 +369,8 @@ static int check_one_conflict(int i, int *type)\n \t}\n \n \t*type = PUNTED;\n-\tif (ce_stage(e) == 1) {\n-\t\tif (active_nr <= ++i)\n-\t\t\treturn i + 1;\n-\t}\n+\tif (ce_stage(e) == 1)\n+\t\ti++;\n \n \t/* Only handle regular files with both stages #2 and #3 */\n \tif (i + 1 < active_nr) {\n-- \n2.5.0-rc0-209-g5e1f148\n"},{"id":"265287","messageId":"1435730699-9124-3-git-send-email-gitster@pobox.com","threadId":"39756","inReplyTo":"1435730699-9124-1-git-send-email-gitster@pobox.com","subject":"[PATCH v2 02/13] rerere: plug conflict ID leaks","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-07-01T06:04:48Z","receivedAt":"2015-07-01T06:04:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The merge_rr string list stores the conflict ID (a hexadecimal\nstring that is used to index into $GIT_DIR/rr-cache) in the .util\nfield of its elements, and when do_plain_rerere() resolves a\nconflict, the field is cleared.  Also, when rerere_forget()\nrecomputes the conflict ID to updates the preimage file, the\nconflict ID for the path is updated.\n\nWe forgot to free the existing conflict ID when we did these two\noperations.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n rerere.c | 7 +++++--\n 1 file changed, 5 insertions(+), 2 deletions(-)\n\ndiff --git a/rerere.c b/rerere.c\nindex e307711..3b9104d 100644\n--- a/rerere.c\n+++ b/rerere.c\n@@ -559,6 +559,7 @@ static int do_plain_rerere(struct string_list *rr, int fd)\n \t\tfprintf(stderr, \"Recorded resolution for '%s'.\\n\", path);\n \t\tcopy_file(rerere_path(name, \"postimage\"), path, 0666);\n \tmark_resolved:\n+\t\tfree(rr->items[i].util);\n \t\trr->items[i].util = NULL;\n \t}\n \n@@ -627,6 +628,7 @@ static int rerere_forget_one_path(const char *path, struct string_list *rr)\n \tchar *hex;\n \tunsigned char sha1[20];\n \tint ret;\n+\tstruct string_list_item *item;\n \n \tret = handle_cache(path, sha1, NULL);\n \tif (ret < 1)\n@@ -641,8 +643,9 @@ static int rerere_forget_one_path(const char *path, struct string_list *rr)\n \thandle_cache(path, sha1, rerere_path(hex, \"preimage\"));\n \tfprintf(stderr, \"Updated preimage for '%s'\\n\", path);\n \n-\n-\tstring_list_insert(rr, path)->util = hex;\n+\titem = string_list_insert(rr, path);\n+\tfree(item->util);\n+\titem->util = hex;\n \tfprintf(stderr, \"Forgot resolution for %s\\n\", path);\n \treturn 0;\n }\n-- \n2.5.0-rc0-209-g5e1f148\n"},{"id":"265286","messageId":"1435730699-9124-4-git-send-email-gitster@pobox.com","threadId":"39756","inReplyTo":"1435730699-9124-1-git-send-email-gitster@pobox.com","subject":"[PATCH v2 03/13] rerere: lift PATH_MAX limitation","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-07-01T06:04:49Z","receivedAt":"2015-07-01T06:04:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The MERGE_RR file records a collection of NUL-terminated entries,\neach of which consists of\n\n - a hash that identifies the conflict\n - a HT\n - the pathname\n\nWe used to read this piece-by-piece, and worse yet, read the\npathname part a byte at a time into a fixed buffer of size PATH_MAX.\n\nInstead, read a whole entry using strbuf_getwholeline() and parse\nout the fields.  This way, we issue fewer read(2) calls and more\nimportantly we do not have to limit the pathname to PATH_MAX.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n rerere.c | 35 +++++++++++++++--------------------\n 1 file changed, 15 insertions(+), 20 deletions(-)\n\ndiff --git a/rerere.c b/rerere.c\nindex 3b9104d..e854985 100644\n--- a/rerere.c\n+++ b/rerere.c\n@@ -35,32 +35,27 @@ static int has_rerere_resolution(const char *hex)\n \n static void read_rr(struct string_list *rr)\n {\n-\tunsigned char sha1[20];\n-\tchar buf[PATH_MAX];\n+\tstruct strbuf buf = STRBUF_INIT;\n \tFILE *in = fopen(merge_rr_path, \"r\");\n+\n \tif (!in)\n \t\treturn;\n-\twhile (fread(buf, 40, 1, in) == 1) {\n-\t\tint i;\n-\t\tchar *name;\n-\t\tif (get_sha1_hex(buf, sha1))\n+\twhile (!strbuf_getwholeline(&buf, in, '\\0')) {\n+\t\tchar *path;\n+\t\tunsigned char sha1[20];\n+\n+\t\t/* There has to be the hash, tab, path and then NUL */\n+\t\tif (buf.len < 42 || get_sha1_hex(buf.buf, sha1))\n \t\t\tdie(\"corrupt MERGE_RR\");\n-\t\tbuf[40] = '\\0';\n-\t\tname = xstrdup(buf);\n-\t\tif (fgetc(in) != '\\t')\n+\n+\t\tif (buf.buf[40] != '\\t')\n \t\t\tdie(\"corrupt MERGE_RR\");\n-\t\tfor (i = 0; i < sizeof(buf); i++) {\n-\t\t\tint c = fgetc(in);\n-\t\t\tif (c < 0)\n-\t\t\t\tdie(\"corrupt MERGE_RR\");\n-\t\t\tbuf[i] = c;\n-\t\t\tif (c == 0)\n-\t\t\t\t break;\n-\t\t}\n-\t\tif (i == sizeof(buf))\n-\t\t\tdie(\"filename too long\");\n-\t\tstring_list_insert(rr, buf)->util = name;\n+\t\tbuf.buf[40] = '\\0';\n+\t\tpath = buf.buf + 41;\n+\n+\t\tstring_list_insert(rr, path)->util = xstrdup(buf.buf);\n \t}\n+\tstrbuf_release(&buf);\n \tfclose(in);\n }\n \n-- \n2.5.0-rc0-209-g5e1f148\n"},{"id":"265290","messageId":"1435730699-9124-5-git-send-email-gitster@pobox.com","threadId":"39756","inReplyTo":"1435730699-9124-1-git-send-email-gitster@pobox.com","subject":"[PATCH v2 04/13] rerere: write out each record of MERGE_RR in one go","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-07-01T06:04:50Z","receivedAt":"2015-07-01T06:04:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Instead of writing the hash for a conflict, a HT, and the path\nwith three separate write_in_full() calls, format them into a\nsingle record into a strbuf and write it out in one go.\n\nAs a more recent \"rerere remaining\" codepath abuses the .util field\nof the merge_rr data to store a sentinel token, make sure that\ncodepath does not call into this function (of course, \"remaining\" is\na read-only operation and currently does not call it).\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n rerere.c | 16 +++++++++-------\n 1 file changed, 9 insertions(+), 7 deletions(-)\n\ndiff --git a/rerere.c b/rerere.c\nindex e854985..27b287d 100644\n--- a/rerere.c\n+++ b/rerere.c\n@@ -65,16 +65,18 @@ static int write_rr(struct string_list *rr, int out_fd)\n {\n \tint i;\n \tfor (i = 0; i < rr->nr; i++) {\n-\t\tconst char *path;\n-\t\tint length;\n+\t\tstruct strbuf buf = STRBUF_INIT;\n+\n+\t\tassert(rr->items[i].util != RERERE_RESOLVED);\n \t\tif (!rr->items[i].util)\n \t\t\tcontinue;\n-\t\tpath = rr->items[i].string;\n-\t\tlength = strlen(path) + 1;\n-\t\tif (write_in_full(out_fd, rr->items[i].util, 40) != 40 ||\n-\t\t    write_str_in_full(out_fd, \"\\t\") != 1 ||\n-\t\t    write_in_full(out_fd, path, length) != length)\n+\t\tstrbuf_addf(&buf, \"%s\\t%s%c\",\n+\t\t\t    (char *)rr->items[i].util,\n+\t\t\t    rr->items[i].string, 0);\n+\t\tif (write_in_full(out_fd, buf.buf, buf.len) != buf.len)\n \t\t\tdie(\"unable to write rerere record\");\n+\n+\t\tstrbuf_release(&buf);\n \t}\n \tif (commit_lock_file(&write_lock) != 0)\n \t\tdie(\"unable to write rerere record\");\n-- \n2.5.0-rc0-209-g5e1f148\n"},{"id":"265289","messageId":"1435730699-9124-6-git-send-email-gitster@pobox.com","threadId":"39756","inReplyTo":"1435730699-9124-1-git-send-email-gitster@pobox.com","subject":"[PATCH v2 05/13] rerere: report autoupdated paths only after actually updating them","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-07-01T06:04:51Z","receivedAt":"2015-07-01T06:04:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Signed-off-by: Junio C Hamano <gitster@pobox.com>\n---\n rerere.c | 22 ++++++++++++----------\n 1 file changed, 12 insertions(+), 10 deletions(-)\n\ndiff --git a/rerere.c b/rerere.c\nindex 27b287d..304df02 100644\n--- a/rerere.c\n+++ b/rerere.c\n@@ -482,6 +482,8 @@ static void update_paths(struct string_list *update)\n \t\tstruct string_list_item *item = &update->items[i];\n \t\tif (add_file_to_cache(item->string, 0))\n \t\t\texit(128);\n+\t\tfprintf(stderr, \"Staged '%s' using previous resolution.\\n\",\n+\t\t\titem->string);\n \t}\n \n \tif (active_cache_changed) {\n@@ -536,16 +538,16 @@ static int do_plain_rerere(struct string_list *rr, int fd)\n \t\tconst char *name = (const char *)rr->items[i].util;\n \n \t\tif (has_rerere_resolution(name)) {\n-\t\t\tif (!merge(name, path)) {\n-\t\t\t\tconst char *msg;\n-\t\t\t\tif (rerere_autoupdate) {\n-\t\t\t\t\tstring_list_insert(&update, path);\n-\t\t\t\t\tmsg = \"Staged '%s' using previous resolution.\\n\";\n-\t\t\t\t} else\n-\t\t\t\t\tmsg = \"Resolved '%s' using previous resolution.\\n\";\n-\t\t\t\tfprintf(stderr, msg, path);\n-\t\t\t\tgoto mark_resolved;\n-\t\t\t}\n+\t\t\tif (merge(name, path))\n+\t\t\t\tcontinue;\n+\n+\t\t\tif (rerere_autoupdate)\n+\t\t\t\tstring_list_insert(&update, path);\n+\t\t\telse\n+\t\t\t\tfprintf(stderr,\n+\t\t\t\t\t\"Resolved '%s' using previous resolution.\\n\",\n+\t\t\t\t\tpath);\n+\t\t\tgoto mark_resolved;\n \t\t}\n \n \t\t/* Let's see if we have resolved it. */\n-- \n2.5.0-rc0-209-g5e1f148\n"},{"id":"265291","messageId":"1435730699-9124-7-git-send-email-gitster@pobox.com","threadId":"39756","inReplyTo":"1435730699-9124-1-git-send-email-gitster@pobox.com","subject":"[PATCH v2 06/13] rerere: drop want_sp parameter from is_cmarker()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-07-01T06:04:52Z","receivedAt":"2015-07-01T06:04:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"As the nature of the conflict marker line determies if there should\na SP and label after it, the caller shouldn't have to pass the\nparameter redundantly.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n rerere.c | 27 ++++++++++++++++++++++-----\n 1 file changed, 22 insertions(+), 5 deletions(-)\n\ndiff --git a/rerere.c b/rerere.c\nindex 304df02..4c45f55 100644\n--- a/rerere.c\n+++ b/rerere.c\n@@ -148,8 +148,25 @@ static int rerere_file_getline(struct strbuf *sb, struct rerere_io *io_)\n \treturn strbuf_getwholeline(sb, io->input, '\\n');\n }\n \n-static int is_cmarker(char *buf, int marker_char, int marker_size, int want_sp)\n+/*\n+ * Require the exact number of conflict marker letters, no more, no\n+ * less, followed by SP or any whitespace\n+ * (including LF).\n+ */\n+static int is_cmarker(char *buf, int marker_char, int marker_size)\n {\n+\tint want_sp;\n+\n+\t/*\n+\t * The beginning of our version and the end of their version\n+\t * always are labeled like \"<<<<< ours\" or \">>>>> theirs\",\n+\t * hence we set want_sp for them.  Note that the version from\n+\t * the common ancestor in diff3-style output is not always\n+\t * labelled (e.g. \"||||| common\" is often seen but \"|||||\"\n+\t * alone is also valid), so we do not set want_sp.\n+\t */\n+\twant_sp = (marker_char == '<') || (marker_char == '>');\n+\n \twhile (marker_size--)\n \t\tif (*buf++ != marker_char)\n \t\t\treturn 0;\n@@ -172,19 +189,19 @@ static int handle_path(unsigned char *sha1, struct rerere_io *io, int marker_siz\n \t\tgit_SHA1_Init(&ctx);\n \n \twhile (!io->getline(&buf, io)) {\n-\t\tif (is_cmarker(buf.buf, '<', marker_size, 1)) {\n+\t\tif (is_cmarker(buf.buf, '<', marker_size)) {\n \t\t\tif (hunk != RR_CONTEXT)\n \t\t\t\tgoto bad;\n \t\t\thunk = RR_SIDE_1;\n-\t\t} else if (is_cmarker(buf.buf, '|', marker_size, 0)) {\n+\t\t} else if (is_cmarker(buf.buf, '|', marker_size)) {\n \t\t\tif (hunk != RR_SIDE_1)\n \t\t\t\tgoto bad;\n \t\t\thunk = RR_ORIGINAL;\n-\t\t} else if (is_cmarker(buf.buf, '=', marker_size, 0)) {\n+\t\t} else if (is_cmarker(buf.buf, '=', marker_size)) {\n \t\t\tif (hunk != RR_SIDE_1 && hunk != RR_ORIGINAL)\n \t\t\t\tgoto bad;\n \t\t\thunk = RR_SIDE_2;\n-\t\t} else if (is_cmarker(buf.buf, '>', marker_size, 1)) {\n+\t\t} else if (is_cmarker(buf.buf, '>', marker_size)) {\n \t\t\tif (hunk != RR_SIDE_2)\n \t\t\t\tgoto bad;\n \t\t\tif (strbuf_cmp(&one, &two) > 0)\n-- \n2.5.0-rc0-209-g5e1f148\n"},{"id":"265292","messageId":"1435730699-9124-8-git-send-email-gitster@pobox.com","threadId":"39756","inReplyTo":"1435730699-9124-1-git-send-email-gitster@pobox.com","subject":"[PATCH v2 07/13] rerere: stop looping unnecessarily","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-07-01T06:04:53Z","receivedAt":"2015-07-01T06:04:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"handle_cache() loops 3 times starting from an index entry that is\nunmerged, while ignoring an entry for a path that is different from\nwhat we are looking for.\n\nAs the index is sorted, once we see a different path, we know we saw\nall stages for the path we are interested in.  Just loop while we\nsee the same path and then break, instead of continuing for 3 times.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n rerere.c | 17 +++++++----------\n 1 file changed, 7 insertions(+), 10 deletions(-)\n\ndiff --git a/rerere.c b/rerere.c\nindex 4c45f55..7b1419c 100644\n--- a/rerere.c\n+++ b/rerere.c\n@@ -329,24 +329,21 @@ static int handle_cache(const char *path, unsigned char *sha1, const char *outpu\n \t\treturn -1;\n \tpos = -pos - 1;\n \n-\tfor (i = 0; i < 3; i++) {\n+\twhile (pos < active_nr) {\n \t\tenum object_type type;\n \t\tunsigned long size;\n-\t\tint j;\n \n-\t\tif (active_nr <= pos)\n-\t\t\tbreak;\n \t\tce = active_cache[pos++];\n \t\tif (ce_namelen(ce) != len || memcmp(ce->name, path, len))\n-\t\t\tcontinue;\n-\t\tj = ce_stage(ce) - 1;\n-\t\tmmfile[j].ptr = read_sha1_file(ce->sha1, &type, &size);\n-\t\tmmfile[j].size = size;\n+\t\t\tbreak;\n+\t\ti = ce_stage(ce) - 1;\n+\t\tmmfile[i].ptr = read_sha1_file(ce->sha1, &type, &size);\n+\t\tmmfile[i].size = size;\n \t}\n-\tfor (i = 0; i < 3; i++) {\n+\tfor (i = 0; i < 3; i++)\n \t\tif (!mmfile[i].ptr && !mmfile[i].size)\n \t\t\tmmfile[i].ptr = xstrdup(\"\");\n-\t}\n+\n \t/*\n \t * NEEDSWORK: handle conflicts from merges with\n \t * merge.renormalize set, too\n-- \n2.5.0-rc0-209-g5e1f148\n"},{"id":"265293","messageId":"1435730699-9124-9-git-send-email-gitster@pobox.com","threadId":"39756","inReplyTo":"1435730699-9124-1-git-send-email-gitster@pobox.com","subject":"[PATCH v2 08/13] rerere: explain the rerere I/O abstraction","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-07-01T06:04:54Z","receivedAt":"2015-07-01T06:04:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Explain the internals of rerere as in-code comments.\n\nThis one covers our thin I/O abstraction to read from either\na file or a memory while optionally writing out to a file.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n rerere.c | 38 +++++++++++++++++++++++++++++++-------\n 1 file changed, 31 insertions(+), 7 deletions(-)\n\ndiff --git a/rerere.c b/rerere.c\nindex 7b1419c..7ed20f1 100644\n--- a/rerere.c\n+++ b/rerere.c\n@@ -83,6 +83,21 @@ static int write_rr(struct string_list *rr, int out_fd)\n \treturn 0;\n }\n \n+/*\n+ * \"rerere\" interacts with conflicted file contents using this I/O\n+ * abstraction.  It reads a conflicted contents from one place via\n+ * \"getline()\" method, and optionally can write it out after\n+ * normalizing the conflicted hunks to the \"output\".  Subclasses of\n+ * rerere_io embed this structure at the beginning of their own\n+ * rerere_io object.\n+ */\n+struct rerere_io {\n+\tint (*getline)(struct strbuf *, struct rerere_io *);\n+\tFILE *output;\n+\tint wrerror;\n+\t/* some more stuff */\n+};\n+\n static void ferr_write(const void *p, size_t count, FILE *fp, int *err)\n {\n \tif (!count || *err)\n@@ -96,19 +111,15 @@ static inline void ferr_puts(const char *s, FILE *fp, int *err)\n \tferr_write(s, strlen(s), fp, err);\n }\n \n-struct rerere_io {\n-\tint (*getline)(struct strbuf *, struct rerere_io *);\n-\tFILE *output;\n-\tint wrerror;\n-\t/* some more stuff */\n-};\n-\n static void rerere_io_putstr(const char *str, struct rerere_io *io)\n {\n \tif (io->output)\n \t\tferr_puts(str, io->output, &io->wrerror);\n }\n \n+/*\n+ * Write a conflict marker to io->output (if defined).\n+ */\n static void rerere_io_putconflict(int ch, int size, struct rerere_io *io)\n {\n \tchar buf[64];\n@@ -137,11 +148,17 @@ static void rerere_io_putmem(const char *mem, size_t sz, struct rerere_io *io)\n \t\tferr_write(mem, sz, io->output, &io->wrerror);\n }\n \n+/*\n+ * Subclass of rerere_io that reads from an on-disk file\n+ */\n struct rerere_io_file {\n \tstruct rerere_io io;\n \tFILE *input;\n };\n \n+/*\n+ * ... and its getline() method implementation\n+ */\n static int rerere_file_getline(struct strbuf *sb, struct rerere_io *io_)\n {\n \tstruct rerere_io_file *io = (struct rerere_io_file *)io_;\n@@ -286,11 +303,18 @@ static int handle_file(const char *path, unsigned char *sha1, const char *output\n \treturn hunk_no;\n }\n \n+/*\n+ * Subclass of rerere_io that reads from an in-core buffer that is a\n+ * strbuf\n+ */\n struct rerere_io_mem {\n \tstruct rerere_io io;\n \tstruct strbuf input;\n };\n \n+/*\n+ * ... and its getline() method implementation\n+ */\n static int rerere_mem_getline(struct strbuf *sb, struct rerere_io *io_)\n {\n \tstruct rerere_io_mem *io = (struct rerere_io_mem *)io_;\n-- \n2.5.0-rc0-209-g5e1f148\n"},{"id":"265294","messageId":"1435730699-9124-10-git-send-email-gitster@pobox.com","threadId":"39756","inReplyTo":"1435730699-9124-1-git-send-email-gitster@pobox.com","subject":"[PATCH v2 09/13] rerere: explain MERGE_RR management helpers","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-07-01T06:04:55Z","receivedAt":"2015-07-01T06:04:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Explain the internals of rerere as in-code comments, while\nsprinkling \"NEEDSWORK\" comment to highlight iffy bits and\nquestionable assumptions.\n\nThis one covers the \"$GIT_DIR/MERGE_RR\" file and in-core merge_rr\nthat are used to keep track of the status of \"rerere\" session in\nprogress.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n rerere.c | 41 +++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 41 insertions(+)\n\ndiff --git a/rerere.c b/rerere.c\nindex 7ed20f1..d54bdb2 100644\n--- a/rerere.c\n+++ b/rerere.c\n@@ -33,6 +33,13 @@ static int has_rerere_resolution(const char *hex)\n \treturn !stat(rerere_path(hex, \"postimage\"), &st);\n }\n \n+/*\n+ * $GIT_DIR/MERGE_RR file is a collection of records, each of which is\n+ * \"conflict ID\", a HT and pathname, terminated with a NUL, and is\n+ * used to keep track of the set of paths that \"rerere\" may need to\n+ * work on (i.e. what is left by the previous invocation of \"git\n+ * rerere\" during the current conflict resolution session).\n+ */\n static void read_rr(struct string_list *rr)\n {\n \tstruct strbuf buf = STRBUF_INIT;\n@@ -394,6 +401,14 @@ static int handle_cache(const char *path, unsigned char *sha1, const char *outpu\n \treturn hunk_no;\n }\n \n+/*\n+ * Look at a cache entry at \"i\" and see if it is not conflicting,\n+ * conflicting and we are willing to handle, or conflicting and\n+ * we are unable to handle, and return the determination in *type.\n+ * Return the cache index to be looked at next, by skipping the\n+ * stages we have already looked at in this invocation of this\n+ * function.\n+ */\n static int check_one_conflict(int i, int *type)\n {\n \tconst struct cache_entry *e = active_cache[i];\n@@ -425,6 +440,17 @@ static int check_one_conflict(int i, int *type)\n \treturn i;\n }\n \n+/*\n+ * Scan the index and find paths that have conflicts that rerere can\n+ * handle, i.e. the ones that has both stages #2 and #3.\n+ *\n+ * NEEDSWORK: we do not record or replay a previous \"resolve by\n+ * deletion\" for a delete-modify conflict, as that is inherently risky\n+ * without knowing what modification is being discarded.  The only\n+ * safe case, i.e. both side doing the deletion and modification that\n+ * are identical to the previous round, might want to be handled,\n+ * though.\n+ */\n static int find_conflict(struct string_list *conflict)\n {\n \tint i;\n@@ -441,6 +467,21 @@ static int find_conflict(struct string_list *conflict)\n \treturn 0;\n }\n \n+/*\n+ * The merge_rr list is meant to hold outstanding conflicted paths\n+ * that rerere could handle.  Abuse the list by adding other types of\n+ * entries to allow the caller to show \"rerere remaining\".\n+ *\n+ * - Conflicted paths that rerere does not handle are added\n+ * - Conflicted paths that have been resolved are marked as such\n+ *   by storing RERERE_RESOLVED to .util field (where conflict ID\n+ *   is expected to be stored).\n+ *\n+ * Do *not* write MERGE_RR file out after calling this function.\n+ *\n+ * NEEDSWORK: we may want to fix the caller that implements \"rerere\n+ * remaining\" to do this without abusing merge_rr.\n+ */\n int rerere_remaining(struct string_list *merge_rr)\n {\n \tint i;\n-- \n2.5.0-rc0-209-g5e1f148\n"},{"id":"265295","messageId":"1435730699-9124-11-git-send-email-gitster@pobox.com","threadId":"39756","inReplyTo":"1435730699-9124-1-git-send-email-gitster@pobox.com","subject":"[PATCH v2 10/13] rerere: explain the primary codepath","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-07-01T06:04:56Z","receivedAt":"2015-07-01T06:04:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Explain the internals of rerere as in-code comments, while\nsprinkling \"NEEDSWORK\" comment to highlight iffy bits and\nquestionable assumptions.\n\nThis one covers the codepath reached from rerere(), the primary\ninterface to the subsystem.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n rerere.c | 95 +++++++++++++++++++++++++++++++++++++++++++++++++++++++---------\n 1 file changed, 82 insertions(+), 13 deletions(-)\n\ndiff --git a/rerere.c b/rerere.c\nindex d54bdb2..3d9c33b 100644\n--- a/rerere.c\n+++ b/rerere.c\n@@ -199,6 +199,21 @@ static int is_cmarker(char *buf, int marker_char, int marker_size)\n \treturn isspace(*buf);\n }\n \n+/*\n+ * Read contents a file with conflicts, normalize the conflicts\n+ * by (1) discarding the common ancestor version in diff3-style,\n+ * (2) reordering our side and their side so that whichever sorts\n+ * alphabetically earlier comes before the other one, while\n+ * computing the \"conflict ID\", which is just an SHA-1 hash of\n+ * one side of the conflict, NUL, the other side of the conflict,\n+ * and NUL concatenated together.\n+ *\n+ * Return the number of conflict hunks found.\n+ *\n+ * NEEDSWORK: the logic and theory of operation behind this conflict\n+ * normalization may deserve to be documented somewhere, perhaps in\n+ * Documentation/technical/rerere.txt.\n+ */\n static int handle_path(unsigned char *sha1, struct rerere_io *io, int marker_size)\n {\n \tgit_SHA_CTX ctx;\n@@ -269,6 +284,10 @@ static int handle_path(unsigned char *sha1, struct rerere_io *io, int marker_siz\n \treturn hunk_no;\n }\n \n+/*\n+ * Scan the path for conflicts, do the \"handle_path()\" thing above, and\n+ * return the number of conflict hunks found.\n+ */\n static int handle_file(const char *path, unsigned char *sha1, const char *output)\n {\n \tint hunk_no = 0;\n@@ -506,29 +525,54 @@ int rerere_remaining(struct string_list *merge_rr)\n \treturn 0;\n }\n \n+/*\n+ * Find the conflict identified by \"name\"; the change between its\n+ * \"preimage\" (i.e. a previous contents with conflict markers) and its\n+ * \"postimage\" (i.e. the corresponding contents with conflicts\n+ * resolved) may apply cleanly to the contents stored in \"path\", i.e.\n+ * the conflict this time around.\n+ *\n+ * Returns 0 for successful replay of recorded resolution, or non-zero\n+ * for failure.\n+ */\n static int merge(const char *name, const char *path)\n {\n \tint ret;\n \tmmfile_t cur = {NULL, 0}, base = {NULL, 0}, other = {NULL, 0};\n \tmmbuffer_t result = {NULL, 0};\n \n+\t/*\n+\t * Normalize the conflicts in path and write it out to\n+\t * \"thisimage\" temporary file.\n+\t */\n \tif (handle_file(path, NULL, rerere_path(name, \"thisimage\")) < 0)\n \t\treturn 1;\n \n \tif (read_mmfile(&cur, rerere_path(name, \"thisimage\")) ||\n-\t\t\tread_mmfile(&base, rerere_path(name, \"preimage\")) ||\n-\t\t\tread_mmfile(&other, rerere_path(name, \"postimage\"))) {\n+\t    read_mmfile(&base, rerere_path(name, \"preimage\")) ||\n+\t    read_mmfile(&other, rerere_path(name, \"postimage\"))) {\n \t\tret = 1;\n \t\tgoto out;\n \t}\n+\n+\t/*\n+\t * A three-way merge. Note that this honors user-customizable\n+\t * low-level merge driver settings.\n+\t */\n \tret = ll_merge(&result, path, &base, NULL, &cur, \"\", &other, \"\", NULL);\n \tif (!ret) {\n \t\tFILE *f;\n \n+\t\t/*\n+\t\t * A successful replay of recorded resolution.\n+\t\t * Mark that \"postimage\" was used to help gc.\n+\t\t */\n \t\tif (utime(rerere_path(name, \"postimage\"), NULL) < 0)\n \t\t\twarning(\"failed utime() on %s: %s\",\n \t\t\t\t\trerere_path(name, \"postimage\"),\n \t\t\t\t\tstrerror(errno));\n+\n+\t\t/* Update \"path\" with the resolution */\n \t\tf = fopen(path, \"w\");\n \t\tif (!f)\n \t\t\treturn error(\"Could not open %s: %s\", path,\n@@ -581,41 +625,61 @@ static int do_plain_rerere(struct string_list *rr, int fd)\n \tfind_conflict(&conflict);\n \n \t/*\n-\t * MERGE_RR records paths with conflicts immediately after merge\n-\t * failed.  Some of the conflicted paths might have been hand resolved\n-\t * in the working tree since then, but the initial run would catch all\n-\t * and register their preimages.\n+\t * MERGE_RR records paths with conflicts immediately after\n+\t * merge failed.  Some of the conflicted paths might have been\n+\t * hand resolved in the working tree since then, but the\n+\t * initial run would catch all and register their preimages.\n \t */\n-\n \tfor (i = 0; i < conflict.nr; i++) {\n \t\tconst char *path = conflict.items[i].string;\n \t\tif (!string_list_has_string(rr, path)) {\n \t\t\tunsigned char sha1[20];\n \t\t\tchar *hex;\n \t\t\tint ret;\n+\n+\t\t\t/*\n+\t\t\t * Ask handle_file() to scan and assign a\n+\t\t\t * conflict ID.  No need to write anything out\n+\t\t\t * yet.\n+\t\t\t */\n \t\t\tret = handle_file(path, sha1, NULL);\n \t\t\tif (ret < 1)\n \t\t\t\tcontinue;\n \t\t\thex = xstrdup(sha1_to_hex(sha1));\n \t\t\tstring_list_insert(rr, path)->util = hex;\n+\n+\t\t\t/*\n+\t\t\t * If the directory does not exist, create\n+\t\t\t * it.  mkdir_in_gitdir() will fail with\n+\t\t\t * EEXIST if there already is one.\n+\t\t\t *\n+\t\t\t * NEEDSWORK: make sure \"gc\" does not remove\n+\t\t\t * preimage without removing the directory.\n+\t\t\t */\n \t\t\tif (mkdir_in_gitdir(git_path(\"rr-cache/%s\", hex)))\n \t\t\t\tcontinue;\n+\n+\t\t\t/*\n+\t\t\t * We are the first to encounter this\n+\t\t\t * conflict.  Ask handle_file() to write the\n+\t\t\t * normalized contents to the \"preimage\" file.\n+\t\t\t */\n \t\t\thandle_file(path, NULL, rerere_path(hex, \"preimage\"));\n \t\t\tfprintf(stderr, \"Recorded preimage for '%s'\\n\", path);\n \t\t}\n \t}\n \n \t/*\n-\t * Now some of the paths that had conflicts earlier might have been\n-\t * hand resolved.  Others may be similar to a conflict already that\n-\t * was resolved before.\n+\t * Some of the paths that had conflicts earlier might have\n+\t * been resolved by the user.  Others may be similar to a\n+\t * conflict already that was resolved before.\n \t */\n-\n \tfor (i = 0; i < rr->nr; i++) {\n \t\tint ret;\n \t\tconst char *path = rr->items[i].string;\n \t\tconst char *name = (const char *)rr->items[i].util;\n \n+\t\t/* Is there a recorded resolution we could attempt to apply? */\n \t\tif (has_rerere_resolution(name)) {\n \t\t\tif (merge(name, path))\n \t\t\t\tcontinue;\n@@ -629,13 +693,13 @@ static int do_plain_rerere(struct string_list *rr, int fd)\n \t\t\tgoto mark_resolved;\n \t\t}\n \n-\t\t/* Let's see if we have resolved it. */\n+\t\t/* Let's see if the user has resolved it. */\n \t\tret = handle_file(path, NULL, NULL);\n \t\tif (ret)\n \t\t\tcontinue;\n \n-\t\tfprintf(stderr, \"Recorded resolution for '%s'.\\n\", path);\n \t\tcopy_file(rerere_path(name, \"postimage\"), path, 0666);\n+\t\tfprintf(stderr, \"Recorded resolution for '%s'.\\n\", path);\n \tmark_resolved:\n \t\tfree(rr->items[i].util);\n \t\trr->items[i].util = NULL;\n@@ -689,6 +753,11 @@ int setup_rerere(struct string_list *merge_rr, int flags)\n \treturn fd;\n }\n \n+/*\n+ * The main entry point that is called internally from codepaths that\n+ * perform mergy operations, possibly leaving conflicted index entries\n+ * and working tree files.\n+ */\n int rerere(int flags)\n {\n \tstruct string_list merge_rr = STRING_LIST_INIT_DUP;\n-- \n2.5.0-rc0-209-g5e1f148\n"},{"id":"265296","messageId":"1435730699-9124-12-git-send-email-gitster@pobox.com","threadId":"39756","inReplyTo":"1435730699-9124-1-git-send-email-gitster@pobox.com","subject":"[PATCH v2 11/13] rerere: explain \"rerere forget\" codepath","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-07-01T06:04:57Z","receivedAt":"2015-07-01T06:04:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Explain the internals of rerere as in-code comments, while\nsprinkling \"NEEDSWORK\" comment to highlight iffy bits and\nquestionable assumptions.\n\nThis covers the codepath that implements \"rerere forget\".\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n rerere.c | 24 ++++++++++++++++++++++++\n 1 file changed, 24 insertions(+)\n\ndiff --git a/rerere.c b/rerere.c\nindex 3d9c33b..3782be6 100644\n--- a/rerere.c\n+++ b/rerere.c\n@@ -413,6 +413,10 @@ static int handle_cache(const char *path, unsigned char *sha1, const char *outpu\n \tstrbuf_init(&io.input, 0);\n \tstrbuf_attach(&io.input, result.ptr, result.size, result.size);\n \n+\t/*\n+\t * Grab the conflict ID and optionally write the original\n+\t * contents with conflict markers out.\n+\t */\n \thunk_no = handle_path(sha1, (struct rerere_io *)&io, marker_size);\n \tstrbuf_release(&io.input);\n \tif (io.io.output)\n@@ -777,9 +781,15 @@ static int rerere_forget_one_path(const char *path, struct string_list *rr)\n \tint ret;\n \tstruct string_list_item *item;\n \n+\t/*\n+\t * Recreate the original conflict from the stages in the\n+\t * index and compute the conflict ID\n+\t */\n \tret = handle_cache(path, sha1, NULL);\n \tif (ret < 1)\n \t\treturn error(\"Could not parse conflict hunks in '%s'\", path);\n+\n+\t/* Nuke the recorded resolution for the conflict */\n \thex = xstrdup(sha1_to_hex(sha1));\n \tfilename = rerere_path(hex, \"postimage\");\n \tif (unlink(filename))\n@@ -787,9 +797,18 @@ static int rerere_forget_one_path(const char *path, struct string_list *rr)\n \t\t\t? error(\"no remembered resolution for %s\", path)\n \t\t\t: error(\"cannot unlink %s: %s\", filename, strerror(errno)));\n \n+\t/*\n+\t * Update the preimage so that the user can resolve the\n+\t * conflict in the working tree, run us again to record\n+\t * the postimage.\n+\t */\n \thandle_cache(path, sha1, rerere_path(hex, \"preimage\"));\n \tfprintf(stderr, \"Updated preimage for '%s'\\n\", path);\n \n+\t/*\n+\t * And remember that we can record resolution for this\n+\t * conflict when the user is done.\n+\t */\n \titem = string_list_insert(rr, path);\n \tfree(item->util);\n \titem->util = hex;\n@@ -808,6 +827,11 @@ int rerere_forget(struct pathspec *pathspec)\n \n \tfd = setup_rerere(&merge_rr, RERERE_NOAUTOUPDATE);\n \n+\t/*\n+\t * The paths may have been resolved (incorrectly);\n+\t * recover the original conflicted state and then\n+\t * find the conflicted paths.\n+\t */\n \tunmerge_cache(pathspec);\n \tfind_conflict(&conflict);\n \tfor (i = 0; i < conflict.nr; i++) {\n-- \n2.5.0-rc0-209-g5e1f148\n"},{"id":"265298","messageId":"1435730699-9124-13-git-send-email-gitster@pobox.com","threadId":"39756","inReplyTo":"1435730699-9124-1-git-send-email-gitster@pobox.com","subject":"[PATCH v2 12/13] rerere: explain the remainder","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-07-01T06:04:58Z","receivedAt":"2015-07-01T06:04:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Explain the internals of rerere as in-code comments, while\nsprinkling \"NEEDSWORK\" comment to highlight iffy bits and\nquestionable assumptions.\n\nThis covers the codepath that implements \"rerere gc\" and \"rerere\nclear\".\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n rerere.c | 20 ++++++++++++++++++++\n 1 file changed, 20 insertions(+)\n\ndiff --git a/rerere.c b/rerere.c\nindex 3782be6..7ef951e 100644\n--- a/rerere.c\n+++ b/rerere.c\n@@ -844,6 +844,9 @@ int rerere_forget(struct pathspec *pathspec)\n \treturn write_rr(&merge_rr, fd);\n }\n \n+/*\n+ * Garbage collection support\n+ */\n static time_t rerere_created_at(const char *name)\n {\n \tstruct stat st;\n@@ -856,11 +859,19 @@ static time_t rerere_last_used_at(const char *name)\n \treturn stat(rerere_path(name, \"postimage\"), &st) ? (time_t) 0 : st.st_mtime;\n }\n \n+/*\n+ * Remove the recorded resolution for a given conflict ID\n+ */\n static void unlink_rr_item(const char *name)\n {\n \tunlink(rerere_path(name, \"thisimage\"));\n \tunlink(rerere_path(name, \"preimage\"));\n \tunlink(rerere_path(name, \"postimage\"));\n+\t/*\n+\t * NEEDSWORK: what if this rmdir() fails?  Wouldn't we then\n+\t * assume that we already have preimage recorded in\n+\t * do_plain_rerere()?\n+\t */\n \trmdir(git_path(\"rr-cache/%s\", name));\n }\n \n@@ -880,6 +891,7 @@ void rerere_gc(struct string_list *rr)\n \tdir = opendir(git_path(\"rr-cache\"));\n \tif (!dir)\n \t\tdie_errno(\"unable to open rr-cache directory\");\n+\t/* Collect stale conflict IDs ... */\n \twhile ((e = readdir(dir))) {\n \t\tif (is_dot_or_dotdot(e->d_name))\n \t\t\tcontinue;\n@@ -897,11 +909,19 @@ void rerere_gc(struct string_list *rr)\n \t\t\tstring_list_append(&to_remove, e->d_name);\n \t}\n \tclosedir(dir);\n+\t/* ... and then remove them one-by-one */\n \tfor (i = 0; i < to_remove.nr; i++)\n \t\tunlink_rr_item(to_remove.items[i].string);\n \tstring_list_clear(&to_remove, 0);\n }\n \n+/*\n+ * During a conflict resolution, after \"rerere\" recorded the\n+ * preimages, abandon them if the user did not resolve them or\n+ * record their resolutions.  And drop $GIT_DIR/MERGE_RR.\n+ *\n+ * NEEDSWORK: shouldn't we be calling this from \"reset --hard\"?\n+ */\n void rerere_clear(struct string_list *merge_rr)\n {\n \tint i;\n-- \n2.5.0-rc0-209-g5e1f148\n"},{"id":"265297","messageId":"1435730699-9124-14-git-send-email-gitster@pobox.com","threadId":"39756","inReplyTo":"1435730699-9124-1-git-send-email-gitster@pobox.com","subject":"[PATCH v2 13/13] rerere: refactor \"replay\" part of do_plain_rerere()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-07-01T06:04:59Z","receivedAt":"2015-07-01T06:04:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Extract the body of a loop that attempts to replay recorded\nresolution for each conflicted path into a helper function, not\nbecause I want to call it from multiple places later, but because\nthe logic has become too deeply nested and hard to read.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n rerere.c | 75 ++++++++++++++++++++++++++++++++++------------------------------\n 1 file changed, 40 insertions(+), 35 deletions(-)\n\ndiff --git a/rerere.c b/rerere.c\nindex 7ef951e..09b72ed 100644\n--- a/rerere.c\n+++ b/rerere.c\n@@ -620,6 +620,44 @@ static void update_paths(struct string_list *update)\n \t\trollback_lock_file(&index_lock);\n }\n \n+/*\n+ * The path indicated by rr_item may still have conflict for which we\n+ * have a recorded resolution, in which case replay it and optionally\n+ * update it.  Or it may have been resolved by the user and we may\n+ * only have the preimage for that conflict, in which case the result\n+ * needs to be recorded as a resolution in a postimage file.\n+ */\n+static void do_rerere_one_path(struct string_list_item *rr_item,\n+\t\t\t       struct string_list *update)\n+{\n+\tconst char *path = rr_item->string;\n+\tconst char *name = (const char *)rr_item->util;\n+\n+\t/* Is there a recorded resolution we could attempt to apply? */\n+\tif (has_rerere_resolution(name)) {\n+\t\tif (merge(name, path))\n+\t\t\treturn; /* failed to replay */\n+\n+\t\tif (rerere_autoupdate)\n+\t\t\tstring_list_insert(update, path);\n+\t\telse\n+\t\t\tfprintf(stderr,\n+\t\t\t\t\"Resolved '%s' using previous resolution.\\n\",\n+\t\t\t\tpath);\n+\t\tgoto mark_resolved;\n+\t}\n+\n+\t/* Let's see if the user has resolved it. */\n+\tif (handle_file(path, NULL, NULL))\n+\t\treturn; /* not yet resolved */\n+\n+\tcopy_file(rerere_path(name, \"postimage\"), path, 0666);\n+\tfprintf(stderr, \"Recorded resolution for '%s'.\\n\", path);\n+mark_resolved:\n+\tfree(rr_item->util);\n+\trr_item->util = NULL;\n+}\n+\n static int do_plain_rerere(struct string_list *rr, int fd)\n {\n \tstruct string_list conflict = STRING_LIST_INIT_DUP;\n@@ -673,41 +711,8 @@ static int do_plain_rerere(struct string_list *rr, int fd)\n \t\t}\n \t}\n \n-\t/*\n-\t * Some of the paths that had conflicts earlier might have\n-\t * been resolved by the user.  Others may be similar to a\n-\t * conflict already that was resolved before.\n-\t */\n-\tfor (i = 0; i < rr->nr; i++) {\n-\t\tint ret;\n-\t\tconst char *path = rr->items[i].string;\n-\t\tconst char *name = (const char *)rr->items[i].util;\n-\n-\t\t/* Is there a recorded resolution we could attempt to apply? */\n-\t\tif (has_rerere_resolution(name)) {\n-\t\t\tif (merge(name, path))\n-\t\t\t\tcontinue;\n-\n-\t\t\tif (rerere_autoupdate)\n-\t\t\t\tstring_list_insert(&update, path);\n-\t\t\telse\n-\t\t\t\tfprintf(stderr,\n-\t\t\t\t\t\"Resolved '%s' using previous resolution.\\n\",\n-\t\t\t\t\tpath);\n-\t\t\tgoto mark_resolved;\n-\t\t}\n-\n-\t\t/* Let's see if the user has resolved it. */\n-\t\tret = handle_file(path, NULL, NULL);\n-\t\tif (ret)\n-\t\t\tcontinue;\n-\n-\t\tcopy_file(rerere_path(name, \"postimage\"), path, 0666);\n-\t\tfprintf(stderr, \"Recorded resolution for '%s'.\\n\", path);\n-\tmark_resolved:\n-\t\tfree(rr->items[i].util);\n-\t\trr->items[i].util = NULL;\n-\t}\n+\tfor (i = 0; i < rr->nr; i++)\n+\t\tdo_rerere_one_path(&rr->items[i], &update);\n \n \tif (update.nr)\n \t\tupdate_paths(&update);\n-- \n2.5.0-rc0-209-g5e1f148\n"},{"id":"266275","messageId":"1437171880-21590-1-git-send-email-gitster@pobox.com","threadId":"39756","inReplyTo":"1435730699-9124-1-git-send-email-gitster@pobox.com","subject":"[PATCH v3 00/18] \"rerere\" preparatory clean-up","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-07-17T22:24:22Z","receivedAt":"2015-07-17T22:24:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This is a resend of v2:\n\n    http://thread.gmane.org/gmane.comp.version-control.git/273117\n\nplus 5 new changes.  I have a few more real-fix changes that build\non top, but they are not sufficiently polished to be published yet\ncompared to these early clean-up bits.\n\nJunio C Hamano (18):\n  rerere: fix an off-by-one non-bug\n  rerere: plug conflict ID leaks\n  rerere: lift PATH_MAX limitation\n  rerere: write out each record of MERGE_RR in one go\n  rerere: report autoupdated paths only after actually updating them\n  rerere: drop want_sp parameter from is_cmarker()\n  rerere: stop looping unnecessarily\n  rerere: explain the rerere I/O abstraction\n  rerere: explain MERGE_RR management helpers\n  rerere: explain the primary codepath\n  rerere: explain \"rerere forget\" codepath\n  rerere: explain the remainder\n  rerere: refactor \"replay\" part of do_plain_rerere()\n  rerere: further de-dent do_plain_rerere()\n  rerere: further clarify do_rerere_one_path()\n  rerere: call conflict-ids IDs\n  rerere: use \"struct rerere_id\" instead of \"char *\" for conflict ID\n  rerere: un-nest merge() further\n\n builtin/rerere.c |   4 +-\n rerere.c         | 544 ++++++++++++++++++++++++++++++++++++++++---------------\n rerere.h         |  12 +-\n 3 files changed, 407 insertions(+), 153 deletions(-)\n\n-- \n2.5.0-rc2-340-g0cccc16\n"},{"id":"266273","messageId":"1437171880-21590-2-git-send-email-gitster@pobox.com","threadId":"39756","inReplyTo":"1437171880-21590-1-git-send-email-gitster@pobox.com","subject":"[PATCH v3 01/18] rerere: fix an off-by-one non-bug","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-07-17T22:24:23Z","receivedAt":"2015-07-17T22:24:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"When ac49f5ca (rerere \"remaining\", 2011-02-16) split out a new\nhelper function check_one_conflict() out of find_conflict()\nfunction, so that the latter will use the returned value from the\nnew helper to update the loop control variable that is an index into\nactive_cache[], the new variable incremented the index by one too\nmany when it found a path with only stage #1 entry at the very end\nof active_cache[].\n\nThis \"strange\" return value does not have any effect on the loop\ncontrol of two callers of this function, as they all notice that\nactive_nr+2 is larger than active_nr just like active_nr+1 is, but\nnevertheless it puzzles the readers when they are trying to figure\nout what the function is trying to do.\n\nIn fact, there is no need to do an early return.  The code that\nfollows after skipping the stage #1 entry is fully prepared to\nhandle a case where the entry is at the very end of active_cache[].\n\nHelp future readers from unnecessary confusion by dropping an early\nreturn.  We skip the stage #1 entry, and if there are stage #2 and\nstage #3 entries for the same path, we diagnose the path as\nTHREE_STAGED (otherwise we say PUNTED), and then we skip all entries\nfor the same path.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n rerere.c | 6 ++----\n 1 file changed, 2 insertions(+), 4 deletions(-)\n\ndiff --git a/rerere.c b/rerere.c\nindex 31644de..e307711 100644\n--- a/rerere.c\n+++ b/rerere.c\n@@ -369,10 +369,8 @@ static int check_one_conflict(int i, int *type)\n \t}\n \n \t*type = PUNTED;\n-\tif (ce_stage(e) == 1) {\n-\t\tif (active_nr <= ++i)\n-\t\t\treturn i + 1;\n-\t}\n+\tif (ce_stage(e) == 1)\n+\t\ti++;\n \n \t/* Only handle regular files with both stages #2 and #3 */\n \tif (i + 1 < active_nr) {\n-- \n2.5.0-rc2-340-g0cccc16\n"},{"id":"266274","messageId":"1437171880-21590-3-git-send-email-gitster@pobox.com","threadId":"39756","inReplyTo":"1437171880-21590-1-git-send-email-gitster@pobox.com","subject":"[PATCH v3 02/18] rerere: plug conflict ID leaks","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-07-17T22:24:24Z","receivedAt":"2015-07-17T22:24:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The merge_rr string list stores the conflict ID (a hexadecimal\nstring that is used to index into $GIT_DIR/rr-cache) in the .util\nfield of its elements, and when do_plain_rerere() resolves a\nconflict, the field is cleared.  Also, when rerere_forget()\nrecomputes the conflict ID to updates the preimage file, the\nconflict ID for the path is updated.\n\nWe forgot to free the existing conflict ID when we did these two\noperations.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n rerere.c | 7 +++++--\n 1 file changed, 5 insertions(+), 2 deletions(-)\n\ndiff --git a/rerere.c b/rerere.c\nindex e307711..3b9104d 100644\n--- a/rerere.c\n+++ b/rerere.c\n@@ -559,6 +559,7 @@ static int do_plain_rerere(struct string_list *rr, int fd)\n \t\tfprintf(stderr, \"Recorded resolution for '%s'.\\n\", path);\n \t\tcopy_file(rerere_path(name, \"postimage\"), path, 0666);\n \tmark_resolved:\n+\t\tfree(rr->items[i].util);\n \t\trr->items[i].util = NULL;\n \t}\n \n@@ -627,6 +628,7 @@ static int rerere_forget_one_path(const char *path, struct string_list *rr)\n \tchar *hex;\n \tunsigned char sha1[20];\n \tint ret;\n+\tstruct string_list_item *item;\n \n \tret = handle_cache(path, sha1, NULL);\n \tif (ret < 1)\n@@ -641,8 +643,9 @@ static int rerere_forget_one_path(const char *path, struct string_list *rr)\n \thandle_cache(path, sha1, rerere_path(hex, \"preimage\"));\n \tfprintf(stderr, \"Updated preimage for '%s'\\n\", path);\n \n-\n-\tstring_list_insert(rr, path)->util = hex;\n+\titem = string_list_insert(rr, path);\n+\tfree(item->util);\n+\titem->util = hex;\n \tfprintf(stderr, \"Forgot resolution for %s\\n\", path);\n \treturn 0;\n }\n-- \n2.5.0-rc2-340-g0cccc16\n"},{"id":"266290","messageId":"1437171880-21590-4-git-send-email-gitster@pobox.com","threadId":"39756","inReplyTo":"1437171880-21590-1-git-send-email-gitster@pobox.com","subject":"[PATCH v3 03/18] rerere: lift PATH_MAX limitation","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-07-17T22:24:25Z","receivedAt":"2015-07-17T22:24:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The MERGE_RR file records a collection of NUL-terminated entries,\neach of which consists of\n\n - a hash that identifies the conflict\n - a HT\n - the pathname\n\nWe used to read this piece-by-piece, and worse yet, read the\npathname part a byte at a time into a fixed buffer of size PATH_MAX.\n\nInstead, read a whole entry using strbuf_getwholeline() and parse\nout the fields.  This way, we issue fewer read(2) calls and more\nimportantly we do not have to limit the pathname to PATH_MAX.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n rerere.c | 35 +++++++++++++++--------------------\n 1 file changed, 15 insertions(+), 20 deletions(-)\n\ndiff --git a/rerere.c b/rerere.c\nindex 3b9104d..e854985 100644\n--- a/rerere.c\n+++ b/rerere.c\n@@ -35,32 +35,27 @@ static int has_rerere_resolution(const char *hex)\n \n static void read_rr(struct string_list *rr)\n {\n-\tunsigned char sha1[20];\n-\tchar buf[PATH_MAX];\n+\tstruct strbuf buf = STRBUF_INIT;\n \tFILE *in = fopen(merge_rr_path, \"r\");\n+\n \tif (!in)\n \t\treturn;\n-\twhile (fread(buf, 40, 1, in) == 1) {\n-\t\tint i;\n-\t\tchar *name;\n-\t\tif (get_sha1_hex(buf, sha1))\n+\twhile (!strbuf_getwholeline(&buf, in, '\\0')) {\n+\t\tchar *path;\n+\t\tunsigned char sha1[20];\n+\n+\t\t/* There has to be the hash, tab, path and then NUL */\n+\t\tif (buf.len < 42 || get_sha1_hex(buf.buf, sha1))\n \t\t\tdie(\"corrupt MERGE_RR\");\n-\t\tbuf[40] = '\\0';\n-\t\tname = xstrdup(buf);\n-\t\tif (fgetc(in) != '\\t')\n+\n+\t\tif (buf.buf[40] != '\\t')\n \t\t\tdie(\"corrupt MERGE_RR\");\n-\t\tfor (i = 0; i < sizeof(buf); i++) {\n-\t\t\tint c = fgetc(in);\n-\t\t\tif (c < 0)\n-\t\t\t\tdie(\"corrupt MERGE_RR\");\n-\t\t\tbuf[i] = c;\n-\t\t\tif (c == 0)\n-\t\t\t\t break;\n-\t\t}\n-\t\tif (i == sizeof(buf))\n-\t\t\tdie(\"filename too long\");\n-\t\tstring_list_insert(rr, buf)->util = name;\n+\t\tbuf.buf[40] = '\\0';\n+\t\tpath = buf.buf + 41;\n+\n+\t\tstring_list_insert(rr, path)->util = xstrdup(buf.buf);\n \t}\n+\tstrbuf_release(&buf);\n \tfclose(in);\n }\n \n-- \n2.5.0-rc2-340-g0cccc16\n"},{"id":"266277","messageId":"1437171880-21590-5-git-send-email-gitster@pobox.com","threadId":"39756","inReplyTo":"1437171880-21590-1-git-send-email-gitster@pobox.com","subject":"[PATCH v3 04/18] rerere: write out each record of MERGE_RR in one go","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-07-17T22:24:26Z","receivedAt":"2015-07-17T22:24:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Instead of writing the hash for a conflict, a HT, and the path\nwith three separate write_in_full() calls, format them into a\nsingle record into a strbuf and write it out in one go.\n\nAs a more recent \"rerere remaining\" codepath abuses the .util field\nof the merge_rr data to store a sentinel token, make sure that\ncodepath does not call into this function (of course, \"remaining\" is\na read-only operation and currently does not call it).\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n rerere.c | 16 +++++++++-------\n 1 file changed, 9 insertions(+), 7 deletions(-)\n\ndiff --git a/rerere.c b/rerere.c\nindex e854985..27b287d 100644\n--- a/rerere.c\n+++ b/rerere.c\n@@ -65,16 +65,18 @@ static int write_rr(struct string_list *rr, int out_fd)\n {\n \tint i;\n \tfor (i = 0; i < rr->nr; i++) {\n-\t\tconst char *path;\n-\t\tint length;\n+\t\tstruct strbuf buf = STRBUF_INIT;\n+\n+\t\tassert(rr->items[i].util != RERERE_RESOLVED);\n \t\tif (!rr->items[i].util)\n \t\t\tcontinue;\n-\t\tpath = rr->items[i].string;\n-\t\tlength = strlen(path) + 1;\n-\t\tif (write_in_full(out_fd, rr->items[i].util, 40) != 40 ||\n-\t\t    write_str_in_full(out_fd, \"\\t\") != 1 ||\n-\t\t    write_in_full(out_fd, path, length) != length)\n+\t\tstrbuf_addf(&buf, \"%s\\t%s%c\",\n+\t\t\t    (char *)rr->items[i].util,\n+\t\t\t    rr->items[i].string, 0);\n+\t\tif (write_in_full(out_fd, buf.buf, buf.len) != buf.len)\n \t\t\tdie(\"unable to write rerere record\");\n+\n+\t\tstrbuf_release(&buf);\n \t}\n \tif (commit_lock_file(&write_lock) != 0)\n \t\tdie(\"unable to write rerere record\");\n-- \n2.5.0-rc2-340-g0cccc16\n"},{"id":"266276","messageId":"1437171880-21590-6-git-send-email-gitster@pobox.com","threadId":"39756","inReplyTo":"1437171880-21590-1-git-send-email-gitster@pobox.com","subject":"[PATCH v3 05/18] rerere: report autoupdated paths only after actually updating them","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-07-17T22:24:27Z","receivedAt":"2015-07-17T22:24:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Signed-off-by: Junio C Hamano <gitster@pobox.com>\n---\n rerere.c | 22 ++++++++++++----------\n 1 file changed, 12 insertions(+), 10 deletions(-)\n\ndiff --git a/rerere.c b/rerere.c\nindex 27b287d..304df02 100644\n--- a/rerere.c\n+++ b/rerere.c\n@@ -482,6 +482,8 @@ static void update_paths(struct string_list *update)\n \t\tstruct string_list_item *item = &update->items[i];\n \t\tif (add_file_to_cache(item->string, 0))\n \t\t\texit(128);\n+\t\tfprintf(stderr, \"Staged '%s' using previous resolution.\\n\",\n+\t\t\titem->string);\n \t}\n \n \tif (active_cache_changed) {\n@@ -536,16 +538,16 @@ static int do_plain_rerere(struct string_list *rr, int fd)\n \t\tconst char *name = (const char *)rr->items[i].util;\n \n \t\tif (has_rerere_resolution(name)) {\n-\t\t\tif (!merge(name, path)) {\n-\t\t\t\tconst char *msg;\n-\t\t\t\tif (rerere_autoupdate) {\n-\t\t\t\t\tstring_list_insert(&update, path);\n-\t\t\t\t\tmsg = \"Staged '%s' using previous resolution.\\n\";\n-\t\t\t\t} else\n-\t\t\t\t\tmsg = \"Resolved '%s' using previous resolution.\\n\";\n-\t\t\t\tfprintf(stderr, msg, path);\n-\t\t\t\tgoto mark_resolved;\n-\t\t\t}\n+\t\t\tif (merge(name, path))\n+\t\t\t\tcontinue;\n+\n+\t\t\tif (rerere_autoupdate)\n+\t\t\t\tstring_list_insert(&update, path);\n+\t\t\telse\n+\t\t\t\tfprintf(stderr,\n+\t\t\t\t\t\"Resolved '%s' using previous resolution.\\n\",\n+\t\t\t\t\tpath);\n+\t\t\tgoto mark_resolved;\n \t\t}\n \n \t\t/* Let's see if we have resolved it. */\n-- \n2.5.0-rc2-340-g0cccc16\n"},{"id":"266289","messageId":"1437171880-21590-7-git-send-email-gitster@pobox.com","threadId":"39756","inReplyTo":"1437171880-21590-1-git-send-email-gitster@pobox.com","subject":"[PATCH v3 06/18] rerere: drop want_sp parameter from is_cmarker()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-07-17T22:24:28Z","receivedAt":"2015-07-17T22:24:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"As the nature of the conflict marker line determies if there should\na SP and label after it, the caller shouldn't have to pass the\nparameter redundantly.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n rerere.c | 27 ++++++++++++++++++++++-----\n 1 file changed, 22 insertions(+), 5 deletions(-)\n\ndiff --git a/rerere.c b/rerere.c\nindex 304df02..4c45f55 100644\n--- a/rerere.c\n+++ b/rerere.c\n@@ -148,8 +148,25 @@ static int rerere_file_getline(struct strbuf *sb, struct rerere_io *io_)\n \treturn strbuf_getwholeline(sb, io->input, '\\n');\n }\n \n-static int is_cmarker(char *buf, int marker_char, int marker_size, int want_sp)\n+/*\n+ * Require the exact number of conflict marker letters, no more, no\n+ * less, followed by SP or any whitespace\n+ * (including LF).\n+ */\n+static int is_cmarker(char *buf, int marker_char, int marker_size)\n {\n+\tint want_sp;\n+\n+\t/*\n+\t * The beginning of our version and the end of their version\n+\t * always are labeled like \"<<<<< ours\" or \">>>>> theirs\",\n+\t * hence we set want_sp for them.  Note that the version from\n+\t * the common ancestor in diff3-style output is not always\n+\t * labelled (e.g. \"||||| common\" is often seen but \"|||||\"\n+\t * alone is also valid), so we do not set want_sp.\n+\t */\n+\twant_sp = (marker_char == '<') || (marker_char == '>');\n+\n \twhile (marker_size--)\n \t\tif (*buf++ != marker_char)\n \t\t\treturn 0;\n@@ -172,19 +189,19 @@ static int handle_path(unsigned char *sha1, struct rerere_io *io, int marker_siz\n \t\tgit_SHA1_Init(&ctx);\n \n \twhile (!io->getline(&buf, io)) {\n-\t\tif (is_cmarker(buf.buf, '<', marker_size, 1)) {\n+\t\tif (is_cmarker(buf.buf, '<', marker_size)) {\n \t\t\tif (hunk != RR_CONTEXT)\n \t\t\t\tgoto bad;\n \t\t\thunk = RR_SIDE_1;\n-\t\t} else if (is_cmarker(buf.buf, '|', marker_size, 0)) {\n+\t\t} else if (is_cmarker(buf.buf, '|', marker_size)) {\n \t\t\tif (hunk != RR_SIDE_1)\n \t\t\t\tgoto bad;\n \t\t\thunk = RR_ORIGINAL;\n-\t\t} else if (is_cmarker(buf.buf, '=', marker_size, 0)) {\n+\t\t} else if (is_cmarker(buf.buf, '=', marker_size)) {\n \t\t\tif (hunk != RR_SIDE_1 && hunk != RR_ORIGINAL)\n \t\t\t\tgoto bad;\n \t\t\thunk = RR_SIDE_2;\n-\t\t} else if (is_cmarker(buf.buf, '>', marker_size, 1)) {\n+\t\t} else if (is_cmarker(buf.buf, '>', marker_size)) {\n \t\t\tif (hunk != RR_SIDE_2)\n \t\t\t\tgoto bad;\n \t\t\tif (strbuf_cmp(&one, &two) > 0)\n-- \n2.5.0-rc2-340-g0cccc16\n"},{"id":"266279","messageId":"1437171880-21590-8-git-send-email-gitster@pobox.com","threadId":"39756","inReplyTo":"1437171880-21590-1-git-send-email-gitster@pobox.com","subject":"[PATCH v3 07/18] rerere: stop looping unnecessarily","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-07-17T22:24:29Z","receivedAt":"2015-07-17T22:24:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"handle_cache() loops 3 times starting from an index entry that is\nunmerged, while ignoring an entry for a path that is different from\nwhat we are looking for.\n\nAs the index is sorted, once we see a different path, we know we saw\nall stages for the path we are interested in.  Just loop while we\nsee the same path and then break, instead of continuing for 3 times.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n rerere.c | 17 +++++++----------\n 1 file changed, 7 insertions(+), 10 deletions(-)\n\ndiff --git a/rerere.c b/rerere.c\nindex 4c45f55..7b1419c 100644\n--- a/rerere.c\n+++ b/rerere.c\n@@ -329,24 +329,21 @@ static int handle_cache(const char *path, unsigned char *sha1, const char *outpu\n \t\treturn -1;\n \tpos = -pos - 1;\n \n-\tfor (i = 0; i < 3; i++) {\n+\twhile (pos < active_nr) {\n \t\tenum object_type type;\n \t\tunsigned long size;\n-\t\tint j;\n \n-\t\tif (active_nr <= pos)\n-\t\t\tbreak;\n \t\tce = active_cache[pos++];\n \t\tif (ce_namelen(ce) != len || memcmp(ce->name, path, len))\n-\t\t\tcontinue;\n-\t\tj = ce_stage(ce) - 1;\n-\t\tmmfile[j].ptr = read_sha1_file(ce->sha1, &type, &size);\n-\t\tmmfile[j].size = size;\n+\t\t\tbreak;\n+\t\ti = ce_stage(ce) - 1;\n+\t\tmmfile[i].ptr = read_sha1_file(ce->sha1, &type, &size);\n+\t\tmmfile[i].size = size;\n \t}\n-\tfor (i = 0; i < 3; i++) {\n+\tfor (i = 0; i < 3; i++)\n \t\tif (!mmfile[i].ptr && !mmfile[i].size)\n \t\t\tmmfile[i].ptr = xstrdup(\"\");\n-\t}\n+\n \t/*\n \t * NEEDSWORK: handle conflicts from merges with\n \t * merge.renormalize set, too\n-- \n2.5.0-rc2-340-g0cccc16\n"},{"id":"266288","messageId":"1437171880-21590-9-git-send-email-gitster@pobox.com","threadId":"39756","inReplyTo":"1437171880-21590-1-git-send-email-gitster@pobox.com","subject":"[PATCH v3 08/18] rerere: explain the rerere I/O abstraction","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-07-17T22:24:30Z","receivedAt":"2015-07-17T22:24:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Explain the internals of rerere as in-code comments.\n\nThis one covers our thin I/O abstraction to read from either\na file or a memory while optionally writing out to a file.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n rerere.c | 38 +++++++++++++++++++++++++++++++-------\n 1 file changed, 31 insertions(+), 7 deletions(-)\n\ndiff --git a/rerere.c b/rerere.c\nindex 7b1419c..7ed20f1 100644\n--- a/rerere.c\n+++ b/rerere.c\n@@ -83,6 +83,21 @@ static int write_rr(struct string_list *rr, int out_fd)\n \treturn 0;\n }\n \n+/*\n+ * \"rerere\" interacts with conflicted file contents using this I/O\n+ * abstraction.  It reads a conflicted contents from one place via\n+ * \"getline()\" method, and optionally can write it out after\n+ * normalizing the conflicted hunks to the \"output\".  Subclasses of\n+ * rerere_io embed this structure at the beginning of their own\n+ * rerere_io object.\n+ */\n+struct rerere_io {\n+\tint (*getline)(struct strbuf *, struct rerere_io *);\n+\tFILE *output;\n+\tint wrerror;\n+\t/* some more stuff */\n+};\n+\n static void ferr_write(const void *p, size_t count, FILE *fp, int *err)\n {\n \tif (!count || *err)\n@@ -96,19 +111,15 @@ static inline void ferr_puts(const char *s, FILE *fp, int *err)\n \tferr_write(s, strlen(s), fp, err);\n }\n \n-struct rerere_io {\n-\tint (*getline)(struct strbuf *, struct rerere_io *);\n-\tFILE *output;\n-\tint wrerror;\n-\t/* some more stuff */\n-};\n-\n static void rerere_io_putstr(const char *str, struct rerere_io *io)\n {\n \tif (io->output)\n \t\tferr_puts(str, io->output, &io->wrerror);\n }\n \n+/*\n+ * Write a conflict marker to io->output (if defined).\n+ */\n static void rerere_io_putconflict(int ch, int size, struct rerere_io *io)\n {\n \tchar buf[64];\n@@ -137,11 +148,17 @@ static void rerere_io_putmem(const char *mem, size_t sz, struct rerere_io *io)\n \t\tferr_write(mem, sz, io->output, &io->wrerror);\n }\n \n+/*\n+ * Subclass of rerere_io that reads from an on-disk file\n+ */\n struct rerere_io_file {\n \tstruct rerere_io io;\n \tFILE *input;\n };\n \n+/*\n+ * ... and its getline() method implementation\n+ */\n static int rerere_file_getline(struct strbuf *sb, struct rerere_io *io_)\n {\n \tstruct rerere_io_file *io = (struct rerere_io_file *)io_;\n@@ -286,11 +303,18 @@ static int handle_file(const char *path, unsigned char *sha1, const char *output\n \treturn hunk_no;\n }\n \n+/*\n+ * Subclass of rerere_io that reads from an in-core buffer that is a\n+ * strbuf\n+ */\n struct rerere_io_mem {\n \tstruct rerere_io io;\n \tstruct strbuf input;\n };\n \n+/*\n+ * ... and its getline() method implementation\n+ */\n static int rerere_mem_getline(struct strbuf *sb, struct rerere_io *io_)\n {\n \tstruct rerere_io_mem *io = (struct rerere_io_mem *)io_;\n-- \n2.5.0-rc2-340-g0cccc16\n"},{"id":"266278","messageId":"1437171880-21590-10-git-send-email-gitster@pobox.com","threadId":"39756","inReplyTo":"1437171880-21590-1-git-send-email-gitster@pobox.com","subject":"[PATCH v3 09/18] rerere: explain MERGE_RR management helpers","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-07-17T22:24:31Z","receivedAt":"2015-07-17T22:24:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Explain the internals of rerere as in-code comments, while\nsprinkling \"NEEDSWORK\" comment to highlight iffy bits and\nquestionable assumptions.\n\nThis one covers the \"$GIT_DIR/MERGE_RR\" file and in-core merge_rr\nthat are used to keep track of the status of \"rerere\" session in\nprogress.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n rerere.c | 41 +++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 41 insertions(+)\n\ndiff --git a/rerere.c b/rerere.c\nindex 7ed20f1..d54bdb2 100644\n--- a/rerere.c\n+++ b/rerere.c\n@@ -33,6 +33,13 @@ static int has_rerere_resolution(const char *hex)\n \treturn !stat(rerere_path(hex, \"postimage\"), &st);\n }\n \n+/*\n+ * $GIT_DIR/MERGE_RR file is a collection of records, each of which is\n+ * \"conflict ID\", a HT and pathname, terminated with a NUL, and is\n+ * used to keep track of the set of paths that \"rerere\" may need to\n+ * work on (i.e. what is left by the previous invocation of \"git\n+ * rerere\" during the current conflict resolution session).\n+ */\n static void read_rr(struct string_list *rr)\n {\n \tstruct strbuf buf = STRBUF_INIT;\n@@ -394,6 +401,14 @@ static int handle_cache(const char *path, unsigned char *sha1, const char *outpu\n \treturn hunk_no;\n }\n \n+/*\n+ * Look at a cache entry at \"i\" and see if it is not conflicting,\n+ * conflicting and we are willing to handle, or conflicting and\n+ * we are unable to handle, and return the determination in *type.\n+ * Return the cache index to be looked at next, by skipping the\n+ * stages we have already looked at in this invocation of this\n+ * function.\n+ */\n static int check_one_conflict(int i, int *type)\n {\n \tconst struct cache_entry *e = active_cache[i];\n@@ -425,6 +440,17 @@ static int check_one_conflict(int i, int *type)\n \treturn i;\n }\n \n+/*\n+ * Scan the index and find paths that have conflicts that rerere can\n+ * handle, i.e. the ones that has both stages #2 and #3.\n+ *\n+ * NEEDSWORK: we do not record or replay a previous \"resolve by\n+ * deletion\" for a delete-modify conflict, as that is inherently risky\n+ * without knowing what modification is being discarded.  The only\n+ * safe case, i.e. both side doing the deletion and modification that\n+ * are identical to the previous round, might want to be handled,\n+ * though.\n+ */\n static int find_conflict(struct string_list *conflict)\n {\n \tint i;\n@@ -441,6 +467,21 @@ static int find_conflict(struct string_list *conflict)\n \treturn 0;\n }\n \n+/*\n+ * The merge_rr list is meant to hold outstanding conflicted paths\n+ * that rerere could handle.  Abuse the list by adding other types of\n+ * entries to allow the caller to show \"rerere remaining\".\n+ *\n+ * - Conflicted paths that rerere does not handle are added\n+ * - Conflicted paths that have been resolved are marked as such\n+ *   by storing RERERE_RESOLVED to .util field (where conflict ID\n+ *   is expected to be stored).\n+ *\n+ * Do *not* write MERGE_RR file out after calling this function.\n+ *\n+ * NEEDSWORK: we may want to fix the caller that implements \"rerere\n+ * remaining\" to do this without abusing merge_rr.\n+ */\n int rerere_remaining(struct string_list *merge_rr)\n {\n \tint i;\n-- \n2.5.0-rc2-340-g0cccc16\n"},{"id":"266280","messageId":"1437171880-21590-11-git-send-email-gitster@pobox.com","threadId":"39756","inReplyTo":"1437171880-21590-1-git-send-email-gitster@pobox.com","subject":"[PATCH v3 10/18] rerere: explain the primary codepath","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-07-17T22:24:32Z","receivedAt":"2015-07-17T22:24:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Explain the internals of rerere as in-code comments, while\nsprinkling \"NEEDSWORK\" comment to highlight iffy bits and\nquestionable assumptions.\n\nThis one covers the codepath reached from rerere(), the primary\ninterface to the subsystem.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n rerere.c | 95 +++++++++++++++++++++++++++++++++++++++++++++++++++++++---------\n 1 file changed, 82 insertions(+), 13 deletions(-)\n\ndiff --git a/rerere.c b/rerere.c\nindex d54bdb2..3d9c33b 100644\n--- a/rerere.c\n+++ b/rerere.c\n@@ -199,6 +199,21 @@ static int is_cmarker(char *buf, int marker_char, int marker_size)\n \treturn isspace(*buf);\n }\n \n+/*\n+ * Read contents a file with conflicts, normalize the conflicts\n+ * by (1) discarding the common ancestor version in diff3-style,\n+ * (2) reordering our side and their side so that whichever sorts\n+ * alphabetically earlier comes before the other one, while\n+ * computing the \"conflict ID\", which is just an SHA-1 hash of\n+ * one side of the conflict, NUL, the other side of the conflict,\n+ * and NUL concatenated together.\n+ *\n+ * Return the number of conflict hunks found.\n+ *\n+ * NEEDSWORK: the logic and theory of operation behind this conflict\n+ * normalization may deserve to be documented somewhere, perhaps in\n+ * Documentation/technical/rerere.txt.\n+ */\n static int handle_path(unsigned char *sha1, struct rerere_io *io, int marker_size)\n {\n \tgit_SHA_CTX ctx;\n@@ -269,6 +284,10 @@ static int handle_path(unsigned char *sha1, struct rerere_io *io, int marker_siz\n \treturn hunk_no;\n }\n \n+/*\n+ * Scan the path for conflicts, do the \"handle_path()\" thing above, and\n+ * return the number of conflict hunks found.\n+ */\n static int handle_file(const char *path, unsigned char *sha1, const char *output)\n {\n \tint hunk_no = 0;\n@@ -506,29 +525,54 @@ int rerere_remaining(struct string_list *merge_rr)\n \treturn 0;\n }\n \n+/*\n+ * Find the conflict identified by \"name\"; the change between its\n+ * \"preimage\" (i.e. a previous contents with conflict markers) and its\n+ * \"postimage\" (i.e. the corresponding contents with conflicts\n+ * resolved) may apply cleanly to the contents stored in \"path\", i.e.\n+ * the conflict this time around.\n+ *\n+ * Returns 0 for successful replay of recorded resolution, or non-zero\n+ * for failure.\n+ */\n static int merge(const char *name, const char *path)\n {\n \tint ret;\n \tmmfile_t cur = {NULL, 0}, base = {NULL, 0}, other = {NULL, 0};\n \tmmbuffer_t result = {NULL, 0};\n \n+\t/*\n+\t * Normalize the conflicts in path and write it out to\n+\t * \"thisimage\" temporary file.\n+\t */\n \tif (handle_file(path, NULL, rerere_path(name, \"thisimage\")) < 0)\n \t\treturn 1;\n \n \tif (read_mmfile(&cur, rerere_path(name, \"thisimage\")) ||\n-\t\t\tread_mmfile(&base, rerere_path(name, \"preimage\")) ||\n-\t\t\tread_mmfile(&other, rerere_path(name, \"postimage\"))) {\n+\t    read_mmfile(&base, rerere_path(name, \"preimage\")) ||\n+\t    read_mmfile(&other, rerere_path(name, \"postimage\"))) {\n \t\tret = 1;\n \t\tgoto out;\n \t}\n+\n+\t/*\n+\t * A three-way merge. Note that this honors user-customizable\n+\t * low-level merge driver settings.\n+\t */\n \tret = ll_merge(&result, path, &base, NULL, &cur, \"\", &other, \"\", NULL);\n \tif (!ret) {\n \t\tFILE *f;\n \n+\t\t/*\n+\t\t * A successful replay of recorded resolution.\n+\t\t * Mark that \"postimage\" was used to help gc.\n+\t\t */\n \t\tif (utime(rerere_path(name, \"postimage\"), NULL) < 0)\n \t\t\twarning(\"failed utime() on %s: %s\",\n \t\t\t\t\trerere_path(name, \"postimage\"),\n \t\t\t\t\tstrerror(errno));\n+\n+\t\t/* Update \"path\" with the resolution */\n \t\tf = fopen(path, \"w\");\n \t\tif (!f)\n \t\t\treturn error(\"Could not open %s: %s\", path,\n@@ -581,41 +625,61 @@ static int do_plain_rerere(struct string_list *rr, int fd)\n \tfind_conflict(&conflict);\n \n \t/*\n-\t * MERGE_RR records paths with conflicts immediately after merge\n-\t * failed.  Some of the conflicted paths might have been hand resolved\n-\t * in the working tree since then, but the initial run would catch all\n-\t * and register their preimages.\n+\t * MERGE_RR records paths with conflicts immediately after\n+\t * merge failed.  Some of the conflicted paths might have been\n+\t * hand resolved in the working tree since then, but the\n+\t * initial run would catch all and register their preimages.\n \t */\n-\n \tfor (i = 0; i < conflict.nr; i++) {\n \t\tconst char *path = conflict.items[i].string;\n \t\tif (!string_list_has_string(rr, path)) {\n \t\t\tunsigned char sha1[20];\n \t\t\tchar *hex;\n \t\t\tint ret;\n+\n+\t\t\t/*\n+\t\t\t * Ask handle_file() to scan and assign a\n+\t\t\t * conflict ID.  No need to write anything out\n+\t\t\t * yet.\n+\t\t\t */\n \t\t\tret = handle_file(path, sha1, NULL);\n \t\t\tif (ret < 1)\n \t\t\t\tcontinue;\n \t\t\thex = xstrdup(sha1_to_hex(sha1));\n \t\t\tstring_list_insert(rr, path)->util = hex;\n+\n+\t\t\t/*\n+\t\t\t * If the directory does not exist, create\n+\t\t\t * it.  mkdir_in_gitdir() will fail with\n+\t\t\t * EEXIST if there already is one.\n+\t\t\t *\n+\t\t\t * NEEDSWORK: make sure \"gc\" does not remove\n+\t\t\t * preimage without removing the directory.\n+\t\t\t */\n \t\t\tif (mkdir_in_gitdir(git_path(\"rr-cache/%s\", hex)))\n \t\t\t\tcontinue;\n+\n+\t\t\t/*\n+\t\t\t * We are the first to encounter this\n+\t\t\t * conflict.  Ask handle_file() to write the\n+\t\t\t * normalized contents to the \"preimage\" file.\n+\t\t\t */\n \t\t\thandle_file(path, NULL, rerere_path(hex, \"preimage\"));\n \t\t\tfprintf(stderr, \"Recorded preimage for '%s'\\n\", path);\n \t\t}\n \t}\n \n \t/*\n-\t * Now some of the paths that had conflicts earlier might have been\n-\t * hand resolved.  Others may be similar to a conflict already that\n-\t * was resolved before.\n+\t * Some of the paths that had conflicts earlier might have\n+\t * been resolved by the user.  Others may be similar to a\n+\t * conflict already that was resolved before.\n \t */\n-\n \tfor (i = 0; i < rr->nr; i++) {\n \t\tint ret;\n \t\tconst char *path = rr->items[i].string;\n \t\tconst char *name = (const char *)rr->items[i].util;\n \n+\t\t/* Is there a recorded resolution we could attempt to apply? */\n \t\tif (has_rerere_resolution(name)) {\n \t\t\tif (merge(name, path))\n \t\t\t\tcontinue;\n@@ -629,13 +693,13 @@ static int do_plain_rerere(struct string_list *rr, int fd)\n \t\t\tgoto mark_resolved;\n \t\t}\n \n-\t\t/* Let's see if we have resolved it. */\n+\t\t/* Let's see if the user has resolved it. */\n \t\tret = handle_file(path, NULL, NULL);\n \t\tif (ret)\n \t\t\tcontinue;\n \n-\t\tfprintf(stderr, \"Recorded resolution for '%s'.\\n\", path);\n \t\tcopy_file(rerere_path(name, \"postimage\"), path, 0666);\n+\t\tfprintf(stderr, \"Recorded resolution for '%s'.\\n\", path);\n \tmark_resolved:\n \t\tfree(rr->items[i].util);\n \t\trr->items[i].util = NULL;\n@@ -689,6 +753,11 @@ int setup_rerere(struct string_list *merge_rr, int flags)\n \treturn fd;\n }\n \n+/*\n+ * The main entry point that is called internally from codepaths that\n+ * perform mergy operations, possibly leaving conflicted index entries\n+ * and working tree files.\n+ */\n int rerere(int flags)\n {\n \tstruct string_list merge_rr = STRING_LIST_INIT_DUP;\n-- \n2.5.0-rc2-340-g0cccc16\n"},{"id":"266281","messageId":"1437171880-21590-12-git-send-email-gitster@pobox.com","threadId":"39756","inReplyTo":"1437171880-21590-1-git-send-email-gitster@pobox.com","subject":"[PATCH v3 11/18] rerere: explain \"rerere forget\" codepath","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-07-17T22:24:33Z","receivedAt":"2015-07-17T22:24:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Explain the internals of rerere as in-code comments, while\nsprinkling \"NEEDSWORK\" comment to highlight iffy bits and\nquestionable assumptions.\n\nThis covers the codepath that implements \"rerere forget\".\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n rerere.c | 24 ++++++++++++++++++++++++\n 1 file changed, 24 insertions(+)\n\ndiff --git a/rerere.c b/rerere.c\nindex 3d9c33b..3782be6 100644\n--- a/rerere.c\n+++ b/rerere.c\n@@ -413,6 +413,10 @@ static int handle_cache(const char *path, unsigned char *sha1, const char *outpu\n \tstrbuf_init(&io.input, 0);\n \tstrbuf_attach(&io.input, result.ptr, result.size, result.size);\n \n+\t/*\n+\t * Grab the conflict ID and optionally write the original\n+\t * contents with conflict markers out.\n+\t */\n \thunk_no = handle_path(sha1, (struct rerere_io *)&io, marker_size);\n \tstrbuf_release(&io.input);\n \tif (io.io.output)\n@@ -777,9 +781,15 @@ static int rerere_forget_one_path(const char *path, struct string_list *rr)\n \tint ret;\n \tstruct string_list_item *item;\n \n+\t/*\n+\t * Recreate the original conflict from the stages in the\n+\t * index and compute the conflict ID\n+\t */\n \tret = handle_cache(path, sha1, NULL);\n \tif (ret < 1)\n \t\treturn error(\"Could not parse conflict hunks in '%s'\", path);\n+\n+\t/* Nuke the recorded resolution for the conflict */\n \thex = xstrdup(sha1_to_hex(sha1));\n \tfilename = rerere_path(hex, \"postimage\");\n \tif (unlink(filename))\n@@ -787,9 +797,18 @@ static int rerere_forget_one_path(const char *path, struct string_list *rr)\n \t\t\t? error(\"no remembered resolution for %s\", path)\n \t\t\t: error(\"cannot unlink %s: %s\", filename, strerror(errno)));\n \n+\t/*\n+\t * Update the preimage so that the user can resolve the\n+\t * conflict in the working tree, run us again to record\n+\t * the postimage.\n+\t */\n \thandle_cache(path, sha1, rerere_path(hex, \"preimage\"));\n \tfprintf(stderr, \"Updated preimage for '%s'\\n\", path);\n \n+\t/*\n+\t * And remember that we can record resolution for this\n+\t * conflict when the user is done.\n+\t */\n \titem = string_list_insert(rr, path);\n \tfree(item->util);\n \titem->util = hex;\n@@ -808,6 +827,11 @@ int rerere_forget(struct pathspec *pathspec)\n \n \tfd = setup_rerere(&merge_rr, RERERE_NOAUTOUPDATE);\n \n+\t/*\n+\t * The paths may have been resolved (incorrectly);\n+\t * recover the original conflicted state and then\n+\t * find the conflicted paths.\n+\t */\n \tunmerge_cache(pathspec);\n \tfind_conflict(&conflict);\n \tfor (i = 0; i < conflict.nr; i++) {\n-- \n2.5.0-rc2-340-g0cccc16\n"},{"id":"266284","messageId":"1437171880-21590-13-git-send-email-gitster@pobox.com","threadId":"39756","inReplyTo":"1437171880-21590-1-git-send-email-gitster@pobox.com","subject":"[PATCH v3 12/18] rerere: explain the remainder","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-07-17T22:24:34Z","receivedAt":"2015-07-17T22:24:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Explain the internals of rerere as in-code comments, while\nsprinkling \"NEEDSWORK\" comment to highlight iffy bits and\nquestionable assumptions.\n\nThis covers the codepath that implements \"rerere gc\" and \"rerere\nclear\".\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n rerere.c | 20 ++++++++++++++++++++\n 1 file changed, 20 insertions(+)\n\ndiff --git a/rerere.c b/rerere.c\nindex 3782be6..7ef951e 100644\n--- a/rerere.c\n+++ b/rerere.c\n@@ -844,6 +844,9 @@ int rerere_forget(struct pathspec *pathspec)\n \treturn write_rr(&merge_rr, fd);\n }\n \n+/*\n+ * Garbage collection support\n+ */\n static time_t rerere_created_at(const char *name)\n {\n \tstruct stat st;\n@@ -856,11 +859,19 @@ static time_t rerere_last_used_at(const char *name)\n \treturn stat(rerere_path(name, \"postimage\"), &st) ? (time_t) 0 : st.st_mtime;\n }\n \n+/*\n+ * Remove the recorded resolution for a given conflict ID\n+ */\n static void unlink_rr_item(const char *name)\n {\n \tunlink(rerere_path(name, \"thisimage\"));\n \tunlink(rerere_path(name, \"preimage\"));\n \tunlink(rerere_path(name, \"postimage\"));\n+\t/*\n+\t * NEEDSWORK: what if this rmdir() fails?  Wouldn't we then\n+\t * assume that we already have preimage recorded in\n+\t * do_plain_rerere()?\n+\t */\n \trmdir(git_path(\"rr-cache/%s\", name));\n }\n \n@@ -880,6 +891,7 @@ void rerere_gc(struct string_list *rr)\n \tdir = opendir(git_path(\"rr-cache\"));\n \tif (!dir)\n \t\tdie_errno(\"unable to open rr-cache directory\");\n+\t/* Collect stale conflict IDs ... */\n \twhile ((e = readdir(dir))) {\n \t\tif (is_dot_or_dotdot(e->d_name))\n \t\t\tcontinue;\n@@ -897,11 +909,19 @@ void rerere_gc(struct string_list *rr)\n \t\t\tstring_list_append(&to_remove, e->d_name);\n \t}\n \tclosedir(dir);\n+\t/* ... and then remove them one-by-one */\n \tfor (i = 0; i < to_remove.nr; i++)\n \t\tunlink_rr_item(to_remove.items[i].string);\n \tstring_list_clear(&to_remove, 0);\n }\n \n+/*\n+ * During a conflict resolution, after \"rerere\" recorded the\n+ * preimages, abandon them if the user did not resolve them or\n+ * record their resolutions.  And drop $GIT_DIR/MERGE_RR.\n+ *\n+ * NEEDSWORK: shouldn't we be calling this from \"reset --hard\"?\n+ */\n void rerere_clear(struct string_list *merge_rr)\n {\n \tint i;\n-- \n2.5.0-rc2-340-g0cccc16\n"},{"id":"266285","messageId":"1437171880-21590-14-git-send-email-gitster@pobox.com","threadId":"39756","inReplyTo":"1437171880-21590-1-git-send-email-gitster@pobox.com","subject":"[PATCH v3 13/18] rerere: refactor \"replay\" part of do_plain_rerere()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-07-17T22:24:35Z","receivedAt":"2015-07-17T22:24:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Extract the body of a loop that attempts to replay recorded\nresolution for each conflicted path into a helper function, not\nbecause I want to call it from multiple places later, but because\nthe logic has become too deeply nested and hard to read.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n rerere.c | 75 ++++++++++++++++++++++++++++++++++------------------------------\n 1 file changed, 40 insertions(+), 35 deletions(-)\n\ndiff --git a/rerere.c b/rerere.c\nindex 7ef951e..09b72ed 100644\n--- a/rerere.c\n+++ b/rerere.c\n@@ -620,6 +620,44 @@ static void update_paths(struct string_list *update)\n \t\trollback_lock_file(&index_lock);\n }\n \n+/*\n+ * The path indicated by rr_item may still have conflict for which we\n+ * have a recorded resolution, in which case replay it and optionally\n+ * update it.  Or it may have been resolved by the user and we may\n+ * only have the preimage for that conflict, in which case the result\n+ * needs to be recorded as a resolution in a postimage file.\n+ */\n+static void do_rerere_one_path(struct string_list_item *rr_item,\n+\t\t\t       struct string_list *update)\n+{\n+\tconst char *path = rr_item->string;\n+\tconst char *name = (const char *)rr_item->util;\n+\n+\t/* Is there a recorded resolution we could attempt to apply? */\n+\tif (has_rerere_resolution(name)) {\n+\t\tif (merge(name, path))\n+\t\t\treturn; /* failed to replay */\n+\n+\t\tif (rerere_autoupdate)\n+\t\t\tstring_list_insert(update, path);\n+\t\telse\n+\t\t\tfprintf(stderr,\n+\t\t\t\t\"Resolved '%s' using previous resolution.\\n\",\n+\t\t\t\tpath);\n+\t\tgoto mark_resolved;\n+\t}\n+\n+\t/* Let's see if the user has resolved it. */\n+\tif (handle_file(path, NULL, NULL))\n+\t\treturn; /* not yet resolved */\n+\n+\tcopy_file(rerere_path(name, \"postimage\"), path, 0666);\n+\tfprintf(stderr, \"Recorded resolution for '%s'.\\n\", path);\n+mark_resolved:\n+\tfree(rr_item->util);\n+\trr_item->util = NULL;\n+}\n+\n static int do_plain_rerere(struct string_list *rr, int fd)\n {\n \tstruct string_list conflict = STRING_LIST_INIT_DUP;\n@@ -673,41 +711,8 @@ static int do_plain_rerere(struct string_list *rr, int fd)\n \t\t}\n \t}\n \n-\t/*\n-\t * Some of the paths that had conflicts earlier might have\n-\t * been resolved by the user.  Others may be similar to a\n-\t * conflict already that was resolved before.\n-\t */\n-\tfor (i = 0; i < rr->nr; i++) {\n-\t\tint ret;\n-\t\tconst char *path = rr->items[i].string;\n-\t\tconst char *name = (const char *)rr->items[i].util;\n-\n-\t\t/* Is there a recorded resolution we could attempt to apply? */\n-\t\tif (has_rerere_resolution(name)) {\n-\t\t\tif (merge(name, path))\n-\t\t\t\tcontinue;\n-\n-\t\t\tif (rerere_autoupdate)\n-\t\t\t\tstring_list_insert(&update, path);\n-\t\t\telse\n-\t\t\t\tfprintf(stderr,\n-\t\t\t\t\t\"Resolved '%s' using previous resolution.\\n\",\n-\t\t\t\t\tpath);\n-\t\t\tgoto mark_resolved;\n-\t\t}\n-\n-\t\t/* Let's see if the user has resolved it. */\n-\t\tret = handle_file(path, NULL, NULL);\n-\t\tif (ret)\n-\t\t\tcontinue;\n-\n-\t\tcopy_file(rerere_path(name, \"postimage\"), path, 0666);\n-\t\tfprintf(stderr, \"Recorded resolution for '%s'.\\n\", path);\n-\tmark_resolved:\n-\t\tfree(rr->items[i].util);\n-\t\trr->items[i].util = NULL;\n-\t}\n+\tfor (i = 0; i < rr->nr; i++)\n+\t\tdo_rerere_one_path(&rr->items[i], &update);\n \n \tif (update.nr)\n \t\tupdate_paths(&update);\n-- \n2.5.0-rc2-340-g0cccc16\n"},{"id":"266286","messageId":"1437171880-21590-15-git-send-email-gitster@pobox.com","threadId":"39756","inReplyTo":"1437171880-21590-1-git-send-email-gitster@pobox.com","subject":"[PATCH v3 14/18] rerere: further de-dent do_plain_rerere()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-07-17T22:24:36Z","receivedAt":"2015-07-17T22:24:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"It's just easier to follow this way.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n rerere.c | 69 ++++++++++++++++++++++++++++++++--------------------------------\n 1 file changed, 35 insertions(+), 34 deletions(-)\n\ndiff --git a/rerere.c b/rerere.c\nindex 09b72ed..1089a9c 100644\n--- a/rerere.c\n+++ b/rerere.c\n@@ -673,42 +673,43 @@ static int do_plain_rerere(struct string_list *rr, int fd)\n \t * initial run would catch all and register their preimages.\n \t */\n \tfor (i = 0; i < conflict.nr; i++) {\n+\t\tunsigned char sha1[20];\n+\t\tchar *hex;\n+\t\tint ret;\n \t\tconst char *path = conflict.items[i].string;\n-\t\tif (!string_list_has_string(rr, path)) {\n-\t\t\tunsigned char sha1[20];\n-\t\t\tchar *hex;\n-\t\t\tint ret;\n-\n-\t\t\t/*\n-\t\t\t * Ask handle_file() to scan and assign a\n-\t\t\t * conflict ID.  No need to write anything out\n-\t\t\t * yet.\n-\t\t\t */\n-\t\t\tret = handle_file(path, sha1, NULL);\n-\t\t\tif (ret < 1)\n-\t\t\t\tcontinue;\n-\t\t\thex = xstrdup(sha1_to_hex(sha1));\n-\t\t\tstring_list_insert(rr, path)->util = hex;\n-\n-\t\t\t/*\n-\t\t\t * If the directory does not exist, create\n-\t\t\t * it.  mkdir_in_gitdir() will fail with\n-\t\t\t * EEXIST if there already is one.\n-\t\t\t *\n-\t\t\t * NEEDSWORK: make sure \"gc\" does not remove\n-\t\t\t * preimage without removing the directory.\n-\t\t\t */\n-\t\t\tif (mkdir_in_gitdir(git_path(\"rr-cache/%s\", hex)))\n-\t\t\t\tcontinue;\n \n-\t\t\t/*\n-\t\t\t * We are the first to encounter this\n-\t\t\t * conflict.  Ask handle_file() to write the\n-\t\t\t * normalized contents to the \"preimage\" file.\n-\t\t\t */\n-\t\t\thandle_file(path, NULL, rerere_path(hex, \"preimage\"));\n-\t\t\tfprintf(stderr, \"Recorded preimage for '%s'\\n\", path);\n-\t\t}\n+\t\tif (string_list_has_string(rr, path))\n+\t\t\tcontinue;\n+\n+\t\t/*\n+\t\t * Ask handle_file() to scan and assign a\n+\t\t * conflict ID.  No need to write anything out\n+\t\t * yet.\n+\t\t */\n+\t\tret = handle_file(path, sha1, NULL);\n+\t\tif (ret < 1)\n+\t\t\tcontinue;\n+\t\thex = xstrdup(sha1_to_hex(sha1));\n+\t\tstring_list_insert(rr, path)->util = hex;\n+\n+\t\t/*\n+\t\t * If the directory does not exist, create\n+\t\t * it.  mkdir_in_gitdir() will fail with\n+\t\t * EEXIST if there already is one.\n+\t\t *\n+\t\t * NEEDSWORK: make sure \"gc\" does not remove\n+\t\t * preimage without removing the directory.\n+\t\t */\n+\t\tif (mkdir_in_gitdir(git_path(\"rr-cache/%s\", hex)))\n+\t\t\tcontinue;\n+\n+\t\t/*\n+\t\t * We are the first to encounter this\n+\t\t * conflict.  Ask handle_file() to write the\n+\t\t * normalized contents to the \"preimage\" file.\n+\t\t */\n+\t\thandle_file(path, NULL, rerere_path(hex, \"preimage\"));\n+\t\tfprintf(stderr, \"Recorded preimage for '%s'\\n\", path);\n \t}\n \n \tfor (i = 0; i < rr->nr; i++)\n-- \n2.5.0-rc2-340-g0cccc16\n"},{"id":"266282","messageId":"1437171880-21590-16-git-send-email-gitster@pobox.com","threadId":"39756","inReplyTo":"1437171880-21590-1-git-send-email-gitster@pobox.com","subject":"[PATCH v3 15/18] rerere: further clarify do_rerere_one_path()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-07-17T22:24:37Z","receivedAt":"2015-07-17T22:24:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Signed-off-by: Junio C Hamano <gitster@pobox.com>\n---\n rerere.c | 15 ++++++---------\n 1 file changed, 6 insertions(+), 9 deletions(-)\n\ndiff --git a/rerere.c b/rerere.c\nindex 1089a9c..30bdfeb 100644\n--- a/rerere.c\n+++ b/rerere.c\n@@ -644,16 +644,13 @@ static void do_rerere_one_path(struct string_list_item *rr_item,\n \t\t\tfprintf(stderr,\n \t\t\t\t\"Resolved '%s' using previous resolution.\\n\",\n \t\t\t\tpath);\n-\t\tgoto mark_resolved;\n+\t} else if (!handle_file(path, NULL, NULL)) {\n+\t\t/* The user has resolved it. */\n+\t\tcopy_file(rerere_path(name, \"postimage\"), path, 0666);\n+\t\tfprintf(stderr, \"Recorded resolution for '%s'.\\n\", path);\n+\t} else {\n+\t\treturn;\n \t}\n-\n-\t/* Let's see if the user has resolved it. */\n-\tif (handle_file(path, NULL, NULL))\n-\t\treturn; /* not yet resolved */\n-\n-\tcopy_file(rerere_path(name, \"postimage\"), path, 0666);\n-\tfprintf(stderr, \"Recorded resolution for '%s'.\\n\", path);\n-mark_resolved:\n \tfree(rr_item->util);\n \trr_item->util = NULL;\n }\n-- \n2.5.0-rc2-340-g0cccc16\n"},{"id":"266291","messageId":"1437171880-21590-17-git-send-email-gitster@pobox.com","threadId":"39756","inReplyTo":"1437171880-21590-1-git-send-email-gitster@pobox.com","subject":"[PATCH v3 16/18] rerere: call conflict-ids IDs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-07-17T22:24:38Z","receivedAt":"2015-07-17T22:24:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Most places we call conflict IDs \"name\" and some others we call them\n\"hex\"; update all of them to \"id\".\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n builtin/rerere.c |  4 +--\n rerere.c         | 76 ++++++++++++++++++++++++++++----------------------------\n rerere.h         |  2 +-\n 3 files changed, 41 insertions(+), 41 deletions(-)\n\ndiff --git a/builtin/rerere.c b/builtin/rerere.c\nindex 98eb8c5..81730bb 100644\n--- a/builtin/rerere.c\n+++ b/builtin/rerere.c\n@@ -103,8 +103,8 @@ int cmd_rerere(int argc, const char **argv, const char *prefix)\n \t} else if (!strcmp(argv[0], \"diff\"))\n \t\tfor (i = 0; i < merge_rr.nr; i++) {\n \t\t\tconst char *path = merge_rr.items[i].string;\n-\t\t\tconst char *name = (const char *)merge_rr.items[i].util;\n-\t\t\tdiff_two(rerere_path(name, \"preimage\"), path, path, path);\n+\t\t\tconst char *id = (const char *)merge_rr.items[i].util;\n+\t\t\tdiff_two(rerere_path(id, \"preimage\"), path, path, path);\n \t\t}\n \telse\n \t\tusage_with_options(rerere_usage, options);\ndiff --git a/rerere.c b/rerere.c\nindex 30bdfeb..e68469a 100644\n--- a/rerere.c\n+++ b/rerere.c\n@@ -22,15 +22,15 @@ static int rerere_autoupdate;\n \n static char *merge_rr_path;\n \n-const char *rerere_path(const char *hex, const char *file)\n+const char *rerere_path(const char *id, const char *file)\n {\n-\treturn git_path(\"rr-cache/%s/%s\", hex, file);\n+\treturn git_path(\"rr-cache/%s/%s\", id, file);\n }\n \n-static int has_rerere_resolution(const char *hex)\n+static int has_rerere_resolution(const char *id)\n {\n \tstruct stat st;\n-\treturn !stat(rerere_path(hex, \"postimage\"), &st);\n+\treturn !stat(rerere_path(id, \"postimage\"), &st);\n }\n \n /*\n@@ -530,7 +530,7 @@ int rerere_remaining(struct string_list *merge_rr)\n }\n \n /*\n- * Find the conflict identified by \"name\"; the change between its\n+ * Find the conflict identified by \"id\"; the change between its\n  * \"preimage\" (i.e. a previous contents with conflict markers) and its\n  * \"postimage\" (i.e. the corresponding contents with conflicts\n  * resolved) may apply cleanly to the contents stored in \"path\", i.e.\n@@ -539,7 +539,7 @@ int rerere_remaining(struct string_list *merge_rr)\n  * Returns 0 for successful replay of recorded resolution, or non-zero\n  * for failure.\n  */\n-static int merge(const char *name, const char *path)\n+static int merge(const char *id, const char *path)\n {\n \tint ret;\n \tmmfile_t cur = {NULL, 0}, base = {NULL, 0}, other = {NULL, 0};\n@@ -549,12 +549,12 @@ static int merge(const char *name, const char *path)\n \t * Normalize the conflicts in path and write it out to\n \t * \"thisimage\" temporary file.\n \t */\n-\tif (handle_file(path, NULL, rerere_path(name, \"thisimage\")) < 0)\n+\tif (handle_file(path, NULL, rerere_path(id, \"thisimage\")) < 0)\n \t\treturn 1;\n \n-\tif (read_mmfile(&cur, rerere_path(name, \"thisimage\")) ||\n-\t    read_mmfile(&base, rerere_path(name, \"preimage\")) ||\n-\t    read_mmfile(&other, rerere_path(name, \"postimage\"))) {\n+\tif (read_mmfile(&cur, rerere_path(id, \"thisimage\")) ||\n+\t    read_mmfile(&base, rerere_path(id, \"preimage\")) ||\n+\t    read_mmfile(&other, rerere_path(id, \"postimage\"))) {\n \t\tret = 1;\n \t\tgoto out;\n \t}\n@@ -571,9 +571,9 @@ static int merge(const char *name, const char *path)\n \t\t * A successful replay of recorded resolution.\n \t\t * Mark that \"postimage\" was used to help gc.\n \t\t */\n-\t\tif (utime(rerere_path(name, \"postimage\"), NULL) < 0)\n+\t\tif (utime(rerere_path(id, \"postimage\"), NULL) < 0)\n \t\t\twarning(\"failed utime() on %s: %s\",\n-\t\t\t\t\trerere_path(name, \"postimage\"),\n+\t\t\t\t\trerere_path(id, \"postimage\"),\n \t\t\t\t\tstrerror(errno));\n \n \t\t/* Update \"path\" with the resolution */\n@@ -631,11 +631,11 @@ static void do_rerere_one_path(struct string_list_item *rr_item,\n \t\t\t       struct string_list *update)\n {\n \tconst char *path = rr_item->string;\n-\tconst char *name = (const char *)rr_item->util;\n+\tconst char *id = (const char *)rr_item->util;\n \n \t/* Is there a recorded resolution we could attempt to apply? */\n-\tif (has_rerere_resolution(name)) {\n-\t\tif (merge(name, path))\n+\tif (has_rerere_resolution(id)) {\n+\t\tif (merge(id, path))\n \t\t\treturn; /* failed to replay */\n \n \t\tif (rerere_autoupdate)\n@@ -646,7 +646,7 @@ static void do_rerere_one_path(struct string_list_item *rr_item,\n \t\t\t\tpath);\n \t} else if (!handle_file(path, NULL, NULL)) {\n \t\t/* The user has resolved it. */\n-\t\tcopy_file(rerere_path(name, \"postimage\"), path, 0666);\n+\t\tcopy_file(rerere_path(id, \"postimage\"), path, 0666);\n \t\tfprintf(stderr, \"Recorded resolution for '%s'.\\n\", path);\n \t} else {\n \t\treturn;\n@@ -671,7 +671,7 @@ static int do_plain_rerere(struct string_list *rr, int fd)\n \t */\n \tfor (i = 0; i < conflict.nr; i++) {\n \t\tunsigned char sha1[20];\n-\t\tchar *hex;\n+\t\tchar *id;\n \t\tint ret;\n \t\tconst char *path = conflict.items[i].string;\n \n@@ -686,8 +686,8 @@ static int do_plain_rerere(struct string_list *rr, int fd)\n \t\tret = handle_file(path, sha1, NULL);\n \t\tif (ret < 1)\n \t\t\tcontinue;\n-\t\thex = xstrdup(sha1_to_hex(sha1));\n-\t\tstring_list_insert(rr, path)->util = hex;\n+\t\tid = xstrdup(sha1_to_hex(sha1));\n+\t\tstring_list_insert(rr, path)->util = id;\n \n \t\t/*\n \t\t * If the directory does not exist, create\n@@ -697,7 +697,7 @@ static int do_plain_rerere(struct string_list *rr, int fd)\n \t\t * NEEDSWORK: make sure \"gc\" does not remove\n \t\t * preimage without removing the directory.\n \t\t */\n-\t\tif (mkdir_in_gitdir(git_path(\"rr-cache/%s\", hex)))\n+\t\tif (mkdir_in_gitdir(git_path(\"rr-cache/%s\", id)))\n \t\t\tcontinue;\n \n \t\t/*\n@@ -705,7 +705,7 @@ static int do_plain_rerere(struct string_list *rr, int fd)\n \t\t * conflict.  Ask handle_file() to write the\n \t\t * normalized contents to the \"preimage\" file.\n \t\t */\n-\t\thandle_file(path, NULL, rerere_path(hex, \"preimage\"));\n+\t\thandle_file(path, NULL, rerere_path(id, \"preimage\"));\n \t\tfprintf(stderr, \"Recorded preimage for '%s'\\n\", path);\n \t}\n \n@@ -779,7 +779,7 @@ int rerere(int flags)\n static int rerere_forget_one_path(const char *path, struct string_list *rr)\n {\n \tconst char *filename;\n-\tchar *hex;\n+\tchar *id;\n \tunsigned char sha1[20];\n \tint ret;\n \tstruct string_list_item *item;\n@@ -793,8 +793,8 @@ static int rerere_forget_one_path(const char *path, struct string_list *rr)\n \t\treturn error(\"Could not parse conflict hunks in '%s'\", path);\n \n \t/* Nuke the recorded resolution for the conflict */\n-\thex = xstrdup(sha1_to_hex(sha1));\n-\tfilename = rerere_path(hex, \"postimage\");\n+\tid = xstrdup(sha1_to_hex(sha1));\n+\tfilename = rerere_path(id, \"postimage\");\n \tif (unlink(filename))\n \t\treturn (errno == ENOENT\n \t\t\t? error(\"no remembered resolution for %s\", path)\n@@ -805,7 +805,7 @@ static int rerere_forget_one_path(const char *path, struct string_list *rr)\n \t * conflict in the working tree, run us again to record\n \t * the postimage.\n \t */\n-\thandle_cache(path, sha1, rerere_path(hex, \"preimage\"));\n+\thandle_cache(path, sha1, rerere_path(id, \"preimage\"));\n \tfprintf(stderr, \"Updated preimage for '%s'\\n\", path);\n \n \t/*\n@@ -814,7 +814,7 @@ static int rerere_forget_one_path(const char *path, struct string_list *rr)\n \t */\n \titem = string_list_insert(rr, path);\n \tfree(item->util);\n-\titem->util = hex;\n+\titem->util = id;\n \tfprintf(stderr, \"Forgot resolution for %s\\n\", path);\n \treturn 0;\n }\n@@ -850,32 +850,32 @@ int rerere_forget(struct pathspec *pathspec)\n /*\n  * Garbage collection support\n  */\n-static time_t rerere_created_at(const char *name)\n+static time_t rerere_created_at(const char *id)\n {\n \tstruct stat st;\n-\treturn stat(rerere_path(name, \"preimage\"), &st) ? (time_t) 0 : st.st_mtime;\n+\treturn stat(rerere_path(id, \"preimage\"), &st) ? (time_t) 0 : st.st_mtime;\n }\n \n-static time_t rerere_last_used_at(const char *name)\n+static time_t rerere_last_used_at(const char *id)\n {\n \tstruct stat st;\n-\treturn stat(rerere_path(name, \"postimage\"), &st) ? (time_t) 0 : st.st_mtime;\n+\treturn stat(rerere_path(id, \"postimage\"), &st) ? (time_t) 0 : st.st_mtime;\n }\n \n /*\n  * Remove the recorded resolution for a given conflict ID\n  */\n-static void unlink_rr_item(const char *name)\n+static void unlink_rr_item(const char *id)\n {\n-\tunlink(rerere_path(name, \"thisimage\"));\n-\tunlink(rerere_path(name, \"preimage\"));\n-\tunlink(rerere_path(name, \"postimage\"));\n+\tunlink(rerere_path(id, \"thisimage\"));\n+\tunlink(rerere_path(id, \"preimage\"));\n+\tunlink(rerere_path(id, \"postimage\"));\n \t/*\n \t * NEEDSWORK: what if this rmdir() fails?  Wouldn't we then\n \t * assume that we already have preimage recorded in\n \t * do_plain_rerere()?\n \t */\n-\trmdir(git_path(\"rr-cache/%s\", name));\n+\trmdir(git_path(\"rr-cache/%s\", id));\n }\n \n void rerere_gc(struct string_list *rr)\n@@ -930,9 +930,9 @@ void rerere_clear(struct string_list *merge_rr)\n \tint i;\n \n \tfor (i = 0; i < merge_rr->nr; i++) {\n-\t\tconst char *name = (const char *)merge_rr->items[i].util;\n-\t\tif (!has_rerere_resolution(name))\n-\t\t\tunlink_rr_item(name);\n+\t\tconst char *id = (const char *)merge_rr->items[i].util;\n+\t\tif (!has_rerere_resolution(id))\n+\t\t\tunlink_rr_item(id);\n \t}\n \tunlink_or_warn(git_path(\"MERGE_RR\"));\n }\ndiff --git a/rerere.h b/rerere.h\nindex 2956c2e..f998eba 100644\n--- a/rerere.h\n+++ b/rerere.h\n@@ -17,7 +17,7 @@ extern void *RERERE_RESOLVED;\n \n extern int setup_rerere(struct string_list *, int);\n extern int rerere(int);\n-extern const char *rerere_path(const char *hex, const char *file);\n+extern const char *rerere_path(const char *id, const char *file);\n extern int rerere_forget(struct pathspec *);\n extern int rerere_remaining(struct string_list *);\n extern void rerere_clear(struct string_list *);\n-- \n2.5.0-rc2-340-g0cccc16\n"},{"id":"266287","messageId":"1437171880-21590-18-git-send-email-gitster@pobox.com","threadId":"39756","inReplyTo":"1437171880-21590-1-git-send-email-gitster@pobox.com","subject":"[PATCH v3 17/18] rerere: use \"struct rerere_id\" instead of \"char *\" for conflict ID","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-07-17T22:24:39Z","receivedAt":"2015-07-17T22:24:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This gives a thin abstraction between the conflict ID that is a hash\nvalue obtained by inspecting the conflicts and the name of the\ndirectory under $GIT_DIR/rr-cache/, in which the previous resolution\nis recorded to be replayed.  The plan is to make sure that the\npresense of the directory does not imply the presense of a previous\nresolution and vice-versa, and later allow us to have more than one\npair of <preimage, postimage> for a given conflict ID.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n builtin/rerere.c |  2 +-\n rerere.c         | 99 +++++++++++++++++++++++++++++++++++++++++---------------\n rerere.h         | 12 ++++++-\n 3 files changed, 85 insertions(+), 28 deletions(-)\n\ndiff --git a/builtin/rerere.c b/builtin/rerere.c\nindex 81730bb..fd229a7 100644\n--- a/builtin/rerere.c\n+++ b/builtin/rerere.c\n@@ -103,7 +103,7 @@ int cmd_rerere(int argc, const char **argv, const char *prefix)\n \t} else if (!strcmp(argv[0], \"diff\"))\n \t\tfor (i = 0; i < merge_rr.nr; i++) {\n \t\t\tconst char *path = merge_rr.items[i].string;\n-\t\t\tconst char *id = (const char *)merge_rr.items[i].util;\n+\t\t\tconst struct rerere_id *id = merge_rr.items[i].util;\n \t\t\tdiff_two(rerere_path(id, \"preimage\"), path, path, path);\n \t\t}\n \telse\ndiff --git a/rerere.c b/rerere.c\nindex e68469a..469d8a8 100644\n--- a/rerere.c\n+++ b/rerere.c\n@@ -22,17 +22,43 @@ static int rerere_autoupdate;\n \n static char *merge_rr_path;\n \n-const char *rerere_path(const char *id, const char *file)\n+static void free_rerere_id(struct string_list_item *item)\n {\n-\treturn git_path(\"rr-cache/%s/%s\", id, file);\n+\tfree(item->util);\n+}\n+\n+static const char *rerere_id_hex(const struct rerere_id *id)\n+{\n+\treturn id->hex;\n+}\n+\n+const char *rerere_path(const struct rerere_id *id, const char *file)\n+{\n+\tif (file)\n+\t\treturn git_path(\"rr-cache/%s/%s\", rerere_id_hex(id), file);\n+\telse\n+\t\treturn git_path(\"rr-cache/%s\", rerere_id_hex(id));\n }\n \n-static int has_rerere_resolution(const char *id)\n+static int has_rerere_resolution(const struct rerere_id *id)\n {\n \tstruct stat st;\n+\n \treturn !stat(rerere_path(id, \"postimage\"), &st);\n }\n \n+static struct rerere_id *new_rerere_id_hex(char *hex)\n+{\n+\tstruct rerere_id *id = xmalloc(sizeof(*id));\n+\tstrcpy(id->hex, hex);\n+\treturn id;\n+}\n+\n+static struct rerere_id *new_rerere_id(unsigned char *sha1)\n+{\n+\treturn new_rerere_id_hex(sha1_to_hex(sha1));\n+}\n+\n /*\n  * $GIT_DIR/MERGE_RR file is a collection of records, each of which is\n  * \"conflict ID\", a HT and pathname, terminated with a NUL, and is\n@@ -50,6 +76,7 @@ static void read_rr(struct string_list *rr)\n \twhile (!strbuf_getwholeline(&buf, in, '\\0')) {\n \t\tchar *path;\n \t\tunsigned char sha1[20];\n+\t\tstruct rerere_id *id;\n \n \t\t/* There has to be the hash, tab, path and then NUL */\n \t\tif (buf.len < 42 || get_sha1_hex(buf.buf, sha1))\n@@ -59,8 +86,8 @@ static void read_rr(struct string_list *rr)\n \t\t\tdie(\"corrupt MERGE_RR\");\n \t\tbuf.buf[40] = '\\0';\n \t\tpath = buf.buf + 41;\n-\n-\t\tstring_list_insert(rr, path)->util = xstrdup(buf.buf);\n+\t\tid = new_rerere_id_hex(buf.buf);\n+\t\tstring_list_insert(rr, path)->util = id;\n \t}\n \tstrbuf_release(&buf);\n \tfclose(in);\n@@ -73,12 +100,15 @@ static int write_rr(struct string_list *rr, int out_fd)\n \tint i;\n \tfor (i = 0; i < rr->nr; i++) {\n \t\tstruct strbuf buf = STRBUF_INIT;\n+\t\tstruct rerere_id *id;\n \n \t\tassert(rr->items[i].util != RERERE_RESOLVED);\n-\t\tif (!rr->items[i].util)\n+\n+\t\tid = rr->items[i].util;\n+\t\tif (!id)\n \t\t\tcontinue;\n \t\tstrbuf_addf(&buf, \"%s\\t%s%c\",\n-\t\t\t    (char *)rr->items[i].util,\n+\t\t\t    rerere_id_hex(id),\n \t\t\t    rr->items[i].string, 0);\n \t\tif (write_in_full(out_fd, buf.buf, buf.len) != buf.len)\n \t\t\tdie(\"unable to write rerere record\");\n@@ -521,7 +551,7 @@ int rerere_remaining(struct string_list *merge_rr)\n \t\t\tstruct string_list_item *it;\n \t\t\tit = string_list_lookup(merge_rr, (const char *)e->name);\n \t\t\tif (it != NULL) {\n-\t\t\t\tfree(it->util);\n+\t\t\t\tfree_rerere_id(it);\n \t\t\t\tit->util = RERERE_RESOLVED;\n \t\t\t}\n \t\t}\n@@ -539,7 +569,7 @@ int rerere_remaining(struct string_list *merge_rr)\n  * Returns 0 for successful replay of recorded resolution, or non-zero\n  * for failure.\n  */\n-static int merge(const char *id, const char *path)\n+static int merge(const struct rerere_id *id, const char *path)\n {\n \tint ret;\n \tmmfile_t cur = {NULL, 0}, base = {NULL, 0}, other = {NULL, 0};\n@@ -573,8 +603,8 @@ static int merge(const char *id, const char *path)\n \t\t */\n \t\tif (utime(rerere_path(id, \"postimage\"), NULL) < 0)\n \t\t\twarning(\"failed utime() on %s: %s\",\n-\t\t\t\t\trerere_path(id, \"postimage\"),\n-\t\t\t\t\tstrerror(errno));\n+\t\t\t\trerere_path(id, \"postimage\"),\n+\t\t\t\tstrerror(errno));\n \n \t\t/* Update \"path\" with the resolution */\n \t\tf = fopen(path, \"w\");\n@@ -631,7 +661,7 @@ static void do_rerere_one_path(struct string_list_item *rr_item,\n \t\t\t       struct string_list *update)\n {\n \tconst char *path = rr_item->string;\n-\tconst char *id = (const char *)rr_item->util;\n+\tconst struct rerere_id *id = rr_item->util;\n \n \t/* Is there a recorded resolution we could attempt to apply? */\n \tif (has_rerere_resolution(id)) {\n@@ -651,7 +681,7 @@ static void do_rerere_one_path(struct string_list_item *rr_item,\n \t} else {\n \t\treturn;\n \t}\n-\tfree(rr_item->util);\n+\tfree_rerere_id(rr_item);\n \trr_item->util = NULL;\n }\n \n@@ -670,10 +700,10 @@ static int do_plain_rerere(struct string_list *rr, int fd)\n \t * initial run would catch all and register their preimages.\n \t */\n \tfor (i = 0; i < conflict.nr; i++) {\n+\t\tstruct rerere_id *id;\n \t\tunsigned char sha1[20];\n-\t\tchar *id;\n-\t\tint ret;\n \t\tconst char *path = conflict.items[i].string;\n+\t\tint ret;\n \n \t\tif (string_list_has_string(rr, path))\n \t\t\tcontinue;\n@@ -686,7 +716,8 @@ static int do_plain_rerere(struct string_list *rr, int fd)\n \t\tret = handle_file(path, sha1, NULL);\n \t\tif (ret < 1)\n \t\t\tcontinue;\n-\t\tid = xstrdup(sha1_to_hex(sha1));\n+\n+\t\tid = new_rerere_id(sha1);\n \t\tstring_list_insert(rr, path)->util = id;\n \n \t\t/*\n@@ -697,7 +728,7 @@ static int do_plain_rerere(struct string_list *rr, int fd)\n \t\t * NEEDSWORK: make sure \"gc\" does not remove\n \t\t * preimage without removing the directory.\n \t\t */\n-\t\tif (mkdir_in_gitdir(git_path(\"rr-cache/%s\", id)))\n+\t\tif (mkdir_in_gitdir(rerere_path(id, NULL)))\n \t\t\tcontinue;\n \n \t\t/*\n@@ -779,7 +810,7 @@ int rerere(int flags)\n static int rerere_forget_one_path(const char *path, struct string_list *rr)\n {\n \tconst char *filename;\n-\tchar *id;\n+\tstruct rerere_id *id;\n \tunsigned char sha1[20];\n \tint ret;\n \tstruct string_list_item *item;\n@@ -793,7 +824,7 @@ static int rerere_forget_one_path(const char *path, struct string_list *rr)\n \t\treturn error(\"Could not parse conflict hunks in '%s'\", path);\n \n \t/* Nuke the recorded resolution for the conflict */\n-\tid = xstrdup(sha1_to_hex(sha1));\n+\tid = new_rerere_id(sha1);\n \tfilename = rerere_path(id, \"postimage\");\n \tif (unlink(filename))\n \t\treturn (errno == ENOENT\n@@ -813,7 +844,7 @@ static int rerere_forget_one_path(const char *path, struct string_list *rr)\n \t * conflict when the user is done.\n \t */\n \titem = string_list_insert(rr, path);\n-\tfree(item->util);\n+\tfree_rerere_id(item);\n \titem->util = id;\n \tfprintf(stderr, \"Forgot resolution for %s\\n\", path);\n \treturn 0;\n@@ -850,22 +881,38 @@ int rerere_forget(struct pathspec *pathspec)\n /*\n  * Garbage collection support\n  */\n-static time_t rerere_created_at(const char *id)\n+\n+/*\n+ * Note that this is not reentrant but is used only one-at-a-time\n+ * so it does not matter right now.\n+ */\n+static struct rerere_id *dirname_to_id(const char *name)\n+{\n+\tstatic struct rerere_id id;\n+\tstrcpy(id.hex, name);\n+\treturn &id;\n+}\n+\n+static time_t rerere_created_at(const char *dir_name)\n {\n \tstruct stat st;\n+\tstruct rerere_id *id = dirname_to_id(dir_name);\n+\n \treturn stat(rerere_path(id, \"preimage\"), &st) ? (time_t) 0 : st.st_mtime;\n }\n \n-static time_t rerere_last_used_at(const char *id)\n+static time_t rerere_last_used_at(const char *dir_name)\n {\n \tstruct stat st;\n+\tstruct rerere_id *id = dirname_to_id(dir_name);\n+\n \treturn stat(rerere_path(id, \"postimage\"), &st) ? (time_t) 0 : st.st_mtime;\n }\n \n /*\n  * Remove the recorded resolution for a given conflict ID\n  */\n-static void unlink_rr_item(const char *id)\n+static void unlink_rr_item(struct rerere_id *id)\n {\n \tunlink(rerere_path(id, \"thisimage\"));\n \tunlink(rerere_path(id, \"preimage\"));\n@@ -875,7 +922,7 @@ static void unlink_rr_item(const char *id)\n \t * assume that we already have preimage recorded in\n \t * do_plain_rerere()?\n \t */\n-\trmdir(git_path(\"rr-cache/%s\", id));\n+\trmdir(rerere_path(id, NULL));\n }\n \n void rerere_gc(struct string_list *rr)\n@@ -914,7 +961,7 @@ void rerere_gc(struct string_list *rr)\n \tclosedir(dir);\n \t/* ... and then remove them one-by-one */\n \tfor (i = 0; i < to_remove.nr; i++)\n-\t\tunlink_rr_item(to_remove.items[i].string);\n+\t\tunlink_rr_item(dirname_to_id(to_remove.items[i].string));\n \tstring_list_clear(&to_remove, 0);\n }\n \n@@ -930,7 +977,7 @@ void rerere_clear(struct string_list *merge_rr)\n \tint i;\n \n \tfor (i = 0; i < merge_rr->nr; i++) {\n-\t\tconst char *id = (const char *)merge_rr->items[i].util;\n+\t\tstruct rerere_id *id = merge_rr->items[i].util;\n \t\tif (!has_rerere_resolution(id))\n \t\t\tunlink_rr_item(id);\n \t}\ndiff --git a/rerere.h b/rerere.h\nindex f998eba..ce545d0 100644\n--- a/rerere.h\n+++ b/rerere.h\n@@ -15,9 +15,19 @@ struct pathspec;\n  */\n extern void *RERERE_RESOLVED;\n \n+struct rerere_id {\n+\tchar hex[41];\n+};\n+\n extern int setup_rerere(struct string_list *, int);\n extern int rerere(int);\n-extern const char *rerere_path(const char *id, const char *file);\n+/*\n+ * Given the conflict ID and the name of a \"file\" used for replaying\n+ * the recorded resolution (e.g. \"preimage\", \"postimage\"), return the\n+ * path to that filesystem entity.  With \"file\" specified with NULL,\n+ * return the path to the directory that houses these files.\n+ */\n+extern const char *rerere_path(const struct rerere_id *, const char *file);\n extern int rerere_forget(struct pathspec *);\n extern int rerere_remaining(struct string_list *);\n extern void rerere_clear(struct string_list *);\n-- \n2.5.0-rc2-340-g0cccc16\n"},{"id":"266283","messageId":"1437171880-21590-19-git-send-email-gitster@pobox.com","threadId":"39756","inReplyTo":"1437171880-21590-1-git-send-email-gitster@pobox.com","subject":"[PATCH v3 18/18] rerere: un-nest merge() further","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-07-17T22:24:40Z","receivedAt":"2015-07-17T22:24:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"By consistently using \"upon failure, set 'ret' and jump to out\"\npattern, flatten the function further.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n rerere.c | 50 ++++++++++++++++++++++++++------------------------\n 1 file changed, 26 insertions(+), 24 deletions(-)\n\ndiff --git a/rerere.c b/rerere.c\nindex 469d8a8..e8d8c02 100644\n--- a/rerere.c\n+++ b/rerere.c\n@@ -571,6 +571,7 @@ int rerere_remaining(struct string_list *merge_rr)\n  */\n static int merge(const struct rerere_id *id, const char *path)\n {\n+\tFILE *f;\n \tint ret;\n \tmmfile_t cur = {NULL, 0}, base = {NULL, 0}, other = {NULL, 0};\n \tmmbuffer_t result = {NULL, 0};\n@@ -579,8 +580,10 @@ static int merge(const struct rerere_id *id, const char *path)\n \t * Normalize the conflicts in path and write it out to\n \t * \"thisimage\" temporary file.\n \t */\n-\tif (handle_file(path, NULL, rerere_path(id, \"thisimage\")) < 0)\n-\t\treturn 1;\n+\tif (handle_file(path, NULL, rerere_path(id, \"thisimage\")) < 0) {\n+\t\tret = 1;\n+\t\tgoto out;\n+\t}\n \n \tif (read_mmfile(&cur, rerere_path(id, \"thisimage\")) ||\n \t    read_mmfile(&base, rerere_path(id, \"preimage\")) ||\n@@ -594,29 +597,28 @@ static int merge(const struct rerere_id *id, const char *path)\n \t * low-level merge driver settings.\n \t */\n \tret = ll_merge(&result, path, &base, NULL, &cur, \"\", &other, \"\", NULL);\n-\tif (!ret) {\n-\t\tFILE *f;\n+\tif (ret)\n+\t\tgoto out;\n \n-\t\t/*\n-\t\t * A successful replay of recorded resolution.\n-\t\t * Mark that \"postimage\" was used to help gc.\n-\t\t */\n-\t\tif (utime(rerere_path(id, \"postimage\"), NULL) < 0)\n-\t\t\twarning(\"failed utime() on %s: %s\",\n-\t\t\t\trerere_path(id, \"postimage\"),\n-\t\t\t\tstrerror(errno));\n-\n-\t\t/* Update \"path\" with the resolution */\n-\t\tf = fopen(path, \"w\");\n-\t\tif (!f)\n-\t\t\treturn error(\"Could not open %s: %s\", path,\n-\t\t\t\t     strerror(errno));\n-\t\tif (fwrite(result.ptr, result.size, 1, f) != 1)\n-\t\t\terror(\"Could not write %s: %s\", path, strerror(errno));\n-\t\tif (fclose(f))\n-\t\t\treturn error(\"Writing %s failed: %s\", path,\n-\t\t\t\t     strerror(errno));\n-\t}\n+\t/*\n+\t * A successful replay of recorded resolution.\n+\t * Mark that \"postimage\" was used to help gc.\n+\t */\n+\tif (utime(rerere_path(id, \"postimage\"), NULL) < 0)\n+\t\twarning(\"failed utime() on %s: %s\",\n+\t\t\trerere_path(id, \"postimage\"),\n+\t\t\tstrerror(errno));\n+\n+\t/* Update \"path\" with the resolution */\n+\tf = fopen(path, \"w\");\n+\tif (!f)\n+\t\treturn error(\"Could not open %s: %s\", path,\n+\t\t\t     strerror(errno));\n+\tif (fwrite(result.ptr, result.size, 1, f) != 1)\n+\t\terror(\"Could not write %s: %s\", path, strerror(errno));\n+\tif (fclose(f))\n+\t\treturn error(\"Writing %s failed: %s\", path,\n+\t\t\t     strerror(errno));\n \n out:\n \tfree(cur.ptr);\n-- \n2.5.0-rc2-340-g0cccc16\n"},{"id":"266322","messageId":"3AAC96959AB14E61B7711B914D963B7F@PhilipOakley","threadId":"39756","inReplyTo":"1437171880-21590-7-git-send-email-gitster@pobox.com","subject":"Re: [PATCH v3 06/18] rerere: drop want_sp parameter from is_cmarker()","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.org","sentAt":"2015-07-18T08:24:04Z","receivedAt":"2015-07-18T08:24:04Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"From: \"Junio C Hamano\" <gitster@pobox.com>\n> As the nature of the conflict marker line determies if there should\n\n\"should be a\"?\ni.e. s/a /be a /    below\n\n> a SP and label after it, the caller shouldn't have to pass the\n> parameter redundantly.\n>\n> Signed-off-by: Junio C Hamano <gitster@pobox.com>\n> ---\n> rerere.c | 27 ++++++++++++++++++++++-----\n> 1 file changed, 22 insertions(+), 5 deletions(-)\n>\n> diff --git a/rerere.c b/rerere.c\n> index 304df02..4c45f55 100644\n> --- a/rerere.c\n> +++ b/rerere.c\n> @@ -148,8 +148,25 @@ static int rerere_file_getline(struct strbuf *sb, \n> struct rerere_io *io_)\n>  return strbuf_getwholeline(sb, io->input, '\\n');\n> }\n>\n> -static int is_cmarker(char *buf, int marker_char, int marker_size, \n> int want_sp)\n> +/*\n> + * Require the exact number of conflict marker letters, no more, no\n> + * less, followed by SP or any whitespace\n> + * (including LF).\n> + */\n> +static int is_cmarker(char *buf, int marker_char, int marker_size)\n> {\n> + int want_sp;\n> +\n> + /*\n> + * The beginning of our version and the end of their version\n> + * always are labeled like \"<<<<< ours\" or \">>>>> theirs\",\n> + * hence we set want_sp for them.  Note that the version from\n> + * the common ancestor in diff3-style output is not always\n> + * labelled (e.g. \"||||| common\" is often seen but \"|||||\"\n> + * alone is also valid), so we do not set want_sp.\n> + */\n> + want_sp = (marker_char == '<') || (marker_char == '>');\n> +\n>  while (marker_size--)\n>  if (*buf++ != marker_char)\n>  return 0;\n> @@ -172,19 +189,19 @@ static int handle_path(unsigned char *sha1, \n> struct rerere_io *io, int marker_siz\n>  git_SHA1_Init(&ctx);\n>\n>  while (!io->getline(&buf, io)) {\n> - if (is_cmarker(buf.buf, '<', marker_size, 1)) {\n> + if (is_cmarker(buf.buf, '<', marker_size)) {\n>  if (hunk != RR_CONTEXT)\n>  goto bad;\n>  hunk = RR_SIDE_1;\n> - } else if (is_cmarker(buf.buf, '|', marker_size, 0)) {\n> + } else if (is_cmarker(buf.buf, '|', marker_size)) {\n>  if (hunk != RR_SIDE_1)\n>  goto bad;\n>  hunk = RR_ORIGINAL;\n> - } else if (is_cmarker(buf.buf, '=', marker_size, 0)) {\n> + } else if (is_cmarker(buf.buf, '=', marker_size)) {\n>  if (hunk != RR_SIDE_1 && hunk != RR_ORIGINAL)\n>  goto bad;\n>  hunk = RR_SIDE_2;\n> - } else if (is_cmarker(buf.buf, '>', marker_size, 1)) {\n> + } else if (is_cmarker(buf.buf, '>', marker_size)) {\n>  if (hunk != RR_SIDE_2)\n>  goto bad;\n>  if (strbuf_cmp(&one, &two) > 0)\n> -- \n> 2.5.0-rc2-340-g0cccc16\n>\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at  http://vger.kernel.org/majordomo-info.html\n> \n"},{"id":"266324","messageId":"CAPig+cSwH_+htWqNphHsoP+SX8YCVS=PgN03m5z3ksSdeJE0kw@mail.gmail.com","threadId":"39756","inReplyTo":"3AAC96959AB14E61B7711B914D963B7F@PhilipOakley","subject":"Re: [PATCH v3 06/18] rerere: drop want_sp parameter from is_cmarker()","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-07-18T08:47:24Z","receivedAt":"2015-07-18T08:47:24Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sat, Jul 18, 2015 at 4:24 AM, Philip Oakley <philipoakley@iee.org> wrote:\n> From: \"Junio C Hamano\" <gitster@pobox.com>\n>> As the nature of the conflict marker line determies if there should\n>\n> \"should be a\"?\n> i.e. s/a /be a /    below\n\nAlso: s/determies/determines/\n"},{"id":"266325","messageId":"CAPig+cRGskV_meE4Fn0FFU-96HJx1yQuEg5zOA2g4N+3+7c4Kw@mail.gmail.com","threadId":"39756","inReplyTo":"1437171880-21590-18-git-send-email-gitster@pobox.com","subject":"Re: [PATCH v3 17/18] rerere: use \"struct rerere_id\" instead of \"char *\" for conflict ID","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-07-18T08:47:59Z","receivedAt":"2015-07-18T08:47:59Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Jul 17, 2015 at 6:24 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> This gives a thin abstraction between the conflict ID that is a hash\n> value obtained by inspecting the conflicts and the name of the\n> directory under $GIT_DIR/rr-cache/, in which the previous resolution\n> is recorded to be replayed.  The plan is to make sure that the\n> presense of the directory does not imply the presense of a previous\n\ns/presense/presence/g\n\n> resolution and vice-versa, and later allow us to have more than one\n> pair of <preimage, postimage> for a given conflict ID.\n>\n> Signed-off-by: Junio C Hamano <gitster@pobox.com>\n"},{"id":"266751","messageId":"xmqq7fppmj95.fsf@gitster.dls.corp.google.com","threadId":"39756","inReplyTo":"1437171880-21590-2-git-send-email-gitster@pobox.com","subject":"Re: [PATCH v3 01/18] rerere: fix an off-by-one non-bug","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-07-24T19:46:14Z","receivedAt":"2015-07-24T19:46:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> When ac49f5ca (rerere \"remaining\", 2011-02-16) split out a new\n> helper function check_one_conflict() out of find_conflict()\n> function, so that the latter will use the returned value from the\n> new helper to update the loop control variable that is an index into\n> active_cache[], the new variable incremented the index by one too\n> many when it found a path with only stage #1 entry at the very end\n> of active_cache[].\n>\n> This \"strange\" return value does not have any effect on the loop\n> control of two callers of this function, as they all notice that\n> active_nr+2 is larger than active_nr just like active_nr+1 is, but\n> nevertheless it puzzles the readers when they are trying to figure\n> out what the function is trying to do.\n>\n> In fact, there is no need to do an early return.  The code that\n> follows after skipping the stage #1 entry is fully prepared to\n> handle a case where the entry is at the very end of active_cache[].\n>\n> Help future readers from unnecessary confusion by dropping an early\n> return.  We skip the stage #1 entry, and if there are stage #2 and\n> stage #3 entries for the same path, we diagnose the path as\n> THREE_STAGED (otherwise we say PUNTED), and then we skip all entries\n> for the same path.\n>\n> Signed-off-by: Junio C Hamano <gitster@pobox.com>\n> ---\n>  rerere.c | 6 ++----\n>  1 file changed, 2 insertions(+), 4 deletions(-)\n>\n> diff --git a/rerere.c b/rerere.c\n> index 31644de..e307711 100644\n> --- a/rerere.c\n> +++ b/rerere.c\n> @@ -369,10 +369,8 @@ static int check_one_conflict(int i, int *type)\n>  \t}\n>  \n>  \t*type = PUNTED;\n> -\tif (ce_stage(e) == 1) {\n> -\t\tif (active_nr <= ++i)\n> -\t\t\treturn i + 1;\n> -\t}\n> +\tif (ce_stage(e) == 1)\n> +\t\ti++;\n\nAs a conflicted index can have multiple stage #1 entries when\ndealing with a criss-cross merge, this should probably be\n\n\twhile (ce_stage(e) == 1)\n        \ti++;\n\ninstead.\n\n>  \t/* Only handle regular files with both stages #2 and #3 */\n>  \tif (i + 1 < active_nr) {\n"},{"id":"266753","messageId":"xmqq380dmiau.fsf@gitster.dls.corp.google.com","threadId":"39756","inReplyTo":"1437171880-21590-8-git-send-email-gitster@pobox.com","subject":"Re: [PATCH v3 07/18] rerere: stop looping unnecessarily","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-07-24T20:06:49Z","receivedAt":"2015-07-24T20:06:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> handle_cache() loops 3 times starting from an index entry that is\n> unmerged, while ignoring an entry for a path that is different from\n> what we are looking for.\n>\n> As the index is sorted, once we see a different path, we know we saw\n> all stages for the path we are interested in.  Just loop while we\n> see the same path and then break, instead of continuing for 3 times.\n>\n> Signed-off-by: Junio C Hamano <gitster@pobox.com>\n> ---\n>  rerere.c | 17 +++++++----------\n>  1 file changed, 7 insertions(+), 10 deletions(-)\n>\n> diff --git a/rerere.c b/rerere.c\n> index 4c45f55..7b1419c 100644\n> --- a/rerere.c\n> +++ b/rerere.c\n> @@ -329,24 +329,21 @@ static int handle_cache(const char *path, unsigned char *sha1, const char *outpu\n>  \t\treturn -1;\n>  \tpos = -pos - 1;\n>  \n> -\tfor (i = 0; i < 3; i++) {\n> +\twhile (pos < active_nr) {\n>  \t\tenum object_type type;\n>  \t\tunsigned long size;\n> -\t\tint j;\n>  \n> -\t\tif (active_nr <= pos)\n> -\t\t\tbreak;\n>  \t\tce = active_cache[pos++];\n>  \t\tif (ce_namelen(ce) != len || memcmp(ce->name, path, len))\n> -\t\t\tcontinue;\n> -\t\tj = ce_stage(ce) - 1;\n> -\t\tmmfile[j].ptr = read_sha1_file(ce->sha1, &type, &size);\n> -\t\tmmfile[j].size = size;\n> +\t\t\tbreak;\n> +\t\ti = ce_stage(ce) - 1;\n> +\t\tmmfile[i].ptr = read_sha1_file(ce->sha1, &type, &size);\n> +\t\tmmfile[i].size = size;\n\nIf the conflicted index has multiple stage #1 entries, this will leak\nwhat was previously read.  As the array is initialized to NULLs, perhaps\n\n\n\ti = ce_stage(ce) - 1;\n        if (!mmfile[i].ptr) {\n\t\tmmfile[i].ptr = read_sha1_file(ce->sha1, &type, &size);\n                mmfile[i].size = size;\n\t}\n\nshould be sufficient.  This does change the semantics, in that we\nused to use the last stage #1 entry as the common ancestor when\ndoing the plain-vanilla three-way merge, but with the leak fix, we\nwill use the first stage #1 entry.  But it does not change it in\nany meaningful way---we are using only one of multiple stage #1\nentries arbitrarily picked either way.\n\n>  \t}\n> -\tfor (i = 0; i < 3; i++) {\n> +\tfor (i = 0; i < 3; i++)\n>  \t\tif (!mmfile[i].ptr && !mmfile[i].size)\n>  \t\t\tmmfile[i].ptr = xstrdup(\"\");\n> -\t}\n> +\n>  \t/*\n>  \t * NEEDSWORK: handle conflicts from merges with\n>  \t * merge.renormalize set, too\n"},{"id":"266757","messageId":"xmqqio99l22p.fsf@gitster.dls.corp.google.com","threadId":"39756","inReplyTo":"1437171880-21590-9-git-send-email-gitster@pobox.com","subject":"Re: [PATCH v3 08/18] rerere: explain the rerere I/O abstraction","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-07-24T20:42:38Z","receivedAt":"2015-07-24T20:42:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> +/*\n> + * Write a conflict marker to io->output (if defined).\n> + */\n>  static void rerere_io_putconflict(int ch, int size, struct rerere_io *io)\n>  {\n>  \tchar buf[64];\n\nThis is unrelated to the topic of this step, but this function seems\nto be very poorly put together with duct tape.\n\n> static void rerere_io_putconflict(int ch, int size, struct rerere_io *io)\n> {\n> \tchar buf[64];\n> \n> \twhile (size) {\n> \t\tif (size < sizeof(buf) - 2) {\n> \t\t\tmemset(buf, ch, size);\n> \t\t\tbuf[size] = '\\n';\n> \t\t\tbuf[size + 1] = '\\0';\n> \t\t\tsize = 0;\n\nThe if() guarding this block has an off-by-one error in the benign\ndirection.  When size is 62, the marker with terminating LF and NUL\nshould still fit within the buf[], so it should use \"<=\", not \"<\",\nto compare.\n\n> \t\t} else {\n> \t\t\tint sz = sizeof(buf) - 1;\n> \t\t\tif (size <= sz)\n> \t\t\t\tsz -= (sz - size) + 1;\n> \t\t\tmemset(buf, ch, sz);\n> \t\t\tbuf[sz] = '\\0';\n> \t\t\tsize -= sz;\n\nThis is an even more awkward construct.  sz is what we have to work\nwith the substring that we cannot emit with a single call (because\nbuf[] is too small), so naturally I would expect it to be more like\n\n\tint to_emit = size;\n        if (sz <= size)\n        \tto_emit = sz;\n\tmemset(buf, ch, to_emit);\n        buf[to_emit] = '\\0';\n\tsize -= to_emit;\n\nwhich makes the \"if (size <= sz)\" in the code look very suspicious.\n\nBut rewriting it to the \"more natural\" version would make it buggy.\nAt a right boundary condition, i.e. size == 63, we may find that the\nconflict marker is too long with LF and NUL to fit in buf[] and come\nhere, and then fill the full conflict marker with NUL just fine in\nbuf[], decrementing size to 0, emit that 63-byte long marker.  The\nnext iteration will exit the loop without emitting the LF.\n\nThe unnatural is what is saving us from such a bug.\n\n\tif (size <= sz)\n\t\tsz -= (sz - size) + 1;\n\nIt ensures that subtraction of sz (i.e. \"to_emit\") from size before\nthe next iteration will never brings size down to zero by reducing\nsz by one too much, forcing another iteration, which will then have\nsize smaller than \"sizeof(buf) - 2\" and have us emit the LF.\n\nNot buggy, but ugly.\n\n> \t\t}\n> \t\trerere_io_putstr(buf, io);\n> \t}\n> }\n"}]}