From patchwork Fri Jul 22 05:15:36 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Elijah Newren X-Patchwork-Id: 12925965 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 2AB81C433EF for ; Fri, 22 Jul 2022 05:15:52 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230113AbiGVFPu (ORCPT ); Fri, 22 Jul 2022 01:15:50 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:51182 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229519AbiGVFPs (ORCPT ); Fri, 22 Jul 2022 01:15:48 -0400 Received: from mail-wr1-x436.google.com (mail-wr1-x436.google.com [IPv6:2a00:1450:4864:20::436]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 1D4F32317D for ; Thu, 21 Jul 2022 22:15:47 -0700 (PDT) Received: by mail-wr1-x436.google.com with SMTP id l15so942751wro.11 for ; Thu, 21 Jul 2022 22:15:47 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=message-id:in-reply-to:references:from:date:subject:fcc :content-transfer-encoding:mime-version:to:cc; bh=gN0tZ+ael5Qgw/uE37c3BdgOrXL44tqLZY6nsQFMh3w=; b=etn8rhLIowTO8pggvh44AmP6DBiPUv3bYuKQay2eKEZRUWLeeARSd1gpcW+iwkqSeg qQGki/0oeHzf7bKORoHZhwhZWT5G8F3dyWOp7aavKb2AS8acMCceUxI33MBuq2Q3knjy 9TtsFV7vu1h6e6TzlErVjbPVpThCUpDIMnqEqOb75Odsa09rS1317B33NlEceg2Z+rB9 JpN5SP/q9AAfL+57w1wPjT0KDE6YXfCFOwuBNbWqgDZ5/2phrHWeeAQKoDay9C7nbmZt e6+WgQpIbXSfxskDmarn/65aRBY/Hph+4qYBVpSOrVbdtMHzn20+eiSOuUapqfcl44kY Lfdg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:in-reply-to:references:from:date :subject:fcc:content-transfer-encoding:mime-version:to:cc; bh=gN0tZ+ael5Qgw/uE37c3BdgOrXL44tqLZY6nsQFMh3w=; b=rmE57TwkPMI4KZH0/qtQCILGklPPr2bV+4JD0/Ts5Fn26vF3913/E4hWQRiDw0ZIEq 7hgBSRuMx+EMl3UtytCLrJuNSWSRXZCYb9w4wSiB1Gm6pBp4io/jljtMAOvWk6m8XHAX JlUp1YwWQROfX2TCUe53Yfjabv1Xrz15t/zSkhqh9iKE6piqOSDsI/+L8Jvf905amhUk IFTYESfl5uASai8Acydiascnj1ynYWzrjFEqrm0oflJsOJlQ43I9bXFEw5giC7lba0Mu rAR4Ne4dHko5Yrg3Ro0xbyylX9bMPVO5poiOefEcaxVmrwySQEF/hfLGAiOGG6VOFeO/ f0pg== X-Gm-Message-State: AJIora/3CWVTvYOKEML6C+ZKOXCjVOlkXH1QggpNrwV0CouhdJQRsaFg EpVmgkb89c1XF5Rr36QJxdKNtz+ZfPc= X-Google-Smtp-Source: AGRyM1vxduFFEkGommBklcJXrceVSSDrLmHOt0NT7YXc6G61SHaFWTJgxJu9ti3BhbvGvFJN+p8wlQ== X-Received: by 2002:a05:6000:49:b0:21d:78fe:34b2 with SMTP id k9-20020a056000004900b0021d78fe34b2mr1066687wrx.200.1658466945299; Thu, 21 Jul 2022 22:15:45 -0700 (PDT) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id q26-20020a056000137a00b0021e0147da47sm3486124wrz.96.2022.07.21.22.15.44 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 21 Jul 2022 22:15:44 -0700 (PDT) Message-Id: In-Reply-To: References: Date: Fri, 22 Jul 2022 05:15:36 +0000 Subject: [PATCH v4 1/7] merge-ort-wrappers: make printed message match the one from recursive Fcc: Sent MIME-Version: 1.0 To: git@vger.kernel.org Cc: ZheNing Hu , Eric Sunshine , Junio C Hamano , Elijah Newren , Elijah Newren , Elijah Newren Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Elijah Newren From: Elijah Newren When the index does not match HEAD, the merge strategies are responsible to detect that condition and abort. The merge-ort-wrappers had code to implement this and meant to copy the error message from merge-recursive but deviated in two ways, both due to the message in merge-recursive being processed by another function that made additional changes: * It added an implicit "error: " prefix * It added an implicit trailing newline We can get these things by making use of the error() function. Signed-off-by: Elijah Newren --- merge-ort-wrappers.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/merge-ort-wrappers.c b/merge-ort-wrappers.c index ad041061695..748924a69ba 100644 --- a/merge-ort-wrappers.c +++ b/merge-ort-wrappers.c @@ -10,8 +10,8 @@ static int unclean(struct merge_options *opt, struct tree *head) struct strbuf sb = STRBUF_INIT; if (head && repo_index_has_changes(opt->repo, head, &sb)) { - fprintf(stderr, _("Your local changes to the following files would be overwritten by merge:\n %s"), - sb.buf); + error(_("Your local changes to the following files would be overwritten by merge:\n %s"), + sb.buf); strbuf_release(&sb); return -1; } From patchwork Fri Jul 22 05:15:37 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Elijah Newren X-Patchwork-Id: 12925966 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 1C968CCA47F for ; Fri, 22 Jul 2022 05:15:56 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231700AbiGVFPx (ORCPT ); Fri, 22 Jul 2022 01:15:53 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:51192 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229593AbiGVFPt (ORCPT ); Fri, 22 Jul 2022 01:15:49 -0400 Received: from mail-wm1-x334.google.com (mail-wm1-x334.google.com [IPv6:2a00:1450:4864:20::334]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 6C0A8237F9 for ; Thu, 21 Jul 2022 22:15:48 -0700 (PDT) Received: by mail-wm1-x334.google.com with SMTP id ay11-20020a05600c1e0b00b003a3013da120so4480833wmb.5 for ; Thu, 21 Jul 2022 22:15:48 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=message-id:in-reply-to:references:from:date:subject:fcc :content-transfer-encoding:mime-version:to:cc; bh=oJKU0DYK+KWZ02t/kXg5ErC65dSKzBhSfDLskTnhx64=; b=SHBSwsJkAxMrtOhwHHW/sfMMBjCefE37AKVyssPKxA8o7VWDBnol4whd2F8I2JhHDX zrOTa8rkOVr4vqjnT2A768J8c33Uo2p0vBJ7X/6I7LE70yk2fMgbTA2atc+7Sly4OjON 7fjW12SJHS49khCYropBQC3eX101gcWV1JaDNjipnx85+W4ezllnRIt0mEZeHp+05ypt 6VKjRh/Ae1ooCely3DGhcTW+Idf74mfP+lDb/eLcHPdiuLzVyDwV01EOCSi0ptZQz42K hAwYvP/f/3918fWNpfQuBFFanxv6GQ2RUnmiE5db8sOIUhZuhTgtd2DMxvIdZ/9ox75v 7mxg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:in-reply-to:references:from:date :subject:fcc:content-transfer-encoding:mime-version:to:cc; bh=oJKU0DYK+KWZ02t/kXg5ErC65dSKzBhSfDLskTnhx64=; b=dmWflHMkp7HeHUGiKLOE2pFvqSaAt4O9ARnydvttpILgsUgMrV2ab7+R2x2zTgoZ1/ BoT18rUTz/LW2ROYdvtx5FmWc4Y29Tenayt6hZkCNBkZhyOgNMukLgk5ZugmFqWXh51G c9wF+Vtq1wBp/R56WbfJJT0feh1o0tz39+AQlOJbNW7GuDqsJMQ+4r7nkRRxTizl/nu6 uJJXJKA7c7J6IX1Mz61Gg7ne077EpijniZhkHJQOrmRYe6lAquNgNEwE84REiWZJAMDj 2wh8SaMydWWiKrp45QYwF14yhu4UAlu/TwX5jggeOy9lmzYCdCCp4IIeg4jWhlbTY1Fr +c2w== X-Gm-Message-State: AJIora+6zbH4bbjFfknYZqh2camrzkg232/x2MDPFjX+2WR845atkeKX FVWhEYDSbJCl2BZfzKzAYnv4ez/lbnM= X-Google-Smtp-Source: AGRyM1uqGxeraCf4ZfJAl0WgZ06dS43KL+BwURV13PvppG1/Jd4d6n8Z6x5RGJOVDNZfUPITZkc1zQ== X-Received: by 2002:a1c:4487:0:b0:3a2:fb76:7981 with SMTP id r129-20020a1c4487000000b003a2fb767981mr10667881wma.98.1658466946359; Thu, 21 Jul 2022 22:15:46 -0700 (PDT) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id p21-20020a05600c205500b003a2eacc8179sm3693135wmg.27.2022.07.21.22.15.45 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 21 Jul 2022 22:15:45 -0700 (PDT) Message-Id: In-Reply-To: References: Date: Fri, 22 Jul 2022 05:15:37 +0000 Subject: [PATCH v4 2/7] merge-resolve: abort if index does not match HEAD Fcc: Sent MIME-Version: 1.0 To: git@vger.kernel.org Cc: ZheNing Hu , Eric Sunshine , Junio C Hamano , Elijah Newren , Elijah Newren , Elijah Newren Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Elijah Newren From: Elijah Newren As noted in commit 9822175d2b ("Ensure index matches head before invoking merge machinery, round N", 2019-08-17), we have had a very long history of problems with failing to enforce the requirement that index matches HEAD when starting a merge. One of the commits referenced in the long tale of issues arising from lax enforcement of this requirement was commit 55f39cf755 ("merge: fix misleading pre-merge check documentation", 2018-06-30), which tried to document the requirement and noted there were some exceptions. As mentioned in that commit message, the `resolve` strategy was the one strategy that did not have an explicit index matching HEAD check, and the reason it didn't was that I wasn't able to discover any cases where the implementation would fail to catch the problem and abort, and didn't want to introduce unnecessary performance overhead of adding another check. Well, today I discovered a testcase where the implementation does not catch the problem and so an explicit check is needed. Add a testcase that previously would have failed, and update git-merge-resolve.sh to have an explicit check. Note that the code is copied from 3ec62ad9ff ("merge-octopus: abort if index does not match HEAD", 2016-04-09), so that we reuse the same message and avoid making translators need to translate some new message. Signed-off-by: Elijah Newren --- builtin/merge.c | 20 ++++++++++++++++++ git-merge-resolve.sh | 10 +++++++++ t/t6424-merge-unrelated-index-changes.sh | 26 ++++++++++++++++++++++++ 3 files changed, 56 insertions(+) diff --git a/builtin/merge.c b/builtin/merge.c index 23170f2d2a6..13884b8e836 100644 --- a/builtin/merge.c +++ b/builtin/merge.c @@ -1599,6 +1599,26 @@ int cmd_merge(int argc, const char **argv, const char *prefix) */ refresh_cache(REFRESH_QUIET); if (allow_trivial && fast_forward != FF_ONLY) { + /* + * Must first ensure that index matches HEAD before + * attempting a trivial merge. + */ + struct tree *head_tree = get_commit_tree(head_commit); + struct strbuf sb = STRBUF_INIT; + + if (repo_index_has_changes(the_repository, head_tree, + &sb)) { + struct strbuf err = STRBUF_INIT; + strbuf_addstr(&err, "error: "); + strbuf_addf(&err, _("Your local changes to the following files would be overwritten by merge:\n %s"), + sb.buf); + strbuf_addch(&err, '\n'); + fputs(err.buf, stderr); + strbuf_release(&err); + strbuf_release(&sb); + return -1; + } + /* See if it is really trivial. */ git_committer_info(IDENT_STRICT); printf(_("Trying really trivial in-index merge...\n")); diff --git a/git-merge-resolve.sh b/git-merge-resolve.sh index 343fe7bccd0..77e93121bf8 100755 --- a/git-merge-resolve.sh +++ b/git-merge-resolve.sh @@ -5,6 +5,16 @@ # # Resolve two trees, using enhanced multi-base read-tree. +. git-sh-setup + +# Abort if index does not match HEAD +if ! git diff-index --quiet --cached HEAD -- +then + gettextln "Error: Your local changes to the following files would be overwritten by merge" + git diff-index --cached --name-only HEAD -- | sed -e 's/^/ /' + exit 2 +fi + # The first parameters up to -- are merge bases; the rest are heads. bases= head= remotes= sep_seen= for arg diff --git a/t/t6424-merge-unrelated-index-changes.sh b/t/t6424-merge-unrelated-index-changes.sh index b6e424a427b..f35d3182b86 100755 --- a/t/t6424-merge-unrelated-index-changes.sh +++ b/t/t6424-merge-unrelated-index-changes.sh @@ -114,6 +114,32 @@ test_expect_success 'resolve, non-trivial' ' test_path_is_missing .git/MERGE_HEAD ' +test_expect_success 'resolve, trivial, related file removed' ' + git reset --hard && + git checkout B^0 && + + git rm a && + test_path_is_missing a && + + test_must_fail git merge -s resolve C^0 && + + test_path_is_missing a && + test_path_is_missing .git/MERGE_HEAD +' + +test_expect_success 'resolve, non-trivial, related file removed' ' + git reset --hard && + git checkout B^0 && + + git rm a && + test_path_is_missing a && + + test_must_fail git merge -s resolve D^0 && + + test_path_is_missing a && + test_path_is_missing .git/MERGE_HEAD +' + test_expect_success 'recursive' ' git reset --hard && git checkout B^0 && From patchwork Fri Jul 22 05:15:38 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Elijah Newren X-Patchwork-Id: 12925968 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 9FEC6CCA487 for ; Fri, 22 Jul 2022 05:15:57 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232768AbiGVFP4 (ORCPT ); Fri, 22 Jul 2022 01:15:56 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:51202 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229941AbiGVFPu (ORCPT ); Fri, 22 Jul 2022 01:15:50 -0400 Received: from mail-wr1-x42e.google.com (mail-wr1-x42e.google.com [IPv6:2a00:1450:4864:20::42e]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 306DC248C5 for ; Thu, 21 Jul 2022 22:15:49 -0700 (PDT) Received: by mail-wr1-x42e.google.com with SMTP id b26so5017119wrc.2 for ; Thu, 21 Jul 2022 22:15:49 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=message-id:in-reply-to:references:from:date:subject:fcc :content-transfer-encoding:mime-version:to:cc; bh=wOGAeuZYYD1ywCQx5iNnhf8S8m5LJhjzC4NvFsDfZec=; b=MrpNaG+IAth1/Mk2+aleIwyDtct4C021FfAj/X2oDlhABPVn9duFFYJUOBvTlFYx/K ejBZoWV6sfNH5k68GfxLyffvePSKhOFY4xf3RVQGyty8SW6i42h/hMh7phtz/Ol9TcJo +G9JutIV9mKTShekQ7cNS4qNFdR4kAqt9a02tDyZvkooyzUeyXfO5aS7B40Sbm8LRMv7 mvBpay24TUb9Gsb6J6WBFAM913n0O0g8t7IiH8JEn3F0m6JMw16WwehR0kIcP4xivVRl RvUtYugnMtEOwYeI6kKr7AMKzloIso48Q7BTXU4TlA80TT9WyBf+G09qARQIfIkS+DDp QRbg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:in-reply-to:references:from:date :subject:fcc:content-transfer-encoding:mime-version:to:cc; bh=wOGAeuZYYD1ywCQx5iNnhf8S8m5LJhjzC4NvFsDfZec=; b=uHzy2gITyuW9GJwvwDLzi2jgu5bXbUBBsfDbNe+0VxLm6To4l0QzrPFjAvmJdWkegn jaW+LGwBG4FBA+stj2DmyDMR/dYqkSyIqIHvxlntgYgv+pADgD1iREzx11v73cfQMHfY 1RwArPiFMZQU/e4JIv2KPEr3aqCxfR85xdGVEs6cFzV90b66sNuVSyb6CkBaEeJMm0r4 1gyp/FVNIh36x3YR8/a1MCO0FDdEMMYB/3bOkQS5c3ltXrxPh/j/U+/S74Ds6iSJ8qSa X2uXtkIKOsz3y8vACICbGj9i3rZeBX5X+93tHEJYODTPXcYyQrWGRtuXTh4egl/lBNCs 0u2A== X-Gm-Message-State: AJIora+Ym7thr56kzm3md72+WyN2qpHRvCTzVJsOHAWeKj0Qj87W8j7s 19Hap233iPPUckeYs07ECM02G+J71cE= X-Google-Smtp-Source: AGRyM1tfc2iB5cPZy/hxLoMD9g9weSNeS8tsmFMgnedppLi054vDsb5fUgttYgGrvv6KD/AqYjSXYA== X-Received: by 2002:adf:f0c6:0:b0:21e:4480:2fa5 with SMTP id x6-20020adff0c6000000b0021e44802fa5mr1121413wro.377.1658466947409; Thu, 21 Jul 2022 22:15:47 -0700 (PDT) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id s2-20020a7bc382000000b003a3253b705dsm3689034wmj.35.2022.07.21.22.15.46 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 21 Jul 2022 22:15:46 -0700 (PDT) Message-Id: <02930448ea1fbf7084b9d78813908b6355304457.1658466942.git.gitgitgadget@gmail.com> In-Reply-To: References: Date: Fri, 22 Jul 2022 05:15:38 +0000 Subject: [PATCH v4 3/7] merge: do not abort early if one strategy fails to handle the merge Fcc: Sent MIME-Version: 1.0 To: git@vger.kernel.org Cc: ZheNing Hu , Eric Sunshine , Junio C Hamano , Elijah Newren , Elijah Newren , Elijah Newren Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Elijah Newren From: Elijah Newren builtin/merge is setup to allow multiple strategies to be specified, and it will find the "best" result and use it. This is defeated if some of the merge strategies abort early when they cannot handle the merge. Fix the logic that calls recursive and ort to not do such an early abort, but instead return "2" or "unhandled" so that the next strategy can try to handle the merge. Coming up with a testcase for this is somewhat difficult, since recursive and ort both handle nearly any two-headed merge (there is a separate code path that checks for non-two-headed merges and already returns "2" for them). So use a somewhat synthetic testcase of having the index not match HEAD before the merge starts, since all merge strategies will abort for that. Signed-off-by: Elijah Newren --- builtin/merge.c | 6 ++++-- t/t6402-merge-rename.sh | 2 +- t/t6424-merge-unrelated-index-changes.sh | 16 ++++++++++++++++ t/t6439-merge-co-error-msgs.sh | 1 + 4 files changed, 22 insertions(+), 3 deletions(-) diff --git a/builtin/merge.c b/builtin/merge.c index 13884b8e836..dec7375bf2a 100644 --- a/builtin/merge.c +++ b/builtin/merge.c @@ -754,8 +754,10 @@ static int try_merge_strategy(const char *strategy, struct commit_list *common, else clean = merge_recursive(&o, head, remoteheads->item, reversed, &result); - if (clean < 0) - exit(128); + if (clean < 0) { + rollback_lock_file(&lock); + return 2; + } if (write_locked_index(&the_index, &lock, COMMIT_LOCK | SKIP_IF_UNCHANGED)) die(_("unable to write %s"), get_index_file()); diff --git a/t/t6402-merge-rename.sh b/t/t6402-merge-rename.sh index 3a32b1a45cf..772238e582c 100755 --- a/t/t6402-merge-rename.sh +++ b/t/t6402-merge-rename.sh @@ -210,7 +210,7 @@ test_expect_success 'updated working tree file should prevent the merge' ' echo >>M one line addition && cat M >M.saved && git update-index M && - test_expect_code 128 git pull --no-rebase . yellow && + test_expect_code 2 git pull --no-rebase . yellow && test_cmp M M.saved && rm -f M.saved ' diff --git a/t/t6424-merge-unrelated-index-changes.sh b/t/t6424-merge-unrelated-index-changes.sh index f35d3182b86..8b749e19083 100755 --- a/t/t6424-merge-unrelated-index-changes.sh +++ b/t/t6424-merge-unrelated-index-changes.sh @@ -268,4 +268,20 @@ test_expect_success 'subtree' ' test_path_is_missing .git/MERGE_HEAD ' +test_expect_success 'resolve && recursive && ort' ' + git reset --hard && + git checkout B^0 && + + test_seq 0 10 >a && + git add a && + + sane_unset GIT_TEST_MERGE_ALGORITHM && + test_must_fail git merge -s resolve -s recursive -s ort C^0 >output 2>&1 && + + grep "Trying merge strategy resolve..." output && + grep "Trying merge strategy recursive..." output && + grep "Trying merge strategy ort..." output && + grep "No merge strategy handled the merge." output +' + test_done diff --git a/t/t6439-merge-co-error-msgs.sh b/t/t6439-merge-co-error-msgs.sh index 5bfb027099a..52cf0c87690 100755 --- a/t/t6439-merge-co-error-msgs.sh +++ b/t/t6439-merge-co-error-msgs.sh @@ -47,6 +47,7 @@ test_expect_success 'untracked files overwritten by merge (fast and non-fast for export GIT_MERGE_VERBOSITY && test_must_fail git merge branch 2>out2 ) && + echo "Merge with strategy ${GIT_TEST_MERGE_ALGORITHM:-ort} failed." >>expect && test_cmp out2 expect && git reset --hard HEAD^ ' From patchwork Fri Jul 22 05:15:39 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Elijah Newren X-Patchwork-Id: 12925969 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 5D040C43334 for ; Fri, 22 Jul 2022 05:16:00 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233608AbiGVFP6 (ORCPT ); Fri, 22 Jul 2022 01:15:58 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:51212 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230318AbiGVFPv (ORCPT ); Fri, 22 Jul 2022 01:15:51 -0400 Received: from mail-wm1-x334.google.com (mail-wm1-x334.google.com [IPv6:2a00:1450:4864:20::334]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 3045822BFD for ; Thu, 21 Jul 2022 22:15:50 -0700 (PDT) Received: by mail-wm1-x334.google.com with SMTP id f24-20020a1cc918000000b003a30178c022so4489424wmb.3 for ; Thu, 21 Jul 2022 22:15:50 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=message-id:in-reply-to:references:from:date:subject:fcc :content-transfer-encoding:mime-version:to:cc; bh=xRjx3GHengqE4lF7ebtLjKNtoNDHdxoCL7MPpYxY6xg=; b=PhYRxlxhHRfoVQFBSfn9lnQBfmPysAvae6Bclo/Tn6lh6JDUMO0/0gL4/Teou5MN23 fq9Udget5egDH0Euw4gnzEZCrzP/nByCGIztZFYBYptg0YpBNAsOXM5+CyIcYy8kq29O WqUiQDRAu6yVAikVGJNchCcQBKkeHay01gU5RsW71VDVqY4XxSyouAPOlOZlwpH9+EYA 8nj9Q4btsbDs1Tp6bra3lpOvkjDLdVBIUn+Wrs6IorZ/W2IV3RpDebxMyqdtYrHtV7gO VVap+/moi9yqIIv71KgaR6io/2rRf3G5V77xdorIfMtUOM1uEOsviQtsR71YEC9cW6hu WOQg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:in-reply-to:references:from:date :subject:fcc:content-transfer-encoding:mime-version:to:cc; bh=xRjx3GHengqE4lF7ebtLjKNtoNDHdxoCL7MPpYxY6xg=; b=pig8dRG1yIAZwuGep9T++D8Brbah4BEm30IPf08snygV/V3WMj8y4WO180IysIW0qW TuHTkk/u2fEq8pl0sSs/HrGHSFUNaz/6UybOfejkzvXma4gcNnCphpF5wNFG9iU6JYPW jYW2zls1tA+rDLjk9/rqWH8DO75V6KbEDTriy7y9YzUMv0reTZPvSPubaiRAAThOIZba pSZBU9CbW3d96VfltedAbHQobOnzrOYAeldGW9Ii/VSRCTWXhSujnGDkJidk5rPiDpoF iigngu7LZnSFsNWTnf/whPn/zGMTLdw4kBNSMQGk8KGCSZwKQQScf5ZIW7EkCQbsm25e 9PXw== X-Gm-Message-State: AJIora/vRGQrzhdlhCSh4Wxc6jCPRwyKBoyPq1bDf4v3OpcjQ5AW1CT7 KQg7htUmwhkM+tyffUV2jehHLVCUv00= X-Google-Smtp-Source: AGRyM1v1rUkkt9o8H1t5dtewq2HM/Q2NjhkYJpWukKVJCEEZG6xpoC1CNESTtmfU6MtpbM4CUFNg4A== X-Received: by 2002:a05:600c:1e04:b0:3a3:11ca:5c0c with SMTP id ay4-20020a05600c1e0400b003a311ca5c0cmr1019341wmb.31.1658466948463; Thu, 21 Jul 2022 22:15:48 -0700 (PDT) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id q184-20020a1c43c1000000b003a302fb9df7sm7359804wma.21.2022.07.21.22.15.47 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 21 Jul 2022 22:15:48 -0700 (PDT) Message-Id: In-Reply-To: References: Date: Fri, 22 Jul 2022 05:15:39 +0000 Subject: [PATCH v4 4/7] merge: fix save_state() to work when there are stat-dirty files Fcc: Sent MIME-Version: 1.0 To: git@vger.kernel.org Cc: ZheNing Hu , Eric Sunshine , Junio C Hamano , Elijah Newren , Elijah Newren , Elijah Newren Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Elijah Newren From: Elijah Newren When there are stat-dirty files, but no files are modified, `git stash create` exits with unsuccessful status. This causes merge to fail. Copy some code from sequencer.c's create_autostash to refresh the index first to avoid this problem. Signed-off-by: Elijah Newren --- builtin/merge.c | 8 ++++++++ t/t6424-merge-unrelated-index-changes.sh | 11 +++++++++++ 2 files changed, 19 insertions(+) diff --git a/builtin/merge.c b/builtin/merge.c index dec7375bf2a..4170c30317e 100644 --- a/builtin/merge.c +++ b/builtin/merge.c @@ -313,8 +313,16 @@ static int save_state(struct object_id *stash) int len; struct child_process cp = CHILD_PROCESS_INIT; struct strbuf buffer = STRBUF_INIT; + struct lock_file lock_file = LOCK_INIT; + int fd; int rc = -1; + fd = repo_hold_locked_index(the_repository, &lock_file, 0); + refresh_cache(REFRESH_QUIET); + if (0 <= fd) + repo_update_index_if_able(the_repository, &lock_file); + rollback_lock_file(&lock_file); + strvec_pushl(&cp.args, "stash", "create", NULL); cp.out = -1; cp.git_cmd = 1; diff --git a/t/t6424-merge-unrelated-index-changes.sh b/t/t6424-merge-unrelated-index-changes.sh index 8b749e19083..3019d030e07 100755 --- a/t/t6424-merge-unrelated-index-changes.sh +++ b/t/t6424-merge-unrelated-index-changes.sh @@ -268,6 +268,17 @@ test_expect_success 'subtree' ' test_path_is_missing .git/MERGE_HEAD ' +test_expect_success 'avoid failure due to stat-dirty files' ' + git reset --hard && + git checkout B^0 && + + # Make "a" be stat-dirty + test-tool chmtime =+1 a && + + # stat-dirty file should not prevent stash creation in builtin/merge.c + git merge -s resolve -s recursive D^0 +' + test_expect_success 'resolve && recursive && ort' ' git reset --hard && git checkout B^0 && From patchwork Fri Jul 22 05:15:40 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Elijah Newren X-Patchwork-Id: 12925970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id CCAFFC433EF for ; Fri, 22 Jul 2022 05:16:01 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233663AbiGVFQA (ORCPT ); Fri, 22 Jul 2022 01:16:00 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:51240 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231522AbiGVFPw (ORCPT ); Fri, 22 Jul 2022 01:15:52 -0400 Received: from mail-wm1-x32d.google.com (mail-wm1-x32d.google.com [IPv6:2a00:1450:4864:20::32d]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 6CF56248C5 for ; Thu, 21 Jul 2022 22:15:51 -0700 (PDT) Received: by mail-wm1-x32d.google.com with SMTP id u14-20020a05600c00ce00b003a323062569so1844919wmm.4 for ; Thu, 21 Jul 2022 22:15:51 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=message-id:in-reply-to:references:from:date:subject:fcc :content-transfer-encoding:mime-version:to:cc; bh=o7GPUzkCZ8mQA/iw0P73pmXqknSvuHaMPI4bL6mBzJM=; b=fxRgjj4UAaKTH9K5Y/BAfrglfbjkgoSm2OsfRLoMs79yiQS6wqi2zKltfxV1kaaR8a sgWn5ySmucyukPLRAJ8aJ8bbfzqIfUaOKQKaeQNuYlSCqtuilEww/b+tEzzkavd0xlMt jVIjp//ZqbdxtJmfrU9PglfhaZRU0RowpwqnkSYWo2XXsku1Q9tfvXJg3H3OonKqw9fz DM00xhAQmK1JDKDymd7Zv+idDdCrfO30m37ukn0eZGfbEetyAbSrKA6wJFYOFGqbG32Y 8JHH5frvdn0K9mhFs3TCR93Px5HpmCF0OB6BwgstJnzXqLLGgTmxNIUtjPHKxJbIje4x 6SkQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:in-reply-to:references:from:date :subject:fcc:content-transfer-encoding:mime-version:to:cc; bh=o7GPUzkCZ8mQA/iw0P73pmXqknSvuHaMPI4bL6mBzJM=; b=btRDQPqArFynGUbA/LZWBINbKFtWvTH77bquykiaPSinM1h3XATXZ6MDSI/QiKPfI0 GhaufkdxRSxLHbM9OAC1jtOgbTuVxyLve1aM9myp29WjWTvfqGLNfr2BgMYGkbGh/o/k cMejYm92X1wDMHRRAFXi/xdEeiF+67Q0nm6fESzEx4kukb+cd5s5+Uzu+i4KZIlNqAOp rRrH6cbkzULMKpaH7gbyQjH5j4swXDM2v/PU6UriHr+pRGY5VeOeYxz7rhMdQSquJ3ZT qLO+sf+lT66Z9D1TRTnlWq0Ij/PT+ahwi+VDb5Q3YyqOeUyYbldD9zobNgEbiGk3fIHz 59hA== X-Gm-Message-State: AJIora9faL1Rk4dM1tt9s2svV6FBuh2Mq7afPro0Z404rxmHG1Ho5Oh8 fSRpcaXwjqWvrWNNMaMDuzO872IuEKE= X-Google-Smtp-Source: AGRyM1t47T33CsL6dDbaXr9TjFhGQKLA0kN+K7Ecb6ExHKZeuNC9VRroOky3NeUI6mHQD8+ZBRxfOw== X-Received: by 2002:a05:600c:19d2:b0:3a3:2cdb:cc02 with SMTP id u18-20020a05600c19d200b003a32cdbcc02mr8269322wmq.182.1658466949398; Thu, 21 Jul 2022 22:15:49 -0700 (PDT) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id k22-20020a5d5256000000b0021e2fccea97sm3711445wrc.64.2022.07.21.22.15.48 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 21 Jul 2022 22:15:48 -0700 (PDT) Message-Id: In-Reply-To: References: Date: Fri, 22 Jul 2022 05:15:40 +0000 Subject: [PATCH v4 5/7] merge: make restore_state() restore staged state too Fcc: Sent MIME-Version: 1.0 To: git@vger.kernel.org Cc: ZheNing Hu , Eric Sunshine , Junio C Hamano , Elijah Newren , Elijah Newren , Elijah Newren Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Elijah Newren From: Elijah Newren There are multiple issues at play here: 1) If `git merge` is invoked with staged changes, it should abort without doing any merging, and the user's working tree and index should be the same as before merge was invoked. 2) Merge strategies are responsible for enforcing the index == HEAD requirement. (See 9822175d2b ("Ensure index matches head before invoking merge machinery, round N", 2019-08-17) for some history around this.) 3) Merge strategies can bail saying they are not an appropriate handler for the merge in question (possibly allowing other strategies to be used instead). 4) Merge strategies can make changes to the index and working tree, and have no expectation to clean up after themselves, *even* if they bail out and say they are not an appropriate handler for the merge in question. (The `octopus` merge strategy does this, for example.) 5) Because of (3) and (4), builtin/merge.c stashes state before trying merge strategies and restores it afterward. Unfortunately, if users had staged changes before calling `git merge`, builtin/merge.c could do the following: * stash the changes, in order to clean up after the strategies * try all the merge strategies in turn, each of which report they cannot function due to the index not matching HEAD * restore the changes via "git stash apply" But that last step would have the net effect of unstaging the user's changes. Fix this by adding the "--index" option to "git stash apply". While at it, also squelch the stash apply output; we already report "Rewinding the tree to pristine..." and don't need a detailed `git status` report afterwards. Signed-off-by: Elijah Newren --- builtin/merge.c | 5 +++-- t/t6424-merge-unrelated-index-changes.sh | 7 ++++++- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/builtin/merge.c b/builtin/merge.c index 4170c30317e..f807bf335bd 100644 --- a/builtin/merge.c +++ b/builtin/merge.c @@ -383,14 +383,15 @@ static void reset_hard(const struct object_id *oid, int verbose) static void restore_state(const struct object_id *head, const struct object_id *stash) { - const char *args[] = { "stash", "apply", NULL, NULL }; + const char *args[] = { "stash", "apply", "--index", "--quiet", + NULL, NULL }; if (is_null_oid(stash)) return; reset_hard(head, 1); - args[2] = oid_to_hex(stash); + args[4] = oid_to_hex(stash); /* * It is OK to ignore error here, for example when there was diff --git a/t/t6424-merge-unrelated-index-changes.sh b/t/t6424-merge-unrelated-index-changes.sh index 3019d030e07..c96649448fa 100755 --- a/t/t6424-merge-unrelated-index-changes.sh +++ b/t/t6424-merge-unrelated-index-changes.sh @@ -285,6 +285,7 @@ test_expect_success 'resolve && recursive && ort' ' test_seq 0 10 >a && git add a && + git rev-parse :a >expect && sane_unset GIT_TEST_MERGE_ALGORITHM && test_must_fail git merge -s resolve -s recursive -s ort C^0 >output 2>&1 && @@ -292,7 +293,11 @@ test_expect_success 'resolve && recursive && ort' ' grep "Trying merge strategy resolve..." output && grep "Trying merge strategy recursive..." output && grep "Trying merge strategy ort..." output && - grep "No merge strategy handled the merge." output + grep "No merge strategy handled the merge." output && + + # Changes to "a" should remain staged + git rev-parse :a >actual && + test_cmp expect actual ' test_done From patchwork Fri Jul 22 05:15:41 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Elijah Newren X-Patchwork-Id: 12925971 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id EC65CC43334 for ; Fri, 22 Jul 2022 05:16:02 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231180AbiGVFQC (ORCPT ); Fri, 22 Jul 2022 01:16:02 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:51252 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231621AbiGVFPx (ORCPT ); Fri, 22 Jul 2022 01:15:53 -0400 Received: from mail-wm1-x329.google.com (mail-wm1-x329.google.com [IPv6:2a00:1450:4864:20::329]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 499D2248D9 for ; Thu, 21 Jul 2022 22:15:52 -0700 (PDT) Received: by mail-wm1-x329.google.com with SMTP id x23-20020a05600c179700b003a30e3e7989so1864842wmo.0 for ; Thu, 21 Jul 2022 22:15:52 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=message-id:in-reply-to:references:from:date:subject:fcc :content-transfer-encoding:mime-version:to:cc; bh=8hOFrkxXUiIasWHVohoJoM+SqtUciFu2UG5GIzqdmgU=; b=QhzcS0EjpJu52JI1LqiLWmes3yNAIJ9s2iuAM4kxVDO7ttNkt8AH8N/LKS5VaNpVDm Eanwjtiytt4HYsBB9X9psWCWidH0fZJJIxpPFJe1SKtQBM5Cb6maTvvZItyl6xJZZml0 XmGpoTnA1i0YekzypbKkqjCh3XvJZFdzjtUWaQslEPLw15vhNhoGhRRegMXn8UgB45ZY X6Y4P8w4it5+uTLpYURYon4o9p8LxBPEtxD9dgvWhSziFkBQqqiRV+oi2jgo0ED9b2Jn i6z2SUmkTc7jCGk7wImaIKJbIXYG/FUahuT641kEzVwl7yVH9VaUOto8Oj40J3riqm0f XZTQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:in-reply-to:references:from:date :subject:fcc:content-transfer-encoding:mime-version:to:cc; bh=8hOFrkxXUiIasWHVohoJoM+SqtUciFu2UG5GIzqdmgU=; b=ggnf6jNQJndD/tfcuXGEuJm1GPiqB5SuUYzYyBRtBN86ci96mfFApOAr51LhWZi0oS tIaEYw1UVGj8MnnQxxP8ZGIJZtVrSevHtG8c7cvuTYFwWMHbCug+HG5qqx8OMJ4LAH8/ WSAynaQcBAnkCwaDeZiNWQUEJEKbc512tX/PkMpjUkpQUvmpjRsNeYX+ZFNyfc64pd68 H6iMNMwfgBHQBpMq5PhdY+VVdSZXiVsVMkwTzLTSlH7jwi57WBLRE2wRBlgyFOyaeHDx NVvEYssZ2mkxdzMEvbq5B6M+8rxqdDndiYh5fu75RSO/39w2C1+IR+jPlT3LSAbSCQzU 55RA== X-Gm-Message-State: AJIora/PegO0nf/yVGRpiVcHxP6X7Fmr8onghiKUqzRg7jjyw3XHBgoN 6SOhy/6dxbhRwvoSklz6Vo6IQhe7Iu0= X-Google-Smtp-Source: AGRyM1t4DY5UxpKej/IHKsa524/XZ64Gzo8YyFSBzvauXi9Jiz6xyE00ZspK01+L12Cle8muQz1tfA== X-Received: by 2002:a05:600c:4f08:b0:3a1:99ed:4f1f with SMTP id l8-20020a05600c4f0800b003a199ed4f1fmr10750143wmq.199.1658466950434; Thu, 21 Jul 2022 22:15:50 -0700 (PDT) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id d6-20020a5d6446000000b0021db2dcd0aasm4258504wrw.108.2022.07.21.22.15.49 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 21 Jul 2022 22:15:49 -0700 (PDT) Message-Id: In-Reply-To: References: Date: Fri, 22 Jul 2022 05:15:41 +0000 Subject: [PATCH v4 6/7] merge: ensure we can actually restore pre-merge state Fcc: Sent MIME-Version: 1.0 To: git@vger.kernel.org Cc: ZheNing Hu , Eric Sunshine , Junio C Hamano , Elijah Newren , Elijah Newren , Elijah Newren Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Elijah Newren From: Elijah Newren Merge strategies can: * succeed with a clean merge * succeed with a conflicted merge * fail to handle the given type of merge If one is thinking in terms of automatic mergeability, they would use the word "fail" instead of "succeed" for the second bullet, but I am focusing here on ability of the merge strategy to handle the given inputs, not on whether the given inputs are mergeable. The third category is about the merge strategy failing to know how to handle the given data; examples include: * Passing more than 2 branches to 'recursive' or 'ort' * Passing 2 or fewer branches to 'octopus' * Trying to do more complicated merges with 'resolve' (I believe directory/file conflicts will cause it to bail.) * Octopus running into a merge conflict for any branch OTHER than the final one (see the "exit 2" codepath of commit 98efc8f3d8 ("octopus: allow manual resolve on the last round.", 2006-01-13)) That final one is particularly interesting, because it shows that the merge strategy can muck with the index and working tree, and THEN bail and say "sorry, this strategy cannot handle this type of merge; use something else". Further, we do not currently expect the individual strategies to clean up after themselves, but instead expect builtin/merge.c to do so. For it to be able to, it needs to save the state before trying the merge strategy so it can have something to restore to. Therefore, remove the shortcut bypassing the save_state() call. There is another bug on the restore_state() side of things, so no testcase will be added until the next commit when we have addressed that issue as well. Signed-off-by: Elijah Newren --- builtin/merge.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/builtin/merge.c b/builtin/merge.c index f807bf335bd..11bb4bab0a1 100644 --- a/builtin/merge.c +++ b/builtin/merge.c @@ -1686,12 +1686,12 @@ int cmd_merge(int argc, const char **argv, const char *prefix) * tree in the index -- this means that the index must be in * sync with the head commit. The strategies are responsible * to ensure this. + * + * Stash away the local changes so that we can try more than one + * and/or recover from merge strategies bailing while leaving the + * index and working tree polluted. */ - if (use_strategies_nr == 1 || - /* - * Stash away the local changes so that we can try more than one. - */ - save_state(&stash)) + if (save_state(&stash)) oidclr(&stash); for (i = 0; !merge_was_ok && i < use_strategies_nr; i++) { From patchwork Fri Jul 22 05:15:42 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Elijah Newren X-Patchwork-Id: 12925972 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id E2C8EC43334 for ; Fri, 22 Jul 2022 05:16:04 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233037AbiGVFQD (ORCPT ); Fri, 22 Jul 2022 01:16:03 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:51296 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231735AbiGVFPz (ORCPT ); Fri, 22 Jul 2022 01:15:55 -0400 Received: from mail-wr1-x433.google.com (mail-wr1-x433.google.com [IPv6:2a00:1450:4864:20::433]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 473B82494E for ; Thu, 21 Jul 2022 22:15:53 -0700 (PDT) Received: by mail-wr1-x433.google.com with SMTP id m17so4712171wrw.7 for ; Thu, 21 Jul 2022 22:15:53 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=message-id:in-reply-to:references:from:date:subject:fcc :content-transfer-encoding:mime-version:to:cc; bh=9InglnYuYAH/AdHw7kf0W+m4eVbt4/lAXFozLEovVSQ=; b=jE1gPPvUY1P1MUXSbseaZPU+JIQkX17XDx/U+On37bGVektk9oyrnOeKCWul3JY8ab 0Gown8Lwt9Ph5JSrcZB1DGHrRlJ33wKT6LybV95+/OQGnwaKduU6eIvLOQrA1cP30Kag RzAh0oaLER4gD0Kams/TUSPaSjjazbWDUaiMasDgIXSi5MSuYBtbykwu/kz9ZviJRRue YDnphQCHFZ8sRVVCp+NAYMELiGrP+uB6mmusKqVHUtKgSigxkg5bM7LDtvBguPXTCELB nFr/RnGNhOJwUHiZm9pXxXfQjnOinDXU9ZY2OdKUbXmCuYK5VitHOiZzSmvd+gx3nl3L ENeA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:in-reply-to:references:from:date :subject:fcc:content-transfer-encoding:mime-version:to:cc; bh=9InglnYuYAH/AdHw7kf0W+m4eVbt4/lAXFozLEovVSQ=; b=nIWMz+jpHurGeB4y3fRxifbNbMKp0JGucnjMI6ymvgUOGomsownuaICnt92DSMFqY2 dGHObZmhSNz2j0uHKpUdY+2Iig6JP1+YGXjo4sSWrKNkaS/9gFmw4193wGSlzNnjBziB YMLD4eTzpBSRGjIIy96qE24Mi5jAsdrE5pZmw2/qh7mCMgMVqUS9wW0vtIFEwnn4yVuJ 4bUH01U2BEu4qYSfDkJggMtQ9jGBL7f+DilzDjy8PSGYSp1irl2g6FZ2xQizS8mxqm2N vPozgOT3l9DCwT0LOQDZ7xUaIHa9xDkg/auIwNTeODwh9JW7naj4r3A2C//d0gwKv5wz xgdQ== X-Gm-Message-State: AJIora8AHUVD6NF7NjHfmfVqHRqsBFNlftJ1xq6j953w/LWXk5Rz47m9 F9LimOMIBO/cpwb+lAc49MypaQ3FV40= X-Google-Smtp-Source: AGRyM1tYxMAh50kSR3AwY3t44NncDYuThKrKvbWwZin7XzXr0hQWW+h34TCQrPourL9bZNeLhSKYVg== X-Received: by 2002:a5d:588f:0:b0:21d:a516:f57b with SMTP id n15-20020a5d588f000000b0021da516f57bmr1085177wrf.685.1658466951512; Thu, 21 Jul 2022 22:15:51 -0700 (PDT) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id k6-20020a5d5186000000b0021e571a99d5sm2964204wrv.17.2022.07.21.22.15.50 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 21 Jul 2022 22:15:51 -0700 (PDT) Message-Id: <6212d572604cd8ec5c57ab0c66da53bd95ed6005.1658466942.git.gitgitgadget@gmail.com> In-Reply-To: References: Date: Fri, 22 Jul 2022 05:15:42 +0000 Subject: [PATCH v4 7/7] merge: do not exit restore_state() prematurely Fcc: Sent MIME-Version: 1.0 To: git@vger.kernel.org Cc: ZheNing Hu , Eric Sunshine , Junio C Hamano , Elijah Newren , Elijah Newren , Elijah Newren Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Elijah Newren From: Elijah Newren Previously, if the user: * Had no local changes before starting the merge * A merge strategy makes changes to the working tree/index but returns with exit status 2 Then we'd call restore_state() to clean up the changes and either let the next merge strategy run (if there is one), or exit telling the user that no merge strategy could handle the merge. Unfortunately, restore_state() did not clean up the changes as expected; that function was a no-op if the stash was a null, and the stash would be null if there were no local changes before starting the merge. So, instead of "Rewinding the tree to pristine..." as the code claimed, restore_state() would leave garbage around in the index and working tree (possibly including conflicts) for either the next merge strategy or for the user after aborting the merge. And in the case of aborting the merge, the user would be unable to run "git merge --abort" to get rid of the unintended leftover conflicts, because the merge control files were not written as it was presumed that we had restored to a clean state already. Fix the main problem by making sure that restore_state() only skips the stash application if the stash is null rather than skipping the whole function. However, there is a secondary problem -- since merge.c forks subprocesses to do the cleanup, the in-memory index is left out-of-sync. While there was a refresh_cache(REFRESH_QUIET) call that attempted to correct that, that function would not handle cases where the previous merge strategy added conflicted entries. We need to drop the index and re-read it to handle such cases. (Alternatively, we could stop forking subprocesses and instead call some appropriate function to do the work which would update the in-memory index automatically. For now, just do the simple fix.) Also, add a testcase checking this, one for which the octopus strategy fails on the first commit it attempts to merge, and thus which it cannot handle at all and must completely bail on (as per the "exit 2" code path of commit 98efc8f3d8 ("octopus: allow manual resolve on the last round.", 2006-01-13)). Reported-by: ZheNing Hu Signed-off-by: Elijah Newren --- builtin/merge.c | 10 ++++++---- t/t7607-merge-state.sh | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 4 deletions(-) create mode 100755 t/t7607-merge-state.sh diff --git a/builtin/merge.c b/builtin/merge.c index 11bb4bab0a1..7fb4414ebb7 100644 --- a/builtin/merge.c +++ b/builtin/merge.c @@ -386,11 +386,11 @@ static void restore_state(const struct object_id *head, const char *args[] = { "stash", "apply", "--index", "--quiet", NULL, NULL }; - if (is_null_oid(stash)) - return; - reset_hard(head, 1); + if (is_null_oid(stash)) + goto refresh_cache; + args[4] = oid_to_hex(stash); /* @@ -399,7 +399,9 @@ static void restore_state(const struct object_id *head, */ run_command_v_opt(args, RUN_GIT_CMD); - refresh_cache(REFRESH_QUIET); +refresh_cache: + if (discard_cache() < 0 || read_cache() < 0) + die(_("could not read index")); } /* This is called when no merge was necessary. */ diff --git a/t/t7607-merge-state.sh b/t/t7607-merge-state.sh new file mode 100755 index 00000000000..fc33d57357b --- /dev/null +++ b/t/t7607-merge-state.sh @@ -0,0 +1,32 @@ +#!/bin/sh + +test_description="Test that merge state is as expected after failed merge" + +GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main +export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME +. ./test-lib.sh + +test_expect_success 'set up custom strategy' ' + test_commit --no-tag "Initial" base base && + + for b in branch1 branch2 branch3 + do + git checkout -b $b main && + test_commit --no-tag "Change on $b" base $b || return 1 + done && + + git checkout branch1 && + # This is a merge that octopus cannot handle. Note, that it does not + # just hit conflicts, it completely fails and says that it cannot + # handle this type of merge. + test_expect_code 2 git merge branch2 branch3 >output 2>&1 && + grep "fatal: merge program failed" output && + grep "Should not be doing an octopus" output && + + # Make sure we did not leave stray changes around when no appropriate + # merge strategy was found + git diff --exit-code --name-status && + test_path_is_missing .git/MERGE_HEAD +' + +test_done