From 9022a495a3c407859560d4ac4a278c0d6ad58837 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Sat, 21 Jun 2008 23:28:58 -0700 Subject: [PATCH 1/5] rerere: rerere_created_at() and has_resolution() abstraction There were too many places in the code how an entry in the rerere database looks like, and the garbage_collect() function that iterates over subdirectories of the rr-cache directory was the worse offender. Introduce two helper functions, rerere_created_at() and has_resolution(), to abstract out the logic a bit better. Incidentally this fixes a small memory leak in garbage_collect() function. The path list to collect the entries to be pruned were defined to strdup the paths but the caller was feeding a path after doing an extra copy. Because the list does not have to be sorted by conflict signature hash, we use path_list_append() instead of path_list_insert(). While we are at it, make a conflicted hunk comparision in handle_file() a bit easier to read. Signed-off-by: Junio C Hamano --- builtin-rerere.c | 57 ++++++++++++++++++++++-------------------------- 1 file changed, 26 insertions(+), 31 deletions(-) diff --git a/builtin-rerere.c b/builtin-rerere.c index 85222d9bc59..610b96a1202 100644 --- a/builtin-rerere.c +++ b/builtin-rerere.c @@ -23,6 +23,18 @@ static const char *rr_path(const char *name, const char *file) return git_path("rr-cache/%s/%s", name, file); } +static time_t rerere_created_at(const char *name) +{ + struct stat st; + return stat(rr_path(name, "preimage"), &st) ? (time_t) 0 : st.st_mtime; +} + +static int has_resolution(const char *name) +{ + struct stat st; + return !stat(rr_path(name, "postimage"), &st); +} + static void read_rr(struct path_list *rr) { unsigned char sha1[20]; @@ -98,13 +110,10 @@ static int handle_file(const char *path, else if (!prefixcmp(buf, "=======")) hunk = 2; else if (!prefixcmp(buf, ">>>>>>> ")) { - int cmp = strbuf_cmp(&one, &two); - + if (strbuf_cmp(&one, &two) > 0) + strbuf_swap(&one, &two); hunk_no++; hunk = 0; - if (cmp > 0) { - strbuf_swap(&one, &two); - } if (out) { fputs("<<<<<<<\n", out); fwrite(one.buf, one.len, 1, out); @@ -201,33 +210,24 @@ static void unlink_rr_item(const char *name) static void garbage_collect(struct path_list *rr) { struct path_list to_remove = { NULL, 0, 0, 1 }; - char buf[1024]; DIR *dir; struct dirent *e; - int len, i, cutoff; + int i, cutoff; time_t now = time(NULL), then; - strlcpy(buf, git_path("rr-cache"), sizeof(buf)); - len = strlen(buf); - dir = opendir(buf); - strcpy(buf + len++, "/"); + dir = opendir(git_path("rr-cache")); while ((e = readdir(dir))) { const char *name = e->d_name; - struct stat st; - if (name[0] == '.' && (name[1] == '\0' || - (name[1] == '.' && name[2] == '\0'))) + if (name[0] == '.' && + (name[1] == '\0' || (name[1] == '.' && name[2] == '\0'))) continue; - i = snprintf(buf + len, sizeof(buf) - len, "%s", name); - strlcpy(buf + len + i, "/preimage", sizeof(buf) - len - i); - if (stat(buf, &st)) + then = rerere_created_at(name); + if (!then) continue; - then = st.st_mtime; - strlcpy(buf + len + i, "/postimage", sizeof(buf) - len - i); - cutoff = stat(buf, &st) ? cutoff_noresolve : cutoff_resolve; - if (then < now - cutoff * 86400) { - buf[len + i] = '\0'; - path_list_insert(xstrdup(name), &to_remove); - } + cutoff = (has_resolution(name) + ? cutoff_resolve : cutoff_noresolve); + if (then < now - cutoff * 86400) + path_list_append(name, &to_remove); } for (i = 0; i < to_remove.nr; i++) unlink_rr_item(to_remove.items[i].path); @@ -306,13 +306,11 @@ static int do_plain_rerere(struct path_list *rr, int fd) */ for (i = 0; i < rr->nr; i++) { - struct stat st; int ret; const char *path = rr->items[i].path; const char *name = (const char *)rr->items[i].util; - if (!stat(rr_path(name, "preimage"), &st) && - !stat(rr_path(name, "postimage"), &st)) { + if (has_resolution(name)) { if (!merge(name, path)) { fprintf(stderr, "Resolved '%s' using " "previous resolution.\n", path); @@ -410,11 +408,8 @@ int cmd_rerere(int argc, const char **argv, const char *prefix) return do_plain_rerere(&merge_rr, fd); else if (!strcmp(argv[1], "clear")) { for (i = 0; i < merge_rr.nr; i++) { - struct stat st; const char *name = (const char *)merge_rr.items[i].util; - if (!stat(git_path("rr-cache/%s", name), &st) && - S_ISDIR(st.st_mode) && - stat(rr_path(name, "postimage"), &st)) + if (!has_resolution(name)) unlink_rr_item(name); } unlink(merge_rr_path); From a1b32fdc3d1d05395f186bfa06e92174519dab8d Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Sun, 22 Jun 2008 00:21:28 -0700 Subject: [PATCH 2/5] git-rerere: detect unparsable conflicts rerere did not detect the case where <<< === >>> markers did not match. Signed-off-by: Junio C Hamano --- builtin-rerere.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/builtin-rerere.c b/builtin-rerere.c index 610b96a1202..addc5c73df0 100644 --- a/builtin-rerere.c +++ b/builtin-rerere.c @@ -144,6 +144,11 @@ static int handle_file(const char *path, fclose(out); if (sha1) SHA1_Final(sha1, &ctx); + if (hunk) { + if (output) + unlink(output); + return error("Could not parse conflict hunks in %s", path); + } return hunk_no; } From 51e0d0a67b8d31e92d52e15b577afb079359ec63 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Sun, 22 Jun 2008 00:27:46 -0700 Subject: [PATCH 3/5] rerere: remove dubious "tail_optimization" It is dubious if it is cheaper to shift entries repeatedly using memmove() to collect entries that needs to be written out in front of an array than simply marking the entries to be skipped. In addition, the label called this "tail optimization", but this obviously is not what people usually call with that name. Signed-off-by: Junio C Hamano --- builtin-rerere.c | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/builtin-rerere.c b/builtin-rerere.c index addc5c73df0..0eec1f93732 100644 --- a/builtin-rerere.c +++ b/builtin-rerere.c @@ -66,8 +66,12 @@ static int write_rr(struct path_list *rr, int out_fd) { int i; for (i = 0; i < rr->nr; i++) { - const char *path = rr->items[i].path; - int length = strlen(path) + 1; + const char *path; + int length; + if (!rr->items[i].util) + continue; + path = rr->items[i].path; + length = strlen(path) + 1; if (write_in_full(out_fd, rr->items[i].util, 40) != 40 || write_in_full(out_fd, "\t", 1) != 1 || write_in_full(out_fd, path, length) != length) @@ -319,7 +323,7 @@ static int do_plain_rerere(struct path_list *rr, int fd) if (!merge(name, path)) { fprintf(stderr, "Resolved '%s' using " "previous resolution.\n", path); - goto tail_optimization; + goto mark_resolved; } } @@ -330,13 +334,8 @@ static int do_plain_rerere(struct path_list *rr, int fd) fprintf(stderr, "Recorded resolution for '%s'.\n", path); copy_file(rr_path(name, "postimage"), path, 0666); -tail_optimization: - if (i < rr->nr - 1) - memmove(rr->items + i, - rr->items + i + 1, - sizeof(rr->items[0]) * (rr->nr - i - 1)); - rr->nr--; - i--; + mark_resolved: + rr->items[i].util = NULL; } return write_rr(rr, fd); From 7f8365f894d60f240edd356f32e3c1bda994ed41 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Sun, 22 Jun 2008 02:03:26 -0700 Subject: [PATCH 4/5] t4200: fix rerere test The test used "diff-files -q" which is not about reporting if there is a difference at all. Instead, make sure that the path remains as conflicting in the index after rerere autoresolves it, as we will be adding rerere.autoupdate configuration with the next patch. --- t/t4200-rerere.sh | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/t/t4200-rerere.sh b/t/t4200-rerere.sh index 85d7e3edcd0..afb3e3d1761 100755 --- a/t/t4200-rerere.sh +++ b/t/t4200-rerere.sh @@ -193,9 +193,9 @@ test_expect_success 'resolution was recorded properly' ' echo Bello > file3 && git add file3 && git commit -m version2 && - ! git merge fifth && - git diff-files -q && - test Cello = "$(cat file3)" + test_must_fail git merge fifth && + test Cello = "$(cat file3)" && + test 0 != $(git ls-files -u | wc -l) ' test_done From 121c813f8d6054dbee25c03301599c0f8d645d7f Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Sun, 22 Jun 2008 02:04:31 -0700 Subject: [PATCH 5/5] rerere.autoupdate When this configuration is set, paths that are autoresolved by git-rerere are updated in the index as well. --- Documentation/config.txt | 5 +++++ builtin-rerere.c | 37 +++++++++++++++++++++++++++++++++++++ t/t4200-rerere.sh | 10 ++++++++++ 3 files changed, 52 insertions(+) diff --git a/Documentation/config.txt b/Documentation/config.txt index 5331b450ea0..0c7cf618ed2 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -650,6 +650,11 @@ gc.rerereunresolved:: kept for this many days when `git rerere gc` is run. The default is 15 days. See linkgit:git-rerere[1]. +rerere.autoupdate:: + When set to true, `git-rerere` updates the index with the + resulting contents after it cleanly resolves conflicts using + previously recorded resolution. Defaults to false. + rerere.enabled:: Activate recording of resolved conflicts, so that identical conflict hunks can be resolved automatically, should they diff --git a/builtin-rerere.c b/builtin-rerere.c index 0eec1f93732..839b26e8e0a 100644 --- a/builtin-rerere.c +++ b/builtin-rerere.c @@ -16,6 +16,9 @@ static int cutoff_resolve = 60; /* if rerere_enabled == -1, fall back to detection of .git/rr-cache */ static int rerere_enabled = -1; +/* automatically update cleanly resolved paths to the index */ +static int rerere_autoupdate; + static char *merge_rr_path; static const char *rr_path(const char *name, const char *file) @@ -276,9 +279,36 @@ static int diff_two(const char *file1, const char *label1, return 0; } +static struct lock_file index_lock; + +static int update_paths(struct path_list *update) +{ + int i; + int fd = hold_locked_index(&index_lock, 0); + int status = 0; + + if (fd < 0) + return -1; + + for (i = 0; i < update->nr; i++) { + struct path_list_item *item = &update->items[i]; + if (add_file_to_cache(item->path, ADD_CACHE_IGNORE_ERRORS)) + status = -1; + } + + if (!status && active_cache_changed) { + if (write_cache(fd, active_cache, active_nr) || + commit_locked_index(&index_lock)) + die("Unable to write new index file"); + } else if (fd >= 0) + rollback_lock_file(&index_lock); + return status; +} + static int do_plain_rerere(struct path_list *rr, int fd) { struct path_list conflict = { NULL, 0, 0, 1 }; + struct path_list update = { NULL, 0, 0, 1 }; int i; find_conflict(&conflict); @@ -323,6 +353,8 @@ static int do_plain_rerere(struct path_list *rr, int fd) if (!merge(name, path)) { fprintf(stderr, "Resolved '%s' using " "previous resolution.\n", path); + if (rerere_autoupdate) + path_list_insert(path, &update); goto mark_resolved; } } @@ -338,6 +370,9 @@ static int do_plain_rerere(struct path_list *rr, int fd) rr->items[i].util = NULL; } + if (update.nr) + update_paths(&update); + return write_rr(rr, fd); } @@ -349,6 +384,8 @@ static int git_rerere_config(const char *var, const char *value, void *cb) cutoff_noresolve = git_config_int(var, value); else if (!strcmp(var, "rerere.enabled")) rerere_enabled = git_config_bool(var, value); + else if (!strcmp(var, "rerere.autoupdate")) + rerere_autoupdate = git_config_bool(var, value); else return git_default_config(var, value, cb); return 0; diff --git a/t/t4200-rerere.sh b/t/t4200-rerere.sh index afb3e3d1761..a64727d5ad0 100755 --- a/t/t4200-rerere.sh +++ b/t/t4200-rerere.sh @@ -193,9 +193,19 @@ test_expect_success 'resolution was recorded properly' ' echo Bello > file3 && git add file3 && git commit -m version2 && + git tag version2 && test_must_fail git merge fifth && test Cello = "$(cat file3)" && test 0 != $(git ls-files -u | wc -l) ' +test_expect_success 'rerere.autoupdate' ' + git config rerere.autoupdate true + git reset --hard && + git checkout version2 && + test_must_fail git merge fifth && + test 0 = $(git ls-files -u | wc -l) + +' + test_done