git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [BUG] git pull --rebase ignores rebase.autostash config when fast-forwarding
@ 2022-01-03 18:08 Tilman Vogel
  2022-01-04 12:59 ` Philip Oakley
  0 siblings, 1 reply; 6+ messages in thread
From: Tilman Vogel @ 2022-01-03 18:08 UTC (permalink / raw)
  To: git

[-- Attachment #1: Type: text/plain, Size: 1011 bytes --]

Hi git-people,

I ran into strange behavior when having rebase.autostash enabled and
doing a git pull --rebase:

> git config rebase.autostash true
> git pull --rebase
Updating cd9ff8a..f3c9840
error: Your local changes to the following files would be overwritten by
merge:
        content
Please commit your changes or stash them before you merge.
Aborting

Confusingly, this fixes the issue:

> git config merge.autostash true
> git pull --rebase
Updating cd9ff8a..f3c9840
Created autostash: c615fda
Fast-forward
 content | 1 +
 1 file changed, 1 insertion(+)
Applied autostash.

Leaving me wonder why merge config options fix rebase behavior.

So, in order to make it easier to check the problem, I added some
test-cases to the git test-suite. Please see the attached patch.

Or here:
https://github.com/tvogel/git/commit/bc941f9357518a34cfa11788dfb8e7fa7f711705

I did not try to find the root-cause as I am not experienced with the
code-base but if there are questions, let me know.

Best regards,

Tilman

[-- Attachment #2: git-pull-rebase-autostash-problem.patch --]
[-- Type: text/x-patch, Size: 3303 bytes --]

From bc941f9357518a34cfa11788dfb8e7fa7f711705 Mon Sep 17 00:00:00 2001
From: Tilman Vogel <tilman.vogel@web.de>
Date: Mon, 3 Jan 2022 18:31:59 +0100
Subject: [PATCH] t5521: Show inconsistent rebase behavior (--autostash vs.
 rebase.autostash)

When pull --rebase can be done as a fast-forward, the rebase.autostash
config is neglected such that the second test-case added in this commit
will fail to autostash and succeed.
In fact, setting merge.autostash as a workaround would cause autostash to
be done.

The other three test-cases added in this commit document that autostashing
works as expected when either requested explicitly with --autostash or when
pull --rebase cannot be done as a fast-forward.

Signed-off-by: Tilman Vogel <tilman.vogel@web.de>
---
 t/t5521-pull-options.sh | 52 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 52 insertions(+)

diff --git a/t/t5521-pull-options.sh b/t/t5521-pull-options.sh
index 66cfcb09c5..4046a74cad 100755
--- a/t/t5521-pull-options.sh
+++ b/t/t5521-pull-options.sh
@@ -252,4 +252,56 @@ test_expect_success 'git pull --no-verify --verify passed to merge' '
 	test_must_fail git -C dst pull --no-ff --no-verify --verify
 '
 
+test_expect_success 'git pull --rebase --autostash succeeds on ff' '
+	test_when_finished "rm -fr src dst actual" &&
+	git init src &&
+	test_commit -C src "initial" file "content" &&
+	git clone src dst &&
+	test_commit -C src --printf "more_content" file "more content\ncontent\n" &&
+	echo "dirty" >>dst/file &&
+	git -C dst pull --rebase --autostash >actual 2>&1 &&
+	grep -q "Fast-forward" actual &&
+	grep -q "Applied autostash." actual
+'
+
+test_expect_success 'git pull --rebase with rebase.autostash succeeds on ff' '
+	test_when_finished "rm -fr src dst actual" &&
+	git init src &&
+	test_commit -C src "initial" file "content" &&
+	git clone src dst &&
+	test_commit -C src --printf "more_content" file "more content\ncontent\n" &&
+	echo "dirty" >>dst/file &&
+	test_config -C dst rebase.autostash true &&
+	git -C dst pull --rebase  >actual 2>&1 &&
+	grep -q "Fast-forward" actual &&
+	grep -q "Applied autostash." actual
+'
+
+test_expect_success 'git pull --rebase --autostash succeeds on non-ff' '
+	test_when_finished "rm -fr src dst actual" &&
+	git init src &&
+	test_commit -C src "initial" file "content" &&
+	git clone src dst &&
+	test_commit -C src --printf "src_content" file "src content\ncontent\n" &&
+	test_commit -C dst --append "dst_content" file "dst content" &&
+	echo "dirty" >>dst/file &&
+	git -C dst pull --rebase --autostash >actual 2>&1 &&
+	grep -q "Successfully rebased and updated refs/heads/main." actual &&
+	grep -q "Applied autostash." actual
+'
+
+test_expect_success 'git pull --rebase with rebase.autostash succeeds on non-ff' '
+	test_when_finished "rm -fr src dst actual" &&
+	git init src &&
+	test_commit -C src "initial" file "content" &&
+	git clone src dst &&
+	test_commit -C src --printf "src_content" file "src content\ncontent\n" &&
+	test_commit -C dst --append "dst_content" file "dst content" &&
+	echo "dirty" >>dst/file &&
+	test_config -C dst rebase.autostash true &&
+	git -C dst pull --rebase >actual 2>&1 &&
+	grep -q "Successfully rebased and updated refs/heads/main." actual &&
+	grep -q "Applied autostash." actual
+'
+
 test_done
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [BUG] git pull --rebase ignores rebase.autostash config when fast-forwarding
  2022-01-03 18:08 [BUG] git pull --rebase ignores rebase.autostash config when fast-forwarding Tilman Vogel
@ 2022-01-04 12:59 ` Philip Oakley
  2022-01-04 18:03   ` Philippe Blain
  0 siblings, 1 reply; 6+ messages in thread
From: Philip Oakley @ 2022-01-04 12:59 UTC (permalink / raw)
  To: Tilman Vogel, git

On 03/01/2022 18:08, Tilman Vogel wrote:
> Hi git-people,
>
> I ran into strange behavior when having rebase.autostash enabled and
> doing a git pull --rebase:
>
>> git config rebase.autostash true
>> git pull --rebase
> Updating cd9ff8a..f3c9840
> error: Your local changes to the following files would be overwritten by
> merge:
>         content
> Please commit your changes or stash them before you merge.
> Aborting
>
> Confusingly, this fixes the issue:
>
>> git config merge.autostash true
>> git pull --rebase
> Updating cd9ff8a..f3c9840
> Created autostash: c615fda
> Fast-forward
>  content | 1 +
>  1 file changed, 1 insertion(+)
> Applied autostash.
>
> Leaving me wonder why merge config options fix rebase behavior.
>
> So, in order to make it easier to check the problem, I added some
> test-cases to the git test-suite. Please see the attached patch.
>
> Or here:
> https://github.com/tvogel/git/commit/bc941f9357518a34cfa11788dfb8e7fa7f711705
>
> I did not try to find the root-cause as I am not experienced with the
> code-base but if there are questions, let me know.

Which version are you running?

There was a problem in 2.33.1 as per thread discussion:
 https://lore.kernel.org/git/YYFEE%2F2g3SiM04zx@hades.panopticon/
 (I'd searched the logs for ".autostash true")

Does that provide any guidance?
--
Philip


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [BUG] git pull --rebase ignores rebase.autostash config when fast-forwarding
  2022-01-04 12:59 ` Philip Oakley
@ 2022-01-04 18:03   ` Philippe Blain
  2022-01-04 18:29     ` Philippe Blain
  2022-01-04 21:03     ` Tilman Vogel
  0 siblings, 2 replies; 6+ messages in thread
From: Philippe Blain @ 2022-01-04 18:03 UTC (permalink / raw)
  To: Philip Oakley, Tilman Vogel, git; +Cc: Felipe Contreras

Hi Tilman,

Le 2022-01-04 à 07:59, Philip Oakley a écrit :
> On 03/01/2022 18:08, Tilman Vogel wrote:
>> Hi git-people,
>>
>> I ran into strange behavior when having rebase.autostash enabled and
>> doing a git pull --rebase:
>>
>>> git config rebase.autostash true
>>> git pull --rebase
>> Updating cd9ff8a..f3c9840
>> error: Your local changes to the following files would be overwritten by
>> merge:
>>          content
>> Please commit your changes or stash them before you merge.
>> Aborting
>>
>> Confusingly, this fixes the issue:
>>
>>> git config merge.autostash true
>>> git pull --rebase
>> Updating cd9ff8a..f3c9840
>> Created autostash: c615fda
>> Fast-forward
>>   content | 1 +
>>   1 file changed, 1 insertion(+)
>> Applied autostash.
>>
>> Leaving me wonder why merge config options fix rebase behavior.
>>
>> So, in order to make it easier to check the problem, I added some
>> test-cases to the git test-suite. Please see the attached patch.

Thanks, this really makes it easier to bisect the issue.

>>
>> Or here:
>> https://github.com/tvogel/git/commit/bc941f9357518a34cfa11788dfb8e7fa7f711705
>>
>> I did not try to find the root-cause as I am not experienced with the
>> code-base but if there are questions, let me know.
> 
> Which version are you running?
> 

That's a good info to include indeed. I'm guessing you are using v2.34.1 as that's the version
indicated at the bottom of your attached patch. I can replicate the behaviour on my side on 2.34.1.
I did not bisect manually but I'm pretty sure it's a regression caused by 340062243a (pull: cleanup autostash
check, 2021-06-17) (author CC'ed). I checked that the parent of that commit passes all 4 of your added tests, provided
this is squashed in:

diff --git a/t/t5521-pull-options.sh b/t/t5521-pull-options.sh
index 4046a74cad..5ad19b1028 100755
--- a/t/t5521-pull-options.sh
+++ b/t/t5521-pull-options.sh
@@ -260,7 +260,6 @@ test_expect_success 'git pull --rebase --autostash succeeds on ff' '
  	test_commit -C src --printf "more_content" file "more content\ncontent\n" &&
  	echo "dirty" >>dst/file &&
  	git -C dst pull --rebase --autostash >actual 2>&1 &&
-	grep -q "Fast-forward" actual &&
  	grep -q "Applied autostash." actual
  '
  
@@ -273,7 +272,6 @@ test_expect_success 'git pull --rebase with rebase.autostash succeeds on ff' '
  	echo "dirty" >>dst/file &&
  	test_config -C dst rebase.autostash true &&
  	git -C dst pull --rebase  >actual 2>&1 &&
-	grep -q "Fast-forward" actual &&
  	grep -q "Applied autostash." actual
  '

After that commit, in case of fast-forward, 'git pull --rebase --autostash' delegates the fast-forward
operation to 'git merge' under the hood, which was not the case before. The '--autostash' flag seems
to be forwarded correctly to that 'git merge' invocation, but the config 'rebase.autostash' seems to not
be passed along.

I did not yet look into why in the code itself. That does explain however why 'merge.autostash' makes it
work - the 'git merge' invocation does check its own config, and if merge.autostash is there the autostash
behaviour is activated.

Philippe.



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [BUG] git pull --rebase ignores rebase.autostash config when fast-forwarding
  2022-01-04 18:03   ` Philippe Blain
@ 2022-01-04 18:29     ` Philippe Blain
       [not found]       ` <BB019E6A-90ED-4D5A-A756-FA8700897DE7@gmail.com>
  2022-01-04 21:03     ` Tilman Vogel
  1 sibling, 1 reply; 6+ messages in thread
From: Philippe Blain @ 2022-01-04 18:29 UTC (permalink / raw)
  To: Philip Oakley, Tilman Vogel, git; +Cc: Felipe Contreras

Hi again,

Le 2022-01-04 à 13:03, Philippe Blain a écrit :
> Hi Tilman,
> 
> Le 2022-01-04 à 07:59, Philip Oakley a écrit :
>> On 03/01/2022 18:08, Tilman Vogel wrote:
>>> Hi git-people,
>>>
>>> I ran into strange behavior when having rebase.autostash enabled and
>>> doing a git pull --rebase:
>>>
>>>> git config rebase.autostash true
>>>> git pull --rebase
>>> Updating cd9ff8a..f3c9840
>>> error: Your local changes to the following files would be overwritten by
>>> merge:
>>>          content
>>> Please commit your changes or stash them before you merge.
>>> Aborting
>>>
>>> Confusingly, this fixes the issue:
>>>
>>>> git config merge.autostash true
>>>> git pull --rebase
>>> Updating cd9ff8a..f3c9840
>>> Created autostash: c615fda
>>> Fast-forward
>>>   content | 1 +
>>>   1 file changed, 1 insertion(+)
>>> Applied autostash.
>>>
>>> Leaving me wonder why merge config options fix rebase behavior.
>>>
>>> So, in order to make it easier to check the problem, I added some
>>> test-cases to the git test-suite. Please see the attached patch.
> 
> Thanks, this really makes it easier to bisect the issue.
> 
>>>
>>> Or here:
>>> https://github.com/tvogel/git/commit/bc941f9357518a34cfa11788dfb8e7fa7f711705
>>>
>>> I did not try to find the root-cause as I am not experienced with the
>>> code-base but if there are questions, let me know.
>>
>> Which version are you running?
>>
> 
> That's a good info to include indeed. I'm guessing you are using v2.34.1 as that's the version
> indicated at the bottom of your attached patch. I can replicate the behaviour on my side on 2.34.1.
> I did not bisect manually but I'm pretty sure it's a regression caused by 340062243a (pull: cleanup autostash
> check, 2021-06-17) (author CC'ed). I checked that the parent of that commit passes all 4 of your added tests, provided
> this is squashed in:
> 
> diff --git a/t/t5521-pull-options.sh b/t/t5521-pull-options.sh
> index 4046a74cad..5ad19b1028 100755
> --- a/t/t5521-pull-options.sh
> +++ b/t/t5521-pull-options.sh
> @@ -260,7 +260,6 @@ test_expect_success 'git pull --rebase --autostash succeeds on ff' '
>       test_commit -C src --printf "more_content" file "more content\ncontent\n" &&
>       echo "dirty" >>dst/file &&
>       git -C dst pull --rebase --autostash >actual 2>&1 &&
> -    grep -q "Fast-forward" actual &&
>       grep -q "Applied autostash." actual
>   '
> 
> @@ -273,7 +272,6 @@ test_expect_success 'git pull --rebase with rebase.autostash succeeds on ff' '
>       echo "dirty" >>dst/file &&
>       test_config -C dst rebase.autostash true &&
>       git -C dst pull --rebase  >actual 2>&1 &&
> -    grep -q "Fast-forward" actual &&
>       grep -q "Applied autostash." actual
>   '
> 
> After that commit, in case of fast-forward, 'git pull --rebase --autostash' delegates the fast-forward
> operation to 'git merge' under the hood, which was not the case before. The '--autostash' flag seems
> to be forwarded correctly to that 'git merge' invocation, but the config 'rebase.autostash' seems to not
> be passed along.
> 
> I did not yet look into why in the code itself

After looking at it a bit, this seems to fix the bug:

diff --git a/builtin/pull.c b/builtin/pull.c
index 1cfaf9f343..0b206bf1d5 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -87,6 +87,7 @@ static char *opt_verify_signatures;
  static char *opt_verify;
  static int opt_autostash = -1;
  static int config_autostash;
+static int autostash = -1;
  static int check_trust_level = 1;
  static struct strvec opt_strategies = STRVEC_INIT;
  static struct strvec opt_strategy_opts = STRVEC_INIT;
@@ -687,9 +688,9 @@ static int run_merge(void)
  	strvec_pushv(&args, opt_strategy_opts.v);
  	if (opt_gpg_sign)
  		strvec_push(&args, opt_gpg_sign);
-	if (opt_autostash == 0)
+	if (autostash == 0)
  		strvec_push(&args, "--no-autostash");
-	else if (opt_autostash == 1)
+	else if (autostash == 1)
  		strvec_push(&args, "--autostash");
  	if (opt_allow_unrelated_histories > 0)
  		strvec_push(&args, "--allow-unrelated-histories");
@@ -1036,7 +1037,7 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
  		oidclr(&orig_head);
  
  	if (opt_rebase) {
-		int autostash = config_autostash;
+		autostash = config_autostash;
  		if (opt_autostash != -1)
  			autostash = opt_autostash;


If nobody beats me to it (if that's the case, be my guest !), I'll try to submit a proper patch later
today or this week.

In case anybody needs it:

Signed-off by: Philippe Blain <levraiphilippeblain@gmail.com>


Cheers,

Philippe.
  


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [BUG] git pull --rebase ignores rebase.autostash config when fast-forwarding
  2022-01-04 18:03   ` Philippe Blain
  2022-01-04 18:29     ` Philippe Blain
@ 2022-01-04 21:03     ` Tilman Vogel
  1 sibling, 0 replies; 6+ messages in thread
From: Tilman Vogel @ 2022-01-04 21:03 UTC (permalink / raw)
  Cc: git

Oh, please excuse my lack of providing the version info: I discovered
the behaviour on 2.34.1 as shipped by current openSUSE Tumbleweed.
Then, I worked on-top of current git master (2ae0a9cb).

Regards and thanks for investigating and fixing this so quickly!

Tilman


Am Di., 4. Jan. 2022 um 19:03 Uhr schrieb Philippe Blain
<levraiphilippeblain@gmail.com>:
>
> Hi Tilman,
>
> Le 2022-01-04 à 07:59, Philip Oakley a écrit :
> > On 03/01/2022 18:08, Tilman Vogel wrote:
> >> Hi git-people,
> >>
> >> I ran into strange behavior when having rebase.autostash enabled and
> >> doing a git pull --rebase:
> >>
> >>> git config rebase.autostash true
> >>> git pull --rebase
> >> Updating cd9ff8a..f3c9840
> >> error: Your local changes to the following files would be overwritten by
> >> merge:
> >>          content
> >> Please commit your changes or stash them before you merge.
> >> Aborting
> >>
> >> Confusingly, this fixes the issue:
> >>
> >>> git config merge.autostash true
> >>> git pull --rebase
> >> Updating cd9ff8a..f3c9840
> >> Created autostash: c615fda
> >> Fast-forward
> >>   content | 1 +
> >>   1 file changed, 1 insertion(+)
> >> Applied autostash.
> >>
> >> Leaving me wonder why merge config options fix rebase behavior.
> >>
> >> So, in order to make it easier to check the problem, I added some
> >> test-cases to the git test-suite. Please see the attached patch.
>
> Thanks, this really makes it easier to bisect the issue.
>
> >>
> >> Or here:
> >> https://github.com/tvogel/git/commit/bc941f9357518a34cfa11788dfb8e7fa7f711705
> >>
> >> I did not try to find the root-cause as I am not experienced with the
> >> code-base but if there are questions, let me know.
> >
> > Which version are you running?
> >
>
> That's a good info to include indeed. I'm guessing you are using v2.34.1 as that's the version
> indicated at the bottom of your attached patch. I can replicate the behaviour on my side on 2.34.1.
> I did not bisect manually but I'm pretty sure it's a regression caused by 340062243a (pull: cleanup autostash
> check, 2021-06-17) (author CC'ed). I checked that the parent of that commit passes all 4 of your added tests, provided
> this is squashed in:
>
> diff --git a/t/t5521-pull-options.sh b/t/t5521-pull-options.sh
> index 4046a74cad..5ad19b1028 100755
> --- a/t/t5521-pull-options.sh
> +++ b/t/t5521-pull-options.sh
> @@ -260,7 +260,6 @@ test_expect_success 'git pull --rebase --autostash succeeds on ff' '
>         test_commit -C src --printf "more_content" file "more content\ncontent\n" &&
>         echo "dirty" >>dst/file &&
>         git -C dst pull --rebase --autostash >actual 2>&1 &&
> -       grep -q "Fast-forward" actual &&
>         grep -q "Applied autostash." actual
>   '
>
> @@ -273,7 +272,6 @@ test_expect_success 'git pull --rebase with rebase.autostash succeeds on ff' '
>         echo "dirty" >>dst/file &&
>         test_config -C dst rebase.autostash true &&
>         git -C dst pull --rebase  >actual 2>&1 &&
> -       grep -q "Fast-forward" actual &&
>         grep -q "Applied autostash." actual
>   '
>
> After that commit, in case of fast-forward, 'git pull --rebase --autostash' delegates the fast-forward
> operation to 'git merge' under the hood, which was not the case before. The '--autostash' flag seems
> to be forwarded correctly to that 'git merge' invocation, but the config 'rebase.autostash' seems to not
> be passed along.
>
> I did not yet look into why in the code itself. That does explain however why 'merge.autostash' makes it
> work - the 'git merge' invocation does check its own config, and if merge.autostash is there the autostash
> behaviour is activated.
>
> Philippe.
>
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [BUG] git pull --rebase ignores rebase.autostash config when fast-forwarding
       [not found]       ` <BB019E6A-90ED-4D5A-A756-FA8700897DE7@gmail.com>
@ 2022-01-04 23:20         ` Philippe Blain
  0 siblings, 0 replies; 6+ messages in thread
From: Philippe Blain @ 2022-01-04 23:20 UTC (permalink / raw)
  To: John Cai; +Cc: git

Hi John,

Le 2022-01-04 à 13:49, John Cai a écrit :
> 
>> On Jan 4, 2022, at 1:29 PM, Philippe Blain <levraiphilippeblain@gmail.com <mailto:levraiphilippeblain@gmail.com>> wrote:
>>
>> Hi again,
>>
>> Le 2022-01-04 à 13:03, Philippe Blain a écrit :
>>> Hi Tilman,
>>> Le 2022-01-04 à 07:59, Philip Oakley a écrit :
>>>> On 03/01/2022 18:08, Tilman Vogel wrote:
>>>>> Hi git-people,
>>>>>
>>>>> I ran into strange behavior when having rebase.autostash enabled and
>>>>> doing a git pull --rebase:
>>>>>
>>>>>> git config rebase.autostash true
>>>>>> git pull --rebase
>>>>> Updating cd9ff8a..f3c9840
>>>>> error: Your local changes to the following files would be overwritten by
>>>>> merge:
>>>>>          content
>>>>> Please commit your changes or stash them before you merge.
>>>>> Aborting
>>>>>
>>>>> Confusingly, this fixes the issue:
>>>>>
>>>>>> git config merge.autostash true
>>>>>> git pull --rebase
>>>>> Updating cd9ff8a..f3c9840
>>>>> Created autostash: c615fda
>>>>> Fast-forward
>>>>>   content | 1 +
>>>>>   1 file changed, 1 insertion(+)
>>>>> Applied autostash.
>>>>>
>>>>> Leaving me wonder why merge config options fix rebase behavior.
>>>>>
>>>>> So, in order to make it easier to check the problem, I added some
>>>>> test-cases to the git test-suite. Please see the attached patch.
>>> Thanks, this really makes it easier to bisect the issue.
>>>>>
>>>>> Or here:
>>>>> https://github.com/tvogel/git/commit/bc941f9357518a34cfa11788dfb8e7fa7f711705 <https://github.com/tvogel/git/commit/bc941f9357518a34cfa11788dfb8e7fa7f711705>
>>>>>
>>>>> I did not try to find the root-cause as I am not experienced with the
>>>>> code-base but if there are questions, let me know.
>>>>
>>>> Which version are you running?
>>>>
>>> That's a good info to include indeed. I'm guessing you are using v2.34.1 as that's the version
>>> indicated at the bottom of your attached patch. I can replicate the behaviour on my side on 2.34.1.
>>> I did not bisect manually but I'm pretty sure it's a regression caused by 340062243a (pull: cleanup autostash
>>> check, 2021-06-17) (author CC'ed). I checked that the parent of that commit passes all 4 of your added tests, provided
>>> this is squashed in:
>>> diff --git a/t/t5521-pull-options.sh b/t/t5521-pull-options.sh
>>> index 4046a74cad..5ad19b1028 100755
>>> --- a/t/t5521-pull-options.sh
>>> +++ b/t/t5521-pull-options.sh
>>> @@ -260,7 +260,6 @@ test_expect_success 'git pull --rebase --autostash succeeds on ff' '
>>>      test_commit -C src --printf "more_content" file "more content\ncontent\n" &&
>>>      echo "dirty" >>dst/file &&
>>>      git -C dst pull --rebase --autostash >actual 2>&1 &&
>>> -    grep -q "Fast-forward" actual &&
>>>      grep -q "Applied autostash." actual
>>>  '
>>> @@ -273,7 +272,6 @@ test_expect_success 'git pull --rebase with rebase.autostash succeeds on ff' '
>>>      echo "dirty" >>dst/file &&
>>>      test_config -C dst rebase.autostash true &&
>>>      git -C dst pull --rebase  >actual 2>&1 &&
>>> -    grep -q "Fast-forward" actual &&
>>>      grep -q "Applied autostash." actual
>>>  '
>>> After that commit, in case of fast-forward, 'git pull --rebase --autostash' delegates the fast-forward
>>> operation to 'git merge' under the hood, which was not the case before. The '--autostash' flag seems
>>> to be forwarded correctly to that 'git merge' invocation, but the config 'rebase.autostash' seems to not
>>> be passed along.
>>> I did not yet look into why in the code itself
>>
>> After looking at it a bit, this seems to fix the bug:
>>
>> diff --git a/builtin/pull.c b/builtin/pull.c
>> index 1cfaf9f343..0b206bf1d5 100644
>> --- a/builtin/pull.c
>> +++ b/builtin/pull.c
>> @@ -87,6 +87,7 @@ static char *opt_verify_signatures;
>> static char *opt_verify;
>> static int opt_autostash = -1;
>> static int config_autostash;
>> +static int autostash = -1;
>> static int check_trust_level = 1;
>> static struct strvec opt_strategies = STRVEC_INIT;
>> static struct strvec opt_strategy_opts = STRVEC_INIT;
>> @@ -687,9 +688,9 @@ static int run_merge(void)
>> strvec_pushv(&args, opt_strategy_opts.v);
>> if (opt_gpg_sign)
>> strvec_push(&args, opt_gpg_sign);
>> -if (opt_autostash == 0)
>> +if (autostash == 0)
>> strvec_push(&args, "--no-autostash");
>> -else if (opt_autostash == 1)
>> +else if (autostash == 1)
>> strvec_push(&args, "--autostash");
>> if (opt_allow_unrelated_histories > 0)
>> strvec_push(&args, "--allow-unrelated-histories");
>> @@ -1036,7 +1037,7 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
>> oidclr(&orig_head);
>> if (opt_rebase) {
>> -int autostash = config_autostash;
>> +autostash = config_autostash;
>> if (opt_autostash != -1)
>> autostash = opt_autostash;

It seems your email client is messing up whitespace (I'm guessing you might be
using the Gmail web UI, it's known to have this problem), you should try to
find a email client that does not have this behaviour :)

>>
>>
>> If nobody beats me to it (if that's the case, be my guest !), I'll try to submit a proper patch later
>> today or this week.
> 
> (Resending due to my previous message containing HTML and being rejected by the mail server)

It seems this here email did not either reach the mailing list archive.

> 
> Hi Philippe,
> 
> Looks like we were working on this in parallel :) I have a PR I was about to submit as a patch through GGG: github.com/git/git/pull/1179 <http://github.com/git/git/pull/1179>
> 
> My fix is very similar to yours. I can add you ad a co-author if you’d like?
> 

Thanks for asking. I saw you sent your patch at [1] with a "Co-authored-by" trailer.
I would have prefered that you wait for my approbation before adding that "Co-authored-by",
since the code you are adding is different than what I posted above.

I'll comment some more on your patch there.

Cheers,

Philippe.

[1] https://lore.kernel.org/git/20220104214522.10692-1-johncai86@gmail.com/T/#t

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2022-01-04 23:20 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-03 18:08 [BUG] git pull --rebase ignores rebase.autostash config when fast-forwarding Tilman Vogel
2022-01-04 12:59 ` Philip Oakley
2022-01-04 18:03   ` Philippe Blain
2022-01-04 18:29     ` Philippe Blain
     [not found]       ` <BB019E6A-90ED-4D5A-A756-FA8700897DE7@gmail.com>
2022-01-04 23:20         ` Philippe Blain
2022-01-04 21:03     ` Tilman Vogel

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).