On 13/04/18 11:12, Phillip Wood wrote:
> On 10/04/18 13:29, Johannes Schindelin wrote:
>> +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;
>> +
>> + 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) {
>> + error(_("could not resolve '%s'"), ref_name.buf);
>> + strbuf_release(&ref_name);
>> + return -1;
>> + }
>> +
>> + if (hold_locked_index(&lock, LOCK_REPORT_ON_ERROR) < 0)
>> + return -1;
>> +
>> + head_commit = lookup_commit_reference_by_name("HEAD");
>> + if (!head_commit) {
>> + rollback_lock_file(&lock);
>> + return error(_("cannot merge without a current revision"));
>> + }
>> +
>> + if (commit) {
>> + const char *message = get_commit_buffer(commit, NULL);
>> + const char *body;
>> + int len;
>> +
>> + if (!message) {
>> + rollback_lock_file(&lock);
>> + return error(_("could not get commit message of '%s'"),
>> + oid_to_hex(&commit->object.oid));
>> + }
>> + write_author_script(message);
>> + find_commit_subject(message, &body);
>> + len = strlen(body);
>> + if (write_message(body, len, git_path_merge_msg(), 0) < 0) {
>> + error_errno(_("could not write '%s'"),
>> + git_path_merge_msg());
>> + unuse_commit_buffer(commit, message);
>> + rollback_lock_file(&lock);
>> + return -1;
>> + }
>> + unuse_commit_buffer(commit, message);
>> + } 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;
>> + }
>> +
>> + if (write_message(p, len, git_path_merge_msg(), 0) < 0) {
>> + error_errno(_("could not write '%s'"),
>> + git_path_merge_msg());
>> + strbuf_release(&buf);
>> + rollback_lock_file(&lock);
>> + return -1;
>> + }
>> + strbuf_release(&buf);
>> + }
>> +
>> + 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)
>> + rerere(opts->allow_rerere_auto);
>> + if (ret <= 0)
>> + fputs(o.obuf.buf, stdout);
>> + strbuf_release(&o.obuf);
>> + if (ret < 0) {
>> + strbuf_release(&ref_name);
>> + rollback_lock_file(&lock);
>> + return error(_("conflicts while merging '%.*s'"),
>> + merge_arg_len, arg);
>> + }
>
> If there are conflicts then ret == 0 rather than -1
>
>> +
>> + if (active_cache_changed &&
>> + write_locked_index(&the_index, &lock, COMMIT_LOCK)) {
>> + strbuf_release(&ref_name);
>> + return error(_("merge: Unable to write new index file"));
>> + }
>> + rollback_lock_file(&lock);
>> +
>> + ret = run_git_commit(git_path_merge_msg(), opts, run_commit_flags);
>
> If there were conflicts this will try and run git commit with unmerged
> cache entries
>
>> + strbuf_release(&ref_name);
>> +
>> + return ret;
>> +}
>> +
>
> If the merge fails with an error rather than conflicts then I think it
> should be rescheduled as we do for picks that fail with an error. The
> patch below does that and also adjusts the logic following the merge so
> that it does not call 'git commit' when there are conflicts. I think we
> may want to say something about fixing the conflicts and running
> 'git rebase --continue' as we do for conflicts when picking.
>
> Best Wishes
>
> Phillip
>
> --->8---
> From: Phillip Wood <phillip.w...@dunelm.org.uk>
> Subject: [PATCH] fixup! sequencer: introduce the `merge` command
>
>
> Signed-off-by: Phillip Wood <phillip.w...@dunelm.org.uk>
> ---
> sequencer.c | 20 ++++++++++----------
> 1 file changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/sequencer.c b/sequencer.c
> index e1b9be7327..511b7fddca 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -2807,27 +2807,26 @@ static int do_merge(struct commit *commit, const char
> *arg, int arg_len,
> o.buffer_output = 2;
>
> ret = merge_recursive(&o, head_commit, merge_commit, reversed, &i);
> - if (!ret)
> - rerere(opts->allow_rerere_auto);
> + strbuf_release(&ref_name);
> if (ret <= 0)
> fputs(o.obuf.buf, stdout);
> strbuf_release(&o.obuf);
> if (ret < 0) {
> - strbuf_release(&ref_name);
> rollback_lock_file(&lock);
> - return error(_("conflicts while merging '%.*s'"),
> - merge_arg_len, arg);
> + return ret;
> }
>
> if (active_cache_changed &&
> - write_locked_index(&the_index, &lock, COMMIT_LOCK)) {
> - strbuf_release(&ref_name);
> + write_locked_index(&the_index, &lock, COMMIT_LOCK))
> return error(_("merge: Unable to write new index file"));
> - }
> rollback_lock_file(&lock);
> + if (!ret) {
> + rerere(opts->allow_rerere_auto);
> + error(_("conflicts while merging '%.*s'"), merge_arg_len, arg);
> + return 1;
> + }
>
> ret = run_git_commit(git_path_merge_msg(), opts, run_commit_flags);
> - strbuf_release(&ref_name);
>
> return ret;
> }
> @@ -3030,7 +3029,8 @@ static int pick_commits(struct todo_list *todo_list,
> struct replay_opts *opts)
> return error(_("unknown command %d"), item->command);
>
> if (res < 0 && (item->command == TODO_LABEL ||
> - item->command == TODO_RESET)) {
> + item->command == TODO_RESET ||
> + item->command == TODO_MERGE)) {
Unfortunately it's not as simple as that - we only want to reschedule if
merge_recursive() fails, not if run_git_commit() does.
> /* Reschedule */
> todo_list->current--;
> save_todo(todo_list, opts);
>
