Merge branch 'jc/rerere'
authorJunio C Hamano <gitster@pobox.com>
Mon, 5 Oct 2015 19:30:04 +0000 (12:30 -0700)
committerJunio C Hamano <gitster@pobox.com>
Mon, 5 Oct 2015 19:30:05 +0000 (12:30 -0700)
Code clean-up and minor fixes.

* jc/rerere: (21 commits)
  rerere: un-nest merge() further
  rerere: use "struct rerere_id" instead of "char *" for conflict ID
  rerere: call conflict-ids IDs
  rerere: further clarify do_rerere_one_path()
  rerere: further de-dent do_plain_rerere()
  rerere: refactor "replay" part of do_plain_rerere()
  rerere: explain the remainder
  rerere: explain "rerere forget" codepath
  rerere: explain the primary codepath
  rerere: explain MERGE_RR management helpers
  rerere: fix benign off-by-one non-bug and clarify code
  rerere: explain the rerere I/O abstraction
  rerere: do not leak mmfile[] for a path with multiple stage #1 entries
  rerere: stop looping unnecessarily
  rerere: drop want_sp parameter from is_cmarker()
  rerere: report autoupdated paths only after actually updating them
  rerere: write out each record of MERGE_RR in one go
  rerere: lift PATH_MAX limitation
  rerere: plug conflict ID leaks
  rerere: handle conflicts with multiple stage #1 entries
  ...

1  2 
builtin/rerere.c
rerere.c
rerere.h

@@@ -98,15 -100,13 +98,15 @@@ int cmd_rerere(int argc, const char **a
                                 * string_list_clear() */
                                merge_rr.items[i].util = NULL;
                }
 -      } else if (!strcmp(argv[0], "diff"))
 +      } else if (!strcmp(argv[0], "diff")) {
 +              if (setup_rerere(&merge_rr, flags | RERERE_READONLY) < 0)
 +                      return 0;
                for (i = 0; i < merge_rr.nr; i++) {
                        const char *path = merge_rr.items[i].string;
-                       const char *name = (const char *)merge_rr.items[i].util;
-                       diff_two(rerere_path(name, "preimage"), path, path, path);
+                       const struct rerere_id *id = merge_rr.items[i].util;
+                       diff_two(rerere_path(id, "preimage"), path, path, path);
                }
 -      else
 +      else
                usage_with_options(rerere_usage, options);
  
        string_list_clear(&merge_rr, 1);
diff --cc rerere.c
+++ b/rerere.c
@@@ -20,45 -20,76 +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 char *merge_rr_path;
 -
+ 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));
+       strcpy(id->hex, 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(merge_rr_path, "r");
 +      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);
  }
  
@@@ -661,9 -871,12 +873,14 @@@ int rerere_forget(struct pathspec *path
                return error("Could not read index");
  
        fd = setup_rerere(&merge_rr, RERERE_NOAUTOUPDATE);
 +      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++) {
@@@ -732,24 -970,27 +977,32 @@@ 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;
  
 +      if (setup_rerere(merge_rr, 0) < 0)
 +              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"));
 +      unlink_or_warn(git_path_merge_rr());
 +      rollback_lock_file(&write_lock);
  }
diff --cc rerere.h
Simple merge