From bf5c0571d6542d992380467c26cb7bdcab23fcb5 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 25 Apr 2018 14:28:17 +0200 Subject: sequencer: avoid using errno clobbered by rollback_lock_file() As pointed out in a review of the `--rebase-merges` patch series, `rollback_lock_file()` clobbers errno. Therefore, we have to report the error message that uses errno before calling said function. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- sequencer.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) (limited to 'sequencer.c') diff --git a/sequencer.c b/sequencer.c index 5e3a50fafc..674e26bf82 100644 --- a/sequencer.c +++ b/sequencer.c @@ -346,12 +346,14 @@ static int write_message(const void *buf, size_t len, const char *filename, if (msg_fd < 0) return error_errno(_("could not lock '%s'"), filename); if (write_in_full(msg_fd, buf, len) < 0) { + error_errno(_("could not write to '%s'"), filename); rollback_lock_file(&msg_file); - return error_errno(_("could not write to '%s'"), filename); + return -1; } if (append_eol && write(msg_fd, "\n", 1) < 0) { + error_errno(_("could not write eol to '%s'"), filename); rollback_lock_file(&msg_file); - return error_errno(_("could not write eol to '%s'"), filename); + return -1; } if (commit_lock_file(&msg_file) < 0) return error(_("failed to finalize '%s'"), filename); @@ -2125,9 +2127,9 @@ static int save_head(const char *head) written = write_in_full(fd, buf.buf, buf.len); strbuf_release(&buf); if (written < 0) { + error_errno(_("could not write to '%s'"), git_path_head_file()); rollback_lock_file(&head_lock); - return error_errno(_("could not write to '%s'"), - git_path_head_file()); + return -1; } if (commit_lock_file(&head_lock) < 0) return error(_("failed to finalize '%s'"), git_path_head_file()); -- cgit v1.2.3 From 2f6b1d13aa730901fe328fc2d88f208ea44d4f9e Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 25 Apr 2018 14:28:25 +0200 Subject: sequencer: make rearrange_squash() a bit more obvious There are some commands that have to be skipped from rearranging by virtue of not handling any commits. However, the logic was not quite obvious: it skipped commands based on their position in the enum todo_command. Instead, let's make it explicit that we skip all commands that do not handle any commit. With one exception: the `drop` command, because it, well, drops the commit and is therefore not eligible to rearranging. Note: this is a bit academic at the moment because the only time we call `rearrange_squash()` is directly after generating the todo list, when we have nothing but `pick` commands anyway. However, the upcoming `merge` command *will* want to be handled by that function, and it *can* handle commits. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- sequencer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'sequencer.c') diff --git a/sequencer.c b/sequencer.c index 674e26bf82..c131e39fa9 100644 --- a/sequencer.c +++ b/sequencer.c @@ -3403,7 +3403,7 @@ int rearrange_squash(void) struct subject2item_entry *entry; next[i] = tail[i] = -1; - if (item->command >= TODO_EXEC) { + if (!item->commit || item->command == TODO_DROP) { subjects[i] = NULL; continue; } -- cgit v1.2.3 From a01c2a5f59cc0f28daeab33355ae4a45490ce7e6 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 25 Apr 2018 14:28:29 +0200 Subject: sequencer: refactor how original todo list lines are accessed Previously, we did a lot of arithmetic gymnastics to get at the line in the todo list (as stored in todo_list.buf). This might have been fast, but only in terms of execution speed, not in terms of developer time. Let's refactor this to make it a lot easier to read, and hence to reason about the correctness of the code. It is not performance-critical code anyway. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- sequencer.c | 60 ++++++++++++++++++++++++++++++++++++------------------------ 1 file changed, 36 insertions(+), 24 deletions(-) (limited to 'sequencer.c') diff --git a/sequencer.c b/sequencer.c index c131e39fa9..eac1c341c1 100644 --- a/sequencer.c +++ b/sequencer.c @@ -1871,6 +1871,23 @@ static int count_commands(struct todo_list *todo_list) return count; } +static int get_item_line_offset(struct todo_list *todo_list, int index) +{ + return index < todo_list->nr ? + todo_list->items[index].offset_in_buf : todo_list->buf.len; +} + +static const char *get_item_line(struct todo_list *todo_list, int index) +{ + return todo_list->buf.buf + get_item_line_offset(todo_list, index); +} + +static int get_item_line_length(struct todo_list *todo_list, int index) +{ + return get_item_line_offset(todo_list, index + 1) + - get_item_line_offset(todo_list, index); +} + static ssize_t strbuf_read_file_or_whine(struct strbuf *sb, const char *path) { int fd; @@ -2250,29 +2267,27 @@ static int save_todo(struct todo_list *todo_list, struct replay_opts *opts) fd = hold_lock_file_for_update(&todo_lock, todo_path, 0); if (fd < 0) return error_errno(_("could not lock '%s'"), todo_path); - offset = next < todo_list->nr ? - todo_list->items[next].offset_in_buf : todo_list->buf.len; + offset = get_item_line_offset(todo_list, next); if (write_in_full(fd, todo_list->buf.buf + offset, todo_list->buf.len - offset) < 0) return error_errno(_("could not write to '%s'"), todo_path); if (commit_lock_file(&todo_lock) < 0) return error(_("failed to finalize '%s'"), todo_path); - if (is_rebase_i(opts)) { - const char *done_path = rebase_path_done(); - int fd = open(done_path, O_CREAT | O_WRONLY | O_APPEND, 0666); - int prev_offset = !next ? 0 : - todo_list->items[next - 1].offset_in_buf; + if (is_rebase_i(opts) && next > 0) { + const char *done = rebase_path_done(); + int fd = open(done, O_CREAT | O_WRONLY | O_APPEND, 0666); + int ret = 0; - if (fd >= 0 && offset > prev_offset && - write_in_full(fd, todo_list->buf.buf + prev_offset, - offset - prev_offset) < 0) { - close(fd); - return error_errno(_("could not write to '%s'"), - done_path); - } - if (fd >= 0) - close(fd); + if (fd < 0) + return 0; + if (write_in_full(fd, get_item_line(todo_list, next - 1), + get_item_line_length(todo_list, next - 1)) + < 0) + ret = error_errno(_("could not write to '%s'"), done); + if (close(fd) < 0) + ret = error_errno(_("failed to finalize '%s'"), done); + return ret; } return 0; } @@ -3307,8 +3322,7 @@ int skip_unnecessary_picks(void) oid = &item->commit->object.oid; } if (i > 0) { - int offset = i < todo_list.nr ? - todo_list.items[i].offset_in_buf : todo_list.buf.len; + int offset = get_item_line_offset(&todo_list, i); const char *done_path = rebase_path_done(); fd = open(done_path, O_CREAT | O_WRONLY | O_APPEND, 0666); @@ -3488,12 +3502,10 @@ int rearrange_squash(void) continue; while (cur >= 0) { - int offset = todo_list.items[cur].offset_in_buf; - int end_offset = cur + 1 < todo_list.nr ? - todo_list.items[cur + 1].offset_in_buf : - todo_list.buf.len; - char *bol = todo_list.buf.buf + offset; - char *eol = todo_list.buf.buf + end_offset; + const char *bol = + get_item_line(&todo_list, cur); + const char *eol = + get_item_line(&todo_list, cur + 1); /* replace 'pick', by 'fixup' or 'squash' */ command = todo_list.items[cur].command; -- cgit v1.2.3 From cb5206eab196544a3b271afdff9865c103e6dbce Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 25 Apr 2018 14:28:33 +0200 Subject: sequencer: offer helpful advice when a command was rescheduled Previously, we did that just magically, and potentially left some users quite puzzled. Let's err on the safe side instead, telling the user what is happening, and how they are supposed to continue. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- sequencer.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) (limited to 'sequencer.c') diff --git a/sequencer.c b/sequencer.c index eac1c341c1..f9c1ddb538 100644 --- a/sequencer.c +++ b/sequencer.c @@ -2561,6 +2561,17 @@ static const char *reflog_message(struct replay_opts *opts, return buf.buf; } +static const char rescheduled_advice[] = +N_("Could not execute the todo command\n" +"\n" +" %.*s" +"\n" +"It has been rescheduled; To edit the command before continuing, please\n" +"edit the todo list first:\n" +"\n" +" git rebase --edit-todo\n" +" git rebase --continue\n"); + static int pick_commits(struct todo_list *todo_list, struct replay_opts *opts) { int res = 0; @@ -2606,6 +2617,11 @@ static int pick_commits(struct todo_list *todo_list, struct replay_opts *opts) opts, is_final_fixup(todo_list)); if (is_rebase_i(opts) && res < 0) { /* Reschedule */ + advise(_(rescheduled_advice), + get_item_line_length(todo_list, + todo_list->current), + get_item_line(todo_list, + todo_list->current)); todo_list->current--; if (save_todo(todo_list, opts)) return -1; -- cgit v1.2.3 From 9055e401dd63a422a4401a454ef2a0e2ef727ace Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 25 Apr 2018 14:28:47 +0200 Subject: sequencer: introduce new commands to reset the revision In the upcoming commits, we will teach the sequencer to rebase merges. This will be done in a very different way from the unfortunate design of `git rebase --preserve-merges` (which does not allow for reordering commits, or changing the branch topology). The main idea is to introduce new todo list commands, to support labeling the current revision with a given name, resetting the current revision to a previous state, and merging labeled revisions. This idea was developed in Git for Windows' Git garden shears (that are used to maintain Git for Windows' "thicket of branches" on top of upstream Git), and this patch is part of the effort to make it available to a wider audience, as well as to make the entire process more robust (by implementing it in a safe and portable language rather than a Unix shell script). This commit implements the commands to label, and to reset to, given revisions. The syntax is: label reset Internally, the `label ` command creates the ref `refs/rewritten/`. This makes it possible to work with the labeled revisions interactively, or in a scripted fashion (e.g. via the todo list command `exec`). These temporary refs are removed upon sequencer_remove_state(), so that even a `git rebase --abort` cleans them up. We disallow '#' as label because that character will be used as separator in the upcoming `merge` command. Later in this patch series, we will mark the `refs/rewritten/` refs as worktree-local, to allow for interactive rebases to be run in parallel in worktrees linked to the same repository. As typos happen, a failed `label` or `reset` command will be rescheduled immediately. As the previous code to reschedule a command is embedded deeply in the pick/fixup/squash code path, we simply duplicate the few lines. This will allow us to extend the new code path easily for the upcoming `merge` command. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- sequencer.c | 213 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 206 insertions(+), 7 deletions(-) (limited to 'sequencer.c') diff --git a/sequencer.c b/sequencer.c index f9c1ddb538..c9655edffa 100644 --- a/sequencer.c +++ b/sequencer.c @@ -23,6 +23,8 @@ #include "hashmap.h" #include "notes-utils.h" #include "sigchain.h" +#include "unpack-trees.h" +#include "worktree.h" #define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION" @@ -120,6 +122,13 @@ static GIT_PATH_FUNC(rebase_path_stopped_sha, "rebase-merge/stopped-sha") static GIT_PATH_FUNC(rebase_path_rewritten_list, "rebase-merge/rewritten-list") static GIT_PATH_FUNC(rebase_path_rewritten_pending, "rebase-merge/rewritten-pending") + +/* + * The path of the file listing refs that need to be deleted after the rebase + * finishes. This is used by the `label` command to record the need for cleanup. + */ +static GIT_PATH_FUNC(rebase_path_refs_to_delete, "rebase-merge/refs-to-delete") + /* * The following files are written by git-rebase just after parsing the * command-line (and are only consumed, not modified, by the sequencer). @@ -245,18 +254,34 @@ static const char *gpg_sign_opt_quoted(struct replay_opts *opts) int sequencer_remove_state(struct replay_opts *opts) { - struct strbuf dir = STRBUF_INIT; + struct strbuf buf = STRBUF_INIT; int i; + if (is_rebase_i(opts) && + strbuf_read_file(&buf, rebase_path_refs_to_delete(), 0) > 0) { + char *p = buf.buf; + while (*p) { + char *eol = strchr(p, '\n'); + if (eol) + *eol = '\0'; + if (delete_ref("(rebase -i) cleanup", p, NULL, 0) < 0) + warning(_("could not delete '%s'"), p); + if (!eol) + break; + p = eol + 1; + } + } + free(opts->gpg_sign); free(opts->strategy); for (i = 0; i < opts->xopts_nr; i++) free(opts->xopts[i]); free(opts->xopts); - strbuf_addstr(&dir, get_dir(opts)); - remove_dir_recursively(&dir, 0); - strbuf_release(&dir); + strbuf_reset(&buf); + strbuf_addstr(&buf, get_dir(opts)); + remove_dir_recursively(&buf, 0); + strbuf_release(&buf); return 0; } @@ -1280,6 +1305,8 @@ enum todo_command { TODO_SQUASH, /* commands that do something else than handling a single commit */ TODO_EXEC, + TODO_LABEL, + TODO_RESET, /* commands that do nothing but are counted for reporting progress */ TODO_NOOP, TODO_DROP, @@ -1298,6 +1325,8 @@ static struct { { 'f', "fixup" }, { 's', "squash" }, { 'x', "exec" }, + { 'l', "label" }, + { 't', "reset" }, { 0, "noop" }, { 'd', "drop" }, { 0, NULL } @@ -1803,7 +1832,8 @@ static int parse_insn_line(struct todo_item *item, const char *bol, char *eol) return error(_("missing arguments for %s"), command_to_string(item->command)); - if (item->command == TODO_EXEC) { + if (item->command == TODO_EXEC || item->command == TODO_LABEL || + item->command == TODO_RESET) { item->commit = NULL; item->arg = bol; item->arg_len = (int)(eol - bol); @@ -2471,6 +2501,159 @@ static int do_exec(const char *command_line) return status; } +static int safe_append(const char *filename, const char *fmt, ...) +{ + va_list ap; + struct lock_file lock = LOCK_INIT; + int fd = hold_lock_file_for_update(&lock, filename, + LOCK_REPORT_ON_ERROR); + struct strbuf buf = STRBUF_INIT; + + if (fd < 0) + return -1; + + if (strbuf_read_file(&buf, filename, 0) < 0 && errno != ENOENT) { + error_errno(_("could not read '%s'"), filename); + rollback_lock_file(&lock); + return -1; + } + strbuf_complete(&buf, '\n'); + va_start(ap, fmt); + strbuf_vaddf(&buf, fmt, ap); + va_end(ap); + + if (write_in_full(fd, buf.buf, buf.len) < 0) { + error_errno(_("could not write to '%s'"), filename); + strbuf_release(&buf); + rollback_lock_file(&lock); + return -1; + } + if (commit_lock_file(&lock) < 0) { + strbuf_release(&buf); + rollback_lock_file(&lock); + return error(_("failed to finalize '%s'"), filename); + } + + strbuf_release(&buf); + return 0; +} + +static int do_label(const char *name, int len) +{ + struct ref_store *refs = get_main_ref_store(); + struct ref_transaction *transaction; + struct strbuf ref_name = STRBUF_INIT, err = STRBUF_INIT; + struct strbuf msg = STRBUF_INIT; + int ret = 0; + struct object_id head_oid; + + if (len == 1 && *name == '#') + return error("Illegal label name: '%.*s'", len, name); + + strbuf_addf(&ref_name, "refs/rewritten/%.*s", len, name); + strbuf_addf(&msg, "rebase -i (label) '%.*s'", len, name); + + transaction = ref_store_transaction_begin(refs, &err); + if (!transaction) { + error("%s", err.buf); + ret = -1; + } else if (get_oid("HEAD", &head_oid)) { + error(_("could not read HEAD")); + ret = -1; + } else if (ref_transaction_update(transaction, ref_name.buf, &head_oid, + NULL, 0, msg.buf, &err) < 0 || + ref_transaction_commit(transaction, &err)) { + error("%s", err.buf); + ret = -1; + } + ref_transaction_free(transaction); + strbuf_release(&err); + strbuf_release(&msg); + + if (!ret) + ret = safe_append(rebase_path_refs_to_delete(), + "%s\n", ref_name.buf); + strbuf_release(&ref_name); + + return ret; +} + +static const char *reflog_message(struct replay_opts *opts, + const char *sub_action, const char *fmt, ...); + +static int do_reset(const char *name, int len, struct replay_opts *opts) +{ + struct strbuf ref_name = STRBUF_INIT; + struct object_id oid; + struct lock_file lock = LOCK_INIT; + struct tree_desc desc; + struct tree *tree; + struct unpack_trees_options unpack_tree_opts; + int ret = 0, i; + + if (hold_locked_index(&lock, LOCK_REPORT_ON_ERROR) < 0) + return -1; + + /* Determine the length of the label */ + for (i = 0; i < len; i++) + if (isspace(name[i])) + len = i; + + strbuf_addf(&ref_name, "refs/rewritten/%.*s", len, name); + if (get_oid(ref_name.buf, &oid) && + get_oid(ref_name.buf + strlen("refs/rewritten/"), &oid)) { + error(_("could not read '%s'"), ref_name.buf); + rollback_lock_file(&lock); + strbuf_release(&ref_name); + return -1; + } + + memset(&unpack_tree_opts, 0, sizeof(unpack_tree_opts)); + setup_unpack_trees_porcelain(&unpack_tree_opts, "reset"); + unpack_tree_opts.head_idx = 1; + unpack_tree_opts.src_index = &the_index; + unpack_tree_opts.dst_index = &the_index; + unpack_tree_opts.fn = oneway_merge; + unpack_tree_opts.merge = 1; + unpack_tree_opts.update = 1; + + if (read_cache_unmerged()) { + rollback_lock_file(&lock); + strbuf_release(&ref_name); + return error_resolve_conflict(_(action_name(opts))); + } + + if (!fill_tree_descriptor(&desc, &oid)) { + error(_("failed to find tree of %s"), oid_to_hex(&oid)); + rollback_lock_file(&lock); + free((void *)desc.buffer); + strbuf_release(&ref_name); + return -1; + } + + if (unpack_trees(1, &desc, &unpack_tree_opts)) { + rollback_lock_file(&lock); + free((void *)desc.buffer); + strbuf_release(&ref_name); + return -1; + } + + tree = parse_tree_indirect(&oid); + prime_cache_tree(&the_index, tree); + + if (write_locked_index(&the_index, &lock, COMMIT_LOCK) < 0) + ret = error(_("could not write index")); + free((void *)desc.buffer); + + if (!ret) + ret = update_ref(reflog_message(opts, "reset", "'%.*s'", + len, name), "HEAD", &oid, + NULL, 0, UPDATE_REFS_MSG_ON_ERR); + + strbuf_release(&ref_name); + return ret; +} + static int is_final_fixup(struct todo_list *todo_list) { int i = todo_list->current; @@ -2574,7 +2757,7 @@ N_("Could not execute the todo command\n" static int pick_commits(struct todo_list *todo_list, struct replay_opts *opts) { - int res = 0; + int res = 0, reschedule = 0; setenv(GIT_REFLOG_ACTION, action_name(opts), 0); if (opts->allow_ff) @@ -2645,7 +2828,7 @@ static int pick_commits(struct todo_list *todo_list, struct replay_opts *opts) intend_to_amend(); return error_failed_squash(item->commit, opts, item->arg_len, item->arg); - } else if (res && is_rebase_i(opts)) + } else if (res && is_rebase_i(opts) && item->commit) return res | error_with_patch(item->commit, item->arg, item->arg_len, opts, res, item->command == TODO_REWORD); @@ -2671,9 +2854,25 @@ static int pick_commits(struct todo_list *todo_list, struct replay_opts *opts) /* `current` will be incremented below */ todo_list->current = -1; } + } else if (item->command == TODO_LABEL) { + if ((res = do_label(item->arg, item->arg_len))) + reschedule = 1; + } else if (item->command == TODO_RESET) { + if ((res = do_reset(item->arg, item->arg_len, opts))) + reschedule = 1; } else if (!is_noop(item->command)) return error(_("unknown command %d"), item->command); + if (reschedule) { + advise(_(rescheduled_advice), + get_item_line_length(todo_list, + todo_list->current), + get_item_line(todo_list, todo_list->current)); + todo_list->current--; + if (save_todo(todo_list, opts)) + return -1; + } + todo_list->current++; if (res) return res; -- cgit v1.2.3 From 4c68e7ddb59457efe9d3a141dc600feda86cbe67 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 25 Apr 2018 14:28:54 +0200 Subject: sequencer: introduce the `merge` command This patch is part of the effort to reimplement `--preserve-merges` with a substantially improved design, a design that has been developed in the Git for Windows project to maintain the dozens of Windows-specific patch series on top of upstream Git. The previous patch implemented the `label` and `reset` commands to label commits and to reset to labeled commits. This patch adds the `merge` command, with the following syntax: merge [-C ] # The parameter in this instance is the *original* merge commit, whose author and message will be used for the merge commit that is about to be created. The parameter refers to the (possibly rewritten) revision to merge. Let's see an example of a todo list (the initial `label onto` command is an auto-generated convenience so that the label `onto` can be used to refer to the revision onto which we rebase): label onto # Branch abc reset onto pick deadbeef Hello, world! label abc reset onto pick cafecafe And now for something completely different merge -C baaabaaa abc # Merge the branch 'abc' into master To edit the merge commit's message (a "reword" for merges, if you will), use `-c` (lower-case) instead of `-C`; this convention was borrowed from `git commit` that also supports `-c` and `-C` with similar meanings. To create *new* merges, i.e. without copying the commit message from an existing commit, simply omit the `-C ` parameter (which will open an editor for the merge message): merge abc This comes in handy when splitting a branch into two or more branches. Note: this patch only adds support for recursive merges, to keep things simple. Support for octopus merges will be added later in a separate patch series, support for merges using strategies other than the recursive merge is left for the future. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- sequencer.c | 200 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 200 insertions(+) (limited to 'sequencer.c') diff --git a/sequencer.c b/sequencer.c index c9655edffa..94f4831a0c 100644 --- a/sequencer.c +++ b/sequencer.c @@ -1307,6 +1307,7 @@ enum todo_command { TODO_EXEC, TODO_LABEL, TODO_RESET, + TODO_MERGE, /* commands that do nothing but are counted for reporting progress */ TODO_NOOP, TODO_DROP, @@ -1327,6 +1328,7 @@ static struct { { 'x', "exec" }, { 'l', "label" }, { 't', "reset" }, + { 'm', "merge" }, { 0, "noop" }, { 'd', "drop" }, { 0, NULL } @@ -1754,9 +1756,14 @@ static int read_and_refresh_cache(struct replay_opts *opts) return 0; } +enum todo_item_flags { + TODO_EDIT_MERGE_MSG = 1 +}; + struct todo_item { enum todo_command command; struct commit *commit; + unsigned int flags; const char *arg; int arg_len; size_t offset_in_buf; @@ -1791,6 +1798,8 @@ static int parse_insn_line(struct todo_item *item, const char *bol, char *eol) char *end_of_object_name; int i, saved, status, padding; + item->flags = 0; + /* left-trim */ bol += strspn(bol, " \t"); @@ -1840,6 +1849,21 @@ static int parse_insn_line(struct todo_item *item, const char *bol, char *eol) return 0; } + if (item->command == TODO_MERGE) { + if (skip_prefix(bol, "-C", &bol)) + bol += strspn(bol, " \t"); + else if (skip_prefix(bol, "-c", &bol)) { + bol += strspn(bol, " \t"); + item->flags |= TODO_EDIT_MERGE_MSG; + } else { + item->flags |= TODO_EDIT_MERGE_MSG; + item->commit = NULL; + item->arg = bol; + item->arg_len = (int)(eol - bol); + return 0; + } + } + end_of_object_name = (char *) bol + strcspn(bol, " \t\n"); saved = *end_of_object_name; *end_of_object_name = '\0'; @@ -2654,6 +2678,158 @@ static int do_reset(const char *name, int len, struct replay_opts *opts) return ret; } +static int do_merge(struct commit *commit, const char *arg, int arg_len, + int flags, struct replay_opts *opts) +{ + int run_commit_flags = (flags & TODO_EDIT_MERGE_MSG) ? + EDIT_MSG | VERIFY_MSG : 0; + struct strbuf ref_name = STRBUF_INIT; + struct commit *head_commit, *merge_commit, *i; + struct commit_list *bases, *j, *reversed = NULL; + struct merge_options o; + int merge_arg_len, oneline_offset, ret; + static struct lock_file lock; + const char *p; + + if (hold_locked_index(&lock, LOCK_REPORT_ON_ERROR) < 0) { + ret = -1; + goto leave_merge; + } + + head_commit = lookup_commit_reference_by_name("HEAD"); + if (!head_commit) { + ret = error(_("cannot merge without a current revision")); + goto leave_merge; + } + + oneline_offset = arg_len; + merge_arg_len = strcspn(arg, " \t\n"); + p = arg + merge_arg_len; + p += strspn(p, " \t\n"); + if (*p == '#' && (!p[1] || isspace(p[1]))) { + p += 1 + strspn(p + 1, " \t\n"); + oneline_offset = p - arg; + } else if (p - arg < arg_len) + BUG("octopus merges are not supported yet: '%s'", p); + + strbuf_addf(&ref_name, "refs/rewritten/%.*s", merge_arg_len, arg); + merge_commit = lookup_commit_reference_by_name(ref_name.buf); + if (!merge_commit) { + /* fall back to non-rewritten ref or commit */ + strbuf_splice(&ref_name, 0, strlen("refs/rewritten/"), "", 0); + merge_commit = lookup_commit_reference_by_name(ref_name.buf); + } + + if (!merge_commit) { + ret = error(_("could not resolve '%s'"), ref_name.buf); + goto leave_merge; + } + + if (commit) { + const char *message = get_commit_buffer(commit, NULL); + const char *body; + int len; + + if (!message) { + ret = error(_("could not get commit message of '%s'"), + oid_to_hex(&commit->object.oid)); + goto leave_merge; + } + write_author_script(message); + find_commit_subject(message, &body); + len = strlen(body); + ret = write_message(body, len, git_path_merge_msg(), 0); + unuse_commit_buffer(commit, message); + if (ret) { + error_errno(_("could not write '%s'"), + git_path_merge_msg()); + goto leave_merge; + } + } else { + struct strbuf buf = STRBUF_INIT; + int len; + + strbuf_addf(&buf, "author %s", git_author_info(0)); + write_author_script(buf.buf); + strbuf_reset(&buf); + + if (oneline_offset < arg_len) { + p = arg + oneline_offset; + len = arg_len - oneline_offset; + } else { + strbuf_addf(&buf, "Merge branch '%.*s'", + merge_arg_len, arg); + p = buf.buf; + len = buf.len; + } + + ret = write_message(p, len, git_path_merge_msg(), 0); + strbuf_release(&buf); + if (ret) { + error_errno(_("could not write '%s'"), + git_path_merge_msg()); + goto leave_merge; + } + } + + write_message(oid_to_hex(&merge_commit->object.oid), GIT_SHA1_HEXSZ, + git_path_merge_head(), 0); + write_message("no-ff", 5, git_path_merge_mode(), 0); + + bases = get_merge_bases(head_commit, merge_commit); + for (j = bases; j; j = j->next) + commit_list_insert(j->item, &reversed); + free_commit_list(bases); + + read_cache(); + init_merge_options(&o); + o.branch1 = "HEAD"; + o.branch2 = ref_name.buf; + o.buffer_output = 2; + + ret = merge_recursive(&o, head_commit, merge_commit, reversed, &i); + if (ret <= 0) + fputs(o.obuf.buf, stdout); + strbuf_release(&o.obuf); + if (ret < 0) { + error(_("could not even attempt to merge '%.*s'"), + merge_arg_len, arg); + goto leave_merge; + } + /* + * The return value of merge_recursive() is 1 on clean, and 0 on + * unclean merge. + * + * Let's reverse that, so that do_merge() returns 0 upon success and + * 1 upon failed merge (keeping the return value -1 for the cases where + * we will want to reschedule the `merge` command). + */ + ret = !ret; + + if (active_cache_changed && + write_locked_index(&the_index, &lock, COMMIT_LOCK)) { + ret = error(_("merge: Unable to write new index file")); + goto leave_merge; + } + + rollback_lock_file(&lock); + if (ret) + rerere(opts->allow_rerere_auto); + else + /* + * In case of problems, we now want to return a positive + * value (a negative one would indicate that the `merge` + * command needs to be rescheduled). + */ + ret = !!run_git_commit(git_path_merge_msg(), opts, + run_commit_flags); + +leave_merge: + strbuf_release(&ref_name); + rollback_lock_file(&lock); + return ret; +} + static int is_final_fixup(struct todo_list *todo_list) { int i = todo_list->current; @@ -2860,6 +3036,17 @@ static int pick_commits(struct todo_list *todo_list, struct replay_opts *opts) } else if (item->command == TODO_RESET) { if ((res = do_reset(item->arg, item->arg_len, opts))) reschedule = 1; + } else if (item->command == TODO_MERGE) { + if ((res = do_merge(item->commit, + item->arg, item->arg_len, + item->flags, opts)) < 0) + reschedule = 1; + else if (res > 0) + /* failed with merge conflicts */ + return error_with_patch(item->commit, + item->arg, + item->arg_len, opts, + res, 0); } else if (!is_noop(item->command)) return error(_("unknown command %d"), item->command); @@ -2871,6 +3058,11 @@ static int pick_commits(struct todo_list *todo_list, struct replay_opts *opts) todo_list->current--; if (save_todo(todo_list, opts)) return -1; + if (item->commit) + return error_with_patch(item->commit, + item->arg, + item->arg_len, opts, + res, 0); } todo_list->current++; @@ -3356,8 +3548,16 @@ int transform_todos(unsigned flags) short_commit_name(item->commit) : oid_to_hex(&item->commit->object.oid); + if (item->command == TODO_MERGE) { + if (item->flags & TODO_EDIT_MERGE_MSG) + strbuf_addstr(&buf, " -c"); + else + strbuf_addstr(&buf, " -C"); + } + strbuf_addf(&buf, " %s", oid); } + /* add all the rest */ if (!item->arg_len) strbuf_addch(&buf, '\n'); -- cgit v1.2.3 From d1e8b0114bef98e8efe3f8d35430300ff9e00443 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 25 Apr 2018 14:28:56 +0200 Subject: sequencer: fast-forward `merge` commands, if possible Just like with regular `pick` commands, if we are trying to rebase a merge commit, we now test whether the parents of said commit match HEAD and the commits to be merged, and fast-forward if possible. This is not only faster, but also avoids unnecessary proliferation of new objects. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- sequencer.c | 33 ++++++++++++++++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-) (limited to 'sequencer.c') diff --git a/sequencer.c b/sequencer.c index 94f4831a0c..6722095655 100644 --- a/sequencer.c +++ b/sequencer.c @@ -2687,7 +2687,7 @@ static int do_merge(struct commit *commit, const char *arg, int arg_len, struct commit *head_commit, *merge_commit, *i; struct commit_list *bases, *j, *reversed = NULL; struct merge_options o; - int merge_arg_len, oneline_offset, ret; + int merge_arg_len, oneline_offset, can_fast_forward, ret; static struct lock_file lock; const char *p; @@ -2772,6 +2772,37 @@ static int do_merge(struct commit *commit, const char *arg, int arg_len, } } + /* + * If HEAD is not identical to the first parent of the original merge + * commit, we cannot fast-forward. + */ + can_fast_forward = opts->allow_ff && commit && commit->parents && + !oidcmp(&commit->parents->item->object.oid, + &head_commit->object.oid); + + /* + * If the merge head is different from the original one, we cannot + * fast-forward. + */ + if (can_fast_forward) { + struct commit_list *second_parent = commit->parents->next; + + if (second_parent && !second_parent->next && + oidcmp(&merge_commit->object.oid, + &second_parent->item->object.oid)) + can_fast_forward = 0; + } + + if (can_fast_forward && commit->parents->next && + !commit->parents->next->next && + !oidcmp(&commit->parents->next->item->object.oid, + &merge_commit->object.oid)) { + rollback_lock_file(&lock); + ret = fast_forward_to(&commit->object.oid, + &head_commit->object.oid, 0, opts); + goto leave_merge; + } + write_message(oid_to_hex(&merge_commit->object.oid), GIT_SHA1_HEXSZ, git_path_merge_head(), 0); write_message("no-ff", 5, git_path_merge_mode(), 0); -- cgit v1.2.3 From 1644c73c6d4fe35d4d3304b9c03c9586de51626f Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 25 Apr 2018 14:29:03 +0200 Subject: rebase-helper --make-script: introduce a flag to rebase merges The sequencer just learned new commands intended to recreate branch structure (similar in spirit to --preserve-merges, but with a substantially less-broken design). Let's allow the rebase--helper to generate todo lists making use of these commands, triggered by the new --rebase-merges option. For a commit topology like this (where the HEAD points to C): - A - B - C \ / D the generated todo list would look like this: # branch D pick 0123 A label branch-point pick 1234 D label D reset branch-point pick 2345 B merge -C 3456 D # C To keep things simple, we first only implement support for merge commits with exactly two parents, leaving support for octopus merges to a later patch series. All merge-rebasing todo lists start with a hard-coded `label onto` line. This makes it convenient to refer later on to the revision onto which everything is rebased, e.g. as starting point for branches other than the very first one. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- sequencer.c | 346 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 345 insertions(+), 1 deletion(-) (limited to 'sequencer.c') diff --git a/sequencer.c b/sequencer.c index 6722095655..e929712263 100644 --- a/sequencer.c +++ b/sequencer.c @@ -25,6 +25,8 @@ #include "sigchain.h" #include "unpack-trees.h" #include "worktree.h" +#include "oidmap.h" +#include "oidset.h" #define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION" @@ -3448,6 +3450,343 @@ void append_signoff(struct strbuf *msgbuf, int ignore_footer, unsigned flag) strbuf_release(&sob); } +struct labels_entry { + struct hashmap_entry entry; + char label[FLEX_ARRAY]; +}; + +static int labels_cmp(const void *fndata, const struct labels_entry *a, + const struct labels_entry *b, const void *key) +{ + return key ? strcmp(a->label, key) : strcmp(a->label, b->label); +} + +struct string_entry { + struct oidmap_entry entry; + char string[FLEX_ARRAY]; +}; + +struct label_state { + struct oidmap commit2label; + struct hashmap labels; + struct strbuf buf; +}; + +static const char *label_oid(struct object_id *oid, const char *label, + struct label_state *state) +{ + struct labels_entry *labels_entry; + struct string_entry *string_entry; + struct object_id dummy; + size_t len; + int i; + + string_entry = oidmap_get(&state->commit2label, oid); + if (string_entry) + return string_entry->string; + + /* + * For "uninteresting" commits, i.e. commits that are not to be + * rebased, and which can therefore not be labeled, we use a unique + * abbreviation of the commit name. This is slightly more complicated + * than calling find_unique_abbrev() because we also need to make + * sure that the abbreviation does not conflict with any other + * label. + * + * We disallow "interesting" commits to be labeled by a string that + * is a valid full-length hash, to ensure that we always can find an + * abbreviation for any uninteresting commit's names that does not + * clash with any other label. + */ + if (!label) { + char *p; + + strbuf_reset(&state->buf); + strbuf_grow(&state->buf, GIT_SHA1_HEXSZ); + label = p = state->buf.buf; + + find_unique_abbrev_r(p, oid, default_abbrev); + + /* + * We may need to extend the abbreviated hash so that there is + * no conflicting label. + */ + if (hashmap_get_from_hash(&state->labels, strihash(p), p)) { + size_t i = strlen(p) + 1; + + oid_to_hex_r(p, oid); + for (; i < GIT_SHA1_HEXSZ; i++) { + char save = p[i]; + p[i] = '\0'; + if (!hashmap_get_from_hash(&state->labels, + strihash(p), p)) + break; + p[i] = save; + } + } + } else if (((len = strlen(label)) == GIT_SHA1_RAWSZ && + !get_oid_hex(label, &dummy)) || + (len == 1 && *label == '#') || + hashmap_get_from_hash(&state->labels, + strihash(label), label)) { + /* + * If the label already exists, or if the label is a valid full + * OID, or the label is a '#' (which we use as a separator + * between merge heads and oneline), we append a dash and a + * number to make it unique. + */ + struct strbuf *buf = &state->buf; + + strbuf_reset(buf); + strbuf_add(buf, label, len); + + for (i = 2; ; i++) { + strbuf_setlen(buf, len); + strbuf_addf(buf, "-%d", i); + if (!hashmap_get_from_hash(&state->labels, + strihash(buf->buf), + buf->buf)) + break; + } + + label = buf->buf; + } + + FLEX_ALLOC_STR(labels_entry, label, label); + hashmap_entry_init(labels_entry, strihash(label)); + hashmap_add(&state->labels, labels_entry); + + FLEX_ALLOC_STR(string_entry, string, label); + oidcpy(&string_entry->entry.oid, oid); + oidmap_put(&state->commit2label, string_entry); + + return string_entry->string; +} + +static int make_script_with_merges(struct pretty_print_context *pp, + struct rev_info *revs, FILE *out, + unsigned flags) +{ + int keep_empty = flags & TODO_LIST_KEEP_EMPTY; + struct strbuf buf = STRBUF_INIT, oneline = STRBUF_INIT; + struct strbuf label = STRBUF_INIT; + struct commit_list *commits = NULL, **tail = &commits, *iter; + struct commit_list *tips = NULL, **tips_tail = &tips; + struct commit *commit; + struct oidmap commit2todo = OIDMAP_INIT; + struct string_entry *entry; + struct oidset interesting = OIDSET_INIT, child_seen = OIDSET_INIT, + shown = OIDSET_INIT; + struct label_state state = { OIDMAP_INIT, { NULL }, STRBUF_INIT }; + + int abbr = flags & TODO_LIST_ABBREVIATE_CMDS; + const char *cmd_pick = abbr ? "p" : "pick", + *cmd_label = abbr ? "l" : "label", + *cmd_reset = abbr ? "t" : "reset", + *cmd_merge = abbr ? "m" : "merge"; + + oidmap_init(&commit2todo, 0); + oidmap_init(&state.commit2label, 0); + hashmap_init(&state.labels, (hashmap_cmp_fn) labels_cmp, NULL, 0); + strbuf_init(&state.buf, 32); + + if (revs->cmdline.nr && (revs->cmdline.rev[0].flags & BOTTOM)) { + struct object_id *oid = &revs->cmdline.rev[0].item->oid; + FLEX_ALLOC_STR(entry, string, "onto"); + oidcpy(&entry->entry.oid, oid); + oidmap_put(&state.commit2label, entry); + } + + /* + * First phase: + * - get onelines for all commits + * - gather all branch tips (i.e. 2nd or later parents of merges) + * - label all branch tips + */ + while ((commit = get_revision(revs))) { + struct commit_list *to_merge; + int is_octopus; + const char *p1, *p2; + struct object_id *oid; + int is_empty; + + tail = &commit_list_insert(commit, tail)->next; + oidset_insert(&interesting, &commit->object.oid); + + is_empty = is_original_commit_empty(commit); + if (!is_empty && (commit->object.flags & PATCHSAME)) + continue; + + strbuf_reset(&oneline); + pretty_print_commit(pp, commit, &oneline); + + to_merge = commit->parents ? commit->parents->next : NULL; + if (!to_merge) { + /* non-merge commit: easy case */ + strbuf_reset(&buf); + if (!keep_empty && is_empty) + strbuf_addf(&buf, "%c ", comment_line_char); + strbuf_addf(&buf, "%s %s %s", cmd_pick, + oid_to_hex(&commit->object.oid), + oneline.buf); + + FLEX_ALLOC_STR(entry, string, buf.buf); + oidcpy(&entry->entry.oid, &commit->object.oid); + oidmap_put(&commit2todo, entry); + + continue; + } + + is_octopus = to_merge && to_merge->next; + + if (is_octopus) + BUG("Octopus merges not yet supported"); + + /* Create a label */ + strbuf_reset(&label); + if (skip_prefix(oneline.buf, "Merge ", &p1) && + (p1 = strchr(p1, '\'')) && + (p2 = strchr(++p1, '\''))) + strbuf_add(&label, p1, p2 - p1); + else if (skip_prefix(oneline.buf, "Merge pull request ", + &p1) && + (p1 = strstr(p1, " from "))) + strbuf_addstr(&label, p1 + strlen(" from ")); + else + strbuf_addbuf(&label, &oneline); + + for (p1 = label.buf; *p1; p1++) + if (isspace(*p1)) + *(char *)p1 = '-'; + + strbuf_reset(&buf); + strbuf_addf(&buf, "%s -C %s", + cmd_merge, oid_to_hex(&commit->object.oid)); + + /* label the tip of merged branch */ + oid = &to_merge->item->object.oid; + strbuf_addch(&buf, ' '); + + if (!oidset_contains(&interesting, oid)) + strbuf_addstr(&buf, label_oid(oid, NULL, &state)); + else { + tips_tail = &commit_list_insert(to_merge->item, + tips_tail)->next; + + strbuf_addstr(&buf, label_oid(oid, label.buf, &state)); + } + strbuf_addf(&buf, " # %s", oneline.buf); + + FLEX_ALLOC_STR(entry, string, buf.buf); + oidcpy(&entry->entry.oid, &commit->object.oid); + oidmap_put(&commit2todo, entry); + } + + /* + * Second phase: + * - label branch points + * - add HEAD to the branch tips + */ + for (iter = commits; iter; iter = iter->next) { + struct commit_list *parent = iter->item->parents; + for (; parent; parent = parent->next) { + struct object_id *oid = &parent->item->object.oid; + if (!oidset_contains(&interesting, oid)) + continue; + if (!oidset_contains(&child_seen, oid)) + oidset_insert(&child_seen, oid); + else + label_oid(oid, "branch-point", &state); + } + + /* Add HEAD as implict "tip of branch" */ + if (!iter->next) + tips_tail = &commit_list_insert(iter->item, + tips_tail)->next; + } + + /* + * Third phase: output the todo list. This is a bit tricky, as we + * want to avoid jumping back and forth between revisions. To + * accomplish that goal, we walk backwards from the branch tips, + * gathering commits not yet shown, reversing the list on the fly, + * then outputting that list (labeling revisions as needed). + */ + fprintf(out, "%s onto\n", cmd_label); + for (iter = tips; iter; iter = iter->next) { + struct commit_list *list = NULL, *iter2; + + commit = iter->item; + if (oidset_contains(&shown, &commit->object.oid)) + continue; + entry = oidmap_get(&state.commit2label, &commit->object.oid); + + if (entry) + fprintf(out, "\n# Branch %s\n", entry->string); + else + fprintf(out, "\n"); + + while (oidset_contains(&interesting, &commit->object.oid) && + !oidset_contains(&shown, &commit->object.oid)) { + commit_list_insert(commit, &list); + if (!commit->parents) { + commit = NULL; + break; + } + commit = commit->parents->item; + } + + if (!commit) + fprintf(out, "%s onto\n", cmd_reset); + else { + const char *to = NULL; + + entry = oidmap_get(&state.commit2label, + &commit->object.oid); + if (entry) + to = entry->string; + + if (!to || !strcmp(to, "onto")) + fprintf(out, "%s onto\n", cmd_reset); + else { + strbuf_reset(&oneline); + pretty_print_commit(pp, commit, &oneline); + fprintf(out, "%s %s # %s\n", + cmd_reset, to, oneline.buf); + } + } + + for (iter2 = list; iter2; iter2 = iter2->next) { + struct object_id *oid = &iter2->item->object.oid; + entry = oidmap_get(&commit2todo, oid); + /* only show if not already upstream */ + if (entry) + fprintf(out, "%s\n", entry->string); + entry = oidmap_get(&state.commit2label, oid); + if (entry) + fprintf(out, "%s %s\n", + cmd_label, entry->string); + oidset_insert(&shown, oid); + } + + free_commit_list(list); + } + + free_commit_list(commits); + free_commit_list(tips); + + strbuf_release(&label); + strbuf_release(&oneline); + strbuf_release(&buf); + + oidmap_free(&commit2todo, 1); + oidmap_free(&state.commit2label, 1); + hashmap_free(&state.labels, 1); + strbuf_release(&state.buf); + + return 0; +} + int sequencer_make_script(FILE *out, int argc, const char **argv, unsigned flags) { @@ -3458,10 +3797,12 @@ int sequencer_make_script(FILE *out, int argc, const char **argv, struct commit *commit; int keep_empty = flags & TODO_LIST_KEEP_EMPTY; const char *insn = flags & TODO_LIST_ABBREVIATE_CMDS ? "p" : "pick"; + int rebase_merges = flags & TODO_LIST_REBASE_MERGES; init_revisions(&revs, NULL); revs.verbose_header = 1; - revs.max_parents = 1; + if (!rebase_merges) + revs.max_parents = 1; revs.cherry_mark = 1; revs.limited = 1; revs.reverse = 1; @@ -3486,6 +3827,9 @@ int sequencer_make_script(FILE *out, int argc, const char **argv, if (prepare_revision_walk(&revs) < 0) return error(_("make_script: error preparing revisions")); + if (rebase_merges) + return make_script_with_merges(&pp, &revs, out, flags); + while ((commit = get_revision(&revs))) { int is_empty = is_original_commit_empty(commit); -- cgit v1.2.3 From 537e7d61359233e95d2ca6ef6b9059afca8daa81 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 25 Apr 2018 14:29:29 +0200 Subject: sequencer: handle post-rewrite for merge commands In the previous patches, we implemented the basic functionality of the `git rebase -i --rebase-merges` command, in particular the `merge` command to create merge commits in the sequencer. The interactive rebase is a lot more these days, though, than a simple cherry-pick in a loop. For example, it calls the post-rewrite hook (if any) after rebasing with a mapping of the old->new commits. This patch implements the post-rewrite handling for the `merge` command we just introduced. The other commands that were added recently (`label` and `reset`) do not create new commits, therefore post-rewrite hooks do not need to handle them. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- sequencer.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'sequencer.c') diff --git a/sequencer.c b/sequencer.c index e929712263..558efc1af6 100644 --- a/sequencer.c +++ b/sequencer.c @@ -3074,7 +3074,10 @@ static int pick_commits(struct todo_list *todo_list, struct replay_opts *opts) item->arg, item->arg_len, item->flags, opts)) < 0) reschedule = 1; - else if (res > 0) + else if (item->commit) + record_in_rewritten(&item->commit->object.oid, + peek_command(todo_list, 1)); + if (res > 0) /* failed with merge conflicts */ return error_with_patch(item->commit, item->arg, -- cgit v1.2.3 From 7ccdf65b63f2f4a5e751d70b9788af3c9e16b6ab Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 25 Apr 2018 14:29:31 +0200 Subject: rebase --rebase-merges: avoid "empty merges" The `git merge` command does not allow merging commits that are already reachable from HEAD: `git merge HEAD^`, for example, will report that we are already up to date and not change a thing. In an interactive rebase, such a merge could occur previously, e.g. when competing (or slightly modified) versions of a patch series were applied upstream, and the user had to `git rebase --skip` all of the local commits, and the topic branch becomes "empty" as a consequence. Let's teach the todo command `merge` to behave the same as `git merge`. Seeing as it requires some low-level trickery to create such merges with Git's commands in the first place, we do not even have to bother to introduce an option to force `merge` to create such merge commits. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- sequencer.c | 7 +++++++ 1 file changed, 7 insertions(+) (limited to 'sequencer.c') diff --git a/sequencer.c b/sequencer.c index 558efc1af6..afa155c282 100644 --- a/sequencer.c +++ b/sequencer.c @@ -2810,6 +2810,13 @@ static int do_merge(struct commit *commit, const char *arg, int arg_len, write_message("no-ff", 5, git_path_merge_mode(), 0); bases = get_merge_bases(head_commit, merge_commit); + if (bases && !oidcmp(&merge_commit->object.oid, + &bases->item->object.oid)) { + ret = 0; + /* skip merging an ancestor of HEAD */ + goto leave_merge; + } + for (j = bases; j; j = j->next) commit_list_insert(j->item, &reversed); free_commit_list(bases); -- cgit v1.2.3 From 7543f6f4441a0ec76460a54f90ab8674fe424786 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 25 Apr 2018 14:29:40 +0200 Subject: rebase -i: introduce --rebase-merges=[no-]rebase-cousins When running `git rebase --rebase-merges` non-interactively with an ancestor of HEAD as (or leaving the todo list unmodified), we would ideally recreate the exact same commits as before the rebase. However, if there are commits in the commit range .. that do not have as direct ancestor (i.e. if `git log ..` would show commits that are omitted by `git log --ancestry-path ..`), this is currently not the case: we would turn them into commits that have as direct ancestor. Let's illustrate that with a diagram: C / \ A - B - E - F \ / D Currently, after running `git rebase -i --rebase-merges B`, the new branch structure would be (pay particular attention to the commit `D`): --- C' -- / \ A - B ------ E' - F' \ / D' This is not really preserving the branch topology from before! The reason is that the commit `D` does not have `B` as ancestor, and therefore it gets rebased onto `B`. This is unintuitive behavior. Even worse, when recreating branch structure, most use cases would appear to want cousins *not* to be rebased onto the new base commit. For example, Git for Windows (the heaviest user of the Git garden shears, which served as the blueprint for --rebase-merges) frequently merges branches from `next` early, and these branches certainly do *not* want to be rebased. In the example above, the desired outcome would look like this: --- C' -- / \ A - B ------ E' - F' \ / -- D' -- Let's introduce the term "cousins" for such commits ("D" in the example), and let's not rebase them by default. For hypothetical use cases where cousins *do* need to be rebased, `git rebase --rebase=merges=rebase-cousins` needs to be used. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- sequencer.c | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'sequencer.c') diff --git a/sequencer.c b/sequencer.c index afa155c282..e2f8394284 100644 --- a/sequencer.c +++ b/sequencer.c @@ -3578,6 +3578,7 @@ static int make_script_with_merges(struct pretty_print_context *pp, unsigned flags) { int keep_empty = flags & TODO_LIST_KEEP_EMPTY; + int rebase_cousins = flags & TODO_LIST_REBASE_COUSINS; struct strbuf buf = STRBUF_INIT, oneline = STRBUF_INIT; struct strbuf label = STRBUF_INIT; struct commit_list *commits = NULL, **tail = &commits, *iter; @@ -3755,6 +3756,9 @@ static int make_script_with_merges(struct pretty_print_context *pp, &commit->object.oid); if (entry) to = entry->string; + else if (!rebase_cousins) + to = label_oid(&commit->object.oid, NULL, + &state); if (!to || !strcmp(to, "onto")) fprintf(out, "%s onto\n", cmd_reset); -- cgit v1.2.3