From patchwork Wed Sep 6 15:22:45 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Phillip Wood X-Patchwork-Id: 13375733 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 2D6DDEE14A8 for ; Wed, 6 Sep 2023 15:23:09 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S242516AbjIFPXC (ORCPT ); Wed, 6 Sep 2023 11:23:02 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:60400 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S234736AbjIFPXC (ORCPT ); Wed, 6 Sep 2023 11:23:02 -0400 Received: from mail-lf1-x131.google.com (mail-lf1-x131.google.com [IPv6:2a00:1450:4864:20::131]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 81DB0172E for ; Wed, 6 Sep 2023 08:22:57 -0700 (PDT) Received: by mail-lf1-x131.google.com with SMTP id 2adb3069b0e04-5009969be25so5924435e87.3 for ; Wed, 06 Sep 2023 08:22:57 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1694013775; x=1694618575; darn=vger.kernel.org; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:from:to:cc:subject:date :message-id:reply-to; bh=I/exoRSTbEckkziY9jJHRpEMg/fq0sxdmzJvLwkGaFk=; b=aX3wypMB0N3f/lJBjEowdOU0rAPlEhvoxX3Ic1h42b6E52AuLRe0Zl/XeMSK8v5wLq kiY4XTfvcgjJ54ZB5epGXhkU50xzt5G6YLj8RRyrEb8TOzc/Fr4BFgliu9UWJcSIISXs shZGKv70YygN3lRS/85kFmCzlH06Devzw7eWtt9iv+4OiSc4A5rF8Y/vLrarw7gElYmP GcKpQVSa+UEmh873IpHR78tWPyKbCkoSk4gEQG40BSuRYVDeEz1wyGYwvpHTGhlMeaw6 QdLfZx30CdouK0BcIQDviNV28FfiviqMMDVn+cE10ntyDcmalh1QFappvsLW6VhBLeOL 4fDw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1694013775; x=1694618575; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=I/exoRSTbEckkziY9jJHRpEMg/fq0sxdmzJvLwkGaFk=; b=fukwagivRWa117RTFh+uyefdxgH/91m6SMbMmekq0ii9jPHLp3rCfmQ8VuR/e8+ZLp f3L/wzewx7fNteMRwtfbWDkZPtKu+ChAxyAwc3Ve3cbwSQtrJOSKIS3/6UEWQ6DhqFGS 7dJ+8HcVmjd4YJ+s9BHxH7JUsz9bLlHyQpeiKu67WQ5gpUnha+g4umlMq87jWcUN6meh 2zZyG2CWdyBnFGz4f7x5z38V/xBTtGbBb0s2ELBV642aUUCje9uh/MyHBmfAnR0peO41 j+vjQ9CRZZYaKaALjWQLiWArWmy0+xE7X2ysLhrRN9BUOyR2VMuDyI4CW5AZoqgZ0Uxf 8f0w== X-Gm-Message-State: AOJu0Yw5TCXT1u7eXnONBDWy6qqrXkE8y7CZ84hDmXQ9ymjKb6/6MdSC ZDWi0C3QJ8FSH4Q4wI71NZiQu3rSqTk= X-Google-Smtp-Source: AGHT+IE4sq9jzmAXITn45cazkHVV2ybFNJL4ouXxzw8uw5r/3+E6GV27jsk/vFUTpuL+oHhaY+aPxg== X-Received: by 2002:a05:6512:3d26:b0:500:9b26:9760 with SMTP id d38-20020a0565123d2600b005009b269760mr3256787lfv.13.1694013775194; Wed, 06 Sep 2023 08:22:55 -0700 (PDT) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id g10-20020a7bc4ca000000b003fe1a092925sm20036419wmk.19.2023.09.06.08.22.54 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 06 Sep 2023 08:22:54 -0700 (PDT) Message-ID: In-Reply-To: References: Date: Wed, 06 Sep 2023 15:22:45 +0000 Subject: [PATCH v4 1/7] rebase -i: move unlink() calls Fcc: Sent MIME-Version: 1.0 To: git@vger.kernel.org Cc: Johannes Schindelin , Junio C Hamano , Stefan Haller , Phillip Wood , Eric Sunshine , Glen Choo , Phillip Wood , Phillip Wood Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Phillip Wood From: Phillip Wood At the start of each iteration the loop that picks commits removes the state files from the previous pick. However some of these files are only written if there are conflicts in which case we exit the loop before the end of the loop body. Therefore they only need to be removed when the rebase continues, not at the start of each iteration. Signed-off-by: Phillip Wood --- sequencer.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/sequencer.c b/sequencer.c index cc9821ece2c..de66bda9d5b 100644 --- a/sequencer.c +++ b/sequencer.c @@ -4656,6 +4656,10 @@ static int pick_commits(struct repository *r, if (read_and_refresh_cache(r, opts)) return -1; + unlink(rebase_path_message()); + unlink(rebase_path_stopped_sha()); + unlink(rebase_path_amend()); + while (todo_list->current < todo_list->nr) { struct todo_item *item = todo_list->items + todo_list->current; const char *arg = todo_item_get_arg(todo_list, item); @@ -4679,10 +4683,7 @@ static int pick_commits(struct repository *r, todo_list->total_nr, opts->verbose ? "\n" : "\r"); } - unlink(rebase_path_message()); unlink(rebase_path_author_script()); - unlink(rebase_path_stopped_sha()); - unlink(rebase_path_amend()); unlink(git_path_merge_head(r)); unlink(git_path_auto_merge(r)); delete_ref(NULL, "REBASE_HEAD", NULL, REF_NO_DEREF); From patchwork Wed Sep 6 15:22:46 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Phillip Wood X-Patchwork-Id: 13375732 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 5B1A6EE14A5 for ; Wed, 6 Sep 2023 15:23:09 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S242534AbjIFPXE (ORCPT ); Wed, 6 Sep 2023 11:23:04 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:45338 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S242517AbjIFPXC (ORCPT ); Wed, 6 Sep 2023 11:23:02 -0400 Received: from mail-wm1-x330.google.com (mail-wm1-x330.google.com [IPv6:2a00:1450:4864:20::330]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 06900173D for ; Wed, 6 Sep 2023 08:22:58 -0700 (PDT) Received: by mail-wm1-x330.google.com with SMTP id 5b1f17b1804b1-4018af103bcso7540455e9.1 for ; Wed, 06 Sep 2023 08:22:57 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1694013776; x=1694618576; darn=vger.kernel.org; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:from:to:cc:subject:date :message-id:reply-to; bh=c3pGS1eGtJAgJD9MdPr3soUbnoVHR3IJEaWtzaYwrb8=; b=m+dY2zPGC3HEefCOtG18ad67frb37GxA5x/1aMBYnWXBGC6qLit3BZN2FQvuCy0/3a 78OOAroU2t3DymKRT8EvXhZzSrwLpgMWe9L+/wVcOhXfTTIMHtCbrWEFahWrzrc1HyVK OZo49ImQmKJUDY4A2FTWaeEgbVVXJSsd85yTj790JFRvHkFNkbnrBZNdumt1mM87xh8X v93iIT8s+n2F0WrCiXHkmnExOoSEJja1cDQwCt2trfNCS6qwt5YOnev7fKUVRPXDGQab PIe7OxBu/VHI1SYpwr4OmgntUrkd5AkzIn1Ao/sS3/OMp9MwQ5+xzwigLIpP1w7HxNGL IWlA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1694013776; x=1694618576; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=c3pGS1eGtJAgJD9MdPr3soUbnoVHR3IJEaWtzaYwrb8=; b=NqhUg+7j1jzvYWA0uAfo3bBtwoGycLqAVegxxWfc3P8PqCMApH+DoPxdlGgy1dTw+c jF1x9SBQmHy6LTpYiUQXYi2uslbf1x2ntGMvY64+61XlB5ef9h/MEijQTCv/xVBKNvAt dUW/3COJA+RxupujOq7M62RboiBJOQOiheeBqnjaDcblkbtWdGLcQuh3CgqdEtBJuIBV 7SsRdgrGyL9tpWGXK8piAFyGWbwP9EQL/BvScG7o91NtoaMDj13oeA4WIlDOekVQSToN vCQxQTzoNQ0SgCmZTzlTLldZ3v/MwvuKNYuzF3vJk45HQvbTkbGnmy1qoJX8kiJebw2n R5Jw== X-Gm-Message-State: AOJu0YxPQpRYy5wErgOOrtTECbzIvHvfp/ZOVm7FG/XT5raoSsl9lzPO F/JvGpjHCE6XrwYSQdSXqfAahbkGYGg= X-Google-Smtp-Source: AGHT+IFL4rpgUqj+1x1NIZMWQiGh1mkg3QxScK72KrQafwwXm6bcte78BFdWBt4dlQ13l//0qwoZQw== X-Received: by 2002:a05:600c:5247:b0:3fe:f4b0:634a with SMTP id fc7-20020a05600c524700b003fef4b0634amr2831019wmb.19.1694013775917; Wed, 06 Sep 2023 08:22:55 -0700 (PDT) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id e2-20020a05600c218200b003fef5402d2dsm23403338wme.8.2023.09.06.08.22.55 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 06 Sep 2023 08:22:55 -0700 (PDT) Message-ID: In-Reply-To: References: Date: Wed, 06 Sep 2023 15:22:46 +0000 Subject: [PATCH v4 2/7] rebase -i: remove patch file after conflict resolution Fcc: Sent MIME-Version: 1.0 To: git@vger.kernel.org Cc: Johannes Schindelin , Junio C Hamano , Stefan Haller , Phillip Wood , Eric Sunshine , Glen Choo , Phillip Wood , Phillip Wood Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Phillip Wood From: Phillip Wood When a rebase stops for the user to resolve conflicts it writes a patch for the conflicting commit to .git/rebase-merge/patch. This file has been written since the introduction of "git-rebase-interactive.sh" in 1b1dce4bae7 (Teach rebase an interactive mode, 2007-06-25). I assume the idea was to enable the user inspect the conflicting commit in the same way as they could for the patch based rebase. This file should be deleted when the rebase continues as if the rebase stops for a failed "exec" command or a "break" command it is confusing to the user if there is a stale patch lying around from an unrelated command. As the path is now used in two different places rebase_path_patch() is added and used to obtain the path for the patch. To construct the path write_patch() previously used get_dir() which returns different paths depending on whether we're rebasing or cherry-picking/reverting. As this function is only called when rebasing it is safe to use a hard coded string for the directory instead. An assertion is added to make sure we don't starting calling this function when cherry-picking in the future. Signed-off-by: Phillip Wood --- sequencer.c | 16 ++++++++++++---- t/t3418-rebase-continue.sh | 18 ++++++++++++++++++ 2 files changed, 30 insertions(+), 4 deletions(-) diff --git a/sequencer.c b/sequencer.c index de66bda9d5b..c1911b0fc14 100644 --- a/sequencer.c +++ b/sequencer.c @@ -138,6 +138,11 @@ static GIT_PATH_FUNC(rebase_path_amend, "rebase-merge/amend") * the commit object name of the corresponding patch. */ static GIT_PATH_FUNC(rebase_path_stopped_sha, "rebase-merge/stopped-sha") +/* + * When we stop for the user to resolve conflicts this file contains + * the patch of the commit that is being picked. + */ +static GIT_PATH_FUNC(rebase_path_patch, "rebase-merge/patch") /* * For the post-rewrite hook, we make a list of rewritten commits and * their new sha1s. The rewritten-pending list keeps the sha1s of @@ -3502,12 +3507,14 @@ static int make_patch(struct repository *r, char hex[GIT_MAX_HEXSZ + 1]; int res = 0; + if (!is_rebase_i(opts)) + BUG("make_patch should only be called when rebasing"); + oid_to_hex_r(hex, &commit->object.oid); if (write_message(hex, strlen(hex), rebase_path_stopped_sha(), 1) < 0) return -1; res |= write_rebase_head(&commit->object.oid); - strbuf_addf(&buf, "%s/patch", get_dir(opts)); memset(&log_tree_opt, 0, sizeof(log_tree_opt)); repo_init_revisions(r, &log_tree_opt, NULL); log_tree_opt.abbrev = 0; @@ -3515,15 +3522,15 @@ static int make_patch(struct repository *r, log_tree_opt.diffopt.output_format = DIFF_FORMAT_PATCH; log_tree_opt.disable_stdin = 1; log_tree_opt.no_commit_id = 1; - log_tree_opt.diffopt.file = fopen(buf.buf, "w"); + log_tree_opt.diffopt.file = fopen(rebase_path_patch(), "w"); log_tree_opt.diffopt.use_color = GIT_COLOR_NEVER; if (!log_tree_opt.diffopt.file) - res |= error_errno(_("could not open '%s'"), buf.buf); + res |= error_errno(_("could not open '%s'"), + rebase_path_patch()); else { res |= log_tree_commit(&log_tree_opt, commit); fclose(log_tree_opt.diffopt.file); } - strbuf_reset(&buf); strbuf_addf(&buf, "%s/message", get_dir(opts)); if (!file_exists(buf.buf)) { @@ -4659,6 +4666,7 @@ static int pick_commits(struct repository *r, unlink(rebase_path_message()); unlink(rebase_path_stopped_sha()); unlink(rebase_path_amend()); + unlink(rebase_path_patch()); while (todo_list->current < todo_list->nr) { struct todo_item *item = todo_list->items + todo_list->current; diff --git a/t/t3418-rebase-continue.sh b/t/t3418-rebase-continue.sh index 2d0789e554b..261e7cd754c 100755 --- a/t/t3418-rebase-continue.sh +++ b/t/t3418-rebase-continue.sh @@ -244,6 +244,24 @@ test_expect_success 'the todo command "break" works' ' test_path_is_file execed ' +test_expect_success 'patch file is removed before break command' ' + test_when_finished "git rebase --abort" && + cat >todo <<-\EOF && + pick commit-new-file-F2-on-topic-branch + break + EOF + + ( + set_replace_editor todo && + test_must_fail git rebase -i --onto commit-new-file-F2 HEAD + ) && + test_path_is_file .git/rebase-merge/patch && + echo 22>F2 && + git add F2 && + git rebase --continue && + test_path_is_missing .git/rebase-merge/patch +' + test_expect_success '--reschedule-failed-exec' ' test_when_finished "git rebase --abort" && test_must_fail git rebase -x false --reschedule-failed-exec HEAD^ && From patchwork Wed Sep 6 15:22:47 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Phillip Wood X-Patchwork-Id: 13375731 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 92F38EE14A9 for ; Wed, 6 Sep 2023 15:23:03 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S242557AbjIFPXF (ORCPT ); Wed, 6 Sep 2023 11:23:05 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:45378 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S242531AbjIFPXE (ORCPT ); Wed, 6 Sep 2023 11:23:04 -0400 Received: from mail-wr1-x42f.google.com (mail-wr1-x42f.google.com [IPv6:2a00:1450:4864:20::42f]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id A062A1732 for ; Wed, 6 Sep 2023 08:22:58 -0700 (PDT) Received: by mail-wr1-x42f.google.com with SMTP id ffacd0b85a97d-31f4950333cso2734881f8f.3 for ; Wed, 06 Sep 2023 08:22:58 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1694013777; x=1694618577; darn=vger.kernel.org; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:from:to:cc:subject:date :message-id:reply-to; bh=MsucW3xG5qOdWILsBi68x2JFSxc79J2vEF//FKIA6MI=; b=FSktGBeSl3J6yblQbWXlEEFHEEe6mwofIEssWiPC6iWCitZYsJ18Ylgw0pMAFtUMbe +fVdA6Lqfyc2WdHHb5tmiq7CekI/7fiSQ05zFtPsPajexlkIkGM/TCQuNY0yWwoCA8JX WE+TREUJh6BnsUCIFJnXfqobUhpKDsBfSgNP3JMTNAQ97khNiOvbJBep7gjz+pS5LWKB jVJ/iMctb64UZWaWZkQJTsy3kKngcqF47F0bY1Zu9C9NNmUAnGPUF/JE0ds41GgD1G8a f+d3KNRNgr6DkDL+4zEIw/4DENBVjh8RmkRMI78EsXSEzwILWR4Cuvoa0YFCl5koS1gp fGQA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1694013777; x=1694618577; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=MsucW3xG5qOdWILsBi68x2JFSxc79J2vEF//FKIA6MI=; b=bxonkGAGp72RRmW3GBpgLs/NRxgn6pK7OQxx/0EasDtBoO4fujoA0RvyRPSOiP8eC8 6wIfGSLt2hb7Rv8o1GC6UQWGT/ruYvUUoX0PNGqKEUjr5dSTLXPQ5Y2WRfz9iCvLPRaP Mv0pE5gd/EJBWmtD6iNKRp1Ai0EjZ1TqM3PPcg/EO9LdHyDh9LjURHoJ7PZ/LFl7bYcN q3F7NS3lNsLy+eYEiPg5N/HxlPSyIQedYe7kTEmcnmNSjiKZfBfigxqW6NXUj9HqCS1U sgDP4kPhN6goKRrmZfUQ9ikBZWKw37j2QrpqX/IbcE7yqLailYqa3XpEZviQBj40mYJD cTAg== X-Gm-Message-State: AOJu0Yz5vDA4x4dRja/MJyK0feDEUS3zWXe72bxdOCOBOK/EgvyJlUfF lD2kKkrSWoH/ak1bN45yrbe+f1OdpNA= X-Google-Smtp-Source: AGHT+IF8hl8W0+ZCh+OC8tx7TWm3ZlJ6rh4k3aCr5fndkUNWM2BQEd1tc196VkjEtO0ofyBLXKL8Mg== X-Received: by 2002:a5d:550e:0:b0:317:6704:72c with SMTP id b14-20020a5d550e000000b003176704072cmr2822858wrv.52.1694013776597; Wed, 06 Sep 2023 08:22:56 -0700 (PDT) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id w2-20020adff9c2000000b00317ddccb0d1sm20666440wrr.24.2023.09.06.08.22.56 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 06 Sep 2023 08:22:56 -0700 (PDT) Message-ID: <818bdaf772dc34ef7282deb2e78ad9a37fa792f8.1694013772.git.gitgitgadget@gmail.com> In-Reply-To: References: Date: Wed, 06 Sep 2023 15:22:47 +0000 Subject: [PATCH v4 3/7] sequencer: use rebase_path_message() Fcc: Sent MIME-Version: 1.0 To: git@vger.kernel.org Cc: Johannes Schindelin , Junio C Hamano , Stefan Haller , Phillip Wood , Eric Sunshine , Glen Choo , Phillip Wood , Phillip Wood Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Phillip Wood From: Phillip Wood Rather than constructing the path in a struct strbuf use the ready made function to get the path name instead. This was the last remaining use of the strbuf so remove it as well. As with the previous patch we now use a hard coded string rather than git_dir() when constructing the path. This is safe for the same reason (make_patch() is only called when rebasing) and is protected by the assertion added in the previous patch. Signed-off-by: Phillip Wood --- sequencer.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/sequencer.c b/sequencer.c index c1911b0fc14..83be8bf2b6d 100644 --- a/sequencer.c +++ b/sequencer.c @@ -3501,7 +3501,6 @@ static int make_patch(struct repository *r, struct commit *commit, struct replay_opts *opts) { - struct strbuf buf = STRBUF_INIT; struct rev_info log_tree_opt; const char *subject; char hex[GIT_MAX_HEXSZ + 1]; @@ -3532,18 +3531,16 @@ static int make_patch(struct repository *r, fclose(log_tree_opt.diffopt.file); } - strbuf_addf(&buf, "%s/message", get_dir(opts)); - if (!file_exists(buf.buf)) { + if (!file_exists(rebase_path_message())) { const char *encoding = get_commit_output_encoding(); const char *commit_buffer = repo_logmsg_reencode(r, commit, NULL, encoding); find_commit_subject(commit_buffer, &subject); - res |= write_message(subject, strlen(subject), buf.buf, 1); + res |= write_message(subject, strlen(subject), rebase_path_message(), 1); repo_unuse_commit_buffer(r, commit, commit_buffer); } - strbuf_release(&buf); release_revisions(&log_tree_opt); return res; From patchwork Wed Sep 6 15:22:48 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Phillip Wood X-Patchwork-Id: 13375734 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 22627EE14A9 for ; Wed, 6 Sep 2023 15:23:10 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S242568AbjIFPXM (ORCPT ); Wed, 6 Sep 2023 11:23:12 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:45422 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S242555AbjIFPXF (ORCPT ); Wed, 6 Sep 2023 11:23:05 -0400 Received: from mail-wr1-x42b.google.com (mail-wr1-x42b.google.com [IPv6:2a00:1450:4864:20::42b]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 80F511982 for ; Wed, 6 Sep 2023 08:22:59 -0700 (PDT) Received: by mail-wr1-x42b.google.com with SMTP id ffacd0b85a97d-31c6cd238e0so754710f8f.0 for ; Wed, 06 Sep 2023 08:22:59 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1694013778; x=1694618578; darn=vger.kernel.org; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:from:to:cc:subject:date :message-id:reply-to; bh=DkessOCLYjU6qRajBC6xLircpgIT+YOR+Z1at6dbBNU=; b=CxI6MJN8kgpLO6LkMKvA7JFFFu2rqwZB8D3s6ChixPK+TKtpy2ONLmLGg2l+VqHub9 LsgWydN+FuD8ZGyBEm0vYEq6JHidj6/CqQfejp8f9uWX6nbCy+6KbaTgVyNVAQWF+nz6 OEEgk3estbQQTn5ajuzXeQyjhqoUxNi03cc77i9PSPH76cu4VFSEJV+sNqY8R5uQ3rJ7 qytNGbKt3S2V0ply8v8qF9Nv1xGoMfnoTZvswXSmvgQwJmBVKppJywR20coBGW3dDjY7 7xEmXeVwOtkyI1sCgEfqX2AIH9OHcP3y3XNBHMDAYA6srcPAgrH9al5JpqvcaVNhVYcs 5lFw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1694013778; x=1694618578; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=DkessOCLYjU6qRajBC6xLircpgIT+YOR+Z1at6dbBNU=; b=cUmUY1P+L2mNGMAEhUnLuAZPWRWZUlgK+owpK6WSNhiKyeq8YWfRlsbWEwC/TCeXSx Ba5PO7CXf6n75RUxrhh2iH2fC5jvRMNgo+lK9CiF7gEBBIPF/gKaQGDxz+SVF12WNV3k NeI1/q8IdG4iWNpJWOZ6MGW9L+yYbqVS5XaIAIK1vBQ8TvsLct0F3lvu7aR6TGBLB6XT UNALuOlnZ1mT1R9KiD7X3Jjlo82hcZrDfvC/BAqL8Luqu9ngwHOlTaCqoOjNpq3lwc/v i0kKbUhKhgTKgkETQUMYhHLJWdzPHgQs7ONDpYkpWqseyMPAX1qTqXSrN5vYC4pem9KK og0w== X-Gm-Message-State: AOJu0YwdDhkadDw46N3UzI+BtllAX6J+uCqpAJLPHWQUplSYzifr0r+w hHxLoqZ66nOl5OVYqhPYPPMpEyGvfXY= X-Google-Smtp-Source: AGHT+IFdAoZv2MhDkvebpznCJn+/CEoTeg9VaubDw7aZmSW+RPxsQY+rM86M5165IkVoBtxDk/Sz6w== X-Received: by 2002:adf:f50b:0:b0:316:f9f1:bb0f with SMTP id q11-20020adff50b000000b00316f9f1bb0fmr2770619wro.34.1694013777753; Wed, 06 Sep 2023 08:22:57 -0700 (PDT) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id d1-20020adfa401000000b0031980294e9fsm17320959wra.116.2023.09.06.08.22.57 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 06 Sep 2023 08:22:57 -0700 (PDT) Message-ID: In-Reply-To: References: Date: Wed, 06 Sep 2023 15:22:48 +0000 Subject: [PATCH v4 4/7] sequencer: factor out part of pick_commits() Fcc: Sent MIME-Version: 1.0 To: git@vger.kernel.org Cc: Johannes Schindelin , Junio C Hamano , Stefan Haller , Phillip Wood , Eric Sunshine , Glen Choo , Phillip Wood , Phillip Wood Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Phillip Wood From: Phillip Wood This simplifies the next commit. If a pick fails we now return the error at the end of the loop body rather than returning early, a successful "edit" command continues to return early. There are three things to check to ensure that removing the early return for an error does not change the behavior of the code: (1) We could enter the block guarded by "if (reschedule)". This block is not entered because "reschedlue" is always zero when picking a commit. (2) We could enter the block guarded by "else if (is_rebase_i(opts) && check_todo && !res)". This block is not entered when returning an error because "res" is non-zero in that case. (3) todo_list->current could be incremented before returning. That is avoided by moving the increment which is of course a potential change in behavior itself. The move is safe because none of the callers look at todo_list after this function returns. Moving the increment makes it clear we only want to advance the current item if the command was successful. Signed-off-by: Phillip Wood --- sequencer.c | 132 ++++++++++++++++++++++++++++------------------------ 1 file changed, 71 insertions(+), 61 deletions(-) diff --git a/sequencer.c b/sequencer.c index 83be8bf2b6d..b434c5a2570 100644 --- a/sequencer.c +++ b/sequencer.c @@ -4645,6 +4645,72 @@ N_("Could not execute the todo command\n" " git rebase --edit-todo\n" " git rebase --continue\n"); +static int pick_one_commit(struct repository *r, + struct todo_list *todo_list, + struct replay_opts *opts, + int *check_todo) +{ + int res; + struct todo_item *item = todo_list->items + todo_list->current; + const char *arg = todo_item_get_arg(todo_list, item); + if (is_rebase_i(opts)) + opts->reflog_message = reflog_message( + opts, command_to_string(item->command), NULL); + + res = do_pick_commit(r, item, opts, is_final_fixup(todo_list), + check_todo); + if (is_rebase_i(opts) && res < 0) { + /* Reschedule */ + advise(_(rescheduled_advice), + get_item_line_length(todo_list, todo_list->current), + get_item_line(todo_list, todo_list->current)); + todo_list->current--; + if (save_todo(todo_list, opts)) + return -1; + } + if (item->command == TODO_EDIT) { + struct commit *commit = item->commit; + if (!res) { + if (!opts->verbose) + term_clear_line(); + fprintf(stderr, _("Stopped at %s... %.*s\n"), + short_commit_name(commit), item->arg_len, arg); + } + return error_with_patch(r, commit, + arg, item->arg_len, opts, res, !res); + } + if (is_rebase_i(opts) && !res) + record_in_rewritten(&item->commit->object.oid, + peek_command(todo_list, 1)); + if (res && is_fixup(item->command)) { + if (res == 1) + intend_to_amend(); + return error_failed_squash(r, item->commit, opts, + item->arg_len, arg); + } else if (res && is_rebase_i(opts) && item->commit) { + int to_amend = 0; + struct object_id oid; + + /* + * If we are rewording and have either + * fast-forwarded already, or are about to + * create a new root commit, we want to amend, + * otherwise we do not. + */ + if (item->command == TODO_REWORD && + !repo_get_oid(r, "HEAD", &oid) && + (oideq(&item->commit->object.oid, &oid) || + (opts->have_squash_onto && + oideq(&opts->squash_onto, &oid)))) + to_amend = 1; + + return res | error_with_patch(r, item->commit, + arg, item->arg_len, opts, + res, to_amend); + } + return res; +} + static int pick_commits(struct repository *r, struct todo_list *todo_list, struct replay_opts *opts) @@ -4700,66 +4766,9 @@ static int pick_commits(struct repository *r, } } if (item->command <= TODO_SQUASH) { - if (is_rebase_i(opts)) - opts->reflog_message = reflog_message(opts, - command_to_string(item->command), NULL); - - res = do_pick_commit(r, item, opts, - is_final_fixup(todo_list), - &check_todo); - if (is_rebase_i(opts) && res < 0) { - /* Reschedule */ - advise(_(rescheduled_advice), - get_item_line_length(todo_list, - todo_list->current), - get_item_line(todo_list, - todo_list->current)); - todo_list->current--; - if (save_todo(todo_list, opts)) - return -1; - } - if (item->command == TODO_EDIT) { - struct commit *commit = item->commit; - if (!res) { - if (!opts->verbose) - term_clear_line(); - fprintf(stderr, - _("Stopped at %s... %.*s\n"), - short_commit_name(commit), - item->arg_len, arg); - } - return error_with_patch(r, commit, - arg, item->arg_len, opts, res, !res); - } - if (is_rebase_i(opts) && !res) - record_in_rewritten(&item->commit->object.oid, - peek_command(todo_list, 1)); - if (res && is_fixup(item->command)) { - if (res == 1) - intend_to_amend(); - return error_failed_squash(r, item->commit, opts, - item->arg_len, arg); - } else if (res && is_rebase_i(opts) && item->commit) { - int to_amend = 0; - struct object_id oid; - - /* - * If we are rewording and have either - * fast-forwarded already, or are about to - * create a new root commit, we want to amend, - * otherwise we do not. - */ - if (item->command == TODO_REWORD && - !repo_get_oid(r, "HEAD", &oid) && - (oideq(&item->commit->object.oid, &oid) || - (opts->have_squash_onto && - oideq(&opts->squash_onto, &oid)))) - to_amend = 1; - - return res | error_with_patch(r, item->commit, - arg, item->arg_len, opts, - res, to_amend); - } + res = pick_one_commit(r, todo_list, opts, &check_todo); + if (!res && item->command == TODO_EDIT) + return 0; } else if (item->command == TODO_EXEC) { char *end_of_arg = (char *)(arg + item->arg_len); int saved = *end_of_arg; @@ -4820,9 +4829,10 @@ static int pick_commits(struct repository *r, return -1; } - todo_list->current++; if (res) return res; + + todo_list->current++; } if (is_rebase_i(opts)) { From patchwork Wed Sep 6 15:22:49 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Phillip Wood X-Patchwork-Id: 13375735 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 C9022EE14A8 for ; Wed, 6 Sep 2023 15:23:16 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S242586AbjIFPXS (ORCPT ); Wed, 6 Sep 2023 11:23:18 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:45480 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S242567AbjIFPXM (ORCPT ); Wed, 6 Sep 2023 11:23:12 -0400 Received: from mail-lj1-x22a.google.com (mail-lj1-x22a.google.com [IPv6:2a00:1450:4864:20::22a]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id C36D31986 for ; Wed, 6 Sep 2023 08:23:00 -0700 (PDT) Received: by mail-lj1-x22a.google.com with SMTP id 38308e7fff4ca-2bd6611873aso58474971fa.1 for ; Wed, 06 Sep 2023 08:23:00 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1694013779; x=1694618579; darn=vger.kernel.org; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:from:to:cc:subject:date :message-id:reply-to; bh=g2QmTux5NHmE+K2ap2mKnjYaZ6Rf9PEs8HqsiLwir48=; b=GRZljln3v7hsxtxXuUSUOPaTmrHEnY3Kd5IycCZYXr8g6lKjmDPHTaNgp80v+KzDp8 QjdAjZpD3diE7vEqpfjhFzGK83rFJyHI1c5tXdJ2hqsE5wbHSKIboBvXNXFlLcVw59dy JWH41M8n9CoThaQV+phbd/KmfuMHxnaPx7ZPoQqgMdP1+dTSVaR8XzT1qV3Ykxp+4Z84 XiPJ0QePqcUQk6wx058HZxZje9D4vVt0aSaireAMSpIy5ceajbPPyIv/YTrRX0HawebS 2v0loMudn1B16b9bJG2kgQpefoZSJzk7PFlp4+NFkO7e+GYpnOTdMLe2BOVfZipztv29 3Lxg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1694013779; x=1694618579; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=g2QmTux5NHmE+K2ap2mKnjYaZ6Rf9PEs8HqsiLwir48=; b=KhoHFirBYt0a2ofGB1uCKsuilglxqGyuevsjx+yx2nJUU9YUtywcE9EKYZqR4aZMYt XCjHwRHAZlT7eD/TUm0xU1CeA+AaMDsmvl5is1Nj4RXEQ9jTenJ535QuWOPchcD/9jzb gKwlUiko3FmiIk3gAWwWfjHSOhfH7ai+GMT8Nu0T5bHuNvmUmn4pYvM1pNrNrgrTrDW0 oB3BvJ9z6reJsAwB+oPa0xKzct+asHe+oVNBJ1yQiRH15nAr+vzQfV9/p+azBbhWnZYi 8UyqpsPMmoDVeYruixLlGzvt2JwTXBBOE4y3qaa8YsQDoSS775vc7A14hkeGLiC+vQAR Faaw== X-Gm-Message-State: AOJu0Yy7Cb0XHODsPZUolFByuZVMjDQL2RqZbuknLARjyIxHzpnmi6hT aY6dg1QcrbGwtwPwDajPuWM9WyvsOlM= X-Google-Smtp-Source: AGHT+IGx0KkkNCcqzzzosM5Ojven2mQY3S+zIomhjDbkbK3GkELUy3PPxAYhzVAeNbjseeGLUBo1Kw== X-Received: by 2002:a2e:86c5:0:b0:2bc:eea7:834e with SMTP id n5-20020a2e86c5000000b002bceea7834emr2472008ljj.40.1694013778601; Wed, 06 Sep 2023 08:22:58 -0700 (PDT) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id t12-20020a7bc3cc000000b003fef19bb55csm20032932wmj.34.2023.09.06.08.22.57 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 06 Sep 2023 08:22:58 -0700 (PDT) Message-ID: In-Reply-To: References: Date: Wed, 06 Sep 2023 15:22:49 +0000 Subject: [PATCH v4 5/7] rebase: fix rewritten list for failed pick Fcc: Sent MIME-Version: 1.0 To: git@vger.kernel.org Cc: Johannes Schindelin , Junio C Hamano , Stefan Haller , Phillip Wood , Eric Sunshine , Glen Choo , Phillip Wood , Phillip Wood Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Phillip Wood From: Phillip Wood git rebase keeps a list that maps the OID of each commit before it was rebased to the OID of the equivalent commit after the rebase. This list is used to drive the "post-rewrite" hook that is called at the end of a successful rebase. When a rebase stops for the user to resolve merge conflicts the OID of the commit being picked is written to ".git/rebase-merge/stopped-sha". Then when the rebase is continued that OID is added to the list of rewritten commits. Unfortunately if a commit cannot be picked because it would overwrite an untracked file we still write the "stopped-sha1" file. This means that when the rebase is continued the commit is added into the list of rewritten commits even though it has not been picked yet. Fix this by not calling error_with_patch() for failed commands. The pick has failed so there is nothing to commit and therefore we do not want to set up the state files for committing staged changes when the rebase continues. This change means we no-longer write a patch for the failed command or display the error message printed by error_with_patch(). As the command has failed the patch isn't really useful and in any case the user can inspect the commit associated with the failed command by inspecting REBASE_HEAD. Unless the user has disabled it we already print an advice message that is more helpful than the message from error_with_patch() which the user will still see. Even if the advice is disabled the user will see the messages from the merge machinery detailing the problem. The code to add a failed command back into the todo list is duplicated between pick_one_commit() and the loop in pick_commits(). Both sites print advice about the command being rescheduled, decrement the current item and save the todo list. To avoid duplicating this code pick_one_commit() is modified to set a flag to indicate that the command should be rescheduled in the main loop. This simplifies things as only the remaining copy of the code needs to be modified to set REBASE_HEAD rather than calling error_with_patch(). Signed-off-by: Phillip Wood --- sequencer.c | 19 +++++--------- t/t3404-rebase-interactive.sh | 6 +++++ t/t3430-rebase-merges.sh | 4 +-- t/t5407-post-rewrite-hook.sh | 48 +++++++++++++++++++++++++++++++++++ 4 files changed, 63 insertions(+), 14 deletions(-) diff --git a/sequencer.c b/sequencer.c index b434c5a2570..76932ab7b23 100644 --- a/sequencer.c +++ b/sequencer.c @@ -4158,6 +4158,7 @@ static int do_merge(struct repository *r, if (ret < 0) { error(_("could not even attempt to merge '%.*s'"), merge_arg_len, arg); + unlink(git_path_merge_msg(r)); goto leave_merge; } /* @@ -4648,7 +4649,7 @@ N_("Could not execute the todo command\n" static int pick_one_commit(struct repository *r, struct todo_list *todo_list, struct replay_opts *opts, - int *check_todo) + int *check_todo, int* reschedule) { int res; struct todo_item *item = todo_list->items + todo_list->current; @@ -4661,12 +4662,8 @@ static int pick_one_commit(struct repository *r, check_todo); if (is_rebase_i(opts) && res < 0) { /* Reschedule */ - advise(_(rescheduled_advice), - get_item_line_length(todo_list, todo_list->current), - get_item_line(todo_list, todo_list->current)); - todo_list->current--; - if (save_todo(todo_list, opts)) - return -1; + *reschedule = 1; + return -1; } if (item->command == TODO_EDIT) { struct commit *commit = item->commit; @@ -4766,7 +4763,8 @@ static int pick_commits(struct repository *r, } } if (item->command <= TODO_SQUASH) { - res = pick_one_commit(r, todo_list, opts, &check_todo); + res = pick_one_commit(r, todo_list, opts, &check_todo, + &reschedule); if (!res && item->command == TODO_EDIT) return 0; } else if (item->command == TODO_EXEC) { @@ -4820,10 +4818,7 @@ static int pick_commits(struct repository *r, if (save_todo(todo_list, opts)) return -1; if (item->commit) - return error_with_patch(r, - item->commit, - arg, item->arg_len, - opts, res, 0); + write_rebase_head(&item->commit->object.oid); } else if (is_rebase_i(opts) && check_todo && !res && reread_todo_if_changed(r, todo_list, opts)) { return -1; diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh index ff0afad63e2..6d3788c588b 100755 --- a/t/t3404-rebase-interactive.sh +++ b/t/t3404-rebase-interactive.sh @@ -1287,7 +1287,9 @@ test_expect_success 'rebase -i commits that overwrite untracked files (pick)' ' >file6 && test_must_fail git rebase --continue && test_cmp_rev HEAD F && + test_cmp_rev REBASE_HEAD I && rm file6 && + test_path_is_missing .git/rebase-merge/patch && git rebase --continue && test_cmp_rev HEAD I ' @@ -1305,7 +1307,9 @@ test_expect_success 'rebase -i commits that overwrite untracked files (squash)' >file6 && test_must_fail git rebase --continue && test_cmp_rev HEAD F && + test_cmp_rev REBASE_HEAD I && rm file6 && + test_path_is_missing .git/rebase-merge/patch && git rebase --continue && test $(git cat-file commit HEAD | sed -ne \$p) = I && git reset --hard original-branch2 @@ -1323,7 +1327,9 @@ test_expect_success 'rebase -i commits that overwrite untracked files (no ff)' ' >file6 && test_must_fail git rebase --continue && test $(git cat-file commit HEAD | sed -ne \$p) = F && + test_cmp_rev REBASE_HEAD I && rm file6 && + test_path_is_missing .git/rebase-merge/patch && git rebase --continue && test $(git cat-file commit HEAD | sed -ne \$p) = I ' diff --git a/t/t3430-rebase-merges.sh b/t/t3430-rebase-merges.sh index 96ae0edf1e1..4938ebb1c17 100755 --- a/t/t3430-rebase-merges.sh +++ b/t/t3430-rebase-merges.sh @@ -165,12 +165,12 @@ test_expect_success 'failed `merge -C` writes patch (may be rescheduled, too)' ' test_config sequence.editor \""$PWD"/replace-editor.sh\" && test_tick && test_must_fail git rebase -ir HEAD && + test_cmp_rev REBASE_HEAD H^0 && grep "^merge -C .* G$" .git/rebase-merge/done && grep "^merge -C .* G$" .git/rebase-merge/git-rebase-todo && - test_path_is_file .git/rebase-merge/patch && + test_path_is_missing .git/rebase-merge/patch && : fail because of merge conflict && - rm G.t .git/rebase-merge/patch && git reset --hard conflicting-G && test_must_fail git rebase --continue && ! grep "^merge -C .* G$" .git/rebase-merge/git-rebase-todo && diff --git a/t/t5407-post-rewrite-hook.sh b/t/t5407-post-rewrite-hook.sh index 5f3ff051ca2..ad7f8c6f002 100755 --- a/t/t5407-post-rewrite-hook.sh +++ b/t/t5407-post-rewrite-hook.sh @@ -17,6 +17,12 @@ test_expect_success 'setup' ' git checkout A^0 && test_commit E bar E && test_commit F foo F && + git checkout B && + git merge E && + git tag merge-E && + test_commit G G && + test_commit H H && + test_commit I I && git checkout main && test_hook --setup post-rewrite <<-EOF @@ -173,6 +179,48 @@ test_fail_interactive_rebase () { ) } +test_expect_success 'git rebase with failed pick' ' + clear_hook_input && + cat >todo <<-\EOF && + exec >bar + merge -C merge-E E + exec >G + pick G + exec >H 2>I + pick H + fixup I + EOF + + ( + set_replace_editor todo && + test_must_fail git rebase -i D D 2>err + ) && + grep "would be overwritten" err && + rm bar && + + test_must_fail git rebase --continue 2>err && + grep "would be overwritten" err && + rm G && + + test_must_fail git rebase --continue 2>err && + grep "would be overwritten" err && + rm H && + + test_must_fail git rebase --continue 2>err && + grep "would be overwritten" err && + rm I && + + git rebase --continue && + echo rebase >expected.args && + cat >expected.data <<-EOF && + $(git rev-parse merge-E) $(git rev-parse HEAD~2) + $(git rev-parse G) $(git rev-parse HEAD~1) + $(git rev-parse H) $(git rev-parse HEAD) + $(git rev-parse I) $(git rev-parse HEAD) + EOF + verify_hook_input +' + test_expect_success 'git rebase -i (unchanged)' ' git reset --hard D && clear_hook_input && From patchwork Wed Sep 6 15:22:50 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Phillip Wood X-Patchwork-Id: 13375736 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 81717EE14A9 for ; Wed, 6 Sep 2023 15:23:19 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S242531AbjIFPXU (ORCPT ); Wed, 6 Sep 2023 11:23:20 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:45464 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S242566AbjIFPXM (ORCPT ); Wed, 6 Sep 2023 11:23:12 -0400 Received: from mail-lj1-x229.google.com (mail-lj1-x229.google.com [IPv6:2a00:1450:4864:20::229]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id BC0E51732 for ; Wed, 6 Sep 2023 08:23:01 -0700 (PDT) Received: by mail-lj1-x229.google.com with SMTP id 38308e7fff4ca-2bcd7a207f7so58166861fa.3 for ; Wed, 06 Sep 2023 08:23:01 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1694013780; x=1694618580; darn=vger.kernel.org; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:from:to:cc:subject:date :message-id:reply-to; bh=vD9GETpmG95sn5RmN4rwg0MjurjAfOhKYJbh8sypqNI=; b=ZkLqchZCqOrWbppYSavQb8WqzcGYM2Oz9ESoxd91sjSA9gDZhy4kls2dCqnEAmrtbV JZlCABenhpf4merwWoZ9x7t+L4zicVPMavr0m7rica9lGYBwg3s9RNjDsoRdBXc4rBp0 YE0MYyQoWFZLnhMyW1QpLt8RvwxYVDLvKTUJllTxbtUjgOIqsUUrmK2m6sE9q+gv4Mx3 WoKG1LNlSSwMckt/bQa0iT05ot0II4X8KQSYEym9Bp8ceBidkIf40JEpMVpTdkr+HmPo RGXJScwJBM5v089ryon6VeFbydol1LDRJRxzrKrSLSB+SDxv5Yu8BvXb2mXcSRFuv4o4 2MDw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1694013780; x=1694618580; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=vD9GETpmG95sn5RmN4rwg0MjurjAfOhKYJbh8sypqNI=; b=CxwHafCfZfr79hbZdYrnkfYu53AUkYfxbYYo3otUbUOqLX3UGi49bLOCI+fDt8PXGT TnJTiWD4YU7Hl20OhAwm86fw1iVwtndLW5jl8y+6+uAPLzifuOXH8VnwRzrIPzQMIkpS xzRMT++bAUz7WIbuPHcVXdwEPnCAyGGzHcUpzIxznoDHN/2r3mtDuwzkL2zeEvxi1XbP 4DxrI6o/8dc9elS6khxyPdsnvDWUOiAdKdRupR4OJgOjEM/2LYMC8yWoUKvF17PvqYr0 UURKzNEsL41E5ODV28uGvTvh3t8CiWKWNw3USSWmJNtXqBrFTMYSpOFauWauwCyTF6ZK 2QiQ== X-Gm-Message-State: AOJu0YybKhTKe57JUYOVdE1gkPycph4EDwZud19Q/FP8Wac4ODIRZXrj X7AsLseNQ5KvBWDwNN69OIi5A8dYaU0= X-Google-Smtp-Source: AGHT+IEwdJSFevdUBH2MD3tTbjOOKpaA8XjEwoLIRAD53CQucYKZdYeKwbmmVwhGhwsOxOLjG94XKA== X-Received: by 2002:a2e:9c9a:0:b0:2bc:dcc2:b186 with SMTP id x26-20020a2e9c9a000000b002bcdcc2b186mr2381647lji.36.1694013779610; Wed, 06 Sep 2023 08:22:59 -0700 (PDT) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id 10-20020a05600c234a00b003fc06169ab3sm23168749wmq.20.2023.09.06.08.22.58 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 06 Sep 2023 08:22:58 -0700 (PDT) Message-ID: <0ca5fccca17af22de6da846d82e960ebe1aa87b1.1694013772.git.gitgitgadget@gmail.com> In-Reply-To: References: Date: Wed, 06 Sep 2023 15:22:50 +0000 Subject: [PATCH v4 6/7] rebase --continue: refuse to commit after failed command Fcc: Sent MIME-Version: 1.0 To: git@vger.kernel.org Cc: Johannes Schindelin , Junio C Hamano , Stefan Haller , Phillip Wood , Eric Sunshine , Glen Choo , Phillip Wood , Phillip Wood Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Phillip Wood From: Phillip Wood If a commit cannot be picked because it would overwrite an untracked file then "git rebase --continue" should refuse to commit any staged changes as the commit was not picked. This is implemented by refusing to commit if the message file is missing. The message file is chosen for this check because it is only written when "git rebase" stops for the user to resolve merge conflicts. Existing commands that refuse to commit staged changes when continuing such as a failed "exec" rely on checking for the absence of the author script in run_git_commit(). This prevents the staged changes from being committed but prints error: could not open '.git/rebase-merge/author-script' for reading before the message about not being able to commit. This is confusing to users and so checking for the message file instead improves the user experience. The existing test for refusing to commit after a failed exec is updated to check that we do not print the error message about a missing author script anymore. Signed-off-by: Phillip Wood --- sequencer.c | 5 +++++ t/t3404-rebase-interactive.sh | 18 +++++++++++++++++- t/t3430-rebase-merges.sh | 4 ++++ 3 files changed, 26 insertions(+), 1 deletion(-) diff --git a/sequencer.c b/sequencer.c index 76932ab7b23..38b0f213157 100644 --- a/sequencer.c +++ b/sequencer.c @@ -4980,6 +4980,11 @@ static int commit_staged_changes(struct repository *r, is_clean = !has_uncommitted_changes(r, 0); + if (!is_clean && !file_exists(rebase_path_message())) { + const char *gpg_opt = gpg_sign_opt_quoted(opts); + + return error(_(staged_changes_advice), gpg_opt, gpg_opt); + } if (file_exists(rebase_path_amend())) { struct strbuf rev = STRBUF_INIT; struct object_id head, to_amend; diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh index 6d3788c588b..a8ad398956a 100755 --- a/t/t3404-rebase-interactive.sh +++ b/t/t3404-rebase-interactive.sh @@ -604,7 +604,8 @@ test_expect_success 'clean error after failed "exec"' ' echo "edited again" > file7 && git add file7 && test_must_fail git rebase --continue 2>error && - test_i18ngrep "you have staged changes in your working tree" error + test_i18ngrep "you have staged changes in your working tree" error && + test_i18ngrep ! "could not open.*for reading" error ' test_expect_success 'rebase a detached HEAD' ' @@ -1290,6 +1291,11 @@ test_expect_success 'rebase -i commits that overwrite untracked files (pick)' ' test_cmp_rev REBASE_HEAD I && rm file6 && test_path_is_missing .git/rebase-merge/patch && + echo changed >file1 && + git add file1 && + test_must_fail git rebase --continue 2>err && + grep "error: you have staged changes in your working tree" err && + git reset --hard HEAD && git rebase --continue && test_cmp_rev HEAD I ' @@ -1310,6 +1316,11 @@ test_expect_success 'rebase -i commits that overwrite untracked files (squash)' test_cmp_rev REBASE_HEAD I && rm file6 && test_path_is_missing .git/rebase-merge/patch && + echo changed >file1 && + git add file1 && + test_must_fail git rebase --continue 2>err && + grep "error: you have staged changes in your working tree" err && + git reset --hard HEAD && git rebase --continue && test $(git cat-file commit HEAD | sed -ne \$p) = I && git reset --hard original-branch2 @@ -1330,6 +1341,11 @@ test_expect_success 'rebase -i commits that overwrite untracked files (no ff)' ' test_cmp_rev REBASE_HEAD I && rm file6 && test_path_is_missing .git/rebase-merge/patch && + echo changed >file1 && + git add file1 && + test_must_fail git rebase --continue 2>err && + grep "error: you have staged changes in your working tree" err && + git reset --hard HEAD && git rebase --continue && test $(git cat-file commit HEAD | sed -ne \$p) = I ' diff --git a/t/t3430-rebase-merges.sh b/t/t3430-rebase-merges.sh index 4938ebb1c17..804ff819782 100755 --- a/t/t3430-rebase-merges.sh +++ b/t/t3430-rebase-merges.sh @@ -169,6 +169,10 @@ test_expect_success 'failed `merge -C` writes patch (may be rescheduled, too)' ' grep "^merge -C .* G$" .git/rebase-merge/done && grep "^merge -C .* G$" .git/rebase-merge/git-rebase-todo && test_path_is_missing .git/rebase-merge/patch && + echo changed >file1 && + git add file1 && + test_must_fail git rebase --continue 2>err && + grep "error: you have staged changes in your working tree" err && : fail because of merge conflict && git reset --hard conflicting-G && From patchwork Wed Sep 6 15:22:51 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Phillip Wood X-Patchwork-Id: 13375737 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 D1C6BEE14A5 for ; Wed, 6 Sep 2023 15:23:20 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234507AbjIFPXW (ORCPT ); Wed, 6 Sep 2023 11:23:22 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:45532 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S242540AbjIFPXM (ORCPT ); Wed, 6 Sep 2023 11:23:12 -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 4CAF01989 for ; Wed, 6 Sep 2023 08:23:02 -0700 (PDT) Received: by mail-wm1-x329.google.com with SMTP id 5b1f17b1804b1-401d6f6b2e0so7194945e9.1 for ; Wed, 06 Sep 2023 08:23:02 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1694013780; x=1694618580; darn=vger.kernel.org; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:from:to:cc:subject:date :message-id:reply-to; bh=KD4f+N5/xgNshrjkT9c/ljbvyAF7IBGhQjO0ODJhwpM=; b=SrnP+izqXgxdtqtZPd2s7fEB06nUsnyZ9SxCXbhK99Qs9J5q7nTUHbC8/0v79J1zwn j7m1sigCHW/bh9fO/AzBcLmqBY8u17/Cz2Bcv0T5+j3Vemdc6h0aGzPiJX5ye+aKN0hO P4w1XMKeWu/t+N9CSKeda1l+rDVYkbicwu51nPKplK0cLrIckjLoiaw/CYmGrzQ2lend d5i2BRJCPK313MxNq3Q9jrmTqm33h9MSSyBT3yeMTgHgdy3q6CSO7fS3LVKr6dRp2ubU +5W1Rw/eEAnwNkG+o8XRgWt066UQ9PCaglk+kEXweh1ZODHedd9K95xk6AjhnEvKD7sK NQZQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1694013780; x=1694618580; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=KD4f+N5/xgNshrjkT9c/ljbvyAF7IBGhQjO0ODJhwpM=; b=eWGdTA0S6Sb3TQJMUDBt9566CBCMpO0IxOaivB5Rlc3FmPjr7oZ9ec7QGp35C73Ptb AJ2mnZzwV7iJxRsZ661ziuwzbPHYa+KcdFKpyJ94dv5gZcugUY8u9X4Lx/XMs+R8S0Wh Kj7AARRGihESbqUWY1eNb7Nk1FKoThQFw99UG6c+rDSue7NK0sMp5EZ35cDkR8LOziRV scbklAtZ2X9scx7z4kMTRaUg3fGrSfuvrj7G4392Boe5UStnRlY0VA2mgHe1msRtf9rz c5wrdMJUKv/0gNV0x8b5+FbrY0dXSCTRymNvHN1H3wJq8z4Ughhz2WkIUVLkIJhum4VH HpFA== X-Gm-Message-State: AOJu0Yx3sj1YwEu2fLw25uJokccNovqjoLrLxFuZyjGyCcYoYwV6Rue3 lEl1eqOfeQvbHZPun8EKPRYi1LFBgCE= X-Google-Smtp-Source: AGHT+IHxB+F8uUcs9KMYtT3iF0rh5F8In6gLUC+gdR3Edzrgbl/XXfmdg3mYFiCzusCRSldIlxJegg== X-Received: by 2002:adf:e94a:0:b0:31c:2f95:8056 with SMTP id m10-20020adfe94a000000b0031c2f958056mr2731002wrn.23.1694013780543; Wed, 06 Sep 2023 08:23:00 -0700 (PDT) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id a28-20020a5d457c000000b00317f70240afsm20764033wrc.27.2023.09.06.08.22.59 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 06 Sep 2023 08:23:00 -0700 (PDT) Message-ID: <8d5f6d51e1959c7908ce268692c1ae9d693b93c5.1694013772.git.gitgitgadget@gmail.com> In-Reply-To: References: Date: Wed, 06 Sep 2023 15:22:51 +0000 Subject: [PATCH v4 7/7] rebase -i: fix adding failed command to the todo list Fcc: Sent MIME-Version: 1.0 To: git@vger.kernel.org Cc: Johannes Schindelin , Junio C Hamano , Stefan Haller , Phillip Wood , Eric Sunshine , Glen Choo , Phillip Wood , Phillip Wood Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Phillip Wood From: Phillip Wood When rebasing commands are moved from the todo list in "git-rebase-todo" to the "done" file (which is used by "git status" to show the recently executed commands) just before they are executed. This means that if a command fails because it would overwrite an untracked file it has to be added back into the todo list before the rebase stops for the user to fix the problem. Unfortunately when a failed command is added back into the todo list the command preceding it is erroneously appended to the "done" file. This means that when rebase stops after "pick B" fails the "done" file contains pick A pick B pick A instead of pick A pick B This happens because save_todo() updates the "done" file with the previous command whenever "git-rebase-todo" is updated. When we add the failed pick back into "git-rebase-todo" we do not want to update "done". Fix this by adding a "reschedule" parameter to save_todo() which prevents the "done" file from being updated when adding a failed command back into the "git-rebase-todo" file. A couple of the existing tests are modified to improve their coverage as none of them trigger this bug or check the "done" file. Reported-by: Stefan Haller Signed-off-by: Phillip Wood --- sequencer.c | 12 ++++++------ t/t3404-rebase-interactive.sh | 29 ++++++++++++++++++----------- t/t3430-rebase-merges.sh | 22 ++++++++++++++++------ 3 files changed, 40 insertions(+), 23 deletions(-) diff --git a/sequencer.c b/sequencer.c index 38b0f213157..175addb7ca6 100644 --- a/sequencer.c +++ b/sequencer.c @@ -3397,7 +3397,8 @@ give_advice: return -1; } -static int save_todo(struct todo_list *todo_list, struct replay_opts *opts) +static int save_todo(struct todo_list *todo_list, struct replay_opts *opts, + int reschedule) { struct lock_file todo_lock = LOCK_INIT; const char *todo_path = get_todo_path(opts); @@ -3407,7 +3408,7 @@ static int save_todo(struct todo_list *todo_list, struct replay_opts *opts) * rebase -i writes "git-rebase-todo" without the currently executing * command, appending it to "done" instead. */ - if (is_rebase_i(opts)) + if (is_rebase_i(opts) && !reschedule) next++; fd = hold_lock_file_for_update(&todo_lock, todo_path, 0); @@ -3420,7 +3421,7 @@ static int save_todo(struct todo_list *todo_list, struct replay_opts *opts) if (commit_lock_file(&todo_lock) < 0) return error(_("failed to finalize '%s'"), todo_path); - if (is_rebase_i(opts) && next > 0) { + if (is_rebase_i(opts) && !reschedule && next > 0) { const char *done = rebase_path_done(); int fd = open(done, O_CREAT | O_WRONLY | O_APPEND, 0666); int ret = 0; @@ -4733,7 +4734,7 @@ static int pick_commits(struct repository *r, const char *arg = todo_item_get_arg(todo_list, item); int check_todo = 0; - if (save_todo(todo_list, opts)) + if (save_todo(todo_list, opts, reschedule)) return -1; if (is_rebase_i(opts)) { if (item->command != TODO_COMMENT) { @@ -4814,8 +4815,7 @@ static int pick_commits(struct repository *r, get_item_line_length(todo_list, todo_list->current), get_item_line(todo_list, todo_list->current)); - todo_list->current--; - if (save_todo(todo_list, opts)) + if (save_todo(todo_list, opts, reschedule)) return -1; if (item->commit) write_rebase_head(&item->commit->object.oid); diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh index a8ad398956a..71da9c465a1 100755 --- a/t/t3404-rebase-interactive.sh +++ b/t/t3404-rebase-interactive.sh @@ -1277,19 +1277,24 @@ test_expect_success 'todo count' ' ' test_expect_success 'rebase -i commits that overwrite untracked files (pick)' ' - git checkout --force branch2 && + git checkout --force A && git clean -f && + cat >todo <<-EOF && + exec >file2 + pick $(git rev-parse B) B + pick $(git rev-parse C) C + pick $(git rev-parse D) D + exec cat .git/rebase-merge/done >actual + EOF ( - set_fake_editor && - FAKE_LINES="edit 1 2" git rebase -i A + set_replace_editor todo && + test_must_fail git rebase -i A ) && - test_cmp_rev HEAD F && - test_path_is_missing file6 && - >file6 && - test_must_fail git rebase --continue && - test_cmp_rev HEAD F && - test_cmp_rev REBASE_HEAD I && - rm file6 && + test_cmp_rev HEAD B && + test_cmp_rev REBASE_HEAD C && + head -n3 todo >expect && + test_cmp expect .git/rebase-merge/done && + rm file2 && test_path_is_missing .git/rebase-merge/patch && echo changed >file1 && git add file1 && @@ -1297,7 +1302,9 @@ test_expect_success 'rebase -i commits that overwrite untracked files (pick)' ' grep "error: you have staged changes in your working tree" err && git reset --hard HEAD && git rebase --continue && - test_cmp_rev HEAD I + test_cmp_rev HEAD D && + tail -n3 todo >>expect && + test_cmp expect actual ' test_expect_success 'rebase -i commits that overwrite untracked files (squash)' ' diff --git a/t/t3430-rebase-merges.sh b/t/t3430-rebase-merges.sh index 804ff819782..0b0877b9846 100755 --- a/t/t3430-rebase-merges.sh +++ b/t/t3430-rebase-merges.sh @@ -128,14 +128,24 @@ test_expect_success 'generate correct todo list' ' ' test_expect_success '`reset` refuses to overwrite untracked files' ' - git checkout -b refuse-to-reset && + git checkout B && test_commit dont-overwrite-untracked && - git checkout @{-1} && - : >dont-overwrite-untracked.t && - echo "reset refs/tags/dont-overwrite-untracked" >script-from-scratch && + cat >script-from-scratch <<-EOF && + exec >dont-overwrite-untracked.t + pick $(git rev-parse B) B + reset refs/tags/dont-overwrite-untracked + pick $(git rev-parse C) C + exec cat .git/rebase-merge/done >actual + EOF test_config sequence.editor \""$PWD"/replace-editor.sh\" && - test_must_fail git rebase -ir HEAD && - git rebase --abort + test_must_fail git rebase -ir A && + test_cmp_rev HEAD B && + head -n3 script-from-scratch >expect && + test_cmp expect .git/rebase-merge/done && + rm dont-overwrite-untracked.t && + git rebase --continue && + tail -n3 script-from-scratch >>expect && + test_cmp expect actual ' test_expect_success '`reset` rejects trees' '