{"thread":{"id":"59108","subject":"[BUG?] 'git rebase --update-refs --whitespace=fix' incompatible, but not enforced","startedAt":"2023-01-17T22:23:02Z","lastAt":"2023-01-18T18:08:39Z","messageCount":6,"participants":["Philippe Blain","Derrick Stolee","Phillip Wood","Elijah Newren"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"470547","messageId":"b322c536-5a75-bb0c-8eac-1a99d3ba3230@gmail.com","threadId":"59108","inReplyTo":null,"subject":"[BUG?] 'git rebase --update-refs --whitespace=fix' incompatible, but not enforced","fromName":"Philippe Blain","fromEmail":"levraiphilippeblain@gmail.com","sentAt":"2023-01-17T22:02:07Z","receivedAt":"2023-01-17T22:23:02Z","isPatch":false,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"Hi Elijah and Stolee,\n\nFirst, Stolee, thanks a lot for adding '--update-refs', it is very useful (I've\nadded it to my config so I don't forget to use it).\n\nI recently learned that 'git rebase --whitespace=fix' exists, which is also\ngreat but since it uses the apply backend, it can't be used with --update-refs.\nI understand this, and the fact that adding '--whitespace=fix' to the merge\nbackend would be a big task; this is not what this email is about.\n\nI think there is a bug in that the code does not enforce the fact that\nboth options can't be used together.  Whenever '--whitespace' or '-C' are used,\nthe code defaults to the apply backend:\n\n```builtin/rebase +1502\n        if (options.git_am_opts.nr || options.type == REBASE_APPLY) {\n                /* all am options except -q are compatible only with --apply */\n                for (i = options.git_am_opts.nr - 1; i >= 0; i--)\n                        if (strcmp(options.git_am_opts.v[i], \"-q\"))\n                                break;\n\n                if (i >= 0) {\n                        if (is_merge(&options))\n                                die(_(\"apply options and merge options \"\n                                          \"cannot be used together\"));\n                        else\n                                options.type = REBASE_APPLY;\n                }\n```\n\nbut 'is_merge' only checks if 'opts->type == REBASE_MERGE', so the check only\nworks if --merge was given explicitely, but not when none of '--merge' or '--apply' \nwere given (and so the default \"merge\" backend is used).\n\nI would have expected the code to die telling me --update-refs and --whitespace\nare incompatible. But instead it defaulted to --apply, and (of course) did not\nupdate the refs in my history (which I found out later). \n\nThanks,\n\nPhilippe.\n"},{"id":"470625","messageId":"9445830b-d172-c3b6-ef60-ae4931cab84b@github.com","threadId":"59108","inReplyTo":"b322c536-5a75-bb0c-8eac-1a99d3ba3230@gmail.com","subject":"Re: [BUG?] 'git rebase --update-refs --whitespace=fix' incompatible, but not enforced","fromName":"Derrick Stolee","fromEmail":"derrickstolee@github.com","sentAt":"2023-01-18T15:51:43Z","receivedAt":"2023-01-18T15:56:37Z","isPatch":false,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 1/17/2023 5:02 PM, Philippe Blain wrote:\n> Hi Elijah and Stolee,\n> \n> First, Stolee, thanks a lot for adding '--update-refs', it is very useful (I've\n> added it to my config so I don't forget to use it).\n\nI'm glad you're using it, and thanks for the report!\n \n> I recently learned that 'git rebase --whitespace=fix' exists, which is also\n> great but since it uses the apply backend, it can't be used with --update-refs.\n> I understand this, and the fact that adding '--whitespace=fix' to the merge\n> backend would be a big task; this is not what this email is about.\n\nI also use --whitespace=fix, and am disappointed that it doesn't work with\n--update-refs. I guess I just haven't used it in my workflows that depend on\n--update-refs.\n\n> I think there is a bug in that the code does not enforce the fact that\n> both options can't be used together.  Whenever '--whitespace' or '-C' are used,\n> the code defaults to the apply backend:\n...\n> but 'is_merge' only checks if 'opts->type == REBASE_MERGE', so the check only\n> works if --merge was given explicitely, but not when none of '--merge' or '--apply' \n> were given (and so the default \"merge\" backend is used).\n> \n> I would have expected the code to die telling me --update-refs and --whitespace\n> are incompatible. But instead it defaulted to --apply, and (of course) did not\n> update the refs in my history (which I found out later). \n\nYes, I agree that there should be an error message (and a die(), not just a\nwarning). I quickly whipped up the patch below, which should resolve your\nconcern.\n\nNote that I was a bit worried about users with the config option and not\njust those that specify the option over the command-line. I put in an extra\nwarning for those users, but I could see the community wanting different\nbehavior there.\n\nLet me know what you think. If we need a new version, I'll create a new\nthread for that review.\n\nThanks,\n-Stolee\n\n--- >8 ---\nFrom fe310b37796b0b15554481eb1cfa3942a9200b4b Mon Sep 17 00:00:00 2001\nFrom: Derrick Stolee <derrickstolee@github.com>\nDate: Wed, 18 Jan 2023 10:38:18 -0500\nSubject: [PATCH] rebase: die for both --apply and --update-refs\n\nThe --apply backend is not compatible with the --update-refs option,\nwhich requires the interactive backend. Without any warning, users\nrunning 'git rebase --whitespace=fix' with --update-refs (or\nrebase.updateRefs=true in their config) will realize that their non-HEAD\nbranches did not come along for the ride.\n\nFix this with a die() message in the presence of both options. Since we\ncannot distinguish between the --update-refs option and the config\noption at this point, do an extra check to see if --update-refs was\nimplied by the config and present a helpful warning to use\n--no-update-refs.\n\nIt is possible that users might want to be able to use options such as\n--whitespace=fix with rebase.updateRefs=true in their config without\nexplicitly adding --no-update-refs. However, it is probably safest to\nrequire them to explicitly opt-in to that behavior. Users with the\nconfig option likely expect that their refs will be automatically\nupdated and this will help warn them that the action they are doing is\nlikely destructive to their branch organization.\n\nReported-by: Philippe Blain <levraiphilippeblain@gmail.com>\nSigned-off-by: Derrick Stolee <derrickstolee@github.com>\n---\n builtin/rebase.c              | 21 +++++++++++++++++++++\n t/t3404-rebase-interactive.sh | 12 ++++++++++++\n 2 files changed, 33 insertions(+)\n\ndiff --git a/builtin/rebase.c b/builtin/rebase.c\nindex 1481c5b6a5b..5330258c865 100644\n--- a/builtin/rebase.c\n+++ b/builtin/rebase.c\n@@ -1247,6 +1247,27 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)\n \t\tdie(_(\"The --edit-todo action can only be used during \"\n \t\t      \"interactive rebase.\"));\n \n+\t/* Check for arguments that imply --apply before checking --apply itself. */\n+\tif (options.update_refs) {\n+\t\tconst char *incompatible = NULL;\n+\t\tif (options.git_am_opts.nr)\n+\t\t\tincompatible = options.git_am_opts.v[0];\n+\t\telse if (options.type == REBASE_APPLY)\n+\t\t\tincompatible = \"--apply\";\n+\n+\t\tif (incompatible) {\n+\t\t\tint from_config = 0;\n+\t\t\tif (!git_config_get_bool(\"rebase.updaterefs\", &from_config) &&\n+\t\t\t    from_config) {\n+\t\t\t\twarning(_(\"you have 'rebase.updateRefs' enabled in your config, \"\n+\t\t\t\t\t  \"but it is incompatible with one or more options;\"));\n+\t\t\t\twarning(_(\"run again with '--no-update-refs' to resolve the issue\"));\n+\t\t\t}\n+\t\t\tdie(_(\"options '%s' and '%s' cannot be used together\"),\n+\t\t\t    \"--upate-refs\", incompatible);\n+\t\t}\n+\t}\n+\n \tif (trace2_is_enabled()) {\n \t\tif (is_merge(&options))\n \t\t\ttrace2_cmd_mode(\"interactive\");\ndiff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\nindex 462cefd25df..8681c8a07f8 100755\n--- a/t/t3404-rebase-interactive.sh\n+++ b/t/t3404-rebase-interactive.sh\n@@ -2123,6 +2123,18 @@ test_expect_success '--update-refs: check failed ref update' '\n \ttest_cmp expect err.trimmed\n '\n \n+test_expect_success '--apply options are incompatible with --update-refs' '\n+\tfor opt in \"--whitespace=fix\" \"-C1\" \"--apply\"\n+\tdo\n+\t\ttest_must_fail git rebase \"$opt\" --update-refs HEAD~1 2>err &&\n+\t\tgrep \"options '\\''--upate-refs'\\'' and '\\''$opt'\\'' cannot be used together\" err &&\n+\n+\t\ttest_must_fail git -c rebase.updateRefs=true rebase \"$opt\" HEAD~1 2>err &&\n+\t\tgrep \"options '\\''--upate-refs'\\'' and '\\''$opt'\\'' cannot be used together\" err &&\n+\t\tgrep \"you have '\\''rebase.updateRefs'\\'' enabled\" err || return 1\n+\tdone\n+'\n+\n # This must be the last test in this file\n test_expect_success '$EDITOR and friends are unchanged' '\n \ttest_editor_unchanged\n-- \n2.39.1.vfs.0.0\n\n\n"},{"id":"470639","messageId":"8a86074e-3f76-472c-15e2-9aa53762ef8f@dunelm.org.uk","threadId":"59108","inReplyTo":"9445830b-d172-c3b6-ef60-ae4931cab84b@github.com","subject":"Re: [BUG?] 'git rebase --update-refs --whitespace=fix' incompatible, but not enforced","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2023-01-18T16:16:56Z","receivedAt":"2023-01-18T16:21:00Z","isPatch":false,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Stolee\n\nOn 18/01/2023 15:51, Derrick Stolee wrote:\n> On 1/17/2023 5:02 PM, Philippe Blain wrote:\n> Subject: [PATCH] rebase: die for both --apply and --update-refs\n> \n> The --apply backend is not compatible with the --update-refs option,\n> which requires the interactive backend. Without any warning, users\n> running 'git rebase --whitespace=fix' with --update-refs (or\n> rebase.updateRefs=true in their config) will realize that their non-HEAD\n> branches did not come along for the ride.\n\nI think that for other options such as \"--exec\" that require the \"merge\" \nbackend we call imply_merge() and then have some code that will error \nout if there are also options that require the \"apply\" backend. Is it \npossible to do that here?\n\n> Fix this with a die() message in the presence of both options. Since we\n> cannot distinguish between the --update-refs option and the config\n> option at this point, do an extra check to see if --update-refs was\n> implied by the config and present a helpful warning to use\n> --no-update-refs.\n> \n> It is possible that users might want to be able to use options such as\n> --whitespace=fix with rebase.updateRefs=true in their config without\n> explicitly adding --no-update-refs. However, it is probably safest to\n> require them to explicitly opt-in to that behavior. Users with the\n> config option likely expect that their refs will be automatically\n> updated and this will help warn them that the action they are doing is\n> likely destructive to their branch organization.\n\nI agree that requiring an explicit --no-update-refs in that case is best.\n\nBest Wishes\n\nPhillip\n\n> Reported-by: Philippe Blain <levraiphilippeblain@gmail.com>\n> Signed-off-by: Derrick Stolee <derrickstolee@github.com>\n> ---\n>   builtin/rebase.c              | 21 +++++++++++++++++++++\n>   t/t3404-rebase-interactive.sh | 12 ++++++++++++\n>   2 files changed, 33 insertions(+)\n> \n> diff --git a/builtin/rebase.c b/builtin/rebase.c\n> index 1481c5b6a5b..5330258c865 100644\n> --- a/builtin/rebase.c\n> +++ b/builtin/rebase.c\n> @@ -1247,6 +1247,27 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)\n>   \t\tdie(_(\"The --edit-todo action can only be used during \"\n>   \t\t      \"interactive rebase.\"));\n>   \n> +\t/* Check for arguments that imply --apply before checking --apply itself. */\n> +\tif (options.update_refs) {\n> +\t\tconst char *incompatible = NULL;\n> +\t\tif (options.git_am_opts.nr)\n> +\t\t\tincompatible = options.git_am_opts.v[0];\n> +\t\telse if (options.type == REBASE_APPLY)\n> +\t\t\tincompatible = \"--apply\";\n> +\n> +\t\tif (incompatible) {\n> +\t\t\tint from_config = 0;\n> +\t\t\tif (!git_config_get_bool(\"rebase.updaterefs\", &from_config) &&\n> +\t\t\t    from_config) {\n> +\t\t\t\twarning(_(\"you have 'rebase.updateRefs' enabled in your config, \"\n> +\t\t\t\t\t  \"but it is incompatible with one or more options;\"));\n> +\t\t\t\twarning(_(\"run again with '--no-update-refs' to resolve the issue\"));\n> +\t\t\t}\n> +\t\t\tdie(_(\"options '%s' and '%s' cannot be used together\"),\n> +\t\t\t    \"--upate-refs\", incompatible);\n> +\t\t}\n> +\t}\n> +\n>   \tif (trace2_is_enabled()) {\n>   \t\tif (is_merge(&options))\n>   \t\t\ttrace2_cmd_mode(\"interactive\");\n> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\n> index 462cefd25df..8681c8a07f8 100755\n> --- a/t/t3404-rebase-interactive.sh\n> +++ b/t/t3404-rebase-interactive.sh\n> @@ -2123,6 +2123,18 @@ test_expect_success '--update-refs: check failed ref update' '\n>   \ttest_cmp expect err.trimmed\n>   '\n>   \n> +test_expect_success '--apply options are incompatible with --update-refs' '\n> +\tfor opt in \"--whitespace=fix\" \"-C1\" \"--apply\"\n> +\tdo\n> +\t\ttest_must_fail git rebase \"$opt\" --update-refs HEAD~1 2>err &&\n> +\t\tgrep \"options '\\''--upate-refs'\\'' and '\\''$opt'\\'' cannot be used together\" err &&\n> +\n> +\t\ttest_must_fail git -c rebase.updateRefs=true rebase \"$opt\" HEAD~1 2>err &&\n> +\t\tgrep \"options '\\''--upate-refs'\\'' and '\\''$opt'\\'' cannot be used together\" err &&\n> +\t\tgrep \"you have '\\''rebase.updateRefs'\\'' enabled\" err || return 1\n> +\tdone\n> +'\n> +\n>   # This must be the last test in this file\n>   test_expect_success '$EDITOR and friends are unchanged' '\n>   \ttest_editor_unchanged\n"},{"id":"470642","messageId":"CABPp-BGLVMoGiCeBMvyRhQmUSDEv8U7_U8=4B=Fh94=p_=QJVQ@mail.gmail.com","threadId":"59108","inReplyTo":"9445830b-d172-c3b6-ef60-ae4931cab84b@github.com","subject":"Re: [BUG?] 'git rebase --update-refs --whitespace=fix' incompatible, but not enforced","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2023-01-18T16:24:07Z","receivedAt":"2023-01-18T16:26:26Z","isPatch":false,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Wed, Jan 18, 2023 at 7:51 AM Derrick Stolee <derrickstolee@github.com> wrote:\n>\n> On 1/17/2023 5:02 PM, Philippe Blain wrote:\n> > Hi Elijah and Stolee,\n> >\n> > First, Stolee, thanks a lot for adding '--update-refs', it is very useful (I've\n> > added it to my config so I don't forget to use it).\n>\n> I'm glad you're using it, and thanks for the report!\n>\n> > I recently learned that 'git rebase --whitespace=fix' exists, which is also\n> > great but since it uses the apply backend, it can't be used with --update-refs.\n> > I understand this, and the fact that adding '--whitespace=fix' to the merge\n> > backend would be a big task; this is not what this email is about.\n>\n> I also use --whitespace=fix, and am disappointed that it doesn't work with\n> --update-refs. I guess I just haven't used it in my workflows that depend on\n> --update-refs.\n>\n> > I think there is a bug in that the code does not enforce the fact that\n> > both options can't be used together.  Whenever '--whitespace' or '-C' are used,\n> > the code defaults to the apply backend:\n> ...\n> > but 'is_merge' only checks if 'opts->type == REBASE_MERGE', so the check only\n> > works if --merge was given explicitely, but not when none of '--merge' or '--apply'\n> > were given (and so the default \"merge\" backend is used).\n> >\n> > I would have expected the code to die telling me --update-refs and --whitespace\n> > are incompatible. But instead it defaulted to --apply, and (of course) did not\n> > update the refs in my history (which I found out later).\n>\n> Yes, I agree that there should be an error message (and a die(), not just a\n> warning). I quickly whipped up the patch below, which should resolve your\n> concern.\n>\n> Note that I was a bit worried about users with the config option and not\n> just those that specify the option over the command-line. I put in an extra\n> warning for those users, but I could see the community wanting different\n> behavior there.\n>\n> Let me know what you think. If we need a new version, I'll create a new\n> thread for that review.\n\nI was just about to post a patch as well --\nhttps://github.com/gitgitgadget/git/pull/1466.\n\n> --- >8 ---\n> From fe310b37796b0b15554481eb1cfa3942a9200b4b Mon Sep 17 00:00:00 2001\n> From: Derrick Stolee <derrickstolee@github.com>\n> Date: Wed, 18 Jan 2023 10:38:18 -0500\n> Subject: [PATCH] rebase: die for both --apply and --update-refs\n>\n> The --apply backend is not compatible with the --update-refs option,\n> which requires the interactive backend. Without any warning, users\n> running 'git rebase --whitespace=fix' with --update-refs (or\n> rebase.updateRefs=true in their config) will realize that their non-HEAD\n> branches did not come along for the ride.\n>\n> Fix this with a die() message in the presence of both options. Since we\n> cannot distinguish between the --update-refs option and the config\n> option at this point, do an extra check to see if --update-refs was\n> implied by the config and present a helpful warning to use\n> --no-update-refs.\n>\n> It is possible that users might want to be able to use options such as\n> --whitespace=fix with rebase.updateRefs=true in their config without\n> explicitly adding --no-update-refs. However, it is probably safest to\n> require them to explicitly opt-in to that behavior. Users with the\n> config option likely expect that their refs will be automatically\n> updated and this will help warn them that the action they are doing is\n> likely destructive to their branch organization.\n>\n> Reported-by: Philippe Blain <levraiphilippeblain@gmail.com>\n> Signed-off-by: Derrick Stolee <derrickstolee@github.com>\n> ---\n>  builtin/rebase.c              | 21 +++++++++++++++++++++\n>  t/t3404-rebase-interactive.sh | 12 ++++++++++++\n>  2 files changed, 33 insertions(+)\n>\n> diff --git a/builtin/rebase.c b/builtin/rebase.c\n> index 1481c5b6a5b..5330258c865 100644\n> --- a/builtin/rebase.c\n> +++ b/builtin/rebase.c\n> @@ -1247,6 +1247,27 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)\n>                 die(_(\"The --edit-todo action can only be used during \"\n>                       \"interactive rebase.\"));\n>\n> +       /* Check for arguments that imply --apply before checking --apply itself. */\n> +       if (options.update_refs) {\n> +               const char *incompatible = NULL;\n> +               if (options.git_am_opts.nr)\n> +                       incompatible = options.git_am_opts.v[0];\n> +               else if (options.type == REBASE_APPLY)\n> +                       incompatible = \"--apply\";\n\ngit_am_opts can include \"-q\" which is not incompatible since it's also\nsupported under the merge backend.\n\n> +\n> +               if (incompatible) {\n> +                       int from_config = 0;\n> +                       if (!git_config_get_bool(\"rebase.updaterefs\", &from_config) &&\n> +                           from_config) {\n> +                               warning(_(\"you have 'rebase.updateRefs' enabled in your config, \"\n> +                                         \"but it is incompatible with one or more options;\"));\n> +                               warning(_(\"run again with '--no-update-refs' to resolve the issue\"));\n> +                       }\n> +                       die(_(\"options '%s' and '%s' cannot be used together\"),\n> +                           \"--upate-refs\", incompatible);\n> +               }\n> +       }\n\nWe already have imply_merge() to catch the range of incompatibilities;\ncan we just use it?\n\n> +\n>         if (trace2_is_enabled()) {\n>                 if (is_merge(&options))\n>                         trace2_cmd_mode(\"interactive\");\n> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\n> index 462cefd25df..8681c8a07f8 100755\n> --- a/t/t3404-rebase-interactive.sh\n> +++ b/t/t3404-rebase-interactive.sh\n> @@ -2123,6 +2123,18 @@ test_expect_success '--update-refs: check failed ref update' '\n>         test_cmp expect err.trimmed\n>  '\n>\n> +test_expect_success '--apply options are incompatible with --update-refs' '\n> +       for opt in \"--whitespace=fix\" \"-C1\" \"--apply\"\n> +       do\n> +               test_must_fail git rebase \"$opt\" --update-refs HEAD~1 2>err &&\n> +               grep \"options '\\''--upate-refs'\\'' and '\\''$opt'\\'' cannot be used together\" err &&\n> +\n> +               test_must_fail git -c rebase.updateRefs=true rebase \"$opt\" HEAD~1 2>err &&\n> +               grep \"options '\\''--upate-refs'\\'' and '\\''$opt'\\'' cannot be used together\" err &&\n> +               grep \"you have '\\''rebase.updateRefs'\\'' enabled\" err || return 1\n> +       done\n> +'\n> +\n\nt3422 exists specifically for checking for incompatibilities of such\noptions; the test should go over there.\n\nI have both changes over at\nhttps://github.com/gitgitgadget/git/pull/1466; it doesn't include the\n\"--no-update-refs\" hint, but maybe that's good enough?  If so, I can\nsubmit it.  If not, do you want to alter or adopt some parts of my\npatch and submit a v2?\n"},{"id":"470649","messageId":"58a70c93-22ed-901c-44f2-3224734c27c3@github.com","threadId":"59108","inReplyTo":"CABPp-BGLVMoGiCeBMvyRhQmUSDEv8U7_U8=4B=Fh94=p_=QJVQ@mail.gmail.com","subject":"Re: [BUG?] 'git rebase --update-refs --whitespace=fix' incompatible, but not enforced","fromName":"Derrick Stolee","fromEmail":"derrickstolee@github.com","sentAt":"2023-01-18T17:03:01Z","receivedAt":"2023-01-18T17:03:06Z","isPatch":false,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 1/18/2023 11:24 AM, Elijah Newren wrote:\n> On Wed, Jan 18, 2023 at 7:51 AM Derrick Stolee <derrickstolee@github.com> wrote:\n\n>> +       /* Check for arguments that imply --apply before checking --apply itself. */\n>> +       if (options.update_refs) {\n>> +               const char *incompatible = NULL;\n>> +               if (options.git_am_opts.nr)\n>> +                       incompatible = options.git_am_opts.v[0];\n>> +               else if (options.type == REBASE_APPLY)\n>> +                       incompatible = \"--apply\";\n> \n> git_am_opts can include \"-q\" which is not incompatible since it's also\n> supported under the merge backend.\n\nTrue, but I don't think that happens at this point in time. Right now,\nthe only things in there should be those placed by the opts parsing.\nThings like '-q' get translated later. However, it would be good to be\nsure.\n \n>> +\n>> +               if (incompatible) {\n>> +                       int from_config = 0;\n>> +                       if (!git_config_get_bool(\"rebase.updaterefs\", &from_config) &&\n>> +                           from_config) {\n>> +                               warning(_(\"you have 'rebase.updateRefs' enabled in your config, \"\n>> +                                         \"but it is incompatible with one or more options;\"));\n>> +                               warning(_(\"run again with '--no-update-refs' to resolve the issue\"));\n>> +                       }\n>> +                       die(_(\"options '%s' and '%s' cannot be used together\"),\n>> +                           \"--upate-refs\", incompatible);\n>> +               }\n>> +       }\n> \n> We already have imply_merge() to catch the range of incompatibilities;\n> can we just use it?\n\nThat would be great! I didn't realize that.\n\n>> +\n>>         if (trace2_is_enabled()) {\n>>                 if (is_merge(&options))\n>>                         trace2_cmd_mode(\"interactive\");\n>> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\n>> index 462cefd25df..8681c8a07f8 100755\n>> --- a/t/t3404-rebase-interactive.sh\n>> +++ b/t/t3404-rebase-interactive.sh\n>> @@ -2123,6 +2123,18 @@ test_expect_success '--update-refs: check failed ref update' '\n>>         test_cmp expect err.trimmed\n>>  '\n>>\n>> +test_expect_success '--apply options are incompatible with --update-refs' '\n>> +       for opt in \"--whitespace=fix\" \"-C1\" \"--apply\"\n>> +       do\n>> +               test_must_fail git rebase \"$opt\" --update-refs HEAD~1 2>err &&\n>> +               grep \"options '\\''--upate-refs'\\'' and '\\''$opt'\\'' cannot be used together\" err &&\n>> +\n>> +               test_must_fail git -c rebase.updateRefs=true rebase \"$opt\" HEAD~1 2>err &&\n>> +               grep \"options '\\''--upate-refs'\\'' and '\\''$opt'\\'' cannot be used together\" err &&\n>> +               grep \"you have '\\''rebase.updateRefs'\\'' enabled\" err || return 1\n>> +       done\n>> +'\n>> +\n> \n> t3422 exists specifically for checking for incompatibilities of such\n> options; the test should go over there.\n> \n> I have both changes over at\n> https://github.com/gitgitgadget/git/pull/1466; it doesn't include the\n> \"--no-update-refs\" hint, but maybe that's good enough?  If so, I can\n> submit it.  If not, do you want to alter or adopt some parts of my\n> patch and submit a v2?\n\nIt sounds like you have a better handle on this and should take it\nfrom here. I look forward to your patch.\n\nThanks,\n-Stolee\n"},{"id":"470653","messageId":"e16c6bd8-41b2-8912-c327-7b903f6644ef@gmail.com","threadId":"59108","inReplyTo":"58a70c93-22ed-901c-44f2-3224734c27c3@github.com","subject":"Re: [BUG?] 'git rebase --update-refs --whitespace=fix' incompatible, but not enforced","fromName":"Philippe Blain","fromEmail":"levraiphilippeblain@gmail.com","sentAt":"2023-01-18T18:07:38Z","receivedAt":"2023-01-18T18:08:39Z","isPatch":false,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"\n\nLe 2023-01-18 à 12:03, Derrick Stolee a écrit :\n> On 1/18/2023 11:24 AM, Elijah Newren wrote:\n>> On Wed, Jan 18, 2023 at 7:51 AM Derrick Stolee <derrickstolee@github.com> wrote:\n> \n--- 8< ---\n>> I have both changes over at\n>> https://github.com/gitgitgadget/git/pull/1466; it doesn't include the\n>> \"--no-update-refs\" hint, but maybe that's good enough?  If so, I can\n>> submit it.  If not, do you want to alter or adopt some parts of my\n>> patch and submit a v2?\n> \n> It sounds like you have a better handle on this and should take it\n> from here. I look forward to your patch.\n\nThanks both for your quick answers and patches. I do think that the hint\nabout '--no-update-refs' could be valuable.\n\nCheers,\n\nPhilippe.\n"}]}