From patchwork Sat Jul 23 01:53:11 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Elijah Newren X-Patchwork-Id: 12927039 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 7A833CCA47C for ; Sat, 23 Jul 2022 01:53:30 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229461AbiGWBx0 (ORCPT ); Fri, 22 Jul 2022 21:53:26 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:42858 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S234587AbiGWBxY (ORCPT ); Fri, 22 Jul 2022 21:53:24 -0400 Received: from mail-wm1-x32e.google.com (mail-wm1-x32e.google.com [IPv6:2a00:1450:4864:20::32e]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 4CBF876EB7 for ; Fri, 22 Jul 2022 18:53:23 -0700 (PDT) Received: by mail-wm1-x32e.google.com with SMTP id v5so3709766wmj.0 for ; Fri, 22 Jul 2022 18:53:23 -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=qyi3OMDuXfz+J7xH5/E0HWEw9G0MztvJpdtAMDYKja6UTWjm9RkS/xBxYKyY6UMuKj e7p441xT1TCd4CfXk1i8r2qRE/SJiQb6AKEYWxrbWinImTi+mMAxiTBBKjd7ayZbuTbj rKkXwc9ayhul24Dco7i+rRTIPeEIWMtdkIKXpz4H/jCfnYinwvBWtYQ9jCMtk26Augq3 ln57g9QskOpV5466IBhk+4TcOpcXE4iNuIf5wRxRbM40ECCaz8Rxj8rqOKjYzotqV0EY RAj5AE30yvh13zOpRYq3jtAw0o99fZzh/vDdr2648JBQQjxMbHedw+VYdOrGG+IzWLRb MBUA== 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=mjkR5vtb4sTN0u6WqQwPdqARikMAn+vKCWck07yT+GM3Q6F88V9PnQ/+yRyZbliVaX TaX3Aj8GGw5KN7dg6G3zgsQJlje7LTf/1sBCauwrvlsV8aK517ayNb3El9H2o9/KFClg UPLNw7zffEQ+Z7300rA1uiQz2eBd9nW07VsEMzOemfKrKDFywcBHss4kStS09vp5Y7s/ C8fQM2usm4yI0Kq29C7qZKWNXIvfljwthjVrj9ROQ2FvJLSClt6mSHcnVsPZ6zyDLWaV KFZT/YjhIslfFUaK1VejNHI+HGBUzF7p8uLSNzm14lJNIlU+cSZrdFH/+XDxeLJPeVte CCgA== X-Gm-Message-State: AJIora+yZk4kqrTc3DRaErp8DUG6WnnHB9HgU8cGKXsPf/+X2q0tc6+6 YAenBavCadfAHs9zpkSEY66XWJEax0A= X-Google-Smtp-Source: AGRyM1u0KtVcdR1+GQjWfCvsjbg8Ing5w0sTxzPLzezcT8DK6TZNku7lu16ijkIouCAARER6RmF85g== X-Received: by 2002:a05:600c:364f:b0:3a3:20ea:b85d with SMTP id y15-20020a05600c364f00b003a320eab85dmr13847448wmq.125.1658541200979; Fri, 22 Jul 2022 18:53:20 -0700 (PDT) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id y7-20020adfe6c7000000b0021e4f595590sm5991190wrm.28.2022.07.22.18.53.20 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 22 Jul 2022 18:53:20 -0700 (PDT) Message-Id: In-Reply-To: References: Date: Sat, 23 Jul 2022 01:53:11 +0000 Subject: [PATCH v5 1/8] 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 , =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsA==?= Bjarmason , 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 Sat Jul 23 01:53:12 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Elijah Newren X-Patchwork-Id: 12927037 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 6A0E4C43334 for ; Sat, 23 Jul 2022 01:53:30 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S236639AbiGWBx3 (ORCPT ); Fri, 22 Jul 2022 21:53:29 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:42866 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S235516AbiGWBxZ (ORCPT ); Fri, 22 Jul 2022 21:53:25 -0400 Received: from mail-wm1-x331.google.com (mail-wm1-x331.google.com [IPv6:2a00:1450:4864:20::331]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id C9AC274795 for ; Fri, 22 Jul 2022 18:53:23 -0700 (PDT) Received: by mail-wm1-x331.google.com with SMTP id ay11-20020a05600c1e0b00b003a3013da120so6121030wmb.5 for ; Fri, 22 Jul 2022 18:53:23 -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=MypplCpsyA0RCBKsYZ9G7TvpGX99JF/cC1HHk0jqvmk=; b=iYqXyEY7KxwlqLYbsZ/hvk5IAs1Sc0/be2uM6IW4nmAyaAJmx2Q7eBoSzjNsXSWt79 n7878oL7h7nFFp16Kkvi/RnV/aeNJBf1rRuU9eRhjwdqtjrYffdfCt4B/SLsixRlNSHG q/jlWRq/BLmHnV5Ee6HODua3/eWtOeYnWpzu5njfnkYwNNxVQjRd7YAQQTCpjK3J1JDH mhg0cHBhqzDfV+HLi+K411ZdrNG1wWiyp7MpPBRzdl6JuR+UFeXFHGurk3BGpJtyiCj9 /pNCIpxkYgK8M059VkP8XqCxw1fBNX5i0YFl4IkJXBmi7nm62nGC1E+ePgUgf7LC86Cc MOjg== 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=MypplCpsyA0RCBKsYZ9G7TvpGX99JF/cC1HHk0jqvmk=; b=aLzhh3RzcIT1kFApeanUJDQejgoMhH9mJdqWgvZ+D4mC5w1Mk24pP9D2Jn8h+aqxur E5IB406IJp8GNXyU+MDQIkdx5Gd9EqQrVgcYl4hp7TOH1D95Xg9TaoTS5N3QubuZDP1S cOHyO7A47WxDvRi1vxPo8bOMRSTDKEX9AVKlPbXCwF9CXa4BjleglFvA0oj5YcIlY490 Z6aSNRojxOwhpcMLM5SFm2wW/jF266EocD4axNXnuQP9zzZp7yGqzdAYT5SEV+2jtcze LG9S/DHJaguWWVaZnIrNG0t56tfVCwxLaiHi3TYWfmrIbU9hhstLEg4k/SlAiZT8o2AW jIlw== X-Gm-Message-State: AJIora/UzdKM2Qm+iG2lWva97B5ilgZjxW78AKROzLBTgLULaPel+DHL KlXlbo92RvAqPyPxGyVmYcZmNwW3b+0= X-Google-Smtp-Source: AGRyM1t6OIZ89/VTfztOYGCLhNcIjPddZi+9v0CwzYlXMWREcFs++v9pZTFP5d/w7XIBt+jqoB6SkQ== X-Received: by 2002:a05:600c:22c1:b0:3a3:170a:7ae1 with SMTP id 1-20020a05600c22c100b003a3170a7ae1mr14198756wmg.192.1658541201894; Fri, 22 Jul 2022 18:53:21 -0700 (PDT) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id t21-20020a1c7715000000b003a331c6bffdsm5931876wmi.47.2022.07.22.18.53.21 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 22 Jul 2022 18:53:21 -0700 (PDT) Message-Id: In-Reply-To: References: Date: Sat, 23 Jul 2022 01:53:12 +0000 Subject: [PATCH v5 2/8] 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 , =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsA==?= Bjarmason , 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 --- git-merge-resolve.sh | 10 ++++++++++ t/t6424-merge-unrelated-index-changes.sh | 13 +++++++++++++ 2 files changed, 23 insertions(+) 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..eabe6bda832 100755 --- a/t/t6424-merge-unrelated-index-changes.sh +++ b/t/t6424-merge-unrelated-index-changes.sh @@ -114,6 +114,19 @@ test_expect_success 'resolve, non-trivial' ' 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 Sat Jul 23 01:53:13 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Elijah Newren X-Patchwork-Id: 12927041 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 66D61CCA47C for ; Sat, 23 Jul 2022 01:53:36 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S236724AbiGWBxa (ORCPT ); Fri, 22 Jul 2022 21:53:30 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:42884 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S236527AbiGWBxZ (ORCPT ); Fri, 22 Jul 2022 21:53:25 -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 C1E0373928 for ; Fri, 22 Jul 2022 18:53:24 -0700 (PDT) Received: by mail-wr1-x433.google.com with SMTP id b26so8663633wrc.2 for ; Fri, 22 Jul 2022 18:53:24 -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=yYPAijwQFOoTdhdS8fGV4HlbUPk99D9N/ORJVdpJBz4=; b=CsEa12xcNpcmxoRM7SKcklq9Rc570kt7AbjOxuYHcVkLUulaUIBCUYVcVyVEG4iBdd ue345rrRLlRlWNejNCSbg2taqIEJWujBf+3ukBtNtibtU9i3qIHtve9ZDYkzrT9gToWR VXVr9M/udAsLTUZ8DGeYFppods24D+jrHnewIii8IiCwpMDpwlUKa0FzsgFAFW43MGMa VUxxXKAg1i9iMimNQKkC50PmoByNVD//K7zYpdv5qhKR9WpXWVT0vwl2lWG7iQ1ImQZN f0zEBzTHiSrW1pEJ24jx30bHPyvg7j7kwgG007P+pAqtt3oAKFHad3sGeZaGs29NHyrS 86aw== 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=yYPAijwQFOoTdhdS8fGV4HlbUPk99D9N/ORJVdpJBz4=; b=OvGyUc8tTiCQY+P28ZMsPdjzjI9KbxXD/hAfbqa1WHC36n9E76yki/XPdfPPFNOpWC JtqPNTs576AAz7sFYvHQ4hqETnzf0+wpLVtu1pyLSPlAEAvQ73WzCHujxa9YZgDYfs8l Ok41Uks0o+L1fvy244p+mLxMPt1iP7ntWdK67xJra7nx+/1jAkON8ZbnN9eb9rWYFzet gW4iGrJANHrvlSLhvMwb0i4NFYisq5WhZG25Kz6l29tiB97VYvyBPSMgCJ0FtLRBPUrn eVhS9YPW7y7dZM4Prg4pHm+VgGDTuCjNFcxUh5ogOONtle9wSFKQKNey7b6PjnnE5Ghm B7eQ== X-Gm-Message-State: AJIora/7EvQpObk+X9ewkpxGueAjFySxc/vgvA5jt7nEP8ZaIAioC7aK q+Fwzwo/Hc35vggv56nwrdu0rkCUjUw= X-Google-Smtp-Source: AGRyM1tvlbz5r4CzTl6VyEkRKK8m2hmlNxgMSS8XYgYmNgw7F4MS8AOzyid317dxRX9vqBKGnx+8rQ== X-Received: by 2002:adf:d089:0:b0:21e:52a6:98a7 with SMTP id y9-20020adfd089000000b0021e52a698a7mr1426196wrh.232.1658541202814; Fri, 22 Jul 2022 18:53:22 -0700 (PDT) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id m20-20020a05600c4f5400b003976fbfbf00sm7130279wmq.30.2022.07.22.18.53.22 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 22 Jul 2022 18:53:22 -0700 (PDT) Message-Id: <3adfd9219954109d9d002459599aac4e0b96a04f.1658541198.git.gitgitgadget@gmail.com> In-Reply-To: References: Date: Sat, 23 Jul 2022 01:53:13 +0000 Subject: [PATCH v5 3/8] merge: abort if index does not match HEAD for trivial merges Fcc: Sent MIME-Version: 1.0 To: git@vger.kernel.org Cc: ZheNing Hu , Eric Sunshine , Junio C Hamano , Elijah Newren , =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsA==?= Bjarmason , Elijah Newren , Elijah Newren Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Elijah Newren From: Elijah Newren As noted in the last commit and the links therein (especially 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. The "trivial merge" logic in builtin/merge.c is yet another such case we previously missed. Add a check for it to ensure it aborts if the index does not match HEAD, and add a testcase where this fix is needed. Note that the fix here would also incidentally be an alternative fix for the testcase added in the last patch, but the fix in the last patch is still needed when multiple merge strategies are in use, so tweak the testcase from the previous commit so that it continues to exercise the codepath added in the last commit. Signed-off-by: Elijah Newren --- builtin/merge.c | 15 +++++++++++++++ t/t6424-merge-unrelated-index-changes.sh | 22 +++++++++++++++++++++- 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/builtin/merge.c b/builtin/merge.c index 23170f2d2a6..b43876f68e4 100644 --- a/builtin/merge.c +++ b/builtin/merge.c @@ -1599,6 +1599,21 @@ 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)) { + error(_("Your local changes to the following files would be overwritten by merge:\n %s"), + sb.buf); + strbuf_release(&sb); + return 2; + } + /* See if it is really trivial. */ git_committer_info(IDENT_STRICT); printf(_("Trying really trivial in-index merge...\n")); diff --git a/t/t6424-merge-unrelated-index-changes.sh b/t/t6424-merge-unrelated-index-changes.sh index eabe6bda832..187c761ad84 100755 --- a/t/t6424-merge-unrelated-index-changes.sh +++ b/t/t6424-merge-unrelated-index-changes.sh @@ -114,6 +114,19 @@ 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 && @@ -121,7 +134,14 @@ test_expect_success 'resolve, non-trivial, related file removed' ' git rm a && test_path_is_missing a && - test_must_fail git merge -s resolve D^0 && + # We also ask for recursive in order to turn off the "allow_trivial" + # setting in builtin/merge.c, and ensure that resolve really does + # correctly fail the merge (I guess this also tests that recursive + # correctly fails the merge, but the main thing we are attempting + # to test here is resolve and are just using the side effect of + # adding recursive to ensure that resolve is actually tested rather + # than the trivial merge codepath) + test_must_fail git merge -s resolve -s recursive D^0 && test_path_is_missing a && test_path_is_missing .git/MERGE_HEAD From patchwork Sat Jul 23 01:53:14 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Elijah Newren X-Patchwork-Id: 12927040 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 4083CC43334 for ; Sat, 23 Jul 2022 01:53:36 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S236836AbiGWBxc (ORCPT ); Fri, 22 Jul 2022 21:53:32 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:42902 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S236554AbiGWBx0 (ORCPT ); Fri, 22 Jul 2022 21:53:26 -0400 Received: from mail-wr1-x430.google.com (mail-wr1-x430.google.com [IPv6:2a00:1450:4864:20::430]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 8EED274DC2 for ; Fri, 22 Jul 2022 18:53:25 -0700 (PDT) Received: by mail-wr1-x430.google.com with SMTP id d13so984392wrn.10 for ; Fri, 22 Jul 2022 18:53:25 -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=FbrA92W8T1ED0spel/y6I4Ue4W9deZkR+aTEQG8Dehg=; b=E9Fe3g9wjoO/7VQ0VJfWmkGfI2abt4cH7IYrZf5TC+dBD3UOFdgY0kVrChA/1ZLok7 ECch92JXq5LHwPN2msg0VzMlp6+Kf33QbbmL9l6AG19nmyOUwIvVnL0pp7BI2GoHBf09 2Zcr6FjbbEGR//ocV/zRI84sbvLE9BiYaBQr4gKicyFBizekdg3DJRt8+Uhafv1AiwEb iGw95mY299oU8O+Loczuyf+z3tRxj/WHz7qysvgkzhfEdt/a+w76VycKyUPqRpvZyiVp /Lo4/loVnxljXk9HePOZCGec3xYmQpQ00XIYi8IiCnFXz1/fTU1PvufGyPL2yLflvU+E PjtQ== 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=FbrA92W8T1ED0spel/y6I4Ue4W9deZkR+aTEQG8Dehg=; b=ZEaHI3Z1eYhmWEL17wkWgXzPxykQ5qT71kbB/lgu3aZtC7/m4wdqMbSnkM3eO4F8hQ SuRGiUNFHbN2JHL0jYQDW1Fx2WnV235DV+qnI+jTLsDCMWUjtL8dZZxhqhwTCQiLUETZ VwgiXr3dk/PBP0XQawuHGGifiQ/R+OhQOsDsTdGR1qmsr9Z0bJxtXScnJGkmiHDYFowe le8ffcMYksNv7QvhxkyLU6NZY75nB0TyJPsaynfKIjlv5C0jF3Xt4n0UAjA+4+fU1LmE fmc0HKRKSYoT4JsYAgk6GlpBrSgwwkp7Bge0vVPD8AbTbP/H+r2hAvttHCRhHigdTvRr Yr3A== X-Gm-Message-State: AJIora9dxvMG6Lc4KswGccGqRE4XRoMHKGgMXXpYfEVEbUTUssbeugtc 4Q1mbv1Xb+m/hDdcswqfU9RSb7mpl98= X-Google-Smtp-Source: AGRyM1vf1NxJPkumDEqWCdrQ7raC6WHQc6c5XR3HROx/k+C4iymtGmusanexyWM65srnvNOxJNap0w== X-Received: by 2002:a05:6000:60a:b0:21d:9451:e91 with SMTP id bn10-20020a056000060a00b0021d94510e91mr1551584wrb.73.1658541203883; Fri, 22 Jul 2022 18:53:23 -0700 (PDT) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id m126-20020a1ca384000000b003a03e63e428sm11650701wme.36.2022.07.22.18.53.23 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 22 Jul 2022 18:53:23 -0700 (PDT) Message-Id: In-Reply-To: References: Date: Sat, 23 Jul 2022 01:53:14 +0000 Subject: [PATCH v5 4/8] 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 , =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsA==?= Bjarmason , 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 b43876f68e4..c120ad619c4 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 187c761ad84..615061c7af4 100755 --- a/t/t6424-merge-unrelated-index-changes.sh +++ b/t/t6424-merge-unrelated-index-changes.sh @@ -275,4 +275,20 @@ test_expect_success 'subtree' ' test_path_is_missing .git/MERGE_HEAD ' +test_expect_success 'with multiple strategies, recursive or ort failure do not early abort' ' + 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 recursive -s ort -s octopus C^0 >output 2>&1 && + + grep "Trying merge strategy recursive..." output && + grep "Trying merge strategy ort..." output && + grep "Trying merge strategy octopus..." 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 Sat Jul 23 01:53:15 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Elijah Newren X-Patchwork-Id: 12927043 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 51BC2C433EF for ; Sat, 23 Jul 2022 01:53:36 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S236851AbiGWBxe (ORCPT ); Fri, 22 Jul 2022 21:53:34 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:42926 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230015AbiGWBx1 (ORCPT ); Fri, 22 Jul 2022 21:53:27 -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 A4EA173928 for ; Fri, 22 Jul 2022 18:53:26 -0700 (PDT) Received: by mail-wr1-x436.google.com with SMTP id g2so597201wru.3 for ; Fri, 22 Jul 2022 18:53:26 -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=OckJ/WL0sf1z9wtCzqcjwJ+Dbks/MDiyEOPvfc19iN0=; b=cpdxAdky/BympLTlrz6g9FDrarP+bgImJshBegDzGf6VHw+ddIKcSpZWtE3H/nbLA5 bv3IlCQry/lrzIPtx6KO+VwPW4XuWmfzXqb3ngsxadLu+iNwKa0tNCFadXL39ks2Fmhp xYh+UzUdc26vREXKpgdJTcF3xG1mTjEn6UAbwcrBMnKJUTA0lpOH5ZbM9nJ4PhDglSv/ 2QFqJU26efwZkfqeQ7n4SkguyUhFGDIO5q9YhiSGvUXawSDuBIQODPu+DM+EECTMDFwM RJ8iQMQKATV+aqKdS4XTfWoo081KzqlkrBzwfQggshb14Lw/Pkb8VM3y6Ra2bcKGRHzn SVQg== 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=OckJ/WL0sf1z9wtCzqcjwJ+Dbks/MDiyEOPvfc19iN0=; b=nll1SRW6zRsM08FYd4kQXgBND7h3cIHa6739xAAHwZYTQBISjNZHFmvDo8j2m4mir3 r74b4g7RHDEsdCXpjNw6BmgYZfxjPEQVm07klWNwY/FQaglVU4NlqXwY0xmI98Lg7CH8 mJ3Nb/PS6aCxPFuxL5IBovJx8gkTvb/ufFQatJYpx4HAYSN0NkJ+Qg6ErdVlVOxBZ9LM tAKeCxpogXeU2OhdaIVWg6fj6QrVEW2Nw1TsN5oWUquNFpjERKOHS3VozKHbkYe1Ahac y5HBukuSYqj7w0lTLp9JM3CKqX8LMhbOvXjKbTuCYDfDMjD4K8/5jTpN+jee1J2KpT6m mksw== X-Gm-Message-State: AJIora8DRKtA7uPZIQfU6tTaUfayXztdaGvb7LL1tmMZBmXgKS7miXxK tIqEdUveQjnehRWKpsxb8N1t4oTQ4os= X-Google-Smtp-Source: AGRyM1vl2uDFuwxczlYO8wximvvZw+I2mNTqamBZ/ppxmzhECPxuT4UMaRTeR6+NRnaoqk4eL/HZmA== X-Received: by 2002:adf:fb12:0:b0:20c:79b2:a200 with SMTP id c18-20020adffb12000000b0020c79b2a200mr1490313wrr.617.1658541204769; Fri, 22 Jul 2022 18:53:24 -0700 (PDT) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id q5-20020a1ce905000000b003a0323463absm6669250wmc.45.2022.07.22.18.53.24 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 22 Jul 2022 18:53:24 -0700 (PDT) Message-Id: In-Reply-To: References: Date: Sat, 23 Jul 2022 01:53:15 +0000 Subject: [PATCH v5 5/8] 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 , =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsA==?= Bjarmason , 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 c120ad619c4..780b4b9100a 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 615061c7af4..2c83210f9fd 100755 --- a/t/t6424-merge-unrelated-index-changes.sh +++ b/t/t6424-merge-unrelated-index-changes.sh @@ -275,6 +275,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 'with multiple strategies, recursive or ort failure do not early abort' ' git reset --hard && git checkout B^0 && From patchwork Sat Jul 23 01:53:16 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Elijah Newren X-Patchwork-Id: 12927042 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 B9720CCA489 for ; Sat, 23 Jul 2022 01:53:37 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S236462AbiGWBxh (ORCPT ); Fri, 22 Jul 2022 21:53:37 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:42956 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230188AbiGWBx2 (ORCPT ); Fri, 22 Jul 2022 21:53:28 -0400 Received: from mail-wm1-x32b.google.com (mail-wm1-x32b.google.com [IPv6:2a00:1450:4864:20::32b]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 6D92B7695B for ; Fri, 22 Jul 2022 18:53:27 -0700 (PDT) Received: by mail-wm1-x32b.google.com with SMTP id u14-20020a05600c00ce00b003a323062569so3337668wmm.4 for ; Fri, 22 Jul 2022 18:53:27 -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=4zhEC3zh7o47SI+pu5atHsxDoFrVxDahs1drKtbiMTc=; b=kdObeyy2Cj0KYaINJdBmOkAddaE8jqNbd26wUkChP+eaEpnoigazamGUcdL7QUTKBZ z3wqVtkFdkr+PAsXgiAMj2RLbwEG8oUGDYrOvw0lsyAMpC+v+biPKwyZQ317LxeBkPmw 9Zc98SZzCuFYWviHXm2FTL8bCCyh2UB/5WcuN9WkQ2Erk7hbKmt/sAA5VSvkZ78Q2B8o 7XB5WPa5HYb+FG/pg/jgsQ+iR4L/V/fy4JyVKIv8BP4SAivz91Z639H/Tyx/3xb+4V4X +6igXjyQTVKYMDifI9jXxqeWMXEWCFMRPVut9ZBJJr6+sF9QyvUUxg26lkujH1jdoOUa Q12g== 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=4zhEC3zh7o47SI+pu5atHsxDoFrVxDahs1drKtbiMTc=; b=w7FPX+hkpmPzSR/Sdaskg5pMiKUj+KHAbicIACXGk3Shl5JQKjjOwDNlLeRSyFHTf2 c0PyRoA1mZvXIjm039WBZaleDdUK9eXDxUIQvwle6qVl0RGz1kdtOucXlqY5kaurTghq 0jWrso4xp1/rfLEGzwDusOUKTocSdwwONYVGqo9rjCMrUwg/dR3QOaT16R66RvZjrLWC gb+bYiCJuEnLW9pstyaOTWTBc3UBE3ClO30POZidKvcG/eOWdG4nwnNhA/qCGgwLVKCI oi3LSDZvRAp8hX0rLM4flZ1YbI+MY+R+wNYjRrdp9DDkciFQWcPteRlQVlCS0IJBtCXn aVNw== X-Gm-Message-State: AJIora+j5Bm1bIYHbyIQPBv6VJIZNlLruuEWDqPRDIpSJ9MzESEm/by+ S4s+96D15U6gM4RLcsu/EEEnI9PVaok= X-Google-Smtp-Source: AGRyM1vQPYIy8tb3KFy0BPpUg+Q4eSpuOQHOGNKjmDuvOYhe+VQb31JG+N+UOcVqJKvdHUq9YhbwxQ== X-Received: by 2002:a05:600c:3caa:b0:3a0:18e4:781b with SMTP id bg42-20020a05600c3caa00b003a018e4781bmr1467045wmb.199.1658541205715; Fri, 22 Jul 2022 18:53:25 -0700 (PDT) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id n6-20020adfe346000000b0021d7ad6b9fdsm5800963wrj.57.2022.07.22.18.53.24 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 22 Jul 2022 18:53:25 -0700 (PDT) Message-Id: In-Reply-To: References: Date: Sat, 23 Jul 2022 01:53:16 +0000 Subject: [PATCH v5 6/8] 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 , =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsA==?= Bjarmason , 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. Also while at it, switch to using strvec so folks don't have to count the arguments to ensure we avoided an off-by-one error, and so it's easier to add additional arguments to the command. Signed-off-by: Elijah Newren --- builtin/merge.c | 8 +++++--- t/t6424-merge-unrelated-index-changes.sh | 7 ++++++- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/builtin/merge.c b/builtin/merge.c index 780b4b9100a..e0a3299e92e 100644 --- a/builtin/merge.c +++ b/builtin/merge.c @@ -383,20 +383,22 @@ 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 }; + struct strvec args = STRVEC_INIT; if (is_null_oid(stash)) return; reset_hard(head, 1); - args[2] = oid_to_hex(stash); + strvec_pushl(&args, "stash", "apply", "--index", "--quiet", NULL); + strvec_push(&args, oid_to_hex(stash)); /* * It is OK to ignore error here, for example when there was * nothing to restore. */ - run_command_v_opt(args, RUN_GIT_CMD); + run_command_v_opt(args.v, RUN_GIT_CMD); + strvec_clear(&args); refresh_cache(REFRESH_QUIET); } diff --git a/t/t6424-merge-unrelated-index-changes.sh b/t/t6424-merge-unrelated-index-changes.sh index 2c83210f9fd..a61f20c22fe 100755 --- a/t/t6424-merge-unrelated-index-changes.sh +++ b/t/t6424-merge-unrelated-index-changes.sh @@ -292,6 +292,7 @@ test_expect_success 'with multiple strategies, recursive or ort failure do not e 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 recursive -s ort -s octopus C^0 >output 2>&1 && @@ -299,7 +300,11 @@ test_expect_success 'with multiple strategies, recursive or ort failure do not e grep "Trying merge strategy recursive..." output && grep "Trying merge strategy ort..." output && grep "Trying merge strategy octopus..." 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 Sat Jul 23 01:53:17 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Elijah Newren X-Patchwork-Id: 12927044 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 A9D41C43334 for ; Sat, 23 Jul 2022 01:53:46 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S236912AbiGWBxo (ORCPT ); Fri, 22 Jul 2022 21:53:44 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:43000 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S236696AbiGWBx3 (ORCPT ); Fri, 22 Jul 2022 21:53:29 -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 5B7A681B11 for ; Fri, 22 Jul 2022 18:53:28 -0700 (PDT) Received: by mail-wm1-x32d.google.com with SMTP id u14-20020a05600c00ce00b003a323062569so3337693wmm.4 for ; Fri, 22 Jul 2022 18:53:28 -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=RA0WtZbNa8m/DV3yDd285OCaS+dlQm7fxrD6oQ75olE=; b=CqHEYy6B6GUkgV6eiLDUxdEeEtKQATnhlrRcXk4npVjdm6VLsa/+KxIKDwcA+IjamQ mEFMjid/N6GtVbkbfcirhPlCm5b8JpRgXEJE8dZW/l4JROPpC0LUQPIaob80FYvC0kyz TWM0X1tW61uEBYS2YTMmBhjGGmOVDmGO/OxjpbMVGNM2iXN5AHh6xt0OEbgCKXVbEix/ Rr+WSzJp3hQ9jwHaxdlsRlmTsjAh/QzOVTDE0Esgf2402NR3cNoUrnE7DQ36Y6mTUya+ KpeizR60HUL4dRDPn1IxpVT0KgrDLcce3x74DczMcYnl5VUB5aNkATg88ye9LPVvWGiN Xdlw== 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=RA0WtZbNa8m/DV3yDd285OCaS+dlQm7fxrD6oQ75olE=; b=AltxxWJJbMilgYEhb+wmijwMdirj2CPci/7jM8NwzbtP7SwVaWlPziMxAp4tQRUAF+ 8+RS0QVfK8z3nDJ22cfxEH8RNH2XgS/hDmmkz2hJk7FRG6vH40OpLTLi7EDYR0P4FKZI PP5dRDR4NJ0q8OhRrynFOxbeTZZiIJBoj5SEjk5ZSq0TiAy/Av9hjNWiFukNcEEEtv1y /WhZE7Q5Ca0KMYM/T4C6+DugqCl5zcjsubiGNZA4bMU/d3txwKESFqKX6S2o9WjBmARs Wj0v9UUm6vSb/58YQyJ2WphHi65dQifoQ0UT7xQ09W050UugftgcJAwPDwr5LZz+U+lg UR7A== X-Gm-Message-State: AJIora9ud7OOtKJ8LWslz0cVo+9KKRjzE33OgNorJM/B4ESJlp1TPSPs 4nhp6AATa8Z/tHx2KCVW3wMn1pieros= X-Google-Smtp-Source: AGRyM1u7IV7hfYxh4O1yYOAceGmcb7qjRB1CVgHauIXhrygLJjB+7SKGqMZWjE57ft0KA1TcsJpNOQ== X-Received: by 2002:a05:600c:4304:b0:3a3:2e32:8a8f with SMTP id p4-20020a05600c430400b003a32e328a8fmr9534162wme.104.1658541206659; Fri, 22 Jul 2022 18:53:26 -0700 (PDT) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id h13-20020a5d548d000000b0021d8b1656dfsm5823836wrv.93.2022.07.22.18.53.26 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 22 Jul 2022 18:53:26 -0700 (PDT) Message-Id: <7f5c6884d685f2b007db56333ad3441e8bc22641.1658541198.git.gitgitgadget@gmail.com> In-Reply-To: References: Date: Sat, 23 Jul 2022 01:53:17 +0000 Subject: [PATCH v5 7/8] 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 , =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsA==?= Bjarmason , 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 e0a3299e92e..3c4f415d87e 100644 --- a/builtin/merge.c +++ b/builtin/merge.c @@ -1682,12 +1682,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 Sat Jul 23 01:53:18 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Elijah Newren X-Patchwork-Id: 12927045 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 DD236C433EF for ; Sat, 23 Jul 2022 01:53:47 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S236913AbiGWBxr (ORCPT ); Fri, 22 Jul 2022 21:53:47 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:43048 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S236728AbiGWBxb (ORCPT ); Fri, 22 Jul 2022 21:53:31 -0400 Received: from mail-wr1-x432.google.com (mail-wr1-x432.google.com [IPv6:2a00:1450:4864:20::432]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 6401973928 for ; Fri, 22 Jul 2022 18:53:29 -0700 (PDT) Received: by mail-wr1-x432.google.com with SMTP id d8so8618319wrp.6 for ; Fri, 22 Jul 2022 18:53:29 -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=7kMtOVKh6kLLyGhhgjM8QPcFrZkS6oUGXET9DoaLiTM=; b=H4T9uuubyqmsPgRCvim0fc/+YPqqMhkyyl0JtIRuadzETxw6s8g9JzYaFEpjmuavZ7 p5G5iiRk5MM6sXnHF7orRRMkvdupd60h28lxuetd6bYxXCDZrbjF9gdU22oFVbVABK0v 27n3GfjAgJHfMWEZ6tHcataMJ+nXATEC6eV+l00wBxqd88rRObZl56cDTlcX95O4kv/o vN/fCtfPe38aOa+tONZeDLRGnNx5hbgBuZl7a5jX7IIkMfOlfCxEoSl9nCZyAvGtwSUU 4dUZjCbMvUOk6lBszWL21QbDkOQBjs8dNGiyd6vIz2iI2GAkAC9Eowe0f29+maoSEUWH mQGA== 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=7kMtOVKh6kLLyGhhgjM8QPcFrZkS6oUGXET9DoaLiTM=; b=q6MGO+1EoiCArC37o1G6ZqbFCOCWylzFAJHlxYDheori6AwrJmFjM+WtXDZHBE7nmm zzCDrrQJ67n9CvuVXoHcQkefnRVedoZgMLXRsx5nz/lxxNueGrUCZ56Gb64T1MF/XaWo 5tSYpkolfiyGMj8OvfxXH5g/DygHAZzSAXwZcglu6IMqSGLKYPI+cR3L/dsZX7xodtRj 19jScZtise7QHE1O8SGvtQ4wYLi3GSkFZ02mQEPKf+7IlvxQRkr0AvaOge6eQrJDri0w TeQx+Be1Dv5HLUJN0VPhon5z/OwyniZz+UTy1Ocayly/Jg43qfjMir42aV+YgSeEoKEp /TkQ== X-Gm-Message-State: AJIora9wzLQdVohDob8PTLwYyhUc19HqYGx0ZmzHlW3AYIEGAK9gzTYC 1f3tmiAAN+o9FhsTbnBUzTrywmX95ns= X-Google-Smtp-Source: AGRyM1vOk3hxl2JtVwslZiIl30wyp8AldsybycRjZe4kXqY5QtRTk8hLoHi1ytpdDQtFOFhDinSekw== X-Received: by 2002:a5d:598e:0:b0:21e:7348:69bf with SMTP id n14-20020a5d598e000000b0021e734869bfmr230544wri.157.1658541207548; Fri, 22 Jul 2022 18:53:27 -0700 (PDT) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id n11-20020a05600c304b00b003a320b6d5eesm9494872wmh.15.2022.07.22.18.53.26 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 22 Jul 2022 18:53:27 -0700 (PDT) Message-Id: <954dec526a2f6df591f84c4a866d61e4bccd8782.1658541198.git.gitgitgadget@gmail.com> In-Reply-To: References: Date: Sat, 23 Jul 2022 01:53:18 +0000 Subject: [PATCH v5 8/8] 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 , =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsA==?= Bjarmason , 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 3c4f415d87e..f7c92c0e64f 100644 --- a/builtin/merge.c +++ b/builtin/merge.c @@ -385,11 +385,11 @@ static void restore_state(const struct object_id *head, { struct strvec args = STRVEC_INIT; - if (is_null_oid(stash)) - return; - reset_hard(head, 1); + if (is_null_oid(stash)) + goto refresh_cache; + strvec_pushl(&args, "stash", "apply", "--index", "--quiet", NULL); strvec_push(&args, oid_to_hex(stash)); @@ -400,7 +400,9 @@ static void restore_state(const struct object_id *head, run_command_v_opt(args.v, RUN_GIT_CMD); strvec_clear(&args); - 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..89a62ac53b3 --- /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 'Ensure we restore original state if no merge strategy handles it' ' + 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