git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [BUG?] 'git rebase --update-refs --whitespace=fix' incompatible, but not enforced

From
Derrick Stolee <derrickstolee@github.com>
Date
Jan 18, 2023, 17:03 UTC
Message-ID
<58a70c93-22ed-901c-44f2-3224734c27c3@github.com>
In-Reply-To
<CABPp-BGLVMoGiCeBMvyRhQmUSDEv8U7_U8=4B=Fh94=p_=QJVQ@mail.gmail.com>
On 1/18/2023 11:24 AM, Elijah Newren wrote:
> On Wed, Jan 18, 2023 at 7:51 AM Derrick Stolee <derrickstolee@github.com> wrote:
Show 10 quoted lines
>> +       /* Check for arguments that imply --apply before checking --apply itself. */
>> +       if (options.update_refs) {
>> +               const char *incompatible = NULL;
>> +               if (options.git_am_opts.nr)
>> +                       incompatible = options.git_am_opts.v[0];
>> +               else if (options.type == REBASE_APPLY)
>> +                       incompatible = "--apply";
> 
> git_am_opts can include "-q" which is not incompatible since it's also
> supported under the merge backend.
True, but I don't think that happens at this point in time. Right now,
the only things in there should be those placed by the opts parsing.
Things like '-q' get translated later. However, it would be good to be
sure.
 
Show 16 quoted lines
>> +
>> +               if (incompatible) {
>> +                       int from_config = 0;
>> +                       if (!git_config_get_bool("rebase.updaterefs", &from_config) &&
>> +                           from_config) {
>> +                               warning(_("you have 'rebase.updateRefs' enabled in your config, "
>> +                                         "but it is incompatible with one or more options;"));
>> +                               warning(_("run again with '--no-update-refs' to resolve the issue"));
>> +                       }
>> +                       die(_("options '%s' and '%s' cannot be used together"),
>> +                           "--upate-refs", incompatible);
>> +               }
>> +       }
> 
> We already have imply_merge() to catch the range of incompatibilities;
> can we just use it?
That would be great! I didn't realize that.
Show 33 quoted lines
>> +
>>         if (trace2_is_enabled()) {
>>                 if (is_merge(&options))
>>                         trace2_cmd_mode("interactive");
>> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
>> index 462cefd25df..8681c8a07f8 100755
>> --- a/t/t3404-rebase-interactive.sh
>> +++ b/t/t3404-rebase-interactive.sh
>> @@ -2123,6 +2123,18 @@ test_expect_success '--update-refs: check failed ref update' '
>>         test_cmp expect err.trimmed
>>  '
>>
>> +test_expect_success '--apply options are incompatible with --update-refs' '
>> +       for opt in "--whitespace=fix" "-C1" "--apply"
>> +       do
>> +               test_must_fail git rebase "$opt" --update-refs HEAD~1 2>err &&
>> +               grep "options '\''--upate-refs'\'' and '\''$opt'\'' cannot be used together" err &&
>> +
>> +               test_must_fail git -c rebase.updateRefs=true rebase "$opt" HEAD~1 2>err &&
>> +               grep "options '\''--upate-refs'\'' and '\''$opt'\'' cannot be used together" err &&
>> +               grep "you have '\''rebase.updateRefs'\'' enabled" err || return 1
>> +       done
>> +'
>> +
> 
> t3422 exists specifically for checking for incompatibilities of such
> options; the test should go over there.
> 
> I have both changes over at
> https://github.com/gitgitgadget/git/pull/1466; it doesn't include the
> "--no-update-refs" hint, but maybe that's good enough?  If so, I can
> submit it.  If not, do you want to alter or adopt some parts of my
> patch and submit a v2?

It sounds like you have a better handle on this and should take it from here. I look forward to your patch.

Thanks, -Stolee

Previous: Elijah NewrenNext: Philippe Blain
Message 5 of 6 in “[BUG?] 'git rebase --update-refs --whitespace=fix' incompatible, but not enforced”
  1. Philippe BlainJan 17, 2023
  2. Derrick StoleeJan 18, 2023
  3. Phillip WoodJan 18, 2023
  4. Elijah NewrenJan 18, 2023
  5. Derrick StoleeJan 18, 2023
  6. Philippe BlainJan 18, 2023

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.