Merge branch 'jc/merge-refuse-new-root'
[gitweb.git] / rerere.c
index d90057b7695918e260e1778a522011dabaa3a37b..587b7e2717b14748b92a5960f79947b5fba088f0 100644 (file)
--- a/rerere.c
+++ b/rerere.c
@@ -20,45 +20,74 @@ static int rerere_enabled = -1;
 /* automatically update cleanly resolved paths to the index */
 static int rerere_autoupdate;
 
-const char *rerere_path(const char *hex, const char *file)
+static void free_rerere_id(struct string_list_item *item)
 {
-       return git_path("rr-cache/%s/%s", hex, file);
+       free(item->util);
 }
 
-static int has_rerere_resolution(const char *hex)
+static const char *rerere_id_hex(const struct rerere_id *id)
+{
+       return id->hex;
+}
+
+const char *rerere_path(const struct rerere_id *id, const char *file)
+{
+       if (!file)
+               return git_path("rr-cache/%s", rerere_id_hex(id));
+
+       return git_path("rr-cache/%s/%s", rerere_id_hex(id), file);
+}
+
+static int has_rerere_resolution(const struct rerere_id *id)
 {
        struct stat st;
-       return !stat(rerere_path(hex, "postimage"), &st);
+
+       return !stat(rerere_path(id, "postimage"), &st);
+}
+
+static struct rerere_id *new_rerere_id_hex(char *hex)
+{
+       struct rerere_id *id = xmalloc(sizeof(*id));
+       xsnprintf(id->hex, sizeof(id->hex), "%s", hex);
+       return id;
+}
+
+static struct rerere_id *new_rerere_id(unsigned char *sha1)
+{
+       return new_rerere_id_hex(sha1_to_hex(sha1));
 }
 
+/*
+ * $GIT_DIR/MERGE_RR file is a collection of records, each of which is
+ * "conflict ID", a HT and pathname, terminated with a NUL, and is
+ * used to keep track of the set of paths that "rerere" may need to
+ * work on (i.e. what is left by the previous invocation of "git
+ * rerere" during the current conflict resolution session).
+ */
 static void read_rr(struct string_list *rr)
 {
-       unsigned char sha1[20];
-       char buf[PATH_MAX];
+       struct strbuf buf = STRBUF_INIT;
        FILE *in = fopen(git_path_merge_rr(), "r");
+
        if (!in)
                return;
-       while (fread(buf, 40, 1, in) == 1) {
-               int i;
-               char *name;
-               if (get_sha1_hex(buf, sha1))
+       while (!strbuf_getwholeline(&buf, in, '\0')) {
+               char *path;
+               unsigned char sha1[20];
+               struct rerere_id *id;
+
+               /* There has to be the hash, tab, path and then NUL */
+               if (buf.len < 42 || get_sha1_hex(buf.buf, sha1))
                        die("corrupt MERGE_RR");
-               buf[40] = '\0';
-               name = xstrdup(buf);
-               if (fgetc(in) != '\t')
+
+               if (buf.buf[40] != '\t')
                        die("corrupt MERGE_RR");
-               for (i = 0; i < sizeof(buf); i++) {
-                       int c = fgetc(in);
-                       if (c < 0)
-                               die("corrupt MERGE_RR");
-                       buf[i] = c;
-                       if (c == 0)
-                                break;
-               }
-               if (i == sizeof(buf))
-                       die("filename too long");
-               string_list_insert(rr, buf)->util = name;
+               buf.buf[40] = '\0';
+               path = buf.buf + 41;
+               id = new_rerere_id_hex(buf.buf);
+               string_list_insert(rr, path)->util = id;
        }
+       strbuf_release(&buf);
        fclose(in);
 }
 
@@ -68,22 +97,42 @@ static int write_rr(struct string_list *rr, int out_fd)
 {
        int i;
        for (i = 0; i < rr->nr; i++) {
-               const char *path;
-               int length;
-               if (!rr->items[i].util)
+               struct strbuf buf = STRBUF_INIT;
+               struct rerere_id *id;
+
+               assert(rr->items[i].util != RERERE_RESOLVED);
+
+               id = rr->items[i].util;
+               if (!id)
                        continue;
-               path = rr->items[i].string;
-               length = strlen(path) + 1;
-               if (write_in_full(out_fd, rr->items[i].util, 40) != 40 ||
-                   write_str_in_full(out_fd, "\t") != 1 ||
-                   write_in_full(out_fd, path, length) != length)
+               strbuf_addf(&buf, "%s\t%s%c",
+                           rerere_id_hex(id),
+                           rr->items[i].string, 0);
+               if (write_in_full(out_fd, buf.buf, buf.len) != buf.len)
                        die("unable to write rerere record");
+
+               strbuf_release(&buf);
        }
        if (commit_lock_file(&write_lock) != 0)
                die("unable to write rerere record");
        return 0;
 }
 
+/*
+ * "rerere" interacts with conflicted file contents using this I/O
+ * abstraction.  It reads a conflicted contents from one place via
+ * "getline()" method, and optionally can write it out after
+ * normalizing the conflicted hunks to the "output".  Subclasses of
+ * rerere_io embed this structure at the beginning of their own
+ * rerere_io object.
+ */
+struct rerere_io {
+       int (*getline)(struct strbuf *, struct rerere_io *);
+       FILE *output;
+       int wrerror;
+       /* some more stuff */
+};
+
 static void ferr_write(const void *p, size_t count, FILE *fp, int *err)
 {
        if (!count || *err)
@@ -97,31 +146,34 @@ static inline void ferr_puts(const char *s, FILE *fp, int *err)
        ferr_write(s, strlen(s), fp, err);
 }
 
-struct rerere_io {
-       int (*getline)(struct strbuf *, struct rerere_io *);
-       FILE *output;
-       int wrerror;
-       /* some more stuff */
-};
-
 static void rerere_io_putstr(const char *str, struct rerere_io *io)
 {
        if (io->output)
                ferr_puts(str, io->output, &io->wrerror);
 }
 
+/*
+ * Write a conflict marker to io->output (if defined).
+ */
 static void rerere_io_putconflict(int ch, int size, struct rerere_io *io)
 {
        char buf[64];
 
        while (size) {
-               if (size < sizeof(buf) - 2) {
+               if (size <= sizeof(buf) - 2) {
                        memset(buf, ch, size);
                        buf[size] = '\n';
                        buf[size + 1] = '\0';
                        size = 0;
                } else {
                        int sz = sizeof(buf) - 1;
+
+                       /*
+                        * Make sure we will not write everything out
+                        * in this round by leaving at least 1 byte
+                        * for the next round, giving the next round
+                        * a chance to add the terminating LF.  Yuck.
+                        */
                        if (size <= sz)
                                sz -= (sz - size) + 1;
                        memset(buf, ch, sz);
@@ -138,19 +190,42 @@ static void rerere_io_putmem(const char *mem, size_t sz, struct rerere_io *io)
                ferr_write(mem, sz, io->output, &io->wrerror);
 }
 
+/*
+ * Subclass of rerere_io that reads from an on-disk file
+ */
 struct rerere_io_file {
        struct rerere_io io;
        FILE *input;
 };
 
+/*
+ * ... and its getline() method implementation
+ */
 static int rerere_file_getline(struct strbuf *sb, struct rerere_io *io_)
 {
        struct rerere_io_file *io = (struct rerere_io_file *)io_;
        return strbuf_getwholeline(sb, io->input, '\n');
 }
 
-static int is_cmarker(char *buf, int marker_char, int marker_size, int want_sp)
+/*
+ * Require the exact number of conflict marker letters, no more, no
+ * less, followed by SP or any whitespace
+ * (including LF).
+ */
+static int is_cmarker(char *buf, int marker_char, int marker_size)
 {
+       int want_sp;
+
+       /*
+        * The beginning of our version and the end of their version
+        * always are labeled like "<<<<< ours" or ">>>>> theirs",
+        * hence we set want_sp for them.  Note that the version from
+        * the common ancestor in diff3-style output is not always
+        * labelled (e.g. "||||| common" is often seen but "|||||"
+        * alone is also valid), so we do not set want_sp.
+        */
+       want_sp = (marker_char == '<') || (marker_char == '>');
+
        while (marker_size--)
                if (*buf++ != marker_char)
                        return 0;
@@ -159,6 +234,21 @@ static int is_cmarker(char *buf, int marker_char, int marker_size, int want_sp)
        return isspace(*buf);
 }
 
+/*
+ * Read contents a file with conflicts, normalize the conflicts
+ * by (1) discarding the common ancestor version in diff3-style,
+ * (2) reordering our side and their side so that whichever sorts
+ * alphabetically earlier comes before the other one, while
+ * computing the "conflict ID", which is just an SHA-1 hash of
+ * one side of the conflict, NUL, the other side of the conflict,
+ * and NUL concatenated together.
+ *
+ * Return the number of conflict hunks found.
+ *
+ * NEEDSWORK: the logic and theory of operation behind this conflict
+ * normalization may deserve to be documented somewhere, perhaps in
+ * Documentation/technical/rerere.txt.
+ */
 static int handle_path(unsigned char *sha1, struct rerere_io *io, int marker_size)
 {
        git_SHA_CTX ctx;
@@ -173,19 +263,19 @@ static int handle_path(unsigned char *sha1, struct rerere_io *io, int marker_siz
                git_SHA1_Init(&ctx);
 
        while (!io->getline(&buf, io)) {
-               if (is_cmarker(buf.buf, '<', marker_size, 1)) {
+               if (is_cmarker(buf.buf, '<', marker_size)) {
                        if (hunk != RR_CONTEXT)
                                goto bad;
                        hunk = RR_SIDE_1;
-               } else if (is_cmarker(buf.buf, '|', marker_size, 0)) {
+               } else if (is_cmarker(buf.buf, '|', marker_size)) {
                        if (hunk != RR_SIDE_1)
                                goto bad;
                        hunk = RR_ORIGINAL;
-               } else if (is_cmarker(buf.buf, '=', marker_size, 0)) {
+               } else if (is_cmarker(buf.buf, '=', marker_size)) {
                        if (hunk != RR_SIDE_1 && hunk != RR_ORIGINAL)
                                goto bad;
                        hunk = RR_SIDE_2;
-               } else if (is_cmarker(buf.buf, '>', marker_size, 1)) {
+               } else if (is_cmarker(buf.buf, '>', marker_size)) {
                        if (hunk != RR_SIDE_2)
                                goto bad;
                        if (strbuf_cmp(&one, &two) > 0)
@@ -229,6 +319,10 @@ static int handle_path(unsigned char *sha1, struct rerere_io *io, int marker_siz
        return hunk_no;
 }
 
+/*
+ * Scan the path for conflicts, do the "handle_path()" thing above, and
+ * return the number of conflict hunks found.
+ */
 static int handle_file(const char *path, unsigned char *sha1, const char *output)
 {
        int hunk_no = 0;
@@ -270,11 +364,18 @@ static int handle_file(const char *path, unsigned char *sha1, const char *output
        return hunk_no;
 }
 
+/*
+ * Subclass of rerere_io that reads from an in-core buffer that is a
+ * strbuf
+ */
 struct rerere_io_mem {
        struct rerere_io io;
        struct strbuf input;
 };
 
+/*
+ * ... and its getline() method implementation
+ */
 static int rerere_mem_getline(struct strbuf *sb, struct rerere_io *io_)
 {
        struct rerere_io_mem *io = (struct rerere_io_mem *)io_;
@@ -313,24 +414,23 @@ static int handle_cache(const char *path, unsigned char *sha1, const char *outpu
                return -1;
        pos = -pos - 1;
 
-       for (i = 0; i < 3; i++) {
+       while (pos < active_nr) {
                enum object_type type;
                unsigned long size;
-               int j;
 
-               if (active_nr <= pos)
-                       break;
                ce = active_cache[pos++];
                if (ce_namelen(ce) != len || memcmp(ce->name, path, len))
-                       continue;
-               j = ce_stage(ce) - 1;
-               mmfile[j].ptr = read_sha1_file(ce->sha1, &type, &size);
-               mmfile[j].size = size;
+                       break;
+               i = ce_stage(ce) - 1;
+               if (!mmfile[i].ptr) {
+                       mmfile[i].ptr = read_sha1_file(ce->sha1, &type, &size);
+                       mmfile[i].size = size;
+               }
        }
-       for (i = 0; i < 3; i++) {
+       for (i = 0; i < 3; i++)
                if (!mmfile[i].ptr && !mmfile[i].size)
                        mmfile[i].ptr = xstrdup("");
-       }
+
        /*
         * NEEDSWORK: handle conflicts from merges with
         * merge.renormalize set, too
@@ -350,6 +450,10 @@ static int handle_cache(const char *path, unsigned char *sha1, const char *outpu
        strbuf_init(&io.input, 0);
        strbuf_attach(&io.input, result.ptr, result.size, result.size);
 
+       /*
+        * Grab the conflict ID and optionally write the original
+        * contents with conflict markers out.
+        */
        hunk_no = handle_path(sha1, (struct rerere_io *)&io, marker_size);
        strbuf_release(&io.input);
        if (io.io.output)
@@ -357,6 +461,14 @@ static int handle_cache(const char *path, unsigned char *sha1, const char *outpu
        return hunk_no;
 }
 
+/*
+ * Look at a cache entry at "i" and see if it is not conflicting,
+ * conflicting and we are willing to handle, or conflicting and
+ * we are unable to handle, and return the determination in *type.
+ * Return the cache index to be looked at next, by skipping the
+ * stages we have already looked at in this invocation of this
+ * function.
+ */
 static int check_one_conflict(int i, int *type)
 {
        const struct cache_entry *e = active_cache[i];
@@ -367,10 +479,8 @@ static int check_one_conflict(int i, int *type)
        }
 
        *type = PUNTED;
-       if (ce_stage(e) == 1) {
-               if (active_nr <= ++i)
-                       return i + 1;
-       }
+       while (ce_stage(active_cache[i]) == 1)
+               i++;
 
        /* Only handle regular files with both stages #2 and #3 */
        if (i + 1 < active_nr) {
@@ -390,6 +500,17 @@ static int check_one_conflict(int i, int *type)
        return i;
 }
 
+/*
+ * Scan the index and find paths that have conflicts that rerere can
+ * handle, i.e. the ones that has both stages #2 and #3.
+ *
+ * NEEDSWORK: we do not record or replay a previous "resolve by
+ * deletion" for a delete-modify conflict, as that is inherently risky
+ * without knowing what modification is being discarded.  The only
+ * safe case, i.e. both side doing the deletion and modification that
+ * are identical to the previous round, might want to be handled,
+ * though.
+ */
 static int find_conflict(struct string_list *conflict)
 {
        int i;
@@ -406,6 +527,21 @@ static int find_conflict(struct string_list *conflict)
        return 0;
 }
 
+/*
+ * The merge_rr list is meant to hold outstanding conflicted paths
+ * that rerere could handle.  Abuse the list by adding other types of
+ * entries to allow the caller to show "rerere remaining".
+ *
+ * - Conflicted paths that rerere does not handle are added
+ * - Conflicted paths that have been resolved are marked as such
+ *   by storing RERERE_RESOLVED to .util field (where conflict ID
+ *   is expected to be stored).
+ *
+ * Do *not* write MERGE_RR file out after calling this function.
+ *
+ * NEEDSWORK: we may want to fix the caller that implements "rerere
+ * remaining" to do this without abusing merge_rr.
+ */
 int rerere_remaining(struct string_list *merge_rr)
 {
        int i;
@@ -424,7 +560,7 @@ int rerere_remaining(struct string_list *merge_rr)
                        struct string_list_item *it;
                        it = string_list_lookup(merge_rr, (const char *)e->name);
                        if (it != NULL) {
-                               free(it->util);
+                               free_rerere_id(it);
                                it->util = RERERE_RESOLVED;
                        }
                }
@@ -432,39 +568,66 @@ int rerere_remaining(struct string_list *merge_rr)
        return 0;
 }
 
-static int merge(const char *name, const char *path)
+/*
+ * Find the conflict identified by "id"; the change between its
+ * "preimage" (i.e. a previous contents with conflict markers) and its
+ * "postimage" (i.e. the corresponding contents with conflicts
+ * resolved) may apply cleanly to the contents stored in "path", i.e.
+ * the conflict this time around.
+ *
+ * Returns 0 for successful replay of recorded resolution, or non-zero
+ * for failure.
+ */
+static int merge(const struct rerere_id *id, const char *path)
 {
+       FILE *f;
        int ret;
        mmfile_t cur = {NULL, 0}, base = {NULL, 0}, other = {NULL, 0};
        mmbuffer_t result = {NULL, 0};
 
-       if (handle_file(path, NULL, rerere_path(name, "thisimage")) < 0)
-               return 1;
+       /*
+        * Normalize the conflicts in path and write it out to
+        * "thisimage" temporary file.
+        */
+       if (handle_file(path, NULL, rerere_path(id, "thisimage")) < 0) {
+               ret = 1;
+               goto out;
+       }
 
-       if (read_mmfile(&cur, rerere_path(name, "thisimage")) ||
-                       read_mmfile(&base, rerere_path(name, "preimage")) ||
-                       read_mmfile(&other, rerere_path(name, "postimage"))) {
+       if (read_mmfile(&cur, rerere_path(id, "thisimage")) ||
+           read_mmfile(&base, rerere_path(id, "preimage")) ||
+           read_mmfile(&other, rerere_path(id, "postimage"))) {
                ret = 1;
                goto out;
        }
+
+       /*
+        * A three-way merge. Note that this honors user-customizable
+        * low-level merge driver settings.
+        */
        ret = ll_merge(&result, path, &base, NULL, &cur, "", &other, "", NULL);
-       if (!ret) {
-               FILE *f;
-
-               if (utime(rerere_path(name, "postimage"), NULL) < 0)
-                       warning("failed utime() on %s: %s",
-                                       rerere_path(name, "postimage"),
-                                       strerror(errno));
-               f = fopen(path, "w");
-               if (!f)
-                       return error("Could not open %s: %s", path,
-                                    strerror(errno));
-               if (fwrite(result.ptr, result.size, 1, f) != 1)
-                       error("Could not write %s: %s", path, strerror(errno));
-               if (fclose(f))
-                       return error("Writing %s failed: %s", path,
-                                    strerror(errno));
-       }
+       if (ret)
+               goto out;
+
+       /*
+        * A successful replay of recorded resolution.
+        * Mark that "postimage" was used to help gc.
+        */
+       if (utime(rerere_path(id, "postimage"), NULL) < 0)
+               warning("failed utime() on %s: %s",
+                       rerere_path(id, "postimage"),
+                       strerror(errno));
+
+       /* Update "path" with the resolution */
+       f = fopen(path, "w");
+       if (!f)
+               return error("Could not open %s: %s", path,
+                            strerror(errno));
+       if (fwrite(result.ptr, result.size, 1, f) != 1)
+               error("Could not write %s: %s", path, strerror(errno));
+       if (fclose(f))
+               return error("Writing %s failed: %s", path,
+                            strerror(errno));
 
 out:
        free(cur.ptr);
@@ -487,6 +650,8 @@ static void update_paths(struct string_list *update)
                struct string_list_item *item = &update->items[i];
                if (add_file_to_cache(item->string, 0))
                        exit(128);
+               fprintf(stderr, "Staged '%s' using previous resolution.\n",
+                       item->string);
        }
 
        if (active_cache_changed) {
@@ -496,6 +661,41 @@ static void update_paths(struct string_list *update)
                rollback_lock_file(&index_lock);
 }
 
+/*
+ * The path indicated by rr_item may still have conflict for which we
+ * have a recorded resolution, in which case replay it and optionally
+ * update it.  Or it may have been resolved by the user and we may
+ * only have the preimage for that conflict, in which case the result
+ * needs to be recorded as a resolution in a postimage file.
+ */
+static void do_rerere_one_path(struct string_list_item *rr_item,
+                              struct string_list *update)
+{
+       const char *path = rr_item->string;
+       const struct rerere_id *id = rr_item->util;
+
+       /* Is there a recorded resolution we could attempt to apply? */
+       if (has_rerere_resolution(id)) {
+               if (merge(id, path))
+                       return; /* failed to replay */
+
+               if (rerere_autoupdate)
+                       string_list_insert(update, path);
+               else
+                       fprintf(stderr,
+                               "Resolved '%s' using previous resolution.\n",
+                               path);
+       } else if (!handle_file(path, NULL, NULL)) {
+               /* The user has resolved it. */
+               copy_file(rerere_path(id, "postimage"), path, 0666);
+               fprintf(stderr, "Recorded resolution for '%s'.\n", path);
+       } else {
+               return;
+       }
+       free_rerere_id(rr_item);
+       rr_item->util = NULL;
+}
+
 static int do_plain_rerere(struct string_list *rr, int fd)
 {
        struct string_list conflict = STRING_LIST_INIT_DUP;
@@ -505,65 +705,55 @@ static int do_plain_rerere(struct string_list *rr, int fd)
        find_conflict(&conflict);
 
        /*
-        * MERGE_RR records paths with conflicts immediately after merge
-        * failed.  Some of the conflicted paths might have been hand resolved
-        * in the working tree since then, but the initial run would catch all
-        * and register their preimages.
+        * MERGE_RR records paths with conflicts immediately after
+        * merge failed.  Some of the conflicted paths might have been
+        * hand resolved in the working tree since then, but the
+        * initial run would catch all and register their preimages.
         */
-
        for (i = 0; i < conflict.nr; i++) {
+               struct rerere_id *id;
+               unsigned char sha1[20];
                const char *path = conflict.items[i].string;
-               if (!string_list_has_string(rr, path)) {
-                       unsigned char sha1[20];
-                       char *hex;
-                       int ret;
-                       ret = handle_file(path, sha1, NULL);
-                       if (ret < 1)
-                               continue;
-                       hex = xstrdup(sha1_to_hex(sha1));
-                       string_list_insert(rr, path)->util = hex;
-                       if (mkdir_in_gitdir(git_path("rr-cache/%s", hex)))
-                               continue;
-                       handle_file(path, NULL, rerere_path(hex, "preimage"));
-                       fprintf(stderr, "Recorded preimage for '%s'\n", path);
-               }
-       }
+               int ret;
 
-       /*
-        * Now some of the paths that had conflicts earlier might have been
-        * hand resolved.  Others may be similar to a conflict already that
-        * was resolved before.
-        */
+               if (string_list_has_string(rr, path))
+                       continue;
 
-       for (i = 0; i < rr->nr; i++) {
-               int ret;
-               const char *path = rr->items[i].string;
-               const char *name = (const char *)rr->items[i].util;
-
-               if (has_rerere_resolution(name)) {
-                       if (!merge(name, path)) {
-                               const char *msg;
-                               if (rerere_autoupdate) {
-                                       string_list_insert(&update, path);
-                                       msg = "Staged '%s' using previous resolution.\n";
-                               } else
-                                       msg = "Resolved '%s' using previous resolution.\n";
-                               fprintf(stderr, msg, path);
-                               goto mark_resolved;
-                       }
-               }
+               /*
+                * Ask handle_file() to scan and assign a
+                * conflict ID.  No need to write anything out
+                * yet.
+                */
+               ret = handle_file(path, sha1, NULL);
+               if (ret < 1)
+                       continue;
 
-               /* Let's see if we have resolved it. */
-               ret = handle_file(path, NULL, NULL);
-               if (ret)
+               id = new_rerere_id(sha1);
+               string_list_insert(rr, path)->util = id;
+
+               /*
+                * If the directory does not exist, create
+                * it.  mkdir_in_gitdir() will fail with
+                * EEXIST if there already is one.
+                *
+                * NEEDSWORK: make sure "gc" does not remove
+                * preimage without removing the directory.
+                */
+               if (mkdir_in_gitdir(rerere_path(id, NULL)))
                        continue;
 
-               fprintf(stderr, "Recorded resolution for '%s'.\n", path);
-               copy_file(rerere_path(name, "postimage"), path, 0666);
-       mark_resolved:
-               rr->items[i].util = NULL;
+               /*
+                * We are the first to encounter this
+                * conflict.  Ask handle_file() to write the
+                * normalized contents to the "preimage" file.
+                */
+               handle_file(path, NULL, rerere_path(id, "preimage"));
+               fprintf(stderr, "Recorded preimage for '%s'\n", path);
        }
 
+       for (i = 0; i < rr->nr; i++)
+               do_rerere_one_path(&rr->items[i], &update);
+
        if (update.nr)
                update_paths(&update);
 
@@ -614,6 +804,11 @@ int setup_rerere(struct string_list *merge_rr, int flags)
        return fd;
 }
 
+/*
+ * The main entry point that is called internally from codepaths that
+ * perform mergy operations, possibly leaving conflicted index entries
+ * and working tree files.
+ */
 int rerere(int flags)
 {
        struct string_list merge_rr = STRING_LIST_INIT_DUP;
@@ -628,25 +823,42 @@ int rerere(int flags)
 static int rerere_forget_one_path(const char *path, struct string_list *rr)
 {
        const char *filename;
-       char *hex;
+       struct rerere_id *id;
        unsigned char sha1[20];
        int ret;
+       struct string_list_item *item;
 
+       /*
+        * Recreate the original conflict from the stages in the
+        * index and compute the conflict ID
+        */
        ret = handle_cache(path, sha1, NULL);
        if (ret < 1)
                return error("Could not parse conflict hunks in '%s'", path);
-       hex = xstrdup(sha1_to_hex(sha1));
-       filename = rerere_path(hex, "postimage");
+
+       /* Nuke the recorded resolution for the conflict */
+       id = new_rerere_id(sha1);
+       filename = rerere_path(id, "postimage");
        if (unlink(filename))
                return (errno == ENOENT
                        ? error("no remembered resolution for %s", path)
                        : error("cannot unlink %s: %s", filename, strerror(errno)));
 
-       handle_cache(path, sha1, rerere_path(hex, "preimage"));
+       /*
+        * Update the preimage so that the user can resolve the
+        * conflict in the working tree, run us again to record
+        * the postimage.
+        */
+       handle_cache(path, sha1, rerere_path(id, "preimage"));
        fprintf(stderr, "Updated preimage for '%s'\n", path);
 
-
-       string_list_insert(rr, path)->util = hex;
+       /*
+        * And remember that we can record resolution for this
+        * conflict when the user is done.
+        */
+       item = string_list_insert(rr, path);
+       free_rerere_id(item);
+       item->util = id;
        fprintf(stderr, "Forgot resolution for %s\n", path);
        return 0;
 }
@@ -664,6 +876,11 @@ int rerere_forget(struct pathspec *pathspec)
        if (fd < 0)
                return 0;
 
+       /*
+        * The paths may have been resolved (incorrectly);
+        * recover the original conflicted state and then
+        * find the conflicted paths.
+        */
        unmerge_cache(pathspec);
        find_conflict(&conflict);
        for (i = 0; i < conflict.nr; i++) {
@@ -676,24 +893,51 @@ int rerere_forget(struct pathspec *pathspec)
        return write_rr(&merge_rr, fd);
 }
 
-static time_t rerere_created_at(const char *name)
+/*
+ * Garbage collection support
+ */
+
+/*
+ * Note that this is not reentrant but is used only one-at-a-time
+ * so it does not matter right now.
+ */
+static struct rerere_id *dirname_to_id(const char *name)
+{
+       static struct rerere_id id;
+       xsnprintf(id.hex, sizeof(id.hex), "%s", name);
+       return &id;
+}
+
+static time_t rerere_created_at(const char *dir_name)
 {
        struct stat st;
-       return stat(rerere_path(name, "preimage"), &st) ? (time_t) 0 : st.st_mtime;
+       struct rerere_id *id = dirname_to_id(dir_name);
+
+       return stat(rerere_path(id, "preimage"), &st) ? (time_t) 0 : st.st_mtime;
 }
 
-static time_t rerere_last_used_at(const char *name)
+static time_t rerere_last_used_at(const char *dir_name)
 {
        struct stat st;
-       return stat(rerere_path(name, "postimage"), &st) ? (time_t) 0 : st.st_mtime;
+       struct rerere_id *id = dirname_to_id(dir_name);
+
+       return stat(rerere_path(id, "postimage"), &st) ? (time_t) 0 : st.st_mtime;
 }
 
-static void unlink_rr_item(const char *name)
+/*
+ * Remove the recorded resolution for a given conflict ID
+ */
+static void unlink_rr_item(struct rerere_id *id)
 {
-       unlink(rerere_path(name, "thisimage"));
-       unlink(rerere_path(name, "preimage"));
-       unlink(rerere_path(name, "postimage"));
-       rmdir(git_path("rr-cache/%s", name));
+       unlink(rerere_path(id, "thisimage"));
+       unlink(rerere_path(id, "preimage"));
+       unlink(rerere_path(id, "postimage"));
+       /*
+        * NEEDSWORK: what if this rmdir() fails?  Wouldn't we then
+        * assume that we already have preimage recorded in
+        * do_plain_rerere()?
+        */
+       rmdir(rerere_path(id, NULL));
 }
 
 void rerere_gc(struct string_list *rr)
@@ -715,6 +959,7 @@ void rerere_gc(struct string_list *rr)
        dir = opendir(git_path("rr-cache"));
        if (!dir)
                die_errno("unable to open rr-cache directory");
+       /* Collect stale conflict IDs ... */
        while ((e = readdir(dir))) {
                if (is_dot_or_dotdot(e->d_name))
                        continue;
@@ -732,12 +977,20 @@ void rerere_gc(struct string_list *rr)
                        string_list_append(&to_remove, e->d_name);
        }
        closedir(dir);
+       /* ... and then remove them one-by-one */
        for (i = 0; i < to_remove.nr; i++)
-               unlink_rr_item(to_remove.items[i].string);
+               unlink_rr_item(dirname_to_id(to_remove.items[i].string));
        string_list_clear(&to_remove, 0);
        rollback_lock_file(&write_lock);
 }
 
+/*
+ * During a conflict resolution, after "rerere" recorded the
+ * preimages, abandon them if the user did not resolve them or
+ * record their resolutions.  And drop $GIT_DIR/MERGE_RR.
+ *
+ * NEEDSWORK: shouldn't we be calling this from "reset --hard"?
+ */
 void rerere_clear(struct string_list *merge_rr)
 {
        int i;
@@ -746,9 +999,9 @@ void rerere_clear(struct string_list *merge_rr)
                return;
 
        for (i = 0; i < merge_rr->nr; i++) {
-               const char *name = (const char *)merge_rr->items[i].util;
-               if (!has_rerere_resolution(name))
-                       unlink_rr_item(name);
+               struct rerere_id *id = merge_rr->items[i].util;
+               if (!has_rerere_resolution(id))
+                       unlink_rr_item(id);
        }
        unlink_or_warn(git_path_merge_rr());
        rollback_lock_file(&write_lock);