From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: AS31976 209.132.180.0/23 X-Spam-Status: No, score=-3.6 required=3.0 tests=AWL,BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, RCVD_IN_DNSWL_HI,T_DKIMWL_WL_MED shortcircuit=no autolearn=ham autolearn_force=no version=3.4.0 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by dcvr.yhbt.net (Postfix) with ESMTP id D05841F424 for ; Fri, 13 Apr 2018 10:12:35 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754144AbeDMKMd (ORCPT ); Fri, 13 Apr 2018 06:12:33 -0400 Received: from smtp-out-6.talktalk.net ([62.24.135.70]:30207 "EHLO smtp-out-6.talktalk.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753931AbeDMKMc (ORCPT ); Fri, 13 Apr 2018 06:12:32 -0400 Received: from [192.168.2.201] ([92.28.131.105]) by smtp.talktalk.net with SMTP id 6vgzfXo4HLjjA6vgzfUHQL; Fri, 13 Apr 2018 11:12:30 +0100 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=talktalk.net; s=cmr1711; t=1523614350; bh=+EP7sUXedEYN969NLnWDCXSmPgsHU2PJXrLi+6pxHN8=; h=Reply-To:Subject:To:Cc:References:From:Date:In-Reply-To; b=MGvYFOolKzqjqWw2ZeuuJMuCxLyuvurw7RDziQUpCG8VwKhJeL463wZFUvhLaifEB 8ftqJWvPX5+O0vdjo+44RLizoG1Jm0lMcYrT2ygqVg2wWupECDl4dQ8TwGnFY+OS8G palYLdiPvovsC39omFXoHcG1Uy/27cgkAEhsdHeo= X-Originating-IP: [92.28.131.105] X-Spam: 0 X-OAuthority: v=2.3 cv=LZYSFAXi c=1 sm=1 tr=0 a=D1tPBkQZhJ8hQpCscnDOhQ==:117 a=D1tPBkQZhJ8hQpCscnDOhQ==:17 a=IkcTkHD0fZMA:10 a=evINK-nbAAAA:8 a=Ud08yLbfuBoa9Tj0uh8A:9 a=zgM-xqpDGvIzRWl-:21 a=M6jvSlbJ8TtQPSvx:21 a=QEXdDO2ut3YA:10 a=RfR_gqz1fSpA9VikTjo0:22 Reply-To: phillip.wood@dunelm.org.uk Subject: Re: [PATCH v6 05/15] sequencer: introduce the `merge` command To: Johannes Schindelin , git@vger.kernel.org Cc: Junio C Hamano , Jacob Keller , Stefan Beller , Philip Oakley , Eric Sunshine , Phillip Wood , Igor Djordjevic , Johannes Sixt , Sergey Organov References: <8a5ae61f603df8d8cb19268e1b8470f0a4993445.1523362469.git.johannes.schindelin@gmx.de> From: Phillip Wood Message-ID: <55216d0b-cb37-6514-7f80-91f4617c105d@talktalk.net> Date: Fri, 13 Apr 2018 11:12:29 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.7.0 MIME-Version: 1.0 In-Reply-To: <8a5ae61f603df8d8cb19268e1b8470f0a4993445.1523362469.git.johannes.schindelin@gmx.de> Content-Type: text/plain; charset=utf-8 Content-Language: en-GB Content-Transfer-Encoding: 7bit X-CMAE-Envelope: MS4wfPXpldxxZHvWFUMqd4CnYNTVPi7KQ6+Hz5zgIF4n9B0Pj9qJGEFYOio3vL0TZtEOixa4zWszKrUBrKCIW3ZPWkg90fDKTlS+YcwaoJGMbeiuQ6QYMQ0V Kia7z33QqtSJA32ZV0DyAFpKkHZdg9BG5iFcLqNtRbdvGeNcME1k1l7G2dbYcyPTzsYwx5jLOHKSNKsFFEL8RgsLkZzzx651puvbeLu0TMxYqnYWMpJgcKNV B8WbyFYrFyZwFX28nfkBp8r00VGjBxavT+vw1ANm3oGsox/XDm2p1d1ect0F4MXFgqJ1upsp0HO/Cvt5VNBeniTvrLPJkEhA3Kt0CqvkNDCx8woe7XOHtq0Y RLnq1CoGt8+5NGhtzOnVofcM5ljvfnjCECfMn/FZL0qfMBmkUiFvcT+4pHOcKLkKJsUQDXsMGxIpngaz0f+4hOrR11igEo0CCzsY7dkbG+W1B5Njhs+ETq2n ksx1I+sXMJFnMmO3IHbGXAD5NIw94UE1aNXS/g== Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org 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 Subject: [PATCH] fixup! sequencer: introduce the `merge` command Signed-off-by: Phillip Wood --- 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)) { /* Reschedule */ todo_list->current--; save_todo(todo_list, opts); -- 2.17.0