From: Phillip Wood <phillip.wood123@gmail.com>
Date: Sat, 21 Jan 2023 19:25:50 GMT
Subject: Re: [PATCH v3 5/7] rebase: add coverage of other incompatible options
Message-ID: <20e759bd-7d13-b7ba-c0df-69b4f827c5bd@dunelm.org.uk>
In-Reply-To: <d5ef5870-baac-14d4-65a5-deb94a848011@dunelm.org.uk>

On 21/01/2023 15:20, Phillip Wood wrote:
>> @@ -1238,6 +1238,17 @@ int cmd_rebase(int argc, const char **argv, 
>> const char *prefix)
>>           if (options.fork_point < 0)
>>               options.fork_point = 0;
>>       }
>> +    /*
>> +     * reapply_cherry_picks is slightly weird.  It starts out with a
>> +     * value of -1.  It will be assigned a value of keep_base below and
>> +     * then handled specially.  The apply backend is hardcoded to
>> +     * behave like reapply_cherry_picks==1,
> 
> I think it is hard coded to 0 not 1. We generate the patches with
> 
>      git format-patch --right-only $upstream...$head

Sorry I somhow managed to omit --cherry-pick from that, it should have been

	git format-patch --right-only --cherry-pick $upstream...$head

Phillip


> so cherry-picks will not be reapplied. I'm hardly an impartial observer 
> but I think the current check is correct. We support
> 
>      --whitespace=fix --no-reapply-cherry-picks
>      --whitespace=fix --keep-base --reapply-cherry-picks
> 
> but not
> 
>      --whitespace=fix --reapply-cherry-picks
> 
>> @@ -1525,6 +1529,9 @@ int cmd_rebase(int argc, const char **argv, 
>> const char *prefix)
>>       if (options.update_refs)
>>           imply_merge(&options, "--update-refs");
>> +    if (options.autosquash)
>> +        imply_merge(&options, "--autosquash");
> 
> Thanks for adding this, it maybe merits a mention in the commit message 
> though as it is a change in behavior for users who have 
> rebase.autosquash set and try to use the apply backend.
> 
> Best Wishes
> 
> Phillip
> 
>>       if (options.type == REBASE_UNSPECIFIED) {
>>           if (!strcmp(options.default_backend, "merge"))
>>               imply_merge(&options, "--merge");
>> diff --git a/t/t3422-rebase-incompatible-options.sh 
>> b/t/t3422-rebase-incompatible-options.sh
>> index f86274990b0..c830025470f 100755
>> --- a/t/t3422-rebase-incompatible-options.sh
>> +++ b/t/t3422-rebase-incompatible-options.sh
>> @@ -50,6 +50,11 @@ test_rebase_am_only () {
>>           test_must_fail git rebase $opt --strategy-option=ours A
>>       "
>> +    test_expect_success "$opt incompatible with --autosquash" "
>> +        git checkout B^0 &&
>> +        test_must_fail git rebase $opt --autosquash A
>> +    "
>> +
>>       test_expect_success "$opt incompatible with --interactive" "
>>           git checkout B^0 &&
>>           test_must_fail git rebase $opt --interactive A
>> @@ -60,6 +65,21 @@ test_rebase_am_only () {
>>           test_must_fail git rebase $opt --exec 'true' A
>>       "
>> +    test_expect_success "$opt incompatible with --keep-empty" "
>> +        git checkout B^0 &&
>> +        test_must_fail git rebase $opt --keep-empty A
>> +    "
>> +
>> +    test_expect_success "$opt incompatible with --empty=..." "
>> +        git checkout B^0 &&
>> +        test_must_fail git rebase $opt --empty=ask A
>> +    "
>> +
>> +    test_expect_success "$opt incompatible with 
>> --no-reapply-cherry-picks" "
>> +        git checkout B^0 &&
>> +        test_must_fail git rebase $opt --no-reapply-cherry-picks A
>> +    "
>> +
>>       test_expect_success "$opt incompatible with --update-refs" "
>>           git checkout B^0 &&
>>           test_must_fail git rebase $opt --update-refs A