Hi John,
Le 2022-01-04 à 13:49, John Cai a écrit :
On Jan 4, 2022, at 1:29 PM, Philippe Blain <levraiphilippeblain@xxxxxxxxx <mailto:levraiphilippeblain@xxxxxxxxx>> 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@xxxxxxxxx/T/#t
