From patchwork Tue Aug 1 15:23:26 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Phillip Wood X-Patchwork-Id: 13336942 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 822A2C0015E for ; Tue, 1 Aug 2023 15:23:50 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233250AbjHAPXn (ORCPT ); Tue, 1 Aug 2023 11:23:43 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:46914 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231332AbjHAPXl (ORCPT ); Tue, 1 Aug 2023 11:23:41 -0400 Received: from mail-wm1-x32a.google.com (mail-wm1-x32a.google.com [IPv6:2a00:1450:4864:20::32a]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 20A572125 for ; Tue, 1 Aug 2023 08:23:37 -0700 (PDT) Received: by mail-wm1-x32a.google.com with SMTP id 5b1f17b1804b1-3fb4146e8fcso39917335e9.0 for ; Tue, 01 Aug 2023 08:23:37 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1690903415; x=1691508215; 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=7FsZlnQ/pdPdGxbCA6lMi0UO2zWupGBHUo0en+6o2o8=; b=nVAvP2Ph8nduFRg6NEVSMshgLGifVaLjccbyiqJy6b0V3xM8NUbLz+GMWGNZfUzzpb kyCSbvAUw7hnhJvQBYFp0TaYZLDDHPM09B4pQ7bc1iLIVGypGOIyYr5nRWRnx8UKjcN6 2glNvRNuleTrwlWhI6TM9dbkdmoHTAIZ7ikAfuLcbyl7l99TSoOW6ZpEEh4PQneRUPbP M2Nftlns8ZFcjasY0g5pOjmpudZNsEISoIAz59I4xzKXfg7jw0J1cdJuns0SUqbVPFKW m9Xwx2ui6KTqqLXqaxvfK61XJE97PunDGl/RaY5M6+Joi7YTHp+pTKfq7AH5HlnrI3fm vG7Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1690903415; x=1691508215; 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=7FsZlnQ/pdPdGxbCA6lMi0UO2zWupGBHUo0en+6o2o8=; b=YcSnjRBs2NOTdv1Lfivy6fw0mQ2bnGklY9j27ZXtCHBA3pqc5pvUC9rbz7/Ly3ourd TO+OcFcAAwbqPeX5H/G2k6yZwTPuzrwcD1ncipns7cJ+h/P05Uuf9/aDWtSDPSLOxmtP s+bm6RPJAbeKkO7Xud5uG5pbHex3PmoN3gZqnpvcDMwQ9QNhvRU6Qez4NLxFLYz/OtXO ETQnOWhv/DCE9yxZaE7LXu3PEJ12Kd8CWTCJW0EfEPyAXvSiauDae/4aWHpRzgiGveuW D4hGBog47P3zPTCC7EEK1OiBzz9v12m+02S6c/LuANH6PQK8YVGGDiZxOEhhmu1EmH8s pkIg== X-Gm-Message-State: ABy/qLbOCXOSr6fcCrew4gmJRA++Nh/rJjyB0SVXpRmIUFhczanMJRQA YxaIrUtGtPOVuYVeOdHYeQ4RQCW7904= X-Google-Smtp-Source: APBJJlGIhkyf/8sWzPuEbQYB9UJdVd4qikWhqvC3N7DPbpfY9RudLldDKPs5ufmS4eNzXciZCX3wDA== X-Received: by 2002:a1c:6a0a:0:b0:3fa:aeac:e978 with SMTP id f10-20020a1c6a0a000000b003faaeace978mr2851242wmc.0.1690903415220; Tue, 01 Aug 2023 08:23:35 -0700 (PDT) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id c13-20020a05600c0acd00b003fe1fe56202sm6296754wmr.33.2023.08.01.08.23.34 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 01 Aug 2023 08:23:35 -0700 (PDT) Message-ID: <1ab1ad2ef07687c25c1d346b5b7b26f38bafe5b9.1690903412.git.gitgitgadget@gmail.com> In-Reply-To: References: Date: Tue, 01 Aug 2023 15:23:26 +0000 Subject: [PATCH v3 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 state files from the previous pick. However some of these are only written if there are conflicts and so we break out of the loop after writing them. Therefore they only need to be removed when the rebase continues, not in 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 Tue Aug 1 15:23:27 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Phillip Wood X-Patchwork-Id: 13336941 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 409ACC0015E for ; Tue, 1 Aug 2023 15:23:47 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232562AbjHAPXq (ORCPT ); Tue, 1 Aug 2023 11:23:46 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:46942 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233797AbjHAPXo (ORCPT ); Tue, 1 Aug 2023 11:23:44 -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 2A1BE2133 for ; Tue, 1 Aug 2023 08:23:38 -0700 (PDT) Received: by mail-wm1-x32e.google.com with SMTP id 5b1f17b1804b1-3fe2bc2701bso5045305e9.2 for ; Tue, 01 Aug 2023 08:23:38 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1690903416; x=1691508216; 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=U2dHNxdeYNlXbs3XDttxVk6u7DRpgrC7B383DHwPTBw=; b=reZga2AqcK2uydsKjwpxcnUpWBFugoeIB2qANEkNiiW4LdGtOeXLWO+x+leSG3KUve 7g9uYpKFe6BXXUzvYeBY2zLtu4yFW+7USYCpo9BRE2ate/NQ02CEli/IgdP0QdGmxWGB hpqYK8Np/+AXBkWUQLWwuH6o+n1v5A+zazrrZ+wzIuZcLPbIyPMyVIkJ2OxxBqe8QHr/ 71aqKwrpY/IeGJ5r/nu88tcovMSGiKt89/zZfDcWIG++bOCGxuHIcHUieD1UBaZXWveU uDeLtE190rLj9cRF97klEyNxu5PD7vPErEoxB7QWSbK0+BNrRcxPQ8dEHcjxf9u5XQ94 iOiw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1690903416; x=1691508216; 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=U2dHNxdeYNlXbs3XDttxVk6u7DRpgrC7B383DHwPTBw=; b=FZVMLCIqWvlHmyAE4m7Pv4c3N8fKHhScPMxCD29fWzrLVZvBnHu2aeElWK5k4bFF+7 lZmSqb+GLjliV5U3vF5nz1XTYcmp5v3Kn6pV2jZSCWgrpiLAP8+CdQ5oN/7UcudKisxq cmXy0izdLaIMfr/n3i2Rti4Rx7Xwjkut18qnZEmNLgx+8kFW+Z8kLjFi00oth2vjTcoV MfEcNfrmDk4t1IN+4dwo4gnfXHgoHMVHAG57ZjYFe71FZbH6sZ4paHq1rahDvRchby0v rMdlJSjxddS8h0+rq6tLHuWKI4k5vxfRuGQzopztP3HK1ICP2fsAg0Mz68ixf4sqBzmQ 904w== X-Gm-Message-State: ABy/qLbkyLpMlYwnOLY7sEhrnSz6AlnCKXzixRQNlHNxOd34HOCmkz2M pmeqUqnptRFyyPjuMYwuehOwtw6vi3I= X-Google-Smtp-Source: APBJJlERRCABJ04GNl2HKYSQN93ma66gsGuED2UwiQVqFSyh8t/y6kzriYgRS8XiU6rTogVNBsuaIA== X-Received: by 2002:a1c:7c03:0:b0:3fe:2140:f504 with SMTP id x3-20020a1c7c03000000b003fe2140f504mr2662422wmc.20.1690903416170; Tue, 01 Aug 2023 08:23:36 -0700 (PDT) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id 3-20020a05600c22c300b003fe13c3ece7sm10201827wmg.10.2023.08.01.08.23.35 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 01 Aug 2023 08:23:35 -0700 (PDT) Message-ID: In-Reply-To: References: Date: Tue, 01 Aug 2023 15:23:27 +0000 Subject: [PATCH v3 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. Signed-off-by: Phillip Wood --- sequencer.c | 13 +++++++++---- t/t3418-rebase-continue.sh | 18 ++++++++++++++++++ 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/sequencer.c b/sequencer.c index de66bda9d5b..70b0a7023b0 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 @@ -3507,7 +3512,6 @@ static int make_patch(struct repository *r, 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 +3519,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 +4663,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 Tue Aug 1 15:23:28 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Phillip Wood X-Patchwork-Id: 13336944 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 D4FC8C0015E for ; Tue, 1 Aug 2023 15:23:54 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234411AbjHAPXx (ORCPT ); Tue, 1 Aug 2023 11:23:53 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:46984 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232540AbjHAPXr (ORCPT ); Tue, 1 Aug 2023 11:23:47 -0400 Received: from mail-wm1-x32c.google.com (mail-wm1-x32c.google.com [IPv6:2a00:1450:4864:20::32c]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 613751FEE for ; Tue, 1 Aug 2023 08:23:39 -0700 (PDT) Received: by mail-wm1-x32c.google.com with SMTP id 5b1f17b1804b1-3fba8e2aa52so63126145e9.1 for ; Tue, 01 Aug 2023 08:23:39 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1690903417; x=1691508217; 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=Kpkv+W1+9kkkw9zCWjfM6lpDPwaFN/ucdioyGSRow5Y=; b=ibQlQIGU0t8Oq1hWgftP/3foZeCN7GoDKpZw/w4JPzWx0yHWn2hWMbl255k3uqCCqQ rurWaN8ivKWjLTVq3IRcN8KRwVQQzgZ5aJF7Zg+y0fa0ZXW4gf5zTUUrQyAFS0VGeedV K/VrkdaxLlm8nOAaEMY7JTAFHmM2zoYWBS6K7Z4hbqE0Phjxi8TIy295uyKHUHJm4gcx +UjpP+iuJpOA/++RZLhqss5ZAWqIbScT/2a6Y1o45x15I3CzEtq0KW1DSOGGUdPkzXQz qmvkAhk+BIXmGQL2+QmVQKYTzhGBgLxFH90L2Dhcb9EyDjujknbE2/FsB/y7DAXC8h5T BSdQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1690903417; x=1691508217; 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=Kpkv+W1+9kkkw9zCWjfM6lpDPwaFN/ucdioyGSRow5Y=; b=CI0o4dTAhvsXYvZ1jwiwOmWHo43KCRFs6sCIOsTogNER8cI9ZREQmadx1ae4VLY5RS 6wREiN5EwWQtYiXQxDDeAXX4KcbnzQgRkNeKHrU+XXFKk28obazfm3oY78xhN4ReYAiE GcZ/NUIjmdwPmFRcw7y4BBsgH4rw+Z6Fo/aXKCUmwvXORFSlDB+ujVoe6xMaqeklX2QO lvE6CUJnJpHLImPh9HGk00kaCoIHKJwahMrCSzCLxq54foAOm8cTfQSRLilhjq+u+A30 6oSIrWAcTN9/kxNY5ZfU0wrgeFBCSS3OVwfkdVzvDPpoGZYTyv4tXTSA8LbUUNEhCzkj mr7A== X-Gm-Message-State: ABy/qLZ4oBphdJ3QzMm86IcVjeqJ+Mj4qmQ1mekIy4sKUquhcdMmLq+A CjnEPBK0XbMOJRYTFXQhqQ9F3ZE1eUk= X-Google-Smtp-Source: APBJJlGTwgV4132ytpPvFKmL1YpG3BwvjxJBKSERzt5BeXNHPpO5skLxfRF3JY+TLpN5EYegna4vAA== X-Received: by 2002:a05:600c:ad3:b0:3fe:173e:4a53 with SMTP id c19-20020a05600c0ad300b003fe173e4a53mr2859115wmr.0.1690903417155; Tue, 01 Aug 2023 08:23:37 -0700 (PDT) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id s15-20020a05600c044f00b003fbb1a9586esm17240394wmb.15.2023.08.01.08.23.36 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 01 Aug 2023 08:23:36 -0700 (PDT) Message-ID: <8f6c0e4056742951ce84acbdb07b0518f7607b2a.1690903412.git.gitgitgadget@gmail.com> In-Reply-To: References: Date: Tue, 01 Aug 2023 15:23:28 +0000 Subject: [PATCH v3 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. Signed-off-by: Phillip Wood --- sequencer.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/sequencer.c b/sequencer.c index 70b0a7023b0..dbddd19b2c2 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]; @@ -3529,18 +3528,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 Tue Aug 1 15:23:29 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Phillip Wood X-Patchwork-Id: 13336945 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 BA38FC04FE0 for ; Tue, 1 Aug 2023 15:23:55 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234516AbjHAPXy (ORCPT ); Tue, 1 Aug 2023 11:23:54 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:46986 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233201AbjHAPXr (ORCPT ); Tue, 1 Aug 2023 11:23:47 -0400 Received: from mail-wm1-x32c.google.com (mail-wm1-x32c.google.com [IPv6:2a00:1450:4864:20::32c]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id D32942139 for ; Tue, 1 Aug 2023 08:23:39 -0700 (PDT) Received: by mail-wm1-x32c.google.com with SMTP id 5b1f17b1804b1-3fe110de46dso36925535e9.1 for ; Tue, 01 Aug 2023 08:23:39 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1690903418; x=1691508218; 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=xOQ278Iqnpfoka+KcTOYBzz1IL5UZGsGFqgUJ1y9rYk=; b=dCNVru/v3kTXrJdHRShK3wAlYtGgWL4srzImkozqHRP5GbaXeDhUBhLy4m7uV2ZeZt hbc44FanhQ4LFRx0u1IgvhB5DnGMN2fpbpbX0Mw1XVqg1CDBZcbyg0F2goi5u8j+gvdq XzTqVg1IiKNyfL3loIP6kQp8jcBWlLFqipxa6S2nsiOWMnW2VnIN97tgzsHvV1x5A3wH kntr4pGxML7M+1FTnJ+u7WUvXvrs3LP/VG7Z6t5hyfWCLxNZCGh6ZCh001OJE1TSG2PW Sqmvg0KCmTpsbrnuNQRcd16AEHo48S/hxhOZqE2iFvGfOevdD3pCu7O+2hSuzgyHay6o jJ6g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1690903418; x=1691508218; 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=xOQ278Iqnpfoka+KcTOYBzz1IL5UZGsGFqgUJ1y9rYk=; b=KGi5iLkxNr4u2520YJu/3wyEOnaz1QdGGFIxutVPAtDqgI16UwBpKeD0GXMVeo+Qo+ PjEqMBCQpGFOkUSeaXtMqDmTN6K49WsoEIVBVULK8TUIRTRsxaAyjgHyKXuN9gkJDbd+ aEjgh2eLfMxtz0l4dXCkACCKG60Sm8KVmG+Fgte0W25pi476uEf4vwQjYdNjNTM82JHb UCROdNT5F6kPxnBo7LdGU7cYkDKLH+uiCZngvH/nfm8wHBvcqaoEjqp4TWOrxPGyTcIx rRMNSdGYDGIobnelJPaHJ2pqjB0gIZHNecY+pdUsT9VPs2tOHoi12Ti5T/moROxvJq0y aBPw== X-Gm-Message-State: ABy/qLZh+3cxG1+pAGkX9MmVZ5kz8/asIdt+dlCYZErDSKF3SqjUyYMU XZTqUnj244NLBnvhhOzTMw4QQ8W3aJg= X-Google-Smtp-Source: APBJJlEQRs2X8XnNxkJEVmSAU9E/JmtuV2tHW/Jwf6iGVG4h7hF0Fbuq6S6pQQX1KJSxHw/yPlGmVw== X-Received: by 2002:a1c:cc08:0:b0:3fb:c9f4:150e with SMTP id h8-20020a1ccc08000000b003fbc9f4150emr2604370wmb.14.1690903417784; Tue, 01 Aug 2023 08:23:37 -0700 (PDT) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id f25-20020a1c6a19000000b003fe24da493dsm3979005wmc.41.2023.08.01.08.23.37 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 01 Aug 2023 08:23:37 -0700 (PDT) Message-ID: In-Reply-To: References: Date: Tue, 01 Aug 2023 15:23:29 +0000 Subject: [PATCH v3 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 dbddd19b2c2..62277e7bcc1 100644 --- a/sequencer.c +++ b/sequencer.c @@ -4642,6 +4642,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) @@ -4697,66 +4763,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; @@ -4817,9 +4826,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 Tue Aug 1 15:23:30 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Phillip Wood X-Patchwork-Id: 13336946 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 DEA3EC0015E for ; Tue, 1 Aug 2023 15:23:57 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233213AbjHAPX4 (ORCPT ); Tue, 1 Aug 2023 11:23:56 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:46990 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233401AbjHAPXs (ORCPT ); Tue, 1 Aug 2023 11:23:48 -0400 Received: from mail-wr1-x42d.google.com (mail-wr1-x42d.google.com [IPv6:2a00:1450:4864:20::42d]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 4E2AA213C for ; Tue, 1 Aug 2023 08:23:40 -0700 (PDT) Received: by mail-wr1-x42d.google.com with SMTP id ffacd0b85a97d-3178dd81ac4so4292235f8f.3 for ; Tue, 01 Aug 2023 08:23:40 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1690903418; x=1691508218; 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=hhxNv+c61rXTFp0ucaqogvcouSv8PQNOyHLWceyno3Q=; b=PCoANMHulI1AirkSDkcyzjOUfvPX8zJyOgfx16BtshQhB/Pv41Mf0jcpKCP9MX3aHz 9EU+3ETVEffnRDnU3wjzinkSK10f+J1XZIjCq9OlhgZxWC19/5DBqOIxvcsuCUsiXBFL 2BUwk2FZRX7IrMGvJgL2QJDdq0fEI1iQMebZfWNMQrcY/rkMYiDLbokzk0/xCG3NPASy grvClSwoHNTbtLVZCInwWFpwH8RTQYCUmMjROPOvw/LOxaad0lrt92/5d/ZQ6RBuf8xq DbnFU+0qgkh8gnTkYLWHI+ckhj0z2yc7wngX8+XSTLL1J9fFmergzH7lu6D/1zeI9nWM dZcQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1690903418; x=1691508218; 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=hhxNv+c61rXTFp0ucaqogvcouSv8PQNOyHLWceyno3Q=; b=bv/9SVmjmtb1pVAdZKXnYUnxCJhpKEY3U6jOwZCoj9hIDfVXZaNYWsU3RyOnXkoOsm jcilXoRvIqgnSjisTO1TBW45eRajpU2IOjm34LwKhtbyP/dSSh3kWzReY+I7UcfVVBJ+ h5GQXZIxArSnlKmGlW2JjgdAqMauUOfkFfuWLCCBkIpCX+QdP07HSGHe8oWOJyt7TRef zSgL6RBW3AdosLE5St3nLMDpx8gQ6ZTCA51LXLH/Woqfui10ydEJ4UiDIf1+A0LUdO1X fBkNfjhctlQrN2RMIRJEDzURtWX8O+83xXHVnb1e2hgxpZtn45MOzUmbdWW/Od+eAhxh V04Q== X-Gm-Message-State: ABy/qLZVX7dy/7la1KffbiGjPO8p93NGBrX+GIH42gogSKFvswHMM0nv fHuJFtqNtyytS6tkvyZMnMdps5sR6Qs= X-Google-Smtp-Source: APBJJlFGW03vJmJU6RdmXJjY+NBKJ8WRw0tW76qwFyY37aTNxBc/ArBVN3+i/7lWdR+mUNhfXqHkLg== X-Received: by 2002:a5d:6905:0:b0:315:9fb7:bd9 with SMTP id t5-20020a5d6905000000b003159fb70bd9mr2600340wru.69.1690903418496; Tue, 01 Aug 2023 08:23:38 -0700 (PDT) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id l13-20020a5d560d000000b00313f9a0c521sm16264200wrv.107.2023.08.01.08.23.38 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 01 Aug 2023 08:23:38 -0700 (PDT) Message-ID: In-Reply-To: References: Date: Tue, 01 Aug 2023 15:23:30 +0000 Subject: [PATCH v3 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. To simplify writing REBASE_HEAD in this case pick_one_commit() is modified to avoid duplicating the code that adds the failed command back into the todo list. 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 62277e7bcc1..e25abfd2fb4 100644 --- a/sequencer.c +++ b/sequencer.c @@ -4155,6 +4155,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; } /* @@ -4645,7 +4646,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; @@ -4658,12 +4659,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; @@ -4763,7 +4760,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) { @@ -4817,10 +4815,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 Tue Aug 1 15:23:31 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Phillip Wood X-Patchwork-Id: 13336947 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 B8085C001E0 for ; Tue, 1 Aug 2023 15:23:59 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233990AbjHAPX7 (ORCPT ); Tue, 1 Aug 2023 11:23:59 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:47074 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S234039AbjHAPXw (ORCPT ); Tue, 1 Aug 2023 11:23:52 -0400 Received: from mail-wm1-x32d.google.com (mail-wm1-x32d.google.com [IPv6:2a00:1450:4864:20::32d]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 4EDF62113 for ; Tue, 1 Aug 2023 08:23:41 -0700 (PDT) Received: by mail-wm1-x32d.google.com with SMTP id 5b1f17b1804b1-3fb4146e8fcso39917965e9.0 for ; Tue, 01 Aug 2023 08:23:41 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1690903419; x=1691508219; 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=Ji5cKYBBBlvNVH0DymsD8L4ogLnGMLVhtR09gQ/Tku4=; b=m//pKeGSljnojHAnmLyA2rk9Un/E5zCOTENm2R7zrMF8SQ4MrJP3n07mG5/eLcYC5j t3J4KOVMllSp7eXIGnNCmK3FCrxWw7P5d4Lhl5B/CnJVrJZe5ppqRhsNbVfExvjOH5xq HvwfG4j+6xUbbSoV3p9+iElbBVn/31zhxAQPQ79qXmr6q4pYKTjJCAjkxssoPpekCM8/ pMdYS4AvUldM2SzN3vFi9Zx/1I3Dzq8PHB57X9zNbsYh4O1VogKd0JkN7dmAhF5ddvst hu70akpnlYNN4qEj3SIT7NHxhgglJ5nPL6Zwm4zcJzRh49E1LI3S0dt9W7Q3J1W3+g0l CUTQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1690903419; x=1691508219; 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=Ji5cKYBBBlvNVH0DymsD8L4ogLnGMLVhtR09gQ/Tku4=; b=knWh6ijwcFhp011mZ8KR/DQ4o0SeREtVfBKKbak77RvWHZ1S8ZUUIQ5eqftWOLqYJx iSMlCaeweQAjx+mU1iVTkI+WrNBBHtXSk9oGmLd8M4vG9sYaSYNsZ4ytqNjiTQrWdCTn EUoyw+7lei6N5XBFZeseeXZmNBmckU+r7pjb9d879PkLXcfPDMDMRJW9vuAX27q+kP1j G3xuGS8JD9wDTZGaHgftKpTep9++waapqcwJUxuhSJ9PW5Q4jx+YAe/l5uDfnxSirXfe me+AvdKtPhKnMdNKw3ud/C4f9B3FXlF2psh87aFPV5t89KKw/wjVM+RrurfL7CR2/ndU go/w== X-Gm-Message-State: ABy/qLYY5qCm9gqBQMMItrKN5zhS8rL2AP3iLTJ3Kps5jpcvdv/ttYrE gwSe2yKKgJX9dA8aw+t3W5t9KZFABvE= X-Google-Smtp-Source: APBJJlGw/K5wSVlgxmx0uemAlp+xaxR6/9JcOeR7oGdFJULRyIcO/rdpJOVYxkgy0IRb6UjdMW6Hww== X-Received: by 2002:a05:600c:4f47:b0:3fa:8c8b:716 with SMTP id m7-20020a05600c4f4700b003fa8c8b0716mr2668918wmq.1.1690903419284; Tue, 01 Aug 2023 08:23:39 -0700 (PDT) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id t20-20020a7bc3d4000000b003fe2a40d287sm1864088wmj.1.2023.08.01.08.23.38 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 01 Aug 2023 08:23:38 -0700 (PDT) Message-ID: <2ed7cbe5fff2d104a1246783909c6c4b75c559f2.1690903412.git.gitgitgadget@gmail.com> In-Reply-To: References: Date: Tue, 01 Aug 2023 15:23:31 +0000 Subject: [PATCH v3 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 e25abfd2fb4..a90b015e79c 100644 --- a/sequencer.c +++ b/sequencer.c @@ -4977,6 +4977,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 Tue Aug 1 15:23:32 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Phillip Wood X-Patchwork-Id: 13336948 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 00719C0015E for ; Tue, 1 Aug 2023 15:24:00 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234109AbjHAPX7 (ORCPT ); Tue, 1 Aug 2023 11:23:59 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:47080 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S234081AbjHAPXw (ORCPT ); Tue, 1 Aug 2023 11:23:52 -0400 Received: from mail-wm1-x32f.google.com (mail-wm1-x32f.google.com [IPv6:2a00:1450:4864:20::32f]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 04046211E for ; Tue, 1 Aug 2023 08:23:42 -0700 (PDT) Received: by mail-wm1-x32f.google.com with SMTP id 5b1f17b1804b1-3fbc5d5742eso63017725e9.3 for ; Tue, 01 Aug 2023 08:23:41 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1690903420; x=1691508220; 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=utc86jQ450LQo2nvFbVNNJSteny3cBUQPuZfLdcUVXA=; b=APLiUF95XhTa+yWqA2IVRA6lQEzEAzhelFfIZQUqXBtZSnPAffewzjokUdE6V1S+HR 2YmHNFoIlNaDSpPPcRIdt0KtkRdb29h10TYnl4Lc7UiUZ3kqsJsncwf/MFD9NRQmL5IW graHaK3krbtm/ukOpTdjEy7WOR/RxTEWwbfMYncU8b3liP726lhdVIrH1AHKEibt4NgP whh6M1cBS4zEvXiy/1tzFZW2dhpRNOf93sU+Wwld/o9WRouJMYV52BgHtrbn5ykSYEQW cW+m5SPnRrFZx27sziNRBVW+OcZO3yzTF43LnkPH5zOdk6HbdbePXIAYc+XsmHT4qQp/ yNHA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1690903420; x=1691508220; 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=utc86jQ450LQo2nvFbVNNJSteny3cBUQPuZfLdcUVXA=; b=XrlwSsDA+4KU74jD5FYq2/i5++vGeXIMCx/+f+d93uqNMy152PmfWWQ7blY2+AwODs 2E2r/UAhbHK2NzR31H58kNHQqiBWHS3ji+b7/ExrfpgiuZo95srjSkETrlBqfec3fcEr qHNI6AWdL2tZ841ScSgfphcExQx+n12jEgyZSH7VxnyL00Wv2it+4bfl69Pi/bhaXM7o mRs+dKvJXAmljRYUUOS020vgM1kwEssphAEqNSE7TeoIk1vkdgXQqRIopzKL+1L66rdr UMz3G+iE++u0ueBmVekR/JJ+/hPhjsxPdBKES4/HPkiwBVasbDvh+lKePVNWbw4TrVpx 2icg== X-Gm-Message-State: ABy/qLZVbc7u4c8n9b8NBzpEJgke9qmufG+9VK8X/fVCOjved4580SrF t/rSklkTlkemmrhQyCgAjyO07DlmL9s= X-Google-Smtp-Source: APBJJlFCLdAVNHPV/blspihhPW0dG0DyoNwrkbUeDIpToRXmMaJI/P0TOizbdd26DbDw5imHoRA6Jw== X-Received: by 2002:a05:600c:11ce:b0:3fe:19cf:93ca with SMTP id b14-20020a05600c11ce00b003fe19cf93camr2806276wmi.8.1690903420193; Tue, 01 Aug 2023 08:23:40 -0700 (PDT) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id l27-20020a05600c1d1b00b003fe1b3e0852sm4364318wms.0.2023.08.01.08.23.39 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 01 Aug 2023 08:23:39 -0700 (PDT) Message-ID: In-Reply-To: References: Date: Tue, 01 Aug 2023 15:23:32 +0000 Subject: [PATCH v3 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 a90b015e79c..4976d159745 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; @@ -4730,7 +4731,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) { @@ -4811,8 +4812,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' '