From patchwork Thu May 19 16:26:21 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Elijah Newren X-Patchwork-Id: 12855764 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 51731C433F5 for ; Thu, 19 May 2022 16:26:31 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S241991AbiESQ03 (ORCPT ); Thu, 19 May 2022 12:26:29 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:57242 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S241098AbiESQ01 (ORCPT ); Thu, 19 May 2022 12:26:27 -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 987555E74E for ; Thu, 19 May 2022 09:26:26 -0700 (PDT) Received: by mail-wm1-x32b.google.com with SMTP id bg25so3165957wmb.4 for ; Thu, 19 May 2022 09:26: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=lt/vUZ0XFMcRtCAMMiib2k753yfBvnatGiqQvguXEkE=; b=kyVaTuPEBpnoNHJXkCWrWCf44ovzn1mvwo6iimU73KPrSOjWdedvU6MD5d+2L9HVNv CUlKVF5d34Wtt7c9jrhmK4WBK2SG6edqYMmxQyLQv56S/8mTDosS9V9YuIgKjrBq/eaO QL3ZIw6JFoG9xUm3LzCXjwZgihicRpy0ztj0pfkZ/G/bX+ge688nQ8J/je95/VtJ+qku 23wqyawSM2K/RSlKFCa4qn8Bx61XcaGX+f8zZ3JEW4IUmwirIhTi6/g5rM9no38Q2+MY ojDCGfHJkZ8WxMi0AKoeimx1cqxthh/g5Vmz1z4mDMZWyBnO8mR3DHVb0Xl/kMOC54Zt ISjQ== 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=lt/vUZ0XFMcRtCAMMiib2k753yfBvnatGiqQvguXEkE=; b=RgXK67yoUlHNSXXL9eFBNnTNKW4oaYyZ1+yBdLAXYX+Xy1Ql1NV4uIopNcNLERMbup RtDdkq6fMuubm1jlnS/x1K/tBx8+elnz8Uk42LO3HlgYeBO2dkGNdAd6d/zpjqUJVF31 YgqmO2hUOfPtU0rDGUsppsYJ1BHQ/mFNAGhOIl4S0lg0ybJzapZT7TIo/9srwM2ImWjs cbZ4b9wi0eS21tXFxpiDDDwjq6vcSSdIC776kn4VinPc3HuvMzRugqlglIg7lxvHhOH3 +UoaEIAMhTdEatkvRJzPQNBIMQ0StEsC+wDvH4s8sHtVF7LCTjpiYD/+jlwLjojFe/70 is5g== X-Gm-Message-State: AOAM533vX3caAYuVGTvRT7EMOGF8ydakCMaN5u1cJaF0gzO35OlNQwN/ sHNo63kwvRehQK8UFP1n4uL8o/NNd+g= X-Google-Smtp-Source: ABdhPJyEbe+fkm014Q4Yw3QgCA+oPAc7b3bBSTr2YCrUXoP/ZLzxlZxlm5D3FueD4mkTKl3eLt1rWA== X-Received: by 2002:a05:600c:a4b:b0:37b:ea2b:5583 with SMTP id c11-20020a05600c0a4b00b0037bea2b5583mr5139200wmq.139.1652977584758; Thu, 19 May 2022 09:26:24 -0700 (PDT) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id r15-20020a05600c35cf00b003970b2fa72dsm521wmq.22.2022.05.19.09.26.23 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 19 May 2022 09:26:23 -0700 (PDT) Message-Id: <042d624b8159364229e95d35e9309f12b67f8173.1652977582.git.gitgitgadget@gmail.com> In-Reply-To: References: Date: Thu, 19 May 2022 16:26:21 +0000 Subject: [PATCH 1/2] merge: remove unused variable Fcc: Sent MIME-Version: 1.0 To: git@vger.kernel.org Cc: ZheNing Hu , Elijah Newren , Elijah Newren Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Elijah Newren From: Elijah Newren restore_state() had a local variable sb that is not used, and in fact, was never used even in the original commit that introduced it, 1c7b76be7d ("Build in merge", 2008-07-07). Remove it. Signed-off-by: Elijah Newren --- builtin/merge.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/builtin/merge.c b/builtin/merge.c index f178f5a3ee1..00de224a2da 100644 --- a/builtin/merge.c +++ b/builtin/merge.c @@ -375,7 +375,6 @@ 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) { - struct strbuf sb = STRBUF_INIT; const char *args[] = { "stash", "apply", NULL, NULL }; if (is_null_oid(stash)) @@ -391,7 +390,6 @@ static void restore_state(const struct object_id *head, */ run_command_v_opt(args, RUN_GIT_CMD); - strbuf_release(&sb); refresh_cache(REFRESH_QUIET); } From patchwork Thu May 19 16:26:22 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Elijah Newren X-Patchwork-Id: 12855766 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 676CDC433EF for ; Thu, 19 May 2022 16:26:37 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S242000AbiESQ0g (ORCPT ); Thu, 19 May 2022 12:26:36 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:57386 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S241966AbiESQ03 (ORCPT ); Thu, 19 May 2022 12:26:29 -0400 Received: from mail-wm1-x333.google.com (mail-wm1-x333.google.com [IPv6:2a00:1450:4864:20::333]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id C9C5F5F8EC for ; Thu, 19 May 2022 09:26:27 -0700 (PDT) Received: by mail-wm1-x333.google.com with SMTP id a14-20020a7bc1ce000000b00393fb52a386so5340555wmj.1 for ; Thu, 19 May 2022 09:26: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=W9Lcb/8aoyOfOE5AerAneohoNficPFd41J8EuEzXE7M=; b=dFa61zenp7SJVuGHwICpHoe9/06sf3wg4c0dwf1yBu2pQrLwB5R5Zu1T2o+kVMnsWB BrsqWf1/ieSFz0pdFJS4keM/+KG4vv8fXqo/WrfGkJWwvkyLii6OIjEfVH28gVFq8xyz 3bVEcDnExMltpayBkXheS4AYELJVBOekG0SiECLC2xLrEf5Jr7PpXl5a36MZEuDWMCNY h/mPW4C4eVl/MxOxNQTEnny0EjZwNCD5PdXSfG15PKJM0DrVSotjaHSxA8NS67TKSEsM mDgd0fHAqfvC7SOmyoDUUJ7JM1I9qMxxefLmrYMlPIkyKzAp0n4Z7VRWejRmhGGhv3uk 8fUw== 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=W9Lcb/8aoyOfOE5AerAneohoNficPFd41J8EuEzXE7M=; b=ZoCcLSAQ3c699yHky/+ZMkdBzV+EK3KwreGvwdSlquFl0ZfeEuVbDA++LNqhn1643s BeKhSAbC5yFglN8I85bULYv/yTZMrHL9Xfa4NmTrYf4c5x1zd1i+tShVMNaBNgKJGtFK pOcIR/09DevuTF0TGbFbL28yveA9r8nKI/VuEwuBZhT1OrbFUjvF2avAc2c50EM3aTE8 iKhHslObzd+0D9sVAk4hBtS1ke6mI/3JcIWGRLOT/20Mk885Oh2USFncBHwtaWUy/YHm j3g9MEVFkLzm7mmwXrPq7XUFp2ImJOGjlbAQo+Cy+lJeopfAoNzMgck9zg7VkrpxcOYO kdfw== X-Gm-Message-State: AOAM5305EDyAfX38BxhXYpZ4GKFe10/x2SDak5lhAFse310TbSm6+w+s QcRSYaxc+Kl4B7rFuyoldjCqkRQZ6p4= X-Google-Smtp-Source: ABdhPJzu9HnlvXGdTIhtCISUmsEd60mdPxXD5Tv8lOyzANNbTH2NpFscVGjwH8VxW16gBHJ1C/cQXw== X-Received: by 2002:a05:600c:2102:b0:394:2765:580c with SMTP id u2-20020a05600c210200b003942765580cmr4525520wml.150.1652977585972; Thu, 19 May 2022 09:26:25 -0700 (PDT) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id r9-20020adfa149000000b0020e62feca05sm48608wrr.32.2022.05.19.09.26.25 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 19 May 2022 09:26:25 -0700 (PDT) Message-Id: <88bdca72a780d70e156e22e1ab96dedd368c761b.1652977582.git.gitgitgadget@gmail.com> In-Reply-To: References: Date: Thu, 19 May 2022 16:26:22 +0000 Subject: [PATCH 2/2] merge: make restore_state() do as its name says Fcc: Sent MIME-Version: 1.0 To: git@vger.kernel.org Cc: ZheNing Hu , 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.) Reported-by: ZheNing Hu Signed-off-by: Elijah Newren --- builtin/merge.c | 10 ++++++---- t/t7607-merge-state.sh | 25 +++++++++++++++++++++++++ 2 files changed, 31 insertions(+), 4 deletions(-) create mode 100755 t/t7607-merge-state.sh diff --git a/builtin/merge.c b/builtin/merge.c index 00de224a2da..ae3ee3a996b 100644 --- a/builtin/merge.c +++ b/builtin/merge.c @@ -377,11 +377,11 @@ static void restore_state(const struct object_id *head, { const char *args[] = { "stash", "apply", NULL, NULL }; - if (is_null_oid(stash)) - return; - reset_hard(head, 1); + if (is_null_oid(stash)) + goto refresh_cache; + args[2] = oid_to_hex(stash); /* @@ -390,7 +390,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..655478cd0b3 --- /dev/null +++ b/t/t7607-merge-state.sh @@ -0,0 +1,25 @@ +#!/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 && +git show-ref && + + for b in branch1 branch2 branch3 + do + git checkout -b $b main && + test_commit --no-tag "Change on $b" base $b + done && + + git checkout branch1 && + test_must_fail git merge branch2 branch3 && + git diff --exit-code --name-status && + test_path_is_missing .git/MERGE_HEAD +' + +test_done