From patchwork Wed Mar 15 15:14:56 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Phillip Wood X-Patchwork-Id: 13176079 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 47825C7618D for ; Wed, 15 Mar 2023 15:15:39 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232550AbjCOPPi (ORCPT ); Wed, 15 Mar 2023 11:15:38 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:48260 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231142AbjCOPPg (ORCPT ); Wed, 15 Mar 2023 11:15:36 -0400 Received: from mail-wr1-x42c.google.com (mail-wr1-x42c.google.com [IPv6:2a00:1450:4864:20::42c]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id D135DEFAA for ; Wed, 15 Mar 2023 08:15:34 -0700 (PDT) Received: by mail-wr1-x42c.google.com with SMTP id l1so17615766wry.12 for ; Wed, 15 Mar 2023 08:15:34 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; t=1678893333; h=content-transfer-encoding:mime-version:reply-to:references :in-reply-to:message-id:date:subject:cc:to:from:from:to:cc:subject :date:message-id:reply-to; bh=Gk3yh4aeo7T8CLcO1xQ0T4v1emGBYXjM1YvGAdHDdSM=; b=fx/zCyAkZhJuknWN0ocHnQyCyV0Lq8yWq/Z7/gXZZHEcu9HhBm9Pk/UOXN7sC4VA6j SGz8gA/F0jiVu6ivRKvCht/4IRDNN62pob46IdZ+KXLJM97WdLPdFaXbgs0MhzkJYnzt gzvAXLERl6iIqI0QlI9tJYr5bxWVTOBtdmjXObYKlF6l5DKMi3PA0urGnH7htnxW6d0d VqD206Fa5vvDBC6KHC3RSDERg15fgjEG+f17SdFC7qMp64qLKJT3zvHU/9Zjn/fl7Xgm 72E/p1eot4+jcksVkbyLDtr9f+YLx8nTbjqPMqEEW2eXIWky4DqV17pcqzwJpII/dSz6 fMTg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1678893333; h=content-transfer-encoding:mime-version:reply-to:references :in-reply-to:message-id:date:subject:cc:to:from:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=Gk3yh4aeo7T8CLcO1xQ0T4v1emGBYXjM1YvGAdHDdSM=; b=DphqWSGiHRgXOz46O4Zvq9LqutXwXk/oGXuTdA2GaElCjOKIoro0sojdzfQwY3ZMVV Dx2WxHP9M5u9kuQIZpg/iUoTTaJf5e8hxtR0RVok7WpiVre+ESKwR1PpafFNqymiq22E 2gWWek3N5KbDVRr8b8J5mDAC+qZGr8JqxbGGbBkSf3UJA//iD6kmboLiywYCJJnpOIDU 9zb9bsbLHp+rcTlQRbuvrcG6XvdAWYkhHj6d21Mo9z9HZAEYXV8UaUYFhCLEAr0jy32w oAAkve8ucQ0D7fWhLfZ6gFLbhCQGDlKIhvdzm6hOkHJEMfW+XHkNJHV3ICruRtXFiSXm TVGA== X-Gm-Message-State: AO0yUKUjvWEcv5UETd9CM7HxvYoFuB3sDk/0laq0Dg9NYIlipG+2To6s r0+KukDJ5aKOLeZRtK8qqGrlP0dvcz4= X-Google-Smtp-Source: AK7set+RZskI7oIeZR4elR2cLYPB3+gojS0aAW5veG9vB2MSsS30STCDUBmpLdseQRM+aQCChB9vYQ== X-Received: by 2002:adf:f48a:0:b0:2c5:5d21:7d4c with SMTP id l10-20020adff48a000000b002c55d217d4cmr2104185wro.43.1678893333189; Wed, 15 Mar 2023 08:15:33 -0700 (PDT) Received: from localhost.localdomain ([90.248.23.119]) by smtp.gmail.com with ESMTPSA id b10-20020a5d550a000000b002c706c754fesm4783191wrv.32.2023.03.15.08.15.32 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 15 Mar 2023 08:15:32 -0700 (PDT) From: Phillip Wood To: git@vger.kernel.org Cc: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= , Phillip Wood , Phillip Wood Subject: [PATCH 1/4] rebase: stop reading and writing unnecessary strategy state Date: Wed, 15 Mar 2023 15:14:56 +0000 Message-Id: <029d0bf2b6249ddc3e79fe1bf3df3e05d22348ad.1678893298.git.phillip.wood@dunelm.org.uk> X-Mailer: git-send-email 2.39.2 In-Reply-To: References: Reply-To: Phillip Wood MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Phillip Wood The state files for "--strategy" and "--strategy-option" are written and read twice, once by builtin/rebase.c and then by sequencer.c. This is an artifact of the scripted rebase and the need to support "rebase --preserve-merges". Now that "--preserve-merges" no-longer exists we only need to read and write these files in sequencer.c. This enables us to remove a call to free() in read_strategy_opts() that was added by f1f4ebf432 (sequencer.c: fix "opts->strategy" leak in read_strategy_opts(), 2022-11-08) as this commit fixes the root cause of that leak. There is further scope for removing duplication in the reading and writing of state files between builtin/rebase.c and sequencer.c but that is left for a follow up series. Signed-off-by: Phillip Wood --- builtin/rebase.c | 24 ------------------------ sequencer.c | 1 - 2 files changed, 25 deletions(-) diff --git a/builtin/rebase.c b/builtin/rebase.c index 6635f10d52..516ad1b12a 100644 --- a/builtin/rebase.c +++ b/builtin/rebase.c @@ -482,24 +482,6 @@ static int read_basic_state(struct rebase_options *opts) opts->gpg_sign_opt = xstrdup(buf.buf); } - if (file_exists(state_dir_path("strategy", opts))) { - strbuf_reset(&buf); - if (!read_oneliner(&buf, state_dir_path("strategy", opts), - READ_ONELINER_WARN_MISSING)) - return -1; - free(opts->strategy); - opts->strategy = xstrdup(buf.buf); - } - - if (file_exists(state_dir_path("strategy_opts", opts))) { - strbuf_reset(&buf); - if (!read_oneliner(&buf, state_dir_path("strategy_opts", opts), - READ_ONELINER_WARN_MISSING)) - return -1; - free(opts->strategy_opts); - opts->strategy_opts = xstrdup(buf.buf); - } - strbuf_release(&buf); return 0; @@ -517,12 +499,6 @@ static int rebase_write_basic_state(struct rebase_options *opts) write_file(state_dir_path("quiet", opts), "%s", ""); if (opts->flags & REBASE_VERBOSE) write_file(state_dir_path("verbose", opts), "%s", ""); - if (opts->strategy) - write_file(state_dir_path("strategy", opts), "%s", - opts->strategy); - if (opts->strategy_opts) - write_file(state_dir_path("strategy_opts", opts), "%s", - opts->strategy_opts); if (opts->allow_rerere_autoupdate > 0) write_file(state_dir_path("allow_rerere_autoupdate", opts), "-%s-rerere-autoupdate", diff --git a/sequencer.c b/sequencer.c index 886fbc3616..55b3ba3a51 100644 --- a/sequencer.c +++ b/sequencer.c @@ -2946,7 +2946,6 @@ static void read_strategy_opts(struct replay_opts *opts, struct strbuf *buf) strbuf_reset(buf); if (!read_oneliner(buf, rebase_path_strategy(), 0)) return; - free(opts->strategy); opts->strategy = strbuf_detach(buf, NULL); if (!read_oneliner(buf, rebase_path_strategy_opts(), 0)) return; From patchwork Wed Mar 15 15:14:57 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Phillip Wood X-Patchwork-Id: 13176080 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 D1CA1C7618B for ; Wed, 15 Mar 2023 15:15:40 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232845AbjCOPPj (ORCPT ); Wed, 15 Mar 2023 11:15:39 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:48258 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232115AbjCOPPh (ORCPT ); Wed, 15 Mar 2023 11:15:37 -0400 Received: from mail-wm1-x335.google.com (mail-wm1-x335.google.com [IPv6:2a00:1450:4864:20::335]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id A00411A954 for ; Wed, 15 Mar 2023 08:15:35 -0700 (PDT) Received: by mail-wm1-x335.google.com with SMTP id p16so12800874wmq.5 for ; Wed, 15 Mar 2023 08:15:35 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; t=1678893334; h=content-transfer-encoding:mime-version:reply-to:references :in-reply-to:message-id:date:subject:cc:to:from:from:to:cc:subject :date:message-id:reply-to; bh=6W/s9RGNic/3VFmjO0Wvh8nW993S8mFHJDfRzcB4y1U=; b=TSr1Y/OXysIduw/NtxFnhe+dN1vr+Q3LVsBB+YCCFQp2XO+RBqnbFbOL8aQk4qur94 hfvk/+f6i+Cw0HlUSIv+NelaDgf38UkOsbbRY0msKK0YVvpR+FOEy0rWl6UJu3sJRkv5 h6NZXmv2VaeoVt0tPdoaCYgqEmjYL5+qEL3wgxUJ2HSMsTDwfRuWzXaIY5rJ0lHJXTyn ggT3qf7OG60SaNOUFJCXUwtZiqXGQ09wqc6sqcIg5488rVeSdDC1f0xLMggHp9o1dtFH TLHoSuhGKZXimAUe13TcFpO4hkvJXDi6vxbyLwlo6GPeAMr9m4FAGuZVwSjfN0/VB6RU npsg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1678893334; h=content-transfer-encoding:mime-version:reply-to:references :in-reply-to:message-id:date:subject:cc:to:from:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=6W/s9RGNic/3VFmjO0Wvh8nW993S8mFHJDfRzcB4y1U=; b=nvz9hsWKFVSM3QbH8wAzYB0gkkJGCC6s0vpAdswT9lrgq5feAeSymp5K1XHj0x4Qga sF371Hwms2o/G4XdqRh4SZieDC5/jFJ0pw1LuaA7YmCuHKlHLiyq+2qpnWemqbUa+0JK +KkQWyyva2YfwJWcPSj+cbP8TCjx009Lg5glKFXkYzG5noU/iETkZx8eYGx4h58F8Qu4 Tn2h4K2xfFgHkVrZZzVbEbi17qoGvfR3kqOOtqp42TSID/rZxzndclNhqp+/I47HvIrk Ow/kTkgCmJslboEb9FtRXVMl56Nu+/Ji7iW0mx+l/FzjSaw+F+vCALoXTEhtC2IVsxWK AbRA== X-Gm-Message-State: AO0yUKWDazHnI0zEpD/WngF16HnY3T5KXl1xRF+7m3g3HDXHtVSQ8HnI GQhC/UbGn/Y0WHSF+64L65CD45Ze7QQ= X-Google-Smtp-Source: AK7set9SnVfGz3Nq/SIwZS94rMOuHNIhMZrG8epbmFmS36ZxS0xbDbLDF9XyWn1bAMmhm/Ee3S8tww== X-Received: by 2002:a05:600c:510a:b0:3ed:307f:1663 with SMTP id o10-20020a05600c510a00b003ed307f1663mr3891173wms.15.1678893334060; Wed, 15 Mar 2023 08:15:34 -0700 (PDT) Received: from localhost.localdomain ([90.248.23.119]) by smtp.gmail.com with ESMTPSA id b10-20020a5d550a000000b002c706c754fesm4783191wrv.32.2023.03.15.08.15.33 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 15 Mar 2023 08:15:33 -0700 (PDT) From: Phillip Wood To: git@vger.kernel.org Cc: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= , Phillip Wood , Phillip Wood Subject: [PATCH 2/4] rebase -m: cleanup --strategy-option handling Date: Wed, 15 Mar 2023 15:14:57 +0000 Message-Id: X-Mailer: git-send-email 2.39.2 In-Reply-To: References: Reply-To: Phillip Wood MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Phillip Wood When handling "--strategy-option" rebase collects the commands into a struct string_list, then concatenates them into a string, prepending "--" to each one before splitting the string and removing the "--" prefix. This is an artifact of the scripted rebase and the need to support "rebase --preserve-merges". Now that "--preserve-merges" no-longer exists we can cleanup the way the argument is handled. The tests for a bad strategy option are adjusted now that parse_strategy_opts() is no-longer called when starting a rebase. The fact that it only errors out when running "git rebase --continue" is a mixed blessing but the next commit will fix the root cause of the parsing problem so lets not worry about that here. Signed-off-by: Phillip Wood --- builtin/rebase.c | 36 +++++++++++++++------------------- sequencer.c | 2 +- sequencer.h | 1 - t/t3436-rebase-more-options.sh | 10 ++++++++-- 4 files changed, 25 insertions(+), 24 deletions(-) diff --git a/builtin/rebase.c b/builtin/rebase.c index 516ad1b12a..5194d64c24 100644 --- a/builtin/rebase.c +++ b/builtin/rebase.c @@ -116,7 +116,8 @@ struct rebase_options { struct string_list exec; int allow_empty_message; int rebase_merges, rebase_cousins; - char *strategy, *strategy_opts; + char *strategy; + struct string_list strategy_opts; struct strbuf git_format_patch_opt; int reschedule_failed_exec; int reapply_cherry_picks; @@ -142,6 +143,7 @@ struct rebase_options { .config_autosquash = -1, \ .update_refs = -1, \ .config_update_refs = -1, \ + .strategy_opts = STRING_LIST_INIT_NODUP \ } static struct replay_opts get_replay_opts(const struct rebase_options *opts) @@ -175,8 +177,14 @@ static struct replay_opts get_replay_opts(const struct rebase_options *opts) replay.default_strategy = NULL; } - if (opts->strategy_opts) - parse_strategy_opts(&replay, opts->strategy_opts); + if (opts->strategy_opts.nr) { + ALLOC_ARRAY(replay.xopts, opts->strategy_opts.nr); + replay.xopts_nr = opts->strategy_opts.nr; + replay.xopts_alloc = opts->strategy_opts.nr; + for (size_t i = 0; i < opts->strategy_opts.nr; i++) + replay.xopts[i] = + xstrdup(opts->strategy_opts.items[i].string); + } if (opts->squash_onto) { oidcpy(&replay.squash_onto, opts->squash_onto); @@ -1012,7 +1020,6 @@ int cmd_rebase(int argc, const char **argv, const char *prefix) int ignore_whitespace = 0; const char *gpg_sign = NULL; const char *rebase_merges = NULL; - struct string_list strategy_options = STRING_LIST_INIT_NODUP; struct object_id squash_onto; char *squash_onto_name = NULL; char *keep_base_onto_name = NULL; @@ -1121,7 +1128,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix) N_("use 'merge-base --fork-point' to refine upstream")), OPT_STRING('s', "strategy", &options.strategy, N_("strategy"), N_("use the given merge strategy")), - OPT_STRING_LIST('X', "strategy-option", &strategy_options, + OPT_STRING_LIST('X', "strategy-option", &options.strategy_opts, N_("option"), N_("pass the argument through to the merge " "strategy")), @@ -1435,23 +1442,13 @@ int cmd_rebase(int argc, const char **argv, const char *prefix) } else { /* REBASE_MERGE */ if (ignore_whitespace) { - string_list_append(&strategy_options, + string_list_append(&options.strategy_opts, "ignore-space-change"); } } - if (strategy_options.nr) { - int i; - - if (!options.strategy) - options.strategy = "ort"; - - strbuf_reset(&buf); - for (i = 0; i < strategy_options.nr; i++) - strbuf_addf(&buf, " --%s", - strategy_options.items[i].string); - options.strategy_opts = xstrdup(buf.buf); - } + if (options.strategy_opts.nr && !options.strategy) + options.strategy = "ort"; if (options.strategy) { options.strategy = xstrdup(options.strategy); @@ -1826,10 +1823,9 @@ int cmd_rebase(int argc, const char **argv, const char *prefix) free(options.gpg_sign_opt); string_list_clear(&options.exec, 0); free(options.strategy); - free(options.strategy_opts); + string_list_clear(&options.strategy_opts, 0); strbuf_release(&options.git_format_patch_opt); free(squash_onto_name); free(keep_base_onto_name); - string_list_clear(&strategy_options, 0); return !!ret; } diff --git a/sequencer.c b/sequencer.c index 55b3ba3a51..83ea1016ae 100644 --- a/sequencer.c +++ b/sequencer.c @@ -2918,7 +2918,7 @@ static int populate_opts_cb(const char *key, const char *value, void *data) return 0; } -void parse_strategy_opts(struct replay_opts *opts, char *raw_opts) +static void parse_strategy_opts(struct replay_opts *opts, char *raw_opts) { int i; int count; diff --git a/sequencer.h b/sequencer.h index 3bcdfa1b58..5de5cfa664 100644 --- a/sequencer.h +++ b/sequencer.h @@ -247,7 +247,6 @@ int read_oneliner(struct strbuf *buf, const char *path, unsigned flags); int read_author_script(const char *path, char **name, char **email, char **date, int allow_missing); -void parse_strategy_opts(struct replay_opts *opts, char *raw_opts); int write_basic_state(struct replay_opts *opts, const char *head_name, struct commit *onto, const struct object_id *orig_head); void sequencer_post_commit_cleanup(struct repository *r, int verbose); diff --git a/t/t3436-rebase-more-options.sh b/t/t3436-rebase-more-options.sh index c3184c9ade..3adf42f47d 100755 --- a/t/t3436-rebase-more-options.sh +++ b/t/t3436-rebase-more-options.sh @@ -41,19 +41,25 @@ test_expect_success 'setup' ' ' test_expect_success 'bad -X arguments: unclosed quote' ' + test_when_finished "test_might_fail git rebase --abort" && cat >expect <<-\EOF && fatal: could not split '\''--bad'\'': unclosed quote EOF - test_expect_code 128 git rebase -X"bad argument\"" side main >out 2>actual && + GIT_SEQUENCE_EDITOR="echo break >" \ + git rebase -i -X"bad argument\"" side main && + test_expect_code 128 git rebase --continue >out 2>actual && test_must_be_empty out && test_cmp expect actual ' test_expect_success 'bad -X arguments: bad escape' ' + test_when_finished "test_might_fail git rebase --abort" && cat >expect <<-\EOF && fatal: could not split '\''--bad'\'': cmdline ends with \ EOF - test_expect_code 128 git rebase -X"bad escape \\" side main >out 2>actual && + GIT_SEQUENCE_EDITOR="echo break >" \ + git rebase -i -X"bad escape \\" side main && + test_expect_code 128 git rebase --continue >out 2>actual && test_must_be_empty out && test_cmp expect actual ' From patchwork Wed Mar 15 15:14:58 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Phillip Wood X-Patchwork-Id: 13176081 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 7AE3EC61DA4 for ; Wed, 15 Mar 2023 15:15:43 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232860AbjCOPPl (ORCPT ); Wed, 15 Mar 2023 11:15:41 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:48258 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232821AbjCOPPj (ORCPT ); Wed, 15 Mar 2023 11:15:39 -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 7B24718AA1 for ; Wed, 15 Mar 2023 08:15:36 -0700 (PDT) Received: by mail-wr1-x42d.google.com with SMTP id v16so17695760wrn.0 for ; Wed, 15 Mar 2023 08:15:36 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; t=1678893335; h=content-transfer-encoding:mime-version:reply-to:references :in-reply-to:message-id:date:subject:cc:to:from:from:to:cc:subject :date:message-id:reply-to; bh=1fZE859NduZbyWuIX3xEGeUxQ0RfFRHfVEG4XDrPQwA=; b=QKA14Z37y++mFTwCwSB0O0VyWZ0dhRvFiSA4lwiAuL7qLg9ivukvxxakr+PHTVt6mi 3v9HCOMKDUIMlaGtDRSolPaOEyQhbuYtFSx2N4e46yTFi+BDO212c0rX1GXGUykOknZU 2F6rrnHqsKHokUS6j+LXzH6sjz2oI7OxdXjzY3/+xasgKqd8k3aATwJqZXiZaDVhyPMV d3L21HuT0QOqTsVw/az6s4x9tsEj40iu6JjlgXoPc+SoQt1b7QsAUqiG4mY3bTf0751H 4MTgwKLb5+6ftNFvsb4IcwelgAUuXQlj/uADkE0YFgIIgfrf1byu1GX7DUJv90NyBtz6 rmqQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1678893335; h=content-transfer-encoding:mime-version:reply-to:references :in-reply-to:message-id:date:subject:cc:to:from:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=1fZE859NduZbyWuIX3xEGeUxQ0RfFRHfVEG4XDrPQwA=; b=ydP0iN8w2YP1xyNjnkQi0TK4lDUXk9nfRvYD5ZhYdOCdHIdKGl17ssmac86V6XAUGt Agk658ZG14G7tnB7p5AU3BQKm2r3UJSwx9KvrQcM09FlVl3CTwLsuUTBCkI2Kz/8lNHE +mFCZtiEtdiaNudcAaE80IcKdz3fMHPSst5dOwvO8V2RJwTQdmx0QgP+SUrhzcXzY6zH LbYo6rK1atl+lahk4cDWGXYP49m8163A7aqeCwyBS2Ok/YfpXzkNfRUZyRC/d8C0i8bH pnxlRzFARx/pLbVg2RnFHIIE6gVcQmxZOmno+VlZGL/R2g7DtRpninnXUsG1WSy6/iys OgYA== X-Gm-Message-State: AO0yUKXePOlj6cJeoXbi29Y5b6gCVG4BQ8fuYvRC6qxya39C/Kvid2qM TSjMeKnigWpKxXQR1AlRHenYhpwpfmw= X-Google-Smtp-Source: AK7set/RJB7O3Pz/r57B33/VUCsdfKWiXE9/ua8MahCPIkrER3n8ntPD7Lppz9TWoH+g894U7hlHlw== X-Received: by 2002:a05:6000:4c:b0:2ce:d152:dae1 with SMTP id k12-20020a056000004c00b002ced152dae1mr2164224wrx.13.1678893334962; Wed, 15 Mar 2023 08:15:34 -0700 (PDT) Received: from localhost.localdomain ([90.248.23.119]) by smtp.gmail.com with ESMTPSA id b10-20020a5d550a000000b002c706c754fesm4783191wrv.32.2023.03.15.08.15.34 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 15 Mar 2023 08:15:34 -0700 (PDT) From: Phillip Wood To: git@vger.kernel.org Cc: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= , Phillip Wood , Phillip Wood Subject: [PATCH 3/4] rebase -m: fix serialization of strategy options Date: Wed, 15 Mar 2023 15:14:58 +0000 Message-Id: <9af68cb065871d9b89e99ef6b48870d322bb5faa.1678893298.git.phillip.wood@dunelm.org.uk> X-Mailer: git-send-email 2.39.2 In-Reply-To: References: Reply-To: Phillip Wood MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Phillip Wood To store the strategy options rebase prepends " --" to each one and writes them to a file. To load them it reads the file and passes the contents to split_cmdline(). This roughly mimics the behavior of the scripted rebase but has a couple of limitations, (1) options containing whitespace are not properly preserved (this is true of the scripted rebase as well) and (2) options containing '"' or '\' are incorrectly parsed and may cause the parser to return an error. Fix these limitations by quoting each option when they are stored so that they can be parsed correctly. Now that "--preserve-merges" no longer exist this change also stops prepending "--" to the options when they are stored as that was an artifact of the scripted rebase. These changes are backwards compatible so the files written by an older version of git can be still be read. They are also forwards compatible, the file can still be parsed by recent versions of git as they treat the "--" prefix as optional. Signed-off-by: Phillip Wood --- sequencer.c | 23 +++++++++++++++++++---- t/t3418-rebase-continue.sh | 34 ++++++++++++++++++++++------------ t/t3436-rebase-more-options.sh | 24 ------------------------ 3 files changed, 41 insertions(+), 40 deletions(-) diff --git a/sequencer.c b/sequencer.c index 83ea1016ae..8890d1f7a1 100644 --- a/sequencer.c +++ b/sequencer.c @@ -2930,7 +2930,7 @@ static void parse_strategy_opts(struct replay_opts *opts, char *raw_opts) count = split_cmdline(strategy_opts_string, (const char ***)&opts->xopts); if (count < 0) - die(_("could not split '%s': %s"), strategy_opts_string, + BUG("could not split '%s': %s", strategy_opts_string, split_cmdline_strerror(count)); opts->xopts_nr = count; for (i = 0; i < opts->xopts_nr; i++) { @@ -3054,12 +3054,27 @@ static int read_populate_opts(struct replay_opts *opts) static void write_strategy_opts(struct replay_opts *opts) { - int i; struct strbuf buf = STRBUF_INIT; - for (i = 0; i < opts->xopts_nr; ++i) - strbuf_addf(&buf, " --%s", opts->xopts[i]); + /* + * Quote strategy options so that they can be read correctly + * by split_cmdline(). + */ + for (size_t i = 0; i < opts->xopts_nr; i++) { + char *arg = opts->xopts[i]; + if (i) + strbuf_addch(&buf, ' '); + strbuf_addch(&buf, '"'); + for (size_t j = 0; arg[j]; j++) { + const char c = arg[j]; + + if (c == '"' || c =='\\') + strbuf_addch(&buf, '\\'); + strbuf_addch(&buf, c); + } + strbuf_addch(&buf, '"'); + } write_file(rebase_path_strategy_opts(), "%s\n", buf.buf); strbuf_release(&buf); } diff --git a/t/t3418-rebase-continue.sh b/t/t3418-rebase-continue.sh index 130e2f9b55..42c3954125 100755 --- a/t/t3418-rebase-continue.sh +++ b/t/t3418-rebase-continue.sh @@ -62,29 +62,39 @@ test_expect_success 'rebase --continue remembers merge strategy and options' ' rm -fr .git/rebase-* && git reset --hard commit-new-file-F2-on-topic-branch && test_commit "commit-new-file-F3-on-topic-branch" F3 32 && - test_when_finished "rm -fr test-bin funny.was.run" && + test_when_finished "rm -fr test-bin" && mkdir test-bin && - cat >test-bin/git-merge-funny <<-EOF && - #!$SHELL_PATH - case "\$1" in --opt) ;; *) exit 2 ;; esac - shift && - >funny.was.run && - exec git merge-recursive "\$@" + + write_script test-bin/git-merge-funny <<-\EOF && + printf "[%s]\n" $# "$1" "$2" "$3" "$5" >actual + shift 3 && + exec git merge-recursive "$@" EOF - chmod +x test-bin/git-merge-funny && + + cat >expect <<-\EOF && + [7] + [--option=arg with space] + [--op"tion\] + [--new + line ] + [--] + EOF + + rm -f actual && ( PATH=./test-bin:$PATH && - test_must_fail git rebase -s funny -Xopt main topic + test_must_fail git rebase -s funny -X"option=arg with space" \ + -Xop\"tion\\ -X"new${LF}line " main topic ) && - test -f funny.was.run && - rm funny.was.run && + test_cmp expect actual && + rm actual && echo "Resolved" >F2 && git add F2 && ( PATH=./test-bin:$PATH && git rebase --continue ) && - test -f funny.was.run + test_cmp expect actual ' test_expect_success 'rebase -i --continue handles merge strategy and options' ' diff --git a/t/t3436-rebase-more-options.sh b/t/t3436-rebase-more-options.sh index 3adf42f47d..94671d3c46 100755 --- a/t/t3436-rebase-more-options.sh +++ b/t/t3436-rebase-more-options.sh @@ -40,30 +40,6 @@ test_expect_success 'setup' ' EOF ' -test_expect_success 'bad -X arguments: unclosed quote' ' - test_when_finished "test_might_fail git rebase --abort" && - cat >expect <<-\EOF && - fatal: could not split '\''--bad'\'': unclosed quote - EOF - GIT_SEQUENCE_EDITOR="echo break >" \ - git rebase -i -X"bad argument\"" side main && - test_expect_code 128 git rebase --continue >out 2>actual && - test_must_be_empty out && - test_cmp expect actual -' - -test_expect_success 'bad -X arguments: bad escape' ' - test_when_finished "test_might_fail git rebase --abort" && - cat >expect <<-\EOF && - fatal: could not split '\''--bad'\'': cmdline ends with \ - EOF - GIT_SEQUENCE_EDITOR="echo break >" \ - git rebase -i -X"bad escape \\" side main && - test_expect_code 128 git rebase --continue >out 2>actual && - test_must_be_empty out && - test_cmp expect actual -' - test_expect_success '--ignore-whitespace works with apply backend' ' test_must_fail git rebase --apply main side && git rebase --abort && From patchwork Wed Mar 15 15:14:59 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Phillip Wood X-Patchwork-Id: 13176082 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 BAE23C6FD1D for ; Wed, 15 Mar 2023 15:15:49 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233050AbjCOPPs (ORCPT ); Wed, 15 Mar 2023 11:15:48 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:48258 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232115AbjCOPPk (ORCPT ); Wed, 15 Mar 2023 11:15:40 -0400 Received: from mail-wr1-x42e.google.com (mail-wr1-x42e.google.com [IPv6:2a00:1450:4864:20::42e]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 38E10EFAA for ; Wed, 15 Mar 2023 08:15:37 -0700 (PDT) Received: by mail-wr1-x42e.google.com with SMTP id v16so17695804wrn.0 for ; Wed, 15 Mar 2023 08:15:37 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; t=1678893335; h=content-transfer-encoding:mime-version:reply-to:references :in-reply-to:message-id:date:subject:cc:to:from:from:to:cc:subject :date:message-id:reply-to; bh=IbE528xeFF7yNa7gwhVWHzFWnFIZWCT4Se6jK4RPIg4=; b=bXBkowDIEe+QzkPSXpxoLbo7yS+jkRZL7PqZ/8ef/2LuOAg5EWfkqcAeGKCcNPemPi KtuxG3PRKFjZKnFaTwCairJEyE5Q4qhvvUxAYkfbT6e3VRJ6WKcu+H6qPsIZ1aly0wnz RoVyD+PhVrKoSnpc/S1sXNLwXbQob3luEIb+9RzXrWRSt5/OvY9ns7qzLnuBvuBGZBV5 mTQ+ioQ7ZVTfosrfrTqg9Ka7pFBMhnw6xf4hL2/6JGvDCcnyTJbWuETq0Ne2caDa9Htc hQV83vrmU/agFVyQQqGQfENSNA8/ta3EuhGRZgjtq9qbGR+Bn50xkVtbjZ2fcsntzhc3 gG+w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1678893335; h=content-transfer-encoding:mime-version:reply-to:references :in-reply-to:message-id:date:subject:cc:to:from:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=IbE528xeFF7yNa7gwhVWHzFWnFIZWCT4Se6jK4RPIg4=; b=JWZhL4Rko1UeBdQRpvdGODyGvjz2vBayuX5BH+OH7L0yLHhoSMBFGAJSFyBL8drY0C lM8+N3ZVEK7d31uyLzJarQSH9mANbsSzSfj2IyEOHfyWPBNF6z8wCNt4RCj+q+Zwlhr3 0WdyTtlHkUT3Tfd9FeWwVZzDF5+KH/26/79aSx67S805EWaLv/Nl1sx6qSYgBJ9KHf3l IMgIsbiBinPNrr6EGZPtSD/UgY6X+uRZdfwAkFAeont1Lf3uSl+vnewucL/CRdsYqsn+ HiFqWyNxOup8vqjrniHul+84JXZq6EuuNd8TWlQtgRrvZsPqan6DL4qucUisdahuOGxb wgCQ== X-Gm-Message-State: AO0yUKVO0wGMoo6zUWQc9ntTThvZPl+oUFumstHH/ATaOZ7ri/VPpBEe u29ff+90dicMD5xcFgVjMiDH1CbwceU= X-Google-Smtp-Source: AK7set97GMz4miq1MIp8BnPKyAC2/0BOfSuj0PmQEK08YNuKVD8w5A7+t0NGipi53VrQAZgTWmON4A== X-Received: by 2002:a5d:63c3:0:b0:2cf:e868:f789 with SMTP id c3-20020a5d63c3000000b002cfe868f789mr2139164wrw.48.1678893335709; Wed, 15 Mar 2023 08:15:35 -0700 (PDT) Received: from localhost.localdomain ([90.248.23.119]) by smtp.gmail.com with ESMTPSA id b10-20020a5d550a000000b002c706c754fesm4783191wrv.32.2023.03.15.08.15.35 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 15 Mar 2023 08:15:35 -0700 (PDT) From: Phillip Wood To: git@vger.kernel.org Cc: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= , Phillip Wood , Phillip Wood Subject: [PATCH 4/4] rebase: remove a couple of redundant strategy tests Date: Wed, 15 Mar 2023 15:14:59 +0000 Message-Id: <3e02eeff78b23711187de47a1a820f9bde683200.1678893298.git.phillip.wood@dunelm.org.uk> X-Mailer: git-send-email 2.39.2 In-Reply-To: References: Reply-To: Phillip Wood MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Phillip Wood The test removed in t3402 has been redundant ever since 80ff47957b (rebase: remember strategy and strategy options, 2011-02-06) which added a new test the first part of which (as noted in the commit message) duplicated the existing test. The test removed in t3418 has been redundant since the merge backend was removed in 68aa495b59 (rebase: implement --merge via the interactive machinery, 2018-12-11) as it now tests the same code paths as the preceding test. Signed-off-by: Phillip Wood --- t/t3402-rebase-merge.sh | 21 --------------------- t/t3418-rebase-continue.sh | 32 -------------------------------- 2 files changed, 53 deletions(-) diff --git a/t/t3402-rebase-merge.sh b/t/t3402-rebase-merge.sh index 7e46f4ca85..79b0640c00 100755 --- a/t/t3402-rebase-merge.sh +++ b/t/t3402-rebase-merge.sh @@ -131,27 +131,6 @@ test_expect_success 'picking rebase' ' esac ' -test_expect_success 'rebase -s funny -Xopt' ' - test_when_finished "rm -fr test-bin funny.was.run" && - mkdir test-bin && - cat >test-bin/git-merge-funny <<-EOF && - #!$SHELL_PATH - case "\$1" in --opt) ;; *) exit 2 ;; esac - shift && - >funny.was.run && - exec git merge-recursive "\$@" - EOF - chmod +x test-bin/git-merge-funny && - git reset --hard && - git checkout -b test-funny main^ && - test_commit funny && - ( - PATH=./test-bin:$PATH && - git rebase -s funny -Xopt main - ) && - test -f funny.was.run -' - test_expect_success 'rebase --skip works with two conflicts in a row' ' git checkout second-side && tr "[A-Z]" "[a-z]" tmp && diff --git a/t/t3418-rebase-continue.sh b/t/t3418-rebase-continue.sh index 42c3954125..2d0789e554 100755 --- a/t/t3418-rebase-continue.sh +++ b/t/t3418-rebase-continue.sh @@ -97,38 +97,6 @@ test_expect_success 'rebase --continue remembers merge strategy and options' ' test_cmp expect actual ' -test_expect_success 'rebase -i --continue handles merge strategy and options' ' - rm -fr .git/rebase-* && - git reset --hard commit-new-file-F2-on-topic-branch && - test_commit "commit-new-file-F3-on-topic-branch-for-dash-i" F3 32 && - test_when_finished "rm -fr test-bin funny.was.run funny.args" && - mkdir test-bin && - cat >test-bin/git-merge-funny <<-EOF && - #!$SHELL_PATH - echo "\$@" >>funny.args - case "\$1" in --opt) ;; *) exit 2 ;; esac - case "\$2" in --foo) ;; *) exit 2 ;; esac - case "\$4" in --) ;; *) exit 2 ;; esac - shift 2 && - >funny.was.run && - exec git merge-recursive "\$@" - EOF - chmod +x test-bin/git-merge-funny && - ( - PATH=./test-bin:$PATH && - test_must_fail git rebase -i -s funny -Xopt -Xfoo main topic - ) && - test -f funny.was.run && - rm funny.was.run && - echo "Resolved" >F2 && - git add F2 && - ( - PATH=./test-bin:$PATH && - git rebase --continue - ) && - test -f funny.was.run -' - test_expect_success 'rebase -r passes merge strategy options correctly' ' rm -fr .git/rebase-* && git reset --hard commit-new-file-F3-on-topic-branch &&