git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [BUG] git pull ignores pull.autostash=true configuration when used with --git-dir and --work-tree flags on a bare repository
@ 2025-07-15  3:26 Bryan Lee
  2025-07-15  4:09 ` Lidong Yan
  0 siblings, 1 reply; 23+ messages in thread
From: Bryan Lee @ 2025-07-15  3:26 UTC (permalink / raw)
  To: git

What did you do before the bug happened? (Steps to reproduce your issue)

I am using a bare Git repository to manage my dotfiles, following a
common pattern where the Git directory is separate from the work tree.
Here are the exact steps to reproduce the issue:

1. Create a bare repository and set up the alias:
   $ git init --bare $HOME/.dotfiles
   Initialized empty Git repository in /Users/bryan/.dotfiles/

   $ alias dot='git --git-dir=$HOME/.dotfiles/ --work-tree=$HOME'

2. Add a remote and create initial tracked file:
   $ dot remote add origin git@github.com:username/dotfiles.git

   $ echo "# My dotfiles" > $HOME/README.md
   $ dot add $HOME/README.md
   $ dot commit -m "Initial commit"
   [main (root-commit) abc1234] Initial commit
    1 file changed, 1 insertion(+)
    create mode 100644 README.md

   $ dot push -u origin main
   Branch 'main' set up to track remote branch 'main' from 'origin'.

3. Set global Git configuration for automatic rebasing and stashing:
   $ git config --global pull.rebase true
   $ git config --global pull.autostash true

   Verify the configuration is set:
   $ git config --global pull.rebase
   true
   $ git config --global pull.autostash
   true

4. Simulate a remote change (on another machine or via GitHub web interface):
   - Edit README.md on remote to add a line: "Updated from remote"
   - This creates a divergence between local and remote

5. Make local unstaged changes:
   $ echo "Local change" >> $HOME/.zshrc

   Verify there are unstaged changes:
   $ dot status
   On branch main
   Changes not staged for commit:
     (use "git add <file>..." to update what will be committed)
     (use "git restore <file>..." to discard changes in working directory)
    modified:   .zshrc

   no changes added to commit (use "git add" and/or "git commit -a")

6. Attempt to pull the remote changes:
   $ dot pull
   error: cannot pull with rebase: You have unstaged changes.
   error: Please commit or stash them.

What did you expect to happen? (Expected behavior)

Since I have configured pull.autostash=true globally, I expected Git to:

1. Automatically create a stash of my unstaged changes (the modified
.zshrc file)
2. Pull the remote changes with rebase (as configured by pull.rebase=true)
3. Apply the stash after the pull completes successfully
4. Show output similar to:
   Created autostash: abc2345
   First, rewinding head to replay your work on top of it...
   Fast-forwarded main to origin/main.
   Applied autostash.

This is the behavior I get when using Git in a regular (non-bare)
repository with the same configuration.

What happened instead? (Actual behavior)

Git immediately fails with an error message:
error: cannot pull with rebase: You have unstaged changes.
error: Please commit or stash them.

The command exits with status code 1 and does not perform any stashing
or pulling.

What's different between what you expected and what actually happened?

The difference is that Git is not honoring the pull.autostash=true
configuration when the repository is accessed using --git-dir and
--work-tree flags. The autostash feature is completely ignored, and
Git behaves as if pull.autostash=false.

To confirm this is specific to the --git-dir/--work-tree usage
pattern, I tested the following workarounds:

1. Explicit --autostash flag works:
   $ dot pull --rebase --autostash
   Created autostash: def3456
   Current branch main is up to date.
   Applied autostash.

2. The same configuration works in a regular repository:
   $ cd /tmp/test-repo
   $ git init
   $ git config pull.rebase true
   $ git config pull.autostash true
   $ echo "test" > file.txt
   $ git add file.txt
   $ git commit -m "test"
   $ echo "change" >> file.txt
   $ git pull origin main  # This would autostash as expected

Additional diagnostic information:

1. The configuration is properly loaded by Git:
   $ dot config --show-origin pull.autostash
   file:/Users/bryan/.config/git/config true

   $ dot config --show-origin pull.rebase
   file:/Users/bryan/.config/git/config true

2. Even setting the configuration directly in the bare repository doesn't help:
   $ dot config pull.autostash true
   $ dot config pull.rebase true
   $ cat $HOME/.dotfiles/config | grep -A2 "\[pull\]"
   [pull]
    rebase = true
    autostash = true

   $ dot pull
   error: cannot pull with rebase: You have unstaged changes.
   error: Please commit or stash them.

3. Using -c flag to override configuration inline also fails:
   $ git --git-dir=$HOME/.dotfiles/ --work-tree=$HOME -c
pull.autostash=true pull
   error: cannot pull with rebase: You have unstaged changes.
   error: Please commit or stash them.

4. GIT_TRACE output shows the pull command is executed but autostash
is not attempted:
   $ GIT_TRACE=1 dot pull 2>&1 | head -5
   11:09:57.474770 git.c:476               trace: built-in: git pull
   error: cannot pull with rebase: You have unstaged changes.
   error: Please commit or stash them.

This appears to be a bug where the autostash functionality is bypassed
when Git is invoked with --git-dir and --work-tree flags, possibly
because the work tree context is not properly established when the
autostash check occurs.

[System Info]
git version:
git version 2.50.1
cpu: arm64
no commit associated with this build
sizeof-long: 8
sizeof-size_t: 8
shell-path: /bin/sh
feature: fsmonitor--daemon
libcurl: 8.7.1
zlib: 1.2.12
SHA-1: SHA1_DC
SHA-256: SHA256_BLK
uname: Darwin 24.5.0 Darwin Kernel Version 24.5.0: Tue Apr 22 19:54:29
PDT 2025; root:xnu-11417.121.6~2/RELEASE_ARM64_T6030 arm64
compiler info: clang: 17.0.0 (clang-1700.0.13.3)
libc info: no libc information available
$SHELL (typically, interactive shell): /bin/zsh

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

* Re: [BUG] git pull ignores pull.autostash=true configuration when used with --git-dir and --work-tree flags on a bare repository
  2025-07-15  3:26 [BUG] git pull ignores pull.autostash=true configuration when used with --git-dir and --work-tree flags on a bare repository Bryan Lee
@ 2025-07-15  4:09 ` Lidong Yan
  2025-07-15  5:31   ` Bryan Lee
  2025-07-15  5:31   ` Bryan Lee
  0 siblings, 2 replies; 23+ messages in thread
From: Lidong Yan @ 2025-07-15  4:09 UTC (permalink / raw)
  To: Bryan Lee; +Cc: git

Bryan Lee <hi@looping.me> wrote:
> 
> 3. Set global Git configuration for automatic rebasing and stashing:
>   $ git config --global pull.rebase true
>   $ git config --global pull.autostash true
> 
>   Verify the configuration is set:
>   $ git config --global pull.rebase
>   true
>   $ git config --global pull.autostash
>   true

Maybe you can try `git config rebase.autostash true` instead.

> The difference is that Git is not honoring the pull.autostash=true
> configuration when the repository is accessed using --git-dir and
> --work-tree flags. The autostash feature is completely ignored, and
> Git behaves as if pull.autostash=false.

I’m not sure why this difference happens either.

- Lidong

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

* Re: [BUG] git pull ignores pull.autostash=true configuration when used with --git-dir and --work-tree flags on a bare repository
  2025-07-15  4:09 ` Lidong Yan
@ 2025-07-15  5:31   ` Bryan Lee
  2025-07-15  5:31   ` Bryan Lee
  1 sibling, 0 replies; 23+ messages in thread
From: Bryan Lee @ 2025-07-15  5:31 UTC (permalink / raw)
  To: Lidong Yan; +Cc: git

Lidong Yan <yldhome2d2@gmail.com> wrote:
> Maybe you can try `git config rebase.autostash true` instead.

Thank you! You're absolutely right. After testing, I can confirm that:

1. `pull.autostash` is not a real Git configuration option - it has no
effect whatsoever
2. The correct configuration is `rebase.autoStash=true` (for rebase operations)
3. This issue is not specific to bare repositories - it happens in
regular repos too

Here's my test in a regular (non-bare) repository that proves this:

```
$ git config --global pull.autostash true
$ git config --global pull.rebase true
$ echo "test" >> README.md  # create unstaged changes
$ git pull
error: cannot pull with rebase: You have unstaged changes.
error: Please commit or stash them.

$ git config --global rebase.autostash true
$ git pull
Updating 9571176..5125236
Created autostash: 9ad0490
Fast-forward
[... changes ...]
Applied autostash.
```

This raises an important issue: Git silently accepts invalid
configuration keys without any warning. Users can waste significant
time debugging "why isn't my configuration working?" when the
configuration key doesn't even exist.

Would it be worthwhile to:
1. Add a warning when users set non-existent configuration keys?
2. Or at least document common misconceptions like `pull.autostash` in
the git-config man page?

Thanks again for pointing me in the right direction!

On Tue, Jul 15, 2025 at 12:09 PM Lidong Yan <yldhome2d2@gmail.com> wrote:
>
> Bryan Lee <hi@looping.me> wrote:
> >
> > 3. Set global Git configuration for automatic rebasing and stashing:
> >   $ git config --global pull.rebase true
> >   $ git config --global pull.autostash true
> >
> >   Verify the configuration is set:
> >   $ git config --global pull.rebase
> >   true
> >   $ git config --global pull.autostash
> >   true
>
> Maybe you can try `git config rebase.autostash true` instead.
>
> > The difference is that Git is not honoring the pull.autostash=true
> > configuration when the repository is accessed using --git-dir and
> > --work-tree flags. The autostash feature is completely ignored, and
> > Git behaves as if pull.autostash=false.
>
> I’m not sure why this difference happens either.
>
> - Lidong

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

* Re: [BUG] git pull ignores pull.autostash=true configuration when used with --git-dir and --work-tree flags on a bare repository
  2025-07-15  4:09 ` Lidong Yan
  2025-07-15  5:31   ` Bryan Lee
@ 2025-07-15  5:31   ` Bryan Lee
  2025-07-15 15:02     ` Lidong Yan
  1 sibling, 1 reply; 23+ messages in thread
From: Bryan Lee @ 2025-07-15  5:31 UTC (permalink / raw)
  To: Lidong Yan; +Cc: git

Lidong Yan <yldhome2d2@gmail.com> wrote:
> Maybe you can try `git config rebase.autostash true` instead.

Thank you! You're absolutely right. After testing, I can confirm that:

1. `pull.autostash` is not a real Git configuration option - it has no
effect whatsoever
2. The correct configuration is `rebase.autoStash=true` (for rebase operations)
3. This issue is not specific to bare repositories - it happens in
regular repos too

Here's my test in a regular (non-bare) repository that proves this:

```
$ git config --global pull.autostash true
$ git config --global pull.rebase true
$ echo "test" >> README.md  # create unstaged changes
$ git pull
error: cannot pull with rebase: You have unstaged changes.
error: Please commit or stash them.

$ git config --global rebase.autostash true
$ git pull
Updating 9571176..5125236
Created autostash: 9ad0490
Fast-forward
[... changes ...]
Applied autostash.
```

This raises an important issue: Git silently accepts invalid
configuration keys without any warning. Users can waste significant
time debugging "why isn't my configuration working?" when the
configuration key doesn't even exist.

Would it be worthwhile to:
1. Add a warning when users set non-existent configuration keys?
2. Or at least document common misconceptions like `pull.autostash` in
the git-config man page?

Thanks again for pointing me in the right direction!

On Tue, Jul 15, 2025 at 12:09 PM Lidong Yan <yldhome2d2@gmail.com> wrote:
>
> Bryan Lee <hi@looping.me> wrote:
> >
> > 3. Set global Git configuration for automatic rebasing and stashing:
> >   $ git config --global pull.rebase true
> >   $ git config --global pull.autostash true
> >
> >   Verify the configuration is set:
> >   $ git config --global pull.rebase
> >   true
> >   $ git config --global pull.autostash
> >   true
>
> Maybe you can try `git config rebase.autostash true` instead.
>
> > The difference is that Git is not honoring the pull.autostash=true
> > configuration when the repository is accessed using --git-dir and
> > --work-tree flags. The autostash feature is completely ignored, and
> > Git behaves as if pull.autostash=false.
>
> I’m not sure why this difference happens either.
>
> - Lidong

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

* Re: [BUG] git pull ignores pull.autostash=true configuration when used with --git-dir and --work-tree flags on a bare repository
  2025-07-15  5:31   ` Bryan Lee
@ 2025-07-15 15:02     ` Lidong Yan
  2025-07-15 20:46       ` Junio C Hamano
  0 siblings, 1 reply; 23+ messages in thread
From: Lidong Yan @ 2025-07-15 15:02 UTC (permalink / raw)
  To: Bryan Lee; +Cc: git

Bryan Lee <hi@looping.me> wrote:
> 
> Would it be worthwhile to:
> 1. Add a warning when users set non-existent configuration keys?
> 2. Or at least document common misconceptions like `pull.autostash` in
> the git-config man page?

I think adding a subcommand like ‘git config verify’ might be a way to
solve this problem.

- Lidong


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

* Re: [BUG] git pull ignores pull.autostash=true configuration when used with --git-dir and --work-tree flags on a bare repository
  2025-07-15 15:02     ` Lidong Yan
@ 2025-07-15 20:46       ` Junio C Hamano
  2025-07-16  1:39         ` Lidong Yan
  0 siblings, 1 reply; 23+ messages in thread
From: Junio C Hamano @ 2025-07-15 20:46 UTC (permalink / raw)
  To: Lidong Yan; +Cc: Bryan Lee, git

Lidong Yan <yldhome2d2@gmail.com> writes:

> Bryan Lee <hi@looping.me> wrote:
>> 
>> Would it be worthwhile to:
>> 1. Add a warning when users set non-existent configuration keys?
>> 2. Or at least document common misconceptions like `pull.autostash` in
>> the git-config man page?
>
> I think adding a subcommand like ‘git config verify’ might be a way to
> solve this problem.

Yes, but I do not know if it is feasible.

There always are end-user or third-party defined keys that are not
known to us, and we cannot tell if an unknown variable is such a
end-user defined one or a typo of a known one.


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

* Re: [BUG] git pull ignores pull.autostash=true configuration when used with --git-dir and --work-tree flags on a bare repository
  2025-07-15 20:46       ` Junio C Hamano
@ 2025-07-16  1:39         ` Lidong Yan
  2025-07-16  5:55           ` Johannes Sixt
  0 siblings, 1 reply; 23+ messages in thread
From: Lidong Yan @ 2025-07-16  1:39 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Bryan Lee, git

Junio C Hamano <gitster@pobox.com> writes:
> 
> Lidong Yan <yldhome2d2@gmail.com> writes:
> 
>> Bryan Lee <hi@looping.me> wrote:
>>> 
>>> Would it be worthwhile to:
>>> 1. Add a warning when users set non-existent configuration keys?
>>> 2. Or at least document common misconceptions like `pull.autostash` in
>>> the git-config man page?
>> 
>> I think adding a subcommand like ‘git config verify’ might be a way to
>> solve this problem.
> 
> Yes, but I do not know if it is feasible.
> 
> There always are end-user or third-party defined keys that are not
> known to us, and we cannot tell if an unknown variable is such a
> end-user defined one or a typo of a known one.

For every git_xxx_config(), we could add a register function like
git_xxx_config_register(), which looks like:

  int git_xxx_config_register()
  {
    struct key_ent ent;

    register_bool_key("key1");
    register_int_key("key2”);
    register_date_key("key3”);

    ent.key = “key4”;
    ent.desp = “key4_desp";
    ent.verify_fn = &verify_key4_value;
    register_key(ent);
  }


And then end-user could define their own register function as well
so that they could also use `git config verify` to verify their own config
<key, value>.

Or end-user could provide a .gitconfigspec and `git config verify` will
load .gitconfigspec to verify whether there exists some invalid config
<k, v> pair.

- Lidong

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

* Re: [BUG] git pull ignores pull.autostash=true configuration when used with --git-dir and --work-tree flags on a bare repository
  2025-07-16  1:39         ` Lidong Yan
@ 2025-07-16  5:55           ` Johannes Sixt
  2025-07-16 11:20             ` Lidong Yan
  2025-07-16 15:16             ` Junio C Hamano
  0 siblings, 2 replies; 23+ messages in thread
From: Johannes Sixt @ 2025-07-16  5:55 UTC (permalink / raw)
  To: Lidong Yan; +Cc: Bryan Lee, git, Junio C Hamano

Am 16.07.25 um 03:39 schrieb Lidong Yan:
> Junio C Hamano <gitster@pobox.com> writes:
>> Lidong Yan <yldhome2d2@gmail.com> writes:
>>> Bryan Lee <hi@looping.me> wrote:
>>>> 2. Or at least document common misconceptions like `pull.autostash` in
>>>> the git-config man page?

>> There always are end-user or third-party defined keys that are not
>> known to us, and we cannot tell if an unknown variable is such a
>> end-user defined one or a typo of a known one.
> 
> For every git_xxx_config(), we could add a register function like
> git_xxx_config_register(), which looks like:

Instead of this complexity, it is most likely a lot easier to fix the
origin of the misconception that `pull.autostash` is the correct
configuration. After all, it isn't even mentioned in the git-config nor
the git-pull man page.

-- Hannes


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

* Re: [BUG] git pull ignores pull.autostash=true configuration when used with --git-dir and --work-tree flags on a bare repository
  2025-07-16  5:55           ` Johannes Sixt
@ 2025-07-16 11:20             ` Lidong Yan
  2025-07-16 15:16             ` Junio C Hamano
  1 sibling, 0 replies; 23+ messages in thread
From: Lidong Yan @ 2025-07-16 11:20 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Bryan Lee, git, Junio C Hamano

Johannes Sixt <j6t@kdbg.org> write:
> 
> Instead of this complexity, it is most likely a lot easier to fix the
> origin of the misconception that `pull.autostash` is the correct
> configuration. After all, it isn't even mentioned in the git-config nor
> the git-pull man page.

Yes, update the document is the more important thing.

- Lidong

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

* Re: [BUG] git pull ignores pull.autostash=true configuration when used with --git-dir and --work-tree flags on a bare repository
  2025-07-16  5:55           ` Johannes Sixt
  2025-07-16 11:20             ` Lidong Yan
@ 2025-07-16 15:16             ` Junio C Hamano
  2025-07-17  3:07               ` [PATCH] pull: add pull.autoStash config option Lidong Yan
  2025-07-17 19:32               ` [BUG] git pull ignores pull.autostash=true configuration when used with --git-dir and --work-tree flags on a bare repository Ben Knoble
  1 sibling, 2 replies; 23+ messages in thread
From: Junio C Hamano @ 2025-07-16 15:16 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Lidong Yan, Bryan Lee, git

Johannes Sixt <j6t@kdbg.org> writes:

> Instead of this complexity, it is most likely a lot easier to fix the
> origin of the misconception that `pull.autostash` is the correct
> configuration. After all, it isn't even mentioned in the git-config nor
> the git-pull man page.

git_pull_config() does pay attention to "rebase.autostash".  

Either it is a bug for the code to do so, or it is a bug that the
documentation does not talk about it.  

The reason why I think "git pull" that pays attention to
rebase.autostash is a bug is because the user is more likely to be
much more familiar with both branches involved and more likely to be
prepared to deal with conflicts potentially created by autostashing
behaviour when making a private merge or rebase of local branches,
than when pulling from other repositories.  So those who show
willingness to accept the responsibility of having to resolve
conflicts that arise when popping autostashed changes by setting
rebase.autostash may not want to be cavalier to the same degree when
running "git pull".  git_pull_config() that pays attention to
"rebase.autostash" breaks that expectation.

There is another curiosity.  git_pull_config() does not pay
attention to "merge.autostash", which seems inconsistent.

If I did not have any existing users, I would actually vote to teach
git_pull_config() stop paying attention to "rebase.autostash", but
we do not live in an ideal world.  Perhaps rectify this at Git 3.0?

We could give a convenience feature in the opposite direction as
well.

It is not inconceivable for the git_pull_config() function to
pretend as if rebase.autostash (when pull.rebase is true) or
merge.autostash (otherwise) is set to true when pull.autostash is
set to true.  It would have prevented this discussion thread from
happening.

I personally think that such an arrangement is backwards, though,
for the same reason why I think git_pull_config() should not pay
attention to "rebase.autostash".  So I am not sure if a new
"pull.autostash" variable is such a good idea to begin with.



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

* [PATCH] pull: add pull.autoStash config option
  2025-07-16 15:16             ` Junio C Hamano
@ 2025-07-17  3:07               ` Lidong Yan
  2025-07-17  3:27                 ` Eric Sunshine
  2025-07-18  3:52                 ` Lidong Yan
  2025-07-17 19:32               ` [BUG] git pull ignores pull.autostash=true configuration when used with --git-dir and --work-tree flags on a bare repository Ben Knoble
  1 sibling, 2 replies; 23+ messages in thread
From: Lidong Yan @ 2025-07-17  3:07 UTC (permalink / raw)
  To: gitster; +Cc: git, hi, j6t, yldhome2d2

Git uses the `rebase.autoStash` option to decide if git-pull is allowed
when the working tree has uncommitted changes. However, since the
documentation does not explicitly state this, users may find it difficult
to associate `rebase.autoStash` with the git-pull command. Add
`pull.autoStash` option along with its documentation.

`pull.autoStash` provides the same functionality as `rebase.autoStash`
but is more user-friendly because its prefix clearly associates it
with git-pull commands. Additionally, when both options are set,
`pull.autoStash` takes precedence and overrides the value of
`rebase.autoStash`.

Signed-off-by: Lidong Yan <yldhome2d2@gmail.com>
---
 Documentation/config/pull.adoc |  9 +++++++++
 builtin/pull.c                 | 10 +++++++---
 2 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/Documentation/config/pull.adoc b/Documentation/config/pull.adoc
index 9349e09261..da9686dbd2 100644
--- a/Documentation/config/pull.adoc
+++ b/Documentation/config/pull.adoc
@@ -13,6 +13,15 @@ pull.rebase::
 	of merging the default branch from the default remote when "git
 	pull" is run. See "branch.<name>.rebase" for setting this on a
 	per-branch basis.
+
+pull.autoStash::
+	When true, Git will automatically perform a `git stash` before the
+	operation and then restore the local changes with `git stash pop`
+	after the merge or rebase is complete. This means that you can run
+	pull on a dirty worktree. Noticed that `rebase.autoStash` provides
+	the same functionality, but `pull.autoStash` overrides its behavior
+	when both are set. This option can be overridden by the `--no-autostash`
+	and `--autostash` options of linkgit:git-pull[1]. Defaults to false.
 +
 When `merges` (or just 'm'), pass the `--rebase-merges` option to 'git rebase'
 so that the local merge commits are included in the rebase (see
diff --git a/builtin/pull.c b/builtin/pull.c
index c593f324fe..dfc3d4656b 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -90,7 +90,8 @@ static char *opt_ff;
 static const char *opt_verify_signatures;
 static const char *opt_verify;
 static int opt_autostash = -1;
-static int config_autostash;
+static int config_rebase_autostash;
+static int config_pull_autostash = -1;
 static int check_trust_level = 1;
 static struct strvec opt_strategies = STRVEC_INIT;
 static struct strvec opt_strategy_opts = STRVEC_INIT;
@@ -367,7 +368,10 @@ static int git_pull_config(const char *var, const char *value,
 			   const struct config_context *ctx, void *cb)
 {
 	if (!strcmp(var, "rebase.autostash")) {
-		config_autostash = git_config_bool(var, value);
+		config_rebase_autostash = git_config_bool(var, value);
+		return 0;
+	} else if (!strcmp(var, "pull.autostash")) {
+		config_pull_autostash = git_config_bool(var, value);
 		return 0;
 	} else if (!strcmp(var, "submodule.recurse")) {
 		recurse_submodules = git_config_bool(var, value) ?
@@ -1052,7 +1056,7 @@ int cmd_pull(int argc,
 
 	if (opt_rebase) {
 		if (opt_autostash == -1)
-			opt_autostash = config_autostash;
+			opt_autostash = config_pull_autostash == -1 ? config_rebase_autostash : config_pull_autostash;
 
 		if (is_null_oid(&orig_head) && !is_index_unborn(the_repository->index))
 			die(_("Updating an unborn branch with changes added to the index."));
-- 
2.39.5 (Apple Git-154)


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

* Re: [PATCH] pull: add pull.autoStash config option
  2025-07-17  3:07               ` [PATCH] pull: add pull.autoStash config option Lidong Yan
@ 2025-07-17  3:27                 ` Eric Sunshine
  2025-07-17  4:09                   ` Lidong Yan
  2025-07-17  4:44                   ` Junio C Hamano
  2025-07-18  3:52                 ` Lidong Yan
  1 sibling, 2 replies; 23+ messages in thread
From: Eric Sunshine @ 2025-07-17  3:27 UTC (permalink / raw)
  To: Lidong Yan; +Cc: gitster, git, hi, j6t

On Wed, Jul 16, 2025 at 11:07 PM Lidong Yan <yldhome2d2@gmail.com> wrote:
> Git uses the `rebase.autoStash` option to decide if git-pull is allowed
> when the working tree has uncommitted changes. However, since the
> documentation does not explicitly state this, users may find it difficult
> to associate `rebase.autoStash` with the git-pull command. Add
> `pull.autoStash` option along with its documentation.

I have no opinion as to whether this is a good path forward.

> `pull.autoStash` provides the same functionality as `rebase.autoStash`
> but is more user-friendly because its prefix clearly associates it
> with git-pull commands.

Rather than "user-friendly", perhaps a better way to phrase it would
be to say that `pull.autoStash` is more *discoverable*.

> Additionally, when both options are set,
> `pull.autoStash` takes precedence and overrides the value of
> `rebase.autoStash`.

This was a question which immediately popped into my head, so it's
nice to see that you considered it and discussed it in the commit
message.

> Signed-off-by: Lidong Yan <yldhome2d2@gmail.com>
> ---
>  Documentation/config/pull.adoc |  9 +++++++++
>  builtin/pull.c                 | 10 +++++++---
>  2 files changed, 16 insertions(+), 3 deletions(-)

You will want to add one or more new tests to a test script to verify
that this new configuration works as expected, and probably also to
verify that `pull.autoStash` takes precedence over `rebase.autoStash`.

> diff --git a/Documentation/config/pull.adoc b/Documentation/config/pull.adoc
> @@ -13,6 +13,15 @@ pull.rebase::
> +pull.autoStash::
> +       When true, Git will automatically perform a `git stash` before the
> +       operation and then restore the local changes with `git stash pop`
> +       after the merge or rebase is complete. This means that you can run

I wonder if you meant "pull" instead of "merge or rebase".

> +       pull on a dirty worktree. Noticed that `rebase.autoStash` provides

s/Noticed/Notice/

> +       the same functionality, but `pull.autoStash` overrides its behavior

Rather: "...same functionality as `pull.autoStash` but overrides the
latter when..."

> +       when both are set. This option can be overridden by the `--no-autostash`
> +       and `--autostash` options of linkgit:git-pull[1]. Defaults to false.
> diff --git a/builtin/pull.c b/builtin/pull.c
> @@ -1052,7 +1056,7 @@ int cmd_pull(int argc,
>         if (opt_rebase) {
>                 if (opt_autostash == -1)
> -                       opt_autostash = config_autostash;
> +                       opt_autostash = config_pull_autostash == -1 ? config_rebase_autostash : config_pull_autostash;

You may want to wrap this over-long line. Perhaps:

    opt_autostash = config_pull_autostash == -1 ?
        config_rebase_autostash : config_pull_autostash;

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

* Re: [PATCH] pull: add pull.autoStash config option
  2025-07-17  3:27                 ` Eric Sunshine
@ 2025-07-17  4:09                   ` Lidong Yan
  2025-07-17  4:37                     ` Junio C Hamano
  2025-07-17  4:44                   ` Junio C Hamano
  1 sibling, 1 reply; 23+ messages in thread
From: Lidong Yan @ 2025-07-17  4:09 UTC (permalink / raw)
  To: Eric Sunshine; +Cc: gitster, git, j6t

Eric Sunshine <sunshine@sunshineco.com> write:
> You will want to add one or more new tests to a test script to verify
> that this new configuration works as expected, and probably also to
> verify that `pull.autoStash` takes precedence over `rebase.autoStash`.

Got it. Always make sure to add tests when introducing new features.

>> diff --git a/Documentation/config/pull.adoc b/Documentation/config/pull.adoc
>> @@ -13,6 +13,15 @@ pull.rebase::
>> +pull.autoStash::
>> +       When true, Git will automatically perform a `git stash` before the
>> +       operation and then restore the local changes with `git stash pop`
>> +       after the merge or rebase is complete. This means that you can run
> 
> I wonder if you meant "pull" instead of "merge or rebase".

Yes, I think I should also say that pull.autoStash only works if we set pull.rebase.

> 
>> +       pull on a dirty worktree. Noticed that `rebase.autoStash` provides
> 
> s/Noticed/Notice/
> 
>> +       the same functionality, but `pull.autoStash` overrides its behavior
> 
> Rather: "...same functionality as `pull.autoStash` but overrides the
> latter when..."

Got it.

> 
>> +       when both are set. This option can be overridden by the `--no-autostash`
>> +       and `--autostash` options of linkgit:git-pull[1]. Defaults to false.
>> diff --git a/builtin/pull.c b/builtin/pull.c
>> @@ -1052,7 +1056,7 @@ int cmd_pull(int argc,
>>        if (opt_rebase) {
>>                if (opt_autostash == -1)
>> -                       opt_autostash = config_autostash;
>> +                       opt_autostash = config_pull_autostash == -1 ? config_rebase_autostash : config_pull_autostash;
> 
> You may want to wrap this over-long line. Perhaps:
> 
>    opt_autostash = config_pull_autostash == -1 ?
>        config_rebase_autostash : config_pull_autostash;

Here's something completely unrelated: the output of clang-format can
sometimes be confusing.

- opt_autostash = config_pull_autostash == -1 ?
-       config_rebase_autostash :
-       config_pull_autostash;
+ opt_autostash = config_pull_autostash == -1 ? config_rebase_autostash : config_pull_autostash;

This made me mistakenly think that Git had set a large line length limit,
So I didn’t break this line here.

Thanks,
Lidong


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

* Re: [PATCH] pull: add pull.autoStash config option
  2025-07-17  4:09                   ` Lidong Yan
@ 2025-07-17  4:37                     ` Junio C Hamano
  2025-07-17  5:01                       ` Lidong Yan
  0 siblings, 1 reply; 23+ messages in thread
From: Junio C Hamano @ 2025-07-17  4:37 UTC (permalink / raw)
  To: Lidong Yan; +Cc: Eric Sunshine, git, j6t

Lidong Yan <yldhome2d2@gmail.com> writes:

>>> +pull.autoStash::
>>> +       When true, Git will automatically perform a `git stash` before the
>>> +       operation and then restore the local changes with `git stash pop`
>>> +       after the merge or rebase is complete. This means that you can run
>> 
>> I wonder if you meant "pull" instead of "merge or rebase".
>
> Yes, I think I should also say that pull.autoStash only works if we set pull.rebase.

Is that wise, though?  When pull.rebase is false, shouldn't pull.autostash
pass --autostash to underlying "git merge" instead?

I've written about the interaction among three commands and
autostash in another message several hours ago, so I won't repeat
it.

https://lore.kernel.org/git/xmqq5xfsdv3w.fsf@gitster.g/


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

* Re: [PATCH] pull: add pull.autoStash config option
  2025-07-17  3:27                 ` Eric Sunshine
  2025-07-17  4:09                   ` Lidong Yan
@ 2025-07-17  4:44                   ` Junio C Hamano
  1 sibling, 0 replies; 23+ messages in thread
From: Junio C Hamano @ 2025-07-17  4:44 UTC (permalink / raw)
  To: Eric Sunshine; +Cc: Lidong Yan, git, hi, j6t

Eric Sunshine <sunshine@sunshineco.com> writes:

>>                 if (opt_autostash == -1)
>> -                       opt_autostash = config_autostash;
>> +                       opt_autostash = config_pull_autostash == -1 ? config_rebase_autostash : config_pull_autostash;
>
> You may want to wrap this over-long line. Perhaps:
>
>     opt_autostash = config_pull_autostash == -1 ?
>         config_rebase_autostash : config_pull_autostash;

Alternatively

	opt_autostash = (config_pull_autostash == -1
			 ? config_rebase_autostash
			 : config_pull_autostash);


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

* Re: [PATCH] pull: add pull.autoStash config option
  2025-07-17  4:37                     ` Junio C Hamano
@ 2025-07-17  5:01                       ` Lidong Yan
  2025-07-17  5:48                         ` Junio C Hamano
  0 siblings, 1 reply; 23+ messages in thread
From: Lidong Yan @ 2025-07-17  5:01 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Eric Sunshine, git, j6t

Junio C Hamano <gitster@pobox.com> write:
> 
> Lidong Yan <yldhome2d2@gmail.com> writes:
> 
>>>> +pull.autoStash::
>>>> +       When true, Git will automatically perform a `git stash` before the
>>>> +       operation and then restore the local changes with `git stash pop`
>>>> +       after the merge or rebase is complete. This means that you can run
>>> 
>>> I wonder if you meant "pull" instead of "merge or rebase".
>> 
>> Yes, I think I should also say that pull.autoStash only works if we set pull.rebase.
> 
> Is that wise, though?  When pull.rebase is false, shouldn't pull.autostash
> pass --autostash to underlying "git merge" instead?

Here set pull.rebase means either set pull.rebase to true or false. And
my patch will autostash if pull.rebase = false. 

> I've written about the interaction among three commands and
> autostash in another message several hours ago, so I won't repeat
> it.
> 
> https://lore.kernel.org/git/xmqq5xfsdv3w.fsf@gitster.g/

Yeah. Though I considered that when pull.rebase = false, checking
merge.autoStash instead of rebase.autoStashmight confuse users
who rely on setting rebase.autoStash for their merge operations.

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

* Re: [PATCH] pull: add pull.autoStash config option
  2025-07-17  5:01                       ` Lidong Yan
@ 2025-07-17  5:48                         ` Junio C Hamano
  0 siblings, 0 replies; 23+ messages in thread
From: Junio C Hamano @ 2025-07-17  5:48 UTC (permalink / raw)
  To: Lidong Yan; +Cc: Eric Sunshine, git, j6t

Lidong Yan <yldhome2d2@gmail.com> writes:

> Yeah. Though I considered that when pull.rebase = false, checking
> merge.autoStash instead of rebase.autoStashmight confuse users
> who rely on setting rebase.autoStash for their merge operations.

Yes, that would be a behaviour change, but it is just one time
thing.  You notice your "git pull" do not autostash and instead
stops, you scratch your head and go read the documentation, and set
pull.autostash to true (which as I said I would not necessarily
recommend) or merge.autostash to true (which may not be as bad) and
move on.

I didn't consider it when I wrote my earlier message, but I like
your idea of defeating rebase.autostash and merge.autostash when
pull.autostash is explicitly set to false very much.  With it, users
can set {rebase,merge}.autostash to true so that their local rebases
and merges, for which they are familiar with what both sides did,
would autostash (and autounstash), and set pull.autostash to false
so their "git pull" would be stopped when they have local changes
that would interfere with the operation, if they wanted to.

Thanks.



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

* Re: [BUG] git pull ignores pull.autostash=true configuration when used with --git-dir and --work-tree flags on a bare repository
  2025-07-16 15:16             ` Junio C Hamano
  2025-07-17  3:07               ` [PATCH] pull: add pull.autoStash config option Lidong Yan
@ 2025-07-17 19:32               ` Ben Knoble
  1 sibling, 0 replies; 23+ messages in thread
From: Ben Knoble @ 2025-07-17 19:32 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Johannes Sixt, Lidong Yan, Bryan Lee, git


> Le 16 juil. 2025 à 11:17, Junio C Hamano <gitster@pobox.com> a écrit :
> 
> Johannes Sixt <j6t@kdbg.org> writes:
> 
>> Instead of this complexity, it is most likely a lot easier to fix the
>> origin of the misconception that `pull.autostash` is the correct
>> configuration. After all, it isn't even mentioned in the git-config nor
>> the git-pull man page.
> 
> git_pull_config() does pay attention to "rebase.autostash".  
> 
> Either it is a bug for the code to do so, or it is a bug that the
> documentation does not talk about it.  
> 
> The reason why I think "git pull" that pays attention to
> rebase.autostash is a bug is because the user is more likely to be
> much more familiar with both branches involved and more likely to be
> prepared to deal with conflicts potentially created by autostashing
> behaviour when making a private merge or rebase of local branches,
> than when pulling from other repositories.  So those who show
> willingness to accept the responsibility of having to resolve
> conflicts that arise when popping autostashed changes by setting
> rebase.autostash may not want to be cavalier to the same degree when
> running "git pull".  git_pull_config() that pays attention to
> "rebase.autostash" breaks that expectation.

On the other hand, a pull that rebases is (conceptually) a fetch followed by a rebase, and there is a lot of description and teaching of pull as fetch+merge. Breaking that expectation is also unnatural.

I would consider it far more inconsistent if pulls that rebase don’t honor rebase configuration. So put me in the camp that pull should probably respect merge.autostash, too. (I don’t have any opinion about pull.autostash, which seems reasonable on the surface.)

> 
> There is another curiosity.  git_pull_config() does not pay
> attention to "merge.autostash", which seems inconsistent.
> 
> If I did not have any existing users, I would actually vote to teach
> git_pull_config() stop paying attention to "rebase.autostash", but
> we do not live in an ideal world.  Perhaps rectify this at Git 3.0?

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

* [PATCH] pull: add pull.autoStash config option
  2025-07-17  3:07               ` [PATCH] pull: add pull.autoStash config option Lidong Yan
  2025-07-17  3:27                 ` Eric Sunshine
@ 2025-07-18  3:52                 ` Lidong Yan
  2025-07-18 22:13                   ` Junio C Hamano
  2025-07-20 12:43                   ` [PATCH v2] " Lidong Yan
  1 sibling, 2 replies; 23+ messages in thread
From: Lidong Yan @ 2025-07-18  3:52 UTC (permalink / raw)
  To: yldhome2d2; +Cc: git, gitster, hi, j6t

Git uses `rebase.autostash` or `merge.autostash` to determine whether a
dirty worktree is allowed during pull. However, this behavior is not
clearly documented, making it difficult for users to discover how to
enable autostash, or causing them to unknowingly enable it. Add new
config option `pull.autostash` along with its documentation and test
cases.

`pull.autostash` provides the same functionality as `rebase.autostash`
and `merge.autostash`, but overrides them when set. If `pull.autostash`
is not set, it falls back to `rebase.autostash` or `merge.autostash`,
depending on the value of `pull.rebase`.

Signed-off-by: Lidong Yan <yldhome2d2@gmail.com>
---
 Documentation/config/pull.adoc | 11 +++++
 builtin/pull.c                 | 20 ++++++--
 t/t5520-pull.sh                | 90 ++++++++++++++++++++++++++++++++++
 3 files changed, 118 insertions(+), 3 deletions(-)

diff --git a/Documentation/config/pull.adoc b/Documentation/config/pull.adoc
index 9349e09261..3aa1e67923 100644
--- a/Documentation/config/pull.adoc
+++ b/Documentation/config/pull.adoc
@@ -13,6 +13,17 @@ pull.rebase::
 	of merging the default branch from the default remote when "git
 	pull" is run. See "branch.<name>.rebase" for setting this on a
 	per-branch basis.
+
+pull.autoStash::
+	When true, Git will automatically perform a `git stash` before the
+	operation and then restore the local changes with `git stash pop`
+	after the pull is complete. This means that you can run pull on a
+	dirty worktree. If `pull.autostash` is set, it takes precedence over
+	`rebase.autostash` and `merge.autostash`. If `pull.autostash` is not
+	set, it falls back to `rebase.autostash` or `merge.autostash`,
+	depending on the value of `pull.rebase`. This option can be
+	overridden by the `--no-autostash` and `--autostash` options of
+	linkgit:git-pull[1]. Defaults to false.
 +
 When `merges` (or just 'm'), pass the `--rebase-merges` option to 'git rebase'
 so that the local merge commits are included in the rebase (see
diff --git a/builtin/pull.c b/builtin/pull.c
index c593f324fe..2a6c2e4a37 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -90,7 +90,8 @@ static char *opt_ff;
 static const char *opt_verify_signatures;
 static const char *opt_verify;
 static int opt_autostash = -1;
-static int config_autostash;
+static int config_rebase_autostash;
+static int config_pull_autostash = -1;
 static int check_trust_level = 1;
 static struct strvec opt_strategies = STRVEC_INIT;
 static struct strvec opt_strategy_opts = STRVEC_INIT;
@@ -367,7 +368,18 @@ static int git_pull_config(const char *var, const char *value,
 			   const struct config_context *ctx, void *cb)
 {
 	if (!strcmp(var, "rebase.autostash")) {
-		config_autostash = git_config_bool(var, value);
+		/*
+		 * run_rebase() also reads this option. The reason we handle it here is
+		 * that when pull.rebase is true, a fast-forward may occur without
+		 * invoking run_rebase(). We need to ensure that autostash is set even
+		 * in the fast-forward case.
+		 *
+		 * run_merge() handles merge.autostash, so we don't handle it here.
+		 */
+		config_rebase_autostash = git_config_bool(var, value);
+		return 0;
+	} else if (!strcmp(var, "pull.autostash")) {
+		config_pull_autostash = git_config_bool(var, value);
 		return 0;
 	} else if (!strcmp(var, "submodule.recurse")) {
 		recurse_submodules = git_config_bool(var, value) ?
@@ -1006,6 +1018,8 @@ int cmd_pull(int argc,
 	}
 
 	argc = parse_options(argc, argv, prefix, pull_options, pull_usage, 0);
+	if (opt_autostash == -1)
+		opt_autostash = config_pull_autostash;
 
 	if (recurse_submodules_cli != RECURSE_SUBMODULES_DEFAULT)
 		recurse_submodules = recurse_submodules_cli;
@@ -1052,7 +1066,7 @@ int cmd_pull(int argc,
 
 	if (opt_rebase) {
 		if (opt_autostash == -1)
-			opt_autostash = config_autostash;
+			opt_autostash = config_rebase_autostash;
 
 		if (is_null_oid(&orig_head) && !is_index_unborn(the_repository->index))
 			die(_("Updating an unborn branch with changes added to the index."));
diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index 63c9a8f04b..134da2185c 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -472,6 +472,96 @@ test_expect_success 'pull --no-autostash & merge.autostash unset' '
 	test_pull_autostash_fail --no-autostash --no-rebase
 '
 
+test_expect_success 'pull succeeds with dirty working directory and pull.autostash set' '
+	test_config pull.autostash true &&
+	test_pull_autostash 1 --rebase &&
+	test_pull_autostash 2 --no-rebase
+'
+
+test_expect_success 'pull --autostash & pull.autostash=true' '
+	test_config pull.autostash true &&
+	test_pull_autostash 1 --autostash --rebase &&
+	test_pull_autostash 2 --autostash --no-rebase
+'
+
+test_expect_success 'pull --autostash & pull.autostash=false' '
+	test_config pull.autostash false &&
+	test_pull_autostash 1 --autostash --rebase &&
+	test_pull_autostash 2 --autostash --no-rebase
+'
+
+test_expect_success 'pull --autostash & pull.autostash unset' '
+	test_unconfig pull.autostash &&
+	test_pull_autostash 1 --autostash --rebase &&
+	test_pull_autostash 2 --autostash --no-rebase
+'
+
+test_expect_success 'pull --no-autostash & pull.autostash=true' '
+	test_config pull.autostash true &&
+	test_pull_autostash_fail --no-autostash --rebase &&
+	test_pull_autostash_fail --no-autostash --no-rebase
+'
+
+test_expect_success 'pull --no-autostash & pull.autostash=false' '
+	test_config pull.autostash false &&
+	test_pull_autostash_fail --no-autostash --rebase &&
+	test_pull_autostash_fail --no-autostash --no-rebase
+'
+
+test_expect_success 'pull --no-autostash & pull.autostash unset' '
+	test_unconfig pull.autostash &&
+	test_pull_autostash_fail --no-autostash --rebase &&
+	test_pull_autostash_fail --no-autostash --no-rebase
+'
+
+test_expect_success 'pull.autostash=true & rebase.autostash=true' '
+	test_config pull.autostash true &&
+	test_config rebase.autostash true &&
+	test_pull_autostash 1 --rebase
+'
+
+test_expect_success 'pull.autostash=true & rebase.autostash=false' '
+	test_config pull.autostash true &&
+	test_config rebase.autostash false &&
+	test_pull_autostash 1 --rebase
+'
+
+test_expect_success 'pull.autostash=false & rebase.autostash=true' '
+	test_config pull.autostash false &&
+	test_config rebase.autostash true &&
+	test_pull_autostash_fail --rebase
+'
+
+test_expect_success 'pull.autostash=false & rebase.autostash=false' '
+	test_config pull.autostash false &&
+	test_config rebase.autostash false &&
+	test_pull_autostash_fail --rebase
+'
+
+test_expect_success 'pull.autostash=true & merge.autostash=true' '
+	test_config pull.autostash true &&
+	test_config merge.autostash true &&
+	test_pull_autostash 2 --no-rebase
+'
+
+test_expect_success 'pull.autostash=true & merge.autostash=false' '
+	test_config pull.autostash true &&
+	test_config merge.autostash false &&
+	test_pull_autostash 2 --no-rebase
+'
+
+test_expect_success 'pull.autostash=false & merge.autostash=true' '
+	test_config pull.autostash false &&
+	test_config merge.autostash true &&
+	test_pull_autostash_fail --no-rebase
+'
+
+test_expect_success 'pull.autostash=false & merge.autostash=false' '
+	test_config pull.autostash false &&
+	test_config merge.autostash false &&
+	test_pull_autostash_fail --no-rebase
+'
+
 test_expect_success 'pull.rebase' '
 	git reset --hard before-rebase &&
 	test_config pull.rebase true &&
-- 
2.39.5 (Apple Git-154)


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

* Re: [PATCH] pull: add pull.autoStash config option
  2025-07-18  3:52                 ` Lidong Yan
@ 2025-07-18 22:13                   ` Junio C Hamano
  2025-07-19  3:14                     ` Lidong Yan
  2025-07-20 12:43                   ` [PATCH v2] " Lidong Yan
  1 sibling, 1 reply; 23+ messages in thread
From: Junio C Hamano @ 2025-07-18 22:13 UTC (permalink / raw)
  To: Lidong Yan; +Cc: git, hi, j6t

Lidong Yan <yldhome2d2@gmail.com> writes:

> Git uses `rebase.autostash` or `merge.autostash` to determine whether a
> dirty worktree is allowed during pull. However, this behavior is not
> clearly documented, making it difficult for users to discover how to
> enable autostash, or causing them to unknowingly enable it. Add new
> config option `pull.autostash` along with its documentation and test
> cases.
>
> `pull.autostash` provides the same functionality as `rebase.autostash`
> and `merge.autostash`, but overrides them when set. If `pull.autostash`
> is not set, it falls back to `rebase.autostash` or `merge.autostash`,
> depending on the value of `pull.rebase`.

Very well reasoned and described.

> diff --git a/Documentation/config/pull.adoc b/Documentation/config/pull.adoc
> index 9349e09261..3aa1e67923 100644
> --- a/Documentation/config/pull.adoc
> +++ b/Documentation/config/pull.adoc
> @@ -13,6 +13,17 @@ pull.rebase::
>  	of merging the default branch from the default remote when "git
>  	pull" is run. See "branch.<name>.rebase" for setting this on a
>  	per-branch basis.
> +
> +pull.autoStash::
> +	When true, Git will automatically perform a `git stash` before the
> +	operation and then restore the local changes with `git stash pop`
> +	after the pull is complete. This means that you can run pull on a
> +	dirty worktree. If `pull.autostash` is set, it takes precedence over
> +	`rebase.autostash` and `merge.autostash`. If `pull.autostash` is not
> +	set, it falls back to `rebase.autostash` or `merge.autostash`,
> +	depending on the value of `pull.rebase`. This option can be
> +	overridden by the `--no-autostash` and `--autostash` options of
> +	linkgit:git-pull[1]. Defaults to false.
>  +
>  When `merges` (or just 'm'), pass the `--rebase-merges` option to 'git rebase'
>  so that the local merge commits are included in the rebase (see

The new text is inserted at a wrong spot.  This "+\nWhen `merges`"
is a continuation of the text that describes `pull.rebase`.  If that
is set to `true`, one thing happens.  If that is set to `merges`,
something else happens.

Insert the text for `pull.autoStash` immediately before the
description of the `pull.octopus` configuration variable.

As to the text itself, "you can run pull on a dirty worktree" may
not be what you want to say here, for a few reasons.

 * (pedantic) Even without the configuration variable set, you can
   run "git pull" in a dirty working tree; it just will refuse to do
   any damage until you stash the local changes away yourself.

 * If your "git pull" merges, it would work even in a dirty working
   tree as long as your local change doesn't overlap with what the
   merge would bring in.  This is quite useful for a maintainer with
   "upcoming" change to GIT-VERSION-GEN always updated locally in
   the working tree and not having to worry about pulling from
   contributors and submaintainers who won't usually be touching
   that file, for example.

 * Not limited to this instance, when you have to say "(This|It)
   means <<B>>" immediately after making a statement <<A>, I would
   like us to think if we can just say <<B>> without saying <<A> at
   all.  In this case, it is not so, which makes me suspect that
   perhaps we do not even want to say <<B>>, as it may not mean
   <<B>> after all.

Here is my attempt.

    When set to true, automatically create a temporary stash entry
    to record the local changes before the operation begins, and
    restore them after the operation completes.  When your "git
    pull" rebases (instead of merges), this may be convenient, since
    unlike merging pull that tolerates local changes that do not
    interfere with the merge, rebasing pull refuses to work with any
    local changes.
+
If `pull.autostash` is set (either to true or false),
`merge.autostash` and `rebase.autostash` are ignored.  If
`pull.autostash` is not set at all, depending on the value of
`pull.rebase`, `merge.autostash` or `rebase.autostash` is used
instead.  Can be overridden by the `--[no-]autostash` command line
option.

> diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
> index 63c9a8f04b..134da2185c 100755
> --- a/t/t5520-pull.sh
> +++ b/t/t5520-pull.sh
> @@ -472,6 +472,96 @@ test_expect_success 'pull --no-autostash & merge.autostash unset' '
>  	test_pull_autostash_fail --no-autostash --no-rebase
>  '
>  
> +test_expect_success 'pull succeeds with dirty working directory and pull.autostash set' '
> +	test_config pull.autostash true &&
> +	test_pull_autostash 1 --rebase &&
> +	test_pull_autostash 2 --no-rebase
> +'

Most trivial case.  No command line override.

> +test_expect_success 'pull --autostash & pull.autostash=true' '
> +	test_config pull.autostash true &&
> +	test_pull_autostash 1 --autostash --rebase &&
> +	test_pull_autostash 2 --autostash --no-rebase
> +'

Command line override specifies the same behaviour as the
configuration, so we cannot learn much from this test.  It still
should keep working, so the test is worth having [*], but I wonder
if makes sense to combine the above two into one test, i.e. set the
configuration variable to true once, and then try --rebase and
--no-rebase with and without --autostash (four combinations).

    [*] In this review, unless I explicitly say "this test is wrong
    and expects an incorrect result", they are not wrong, even
    though what they test may not be as interesting as others, and I
    am not suggesting its removal.  This is one of these tests.

> +test_expect_success 'pull --autostash & pull.autostash=false' '
> +	test_config pull.autostash false &&
> +	test_pull_autostash 1 --autostash --rebase &&
> +	test_pull_autostash 2 --autostash --no-rebase
> +'

Configuration should be overridden by the command line option, which
is a good thing to test.

> +test_expect_success 'pull --autostash & pull.autostash unset' '
> +	test_unconfig pull.autostash &&
> +	test_pull_autostash 1 --autostash --rebase &&
> +	test_pull_autostash 2 --autostash --no-rebase
> +'

Another most trivial case.  Shouldn't we already have an existing
test for this, back from the days before pull.autostash got
introduced, since the command line option has been there all along?

> +test_expect_success 'pull --no-autostash & pull.autostash=true' '
> +	test_config pull.autostash true &&
> +	test_pull_autostash_fail --no-autostash --rebase &&
> +	test_pull_autostash_fail --no-autostash --no-rebase
> +'

Configuration overridden by the option, opposite of what we saw
earlier, which is another good thing to test.

> +test_expect_success 'pull --no-autostash & pull.autostash=false' '
> +	test_config pull.autostash false &&
> +	test_pull_autostash_fail --no-autostash --rebase &&
> +	test_pull_autostash_fail --no-autostash --no-rebase
> +'

Uninteresting test that does not tell us much; we cannot tell which
between the configuration and the command line option caused us not
to auto stash with this test.

Two cases that may be worth adding to this test immediately after
setting pull.autostash to false are:

	test_pull_autostash_fail --rebase &&
	test_pull_autostash_fail --no-rebase &&

> +test_expect_success 'pull --no-autostash & pull.autostash unset' '
> +	test_unconfig pull.autostash &&
> +	test_pull_autostash_fail --no-autostash --rebase &&
> +	test_pull_autostash_fail --no-autostash --no-rebase
> +'

Another uninteresting case that probably should be already covered
by existing test, since this tests "what happens when autostash is
explicitly declined from the command line when there is no
configuration variable to intervene?".

> +test_expect_success 'pull.autostash=true & rebase.autostash=true' '
> +	test_config pull.autostash true &&
> +	test_config rebase.autostash true &&
> +	test_pull_autostash 1 --rebase
> +'

OK.  Perhaps make sure "--no-autostash --rebase" would fail while at
it in the same test?

> +test_expect_success 'pull.autostash=true & rebase.autostash=false' '
> +	test_config pull.autostash true &&
> +	test_config rebase.autostash false &&
> +	test_pull_autostash 1 --rebase
> +'

This is more interesting than the previous one, as we make sure that
pull.* trumps rebase.* with this test.  Perhaps throw --no-autostash
specified on the command line into the mix?

> +test_expect_success 'pull.autostash=false & rebase.autostash=true' '
> +	test_config pull.autostash false &&
> +	test_config rebase.autostash true &&
> +	test_pull_autostash_fail --rebase
> +'

Another good one.  It might be intereseting to test --no-rebase and
make sure it also fails?  I dunno.

> +test_expect_success 'pull.autostash=false & rebase.autostash=false' '
> +	test_config pull.autostash false &&
> +	test_config rebase.autostash false &&
> +	test_pull_autostash_fail --rebase
> +'

Not as interesting as others.

> +test_expect_success 'pull.autostash=true & merge.autostash=true' '
> +	test_config pull.autostash true &&
> +	test_config merge.autostash true &&
> +	test_pull_autostash 2 --no-rebase
> +'

Not as interesting as others.  Throw --no-autostash given on the
command line into the mix as well?

> +test_expect_success 'pull.autostash=true & merge.autostash=false' '
> +	test_config pull.autostash true &&
> +	test_config merge.autostash false &&
> +	test_pull_autostash 2 --no-rebase
> +'

OK.  pull.*=true trumps merge.*=false.  We test the other way around
next.  Good.

> +test_expect_success 'pull.autostash=false & merge.autostash=true' '
> +	test_config pull.autostash false &&
> +	test_config merge.autostash true &&
> +	test_pull_autostash_fail --no-rebase
> +'
> +
> +test_expect_success 'pull.autostash=false & merge.autostash=false' '
> +	test_config pull.autostash false &&
> +	test_config merge.autostash false &&
> +	test_pull_autostash_fail --no-rebase
> +'

Not very interesting.  Throw anothre that gives --autostash from the
command line in the mix, perhaps?

>  test_expect_success 'pull.rebase' '
>  	git reset --hard before-rebase &&
>  	test_config pull.rebase true &&


Whew.

I did not spot anything majory broken (except for the location to
which the new documentation paragraph goes) in the patch.  Nicely
done.

Thanks.

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

* Re: [PATCH] pull: add pull.autoStash config option
  2025-07-18 22:13                   ` Junio C Hamano
@ 2025-07-19  3:14                     ` Lidong Yan
  0 siblings, 0 replies; 23+ messages in thread
From: Lidong Yan @ 2025-07-19  3:14 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, hi, j6t

Junio C Hamano <gitster@pobox.com> writes:
> 
> Lidong Yan <yldhome2d2@gmail.com> writes:
> 
>> Git uses `rebase.autostash` or `merge.autostash` to determine whether a
>> dirty worktree is allowed during pull. However, this behavior is not
>> clearly documented, making it difficult for users to discover how to
>> enable autostash, or causing them to unknowingly enable it. Add new
>> config option `pull.autostash` along with its documentation and test
>> cases.
>> 
>> `pull.autostash` provides the same functionality as `rebase.autostash`
>> and `merge.autostash`, but overrides them when set. If `pull.autostash`
>> is not set, it falls back to `rebase.autostash` or `merge.autostash`,
>> depending on the value of `pull.rebase`.
> 
> Very well reasoned and described.
> 
>> diff --git a/Documentation/config/pull.adoc b/Documentation/config/pull.adoc
>> index 9349e09261..3aa1e67923 100644
>> --- a/Documentation/config/pull.adoc
>> +++ b/Documentation/config/pull.adoc
>> @@ -13,6 +13,17 @@ pull.rebase::
>> of merging the default branch from the default remote when "git
>> pull" is run. See "branch.<name>.rebase" for setting this on a
>> per-branch basis.
>> +
>> +pull.autoStash::
>> + When true, Git will automatically perform a `git stash` before the
>> + operation and then restore the local changes with `git stash pop`
>> + after the pull is complete. This means that you can run pull on a
>> + dirty worktree. If `pull.autostash` is set, it takes precedence over
>> + `rebase.autostash` and `merge.autostash`. If `pull.autostash` is not
>> + set, it falls back to `rebase.autostash` or `merge.autostash`,
>> + depending on the value of `pull.rebase`. This option can be
>> + overridden by the `--no-autostash` and `--autostash` options of
>> + linkgit:git-pull[1]. Defaults to false.
>> +
>> When `merges` (or just 'm'), pass the `--rebase-merges` option to 'git rebase'
>> so that the local merge commits are included in the rebase (see
> 
> The new text is inserted at a wrong spot.  This "+\nWhen `merges`"
> is a continuation of the text that describes `pull.rebase`.  If that
> is set to `true`, one thing happens.  If that is set to `merges`,
> something else happens.
> 
> Insert the text for `pull.autoStash` immediately before the
> description of the `pull.octopus` configuration variable.
> 
> As to the text itself, "you can run pull on a dirty worktree" may
> not be what you want to say here, for a few reasons.
> 
> * (pedantic) Even without the configuration variable set, you can
>   run "git pull" in a dirty working tree; it just will refuse to do
>   any damage until you stash the local changes away yourself.
> 
> * If your "git pull" merges, it would work even in a dirty working
>   tree as long as your local change doesn't overlap with what the
>   merge would bring in.  This is quite useful for a maintainer with
>   "upcoming" change to GIT-VERSION-GEN always updated locally in
>   the working tree and not having to worry about pulling from
>   contributors and submaintainers who won't usually be touching
>   that file, for example.
> 
> * Not limited to this instance, when you have to say "(This|It)
>   means <<B>>" immediately after making a statement <<A>, I would
>   like us to think if we can just say <<B>> without saying <<A> at
>   all.  In this case, it is not so, which makes me suspect that
>   perhaps we do not even want to say <<B>>, as it may not mean
>   <<B>> after all.
> 
> Here is my attempt.
> 
>    When set to true, automatically create a temporary stash entry
>    to record the local changes before the operation begins, and
>    restore them after the operation completes.  When your "git
>    pull" rebases (instead of merges), this may be convenient, since
>    unlike merging pull that tolerates local changes that do not
>    interfere with the merge, rebasing pull refuses to work with any
>    local changes.
> +
> If `pull.autostash` is set (either to true or false),
> `merge.autostash` and `rebase.autostash` are ignored.  If
> `pull.autostash` is not set at all, depending on the value of
> `pull.rebase`, `merge.autostash` or `rebase.autostash` is used
> instead.  Can be overridden by the `--[no-]autostash` command line
> option.
> 
>> diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
>> index 63c9a8f04b..134da2185c 100755
>> --- a/t/t5520-pull.sh
>> +++ b/t/t5520-pull.sh
>> @@ -472,6 +472,96 @@ test_expect_success 'pull --no-autostash & merge.autostash unset' '
>> test_pull_autostash_fail --no-autostash --no-rebase
>> '
>> 
>> +test_expect_success 'pull succeeds with dirty working directory and pull.autostash set' '
>> + test_config pull.autostash true &&
>> + test_pull_autostash 1 --rebase &&
>> + test_pull_autostash 2 --no-rebase
>> +'
> 
> Most trivial case.  No command line override.
> 
>> +test_expect_success 'pull --autostash & pull.autostash=true' '
>> + test_config pull.autostash true &&
>> + test_pull_autostash 1 --autostash --rebase &&
>> + test_pull_autostash 2 --autostash --no-rebase
>> +'
> 
> Command line override specifies the same behaviour as the
> configuration, so we cannot learn much from this test.  It still
> should keep working, so the test is worth having [*], but I wonder
> if makes sense to combine the above two into one test, i.e. set the
> configuration variable to true once, and then try --rebase and
> --no-rebase with and without --autostash (four combinations).
> 
>    [*] In this review, unless I explicitly say "this test is wrong
>    and expects an incorrect result", they are not wrong, even
>    though what they test may not be as interesting as others, and I
>    am not suggesting its removal.  This is one of these tests.
> 
>> +test_expect_success 'pull --autostash & pull.autostash=false' '
>> + test_config pull.autostash false &&
>> + test_pull_autostash 1 --autostash --rebase &&
>> + test_pull_autostash 2 --autostash --no-rebase
>> +'
> 
> Configuration should be overridden by the command line option, which
> is a good thing to test.
> 
>> +test_expect_success 'pull --autostash & pull.autostash unset' '
>> + test_unconfig pull.autostash &&
>> + test_pull_autostash 1 --autostash --rebase &&
>> + test_pull_autostash 2 --autostash --no-rebase
>> +'
> 
> Another most trivial case.  Shouldn't we already have an existing
> test for this, back from the days before pull.autostash got
> introduced, since the command line option has been there all along?
> 
>> +test_expect_success 'pull --no-autostash & pull.autostash=true' '
>> + test_config pull.autostash true &&
>> + test_pull_autostash_fail --no-autostash --rebase &&
>> + test_pull_autostash_fail --no-autostash --no-rebase
>> +'
> 
> Configuration overridden by the option, opposite of what we saw
> earlier, which is another good thing to test.
> 
>> +test_expect_success 'pull --no-autostash & pull.autostash=false' '
>> + test_config pull.autostash false &&
>> + test_pull_autostash_fail --no-autostash --rebase &&
>> + test_pull_autostash_fail --no-autostash --no-rebase
>> +'
> 
> Uninteresting test that does not tell us much; we cannot tell which
> between the configuration and the command line option caused us not
> to auto stash with this test.
> 
> Two cases that may be worth adding to this test immediately after
> setting pull.autostash to false are:
> 
> test_pull_autostash_fail --rebase &&
> test_pull_autostash_fail --no-rebase &&
> 
>> +test_expect_success 'pull --no-autostash & pull.autostash unset' '
>> + test_unconfig pull.autostash &&
>> + test_pull_autostash_fail --no-autostash --rebase &&
>> + test_pull_autostash_fail --no-autostash --no-rebase
>> +'
> 
> Another uninteresting case that probably should be already covered
> by existing test, since this tests "what happens when autostash is
> explicitly declined from the command line when there is no
> configuration variable to intervene?".
> 
>> +test_expect_success 'pull.autostash=true & rebase.autostash=true' '
>> + test_config pull.autostash true &&
>> + test_config rebase.autostash true &&
>> + test_pull_autostash 1 --rebase
>> +'
> 
> OK.  Perhaps make sure "--no-autostash --rebase" would fail while at
> it in the same test?
> 
>> +test_expect_success 'pull.autostash=true & rebase.autostash=false' '
>> + test_config pull.autostash true &&
>> + test_config rebase.autostash false &&
>> + test_pull_autostash 1 --rebase
>> +'
> 
> This is more interesting than the previous one, as we make sure that
> pull.* trumps rebase.* with this test.  Perhaps throw --no-autostash
> specified on the command line into the mix?
> 
>> +test_expect_success 'pull.autostash=false & rebase.autostash=true' '
>> + test_config pull.autostash false &&
>> + test_config rebase.autostash true &&
>> + test_pull_autostash_fail --rebase
>> +'
> 
> Another good one.  It might be intereseting to test --no-rebase and
> make sure it also fails?  I dunno.
> 
>> +test_expect_success 'pull.autostash=false & rebase.autostash=false' '
>> + test_config pull.autostash false &&
>> + test_config rebase.autostash false &&
>> + test_pull_autostash_fail --rebase
>> +'
> 
> Not as interesting as others.
> 
>> +test_expect_success 'pull.autostash=true & merge.autostash=true' '
>> + test_config pull.autostash true &&
>> + test_config merge.autostash true &&
>> + test_pull_autostash 2 --no-rebase
>> +'
> 
> Not as interesting as others.  Throw --no-autostash given on the
> command line into the mix as well?
> 
>> +test_expect_success 'pull.autostash=true & merge.autostash=false' '
>> + test_config pull.autostash true &&
>> + test_config merge.autostash false &&
>> + test_pull_autostash 2 --no-rebase
>> +'
> 
> OK.  pull.*=true trumps merge.*=false.  We test the other way around
> next.  Good.
> 
>> +test_expect_success 'pull.autostash=false & merge.autostash=true' '
>> + test_config pull.autostash false &&
>> + test_config merge.autostash true &&
>> + test_pull_autostash_fail --no-rebase
>> +'
>> +
>> +test_expect_success 'pull.autostash=false & merge.autostash=false' '
>> + test_config pull.autostash false &&
>> + test_config merge.autostash false &&
>> + test_pull_autostash_fail --no-rebase
>> +'
> 
> Not very interesting.  Throw anothre that gives --autostash from the
> command line in the mix, perhaps?
> 
>> test_expect_success 'pull.rebase' '
>> git reset --hard before-rebase &&
>> test_config pull.rebase true &&


Thank you very much for your thorough review. I will reorganize the
documentation and test cases in v2.

- Lidong

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

* [PATCH v2] pull: add pull.autoStash config option
  2025-07-18  3:52                 ` Lidong Yan
  2025-07-18 22:13                   ` Junio C Hamano
@ 2025-07-20 12:43                   ` Lidong Yan
  2025-07-21 22:10                     ` Junio C Hamano
  1 sibling, 1 reply; 23+ messages in thread
From: Lidong Yan @ 2025-07-20 12:43 UTC (permalink / raw)
  To: yldhome2d2; +Cc: git, gitster, hi, j6t, sunshine

Git uses `rebase.autostash` or `merge.autostash` to determine whether a
dirty worktree is allowed during pull. However, this behavior is not
clearly documented, making it difficult for users to discover how to
enable autostash, or causing them to unknowingly enable it. Add new
config option `pull.autostash` along with its documentation and test
cases.

`pull.autostash` provides the same functionality as `rebase.autostash`
and `merge.autostash`, but overrides them when set. If `pull.autostash`
is not set, it falls back to `rebase.autostash` or `merge.autostash`,
depending on the value of `pull.rebase`.

Signed-off-by: Lidong Yan <yldhome2d2@gmail.com>
---
Range-diff against v1:
1:  5b7d10d7e9 ! 1:  51a7c66783 pull: add pull.autoStash config option
    @@ Commit message
         Signed-off-by: Lidong Yan <yldhome2d2@gmail.com>
     
      ## Documentation/config/pull.adoc ##
    -@@ Documentation/config/pull.adoc: pull.rebase::
    - 	of merging the default branch from the default remote when "git
    - 	pull" is run. See "branch.<name>.rebase" for setting this on a
    - 	per-branch basis.
    -+
    +@@ Documentation/config/pull.adoc: pull.octopus::
    + 	The default merge strategy to use when pulling multiple branches
    + 	at once.
    + 
     +pull.autoStash::
    -+	When true, Git will automatically perform a `git stash` before the
    -+	operation and then restore the local changes with `git stash pop`
    -+	after the pull is complete. This means that you can run pull on a
    -+	dirty worktree. If `pull.autostash` is set, it takes precedence over
    -+	`rebase.autostash` and `merge.autostash`. If `pull.autostash` is not
    -+	set, it falls back to `rebase.autostash` or `merge.autostash`,
    -+	depending on the value of `pull.rebase`. This option can be
    -+	overridden by the `--no-autostash` and `--autostash` options of
    -+	linkgit:git-pull[1]. Defaults to false.
    - +
    - When `merges` (or just 'm'), pass the `--rebase-merges` option to 'git rebase'
    - so that the local merge commits are included in the rebase (see
    ++	When set to true, automatically create a temporary stash entry
    ++	to record the local changes before the operation begins, and
    ++	restore them after the operation completes.  When your "git
    ++	pull" rebases (instead of merges), this may be convenient, since
    ++	unlike merging pull that tolerates local changes that do not
    ++	interfere with the merge, rebasing pull refuses to work with any
    ++	local changes.
    +++
    ++If `pull.autostash` is set (either to true or false),
    ++`merge.autostash` and `rebase.autostash` are ignored.  If
    ++`pull.autostash` is not set at all, depending on the value of
    ++`pull.rebase`, `merge.autostash` or `rebase.autostash` is used
    ++instead.  Can be overridden by the `--[no-]autostash` command line
    ++option.
    ++
    + pull.twohead::
    + 	The default merge strategy to use when pulling a single branch.
     
      ## builtin/pull.c ##
     @@ builtin/pull.c: static char *opt_ff;
    @@ t/t5520-pull.sh: test_expect_success 'pull --no-autostash & merge.autostash unse
      	test_pull_autostash_fail --no-autostash --no-rebase
      '
      
    -+test_expect_success 'pull succeeds with dirty working directory and pull.autostash set' '
    ++test_expect_success 'pull succeeds with dirty working directory and pull.autostash=true' '
     +	test_config pull.autostash true &&
     +	test_pull_autostash 1 --rebase &&
    -+	test_pull_autostash 2 --no-rebase
    -+'
    -+
    -+test_expect_success 'pull --autostash & pull.autostash=true' '
    -+	test_config pull.autostash true &&
    ++	test_pull_autostash 2 --no-rebase &&
     +	test_pull_autostash 1 --autostash --rebase &&
     +	test_pull_autostash 2 --autostash --no-rebase
     +'
     +
    -+test_expect_success 'pull --autostash & pull.autostash=false' '
    ++test_expect_success 'pull fails with dirty working directory and pull.autostash=false' '
     +	test_config pull.autostash false &&
    -+	test_pull_autostash 1 --autostash --rebase &&
    -+	test_pull_autostash 2 --autostash --no-rebase
    -+'
    -+
    -+test_expect_success 'pull --autostash & pull.autostash unset' '
    -+	test_unconfig pull.autostash &&
    -+	test_pull_autostash 1 --autostash --rebase &&
    -+	test_pull_autostash 2 --autostash --no-rebase
    -+'
    -+
    -+test_expect_success 'pull --no-autostash & pull.autostash=true' '
    -+	test_config pull.autostash true &&
    ++	test_pull_autostash_fail --rebase &&
    ++	test_pull_autostash_fail --no-rebase &&
     +	test_pull_autostash_fail --no-autostash --rebase &&
     +	test_pull_autostash_fail --no-autostash --no-rebase
     +'
     +
    -+test_expect_success 'pull --no-autostash & pull.autostash=false' '
    ++test_expect_success 'pull --autostash overrides pull.autostash=false' '
     +	test_config pull.autostash false &&
    -+	test_pull_autostash_fail --no-autostash --rebase &&
    -+	test_pull_autostash_fail --no-autostash --no-rebase
    ++	test_pull_autostash 1 --autostash --rebase &&
    ++	test_pull_autostash 2 --autostash --no-rebase
     +'
     +
    -+test_expect_success 'pull --no-autostash & pull.autostash unset' '
    -+	test_unconfig pull.autostash &&
    ++test_expect_success 'pull --no-autostash overrides pull.autostash=true' '
    ++	test_config pull.autostash true &&
     +	test_pull_autostash_fail --no-autostash --rebase &&
     +	test_pull_autostash_fail --no-autostash --no-rebase
     +'
     +
    -+test_expect_success 'pull.autostash=true & rebase.autostash=true' '
    ++test_expect_success 'pull.autostash=true overrides rebase.autostash' '
     +	test_config pull.autostash true &&
     +	test_config rebase.autostash true &&
    -+	test_pull_autostash 1 --rebase
    -+'
    -+
    -+test_expect_success 'pull.autostash=true & rebase.autostash=false' '
    -+	test_config pull.autostash true &&
    ++	test_pull_autostash 1 --rebase &&
     +	test_config rebase.autostash false &&
     +	test_pull_autostash 1 --rebase
     +'
     +
    -+test_expect_success 'pull.autostash=false & rebase.autostash=true' '
    ++test_expect_success 'pull.autostash=false overrides rebase.autostash' '
     +	test_config pull.autostash false &&
     +	test_config rebase.autostash true &&
    -+	test_pull_autostash_fail --rebase
    -+'
    -+
    -+test_expect_success 'pull.autostash=false & rebase.autostash=false' '
    -+	test_config pull.autostash false &&
    ++	test_pull_autostash_fail --rebase &&
     +	test_config rebase.autostash false &&
     +	test_pull_autostash_fail --rebase
     +'
     +
    -+test_expect_success 'pull.autostash=true & merge.autostash=true' '
    ++test_expect_success 'pull.autostash=true overrides merge.autostash' '
     +	test_config pull.autostash true &&
     +	test_config merge.autostash true &&
    -+	test_pull_autostash 2 --no-rebase
    -+'
    -+
    -+test_expect_success 'pull.autostash=true & merge.autostash=false' '
    -+	test_config pull.autostash true &&
    ++	test_pull_autostash 2 --no-rebase &&
     +	test_config merge.autostash false &&
     +	test_pull_autostash 2 --no-rebase
     +'
     +
    -+test_expect_success 'pull.autostash=false & merge.autostash=true' '
    ++test_expect_success 'pull.autostash=false overrides merge.autostash' '
     +	test_config pull.autostash false &&
     +	test_config merge.autostash true &&
    -+	test_pull_autostash_fail --no-rebase
    -+'
    -+
    -+test_expect_success 'pull.autostash=false & merge.autostash=false' '
    -+	test_config pull.autostash false &&
    ++	test_pull_autostash_fail --no-rebase &&
     +	test_config merge.autostash false &&
     +	test_pull_autostash_fail --no-rebase
     +'

 Documentation/config/pull.adoc | 16 +++++++++
 builtin/pull.c                 | 20 ++++++++++--
 t/t5520-pull.sh                | 60 ++++++++++++++++++++++++++++++++++
 3 files changed, 93 insertions(+), 3 deletions(-)

diff --git a/Documentation/config/pull.adoc b/Documentation/config/pull.adoc
index 9349e09261..125c930f72 100644
--- a/Documentation/config/pull.adoc
+++ b/Documentation/config/pull.adoc
@@ -29,5 +29,21 @@ pull.octopus::
 	The default merge strategy to use when pulling multiple branches
 	at once.
 
+pull.autoStash::
+	When set to true, automatically create a temporary stash entry
+	to record the local changes before the operation begins, and
+	restore them after the operation completes.  When your "git
+	pull" rebases (instead of merges), this may be convenient, since
+	unlike merging pull that tolerates local changes that do not
+	interfere with the merge, rebasing pull refuses to work with any
+	local changes.
++
+If `pull.autostash` is set (either to true or false),
+`merge.autostash` and `rebase.autostash` are ignored.  If
+`pull.autostash` is not set at all, depending on the value of
+`pull.rebase`, `merge.autostash` or `rebase.autostash` is used
+instead.  Can be overridden by the `--[no-]autostash` command line
+option.
+
 pull.twohead::
 	The default merge strategy to use when pulling a single branch.
diff --git a/builtin/pull.c b/builtin/pull.c
index c593f324fe..2a6c2e4a37 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -90,7 +90,8 @@ static char *opt_ff;
 static const char *opt_verify_signatures;
 static const char *opt_verify;
 static int opt_autostash = -1;
-static int config_autostash;
+static int config_rebase_autostash;
+static int config_pull_autostash = -1;
 static int check_trust_level = 1;
 static struct strvec opt_strategies = STRVEC_INIT;
 static struct strvec opt_strategy_opts = STRVEC_INIT;
@@ -367,7 +368,18 @@ static int git_pull_config(const char *var, const char *value,
 			   const struct config_context *ctx, void *cb)
 {
 	if (!strcmp(var, "rebase.autostash")) {
-		config_autostash = git_config_bool(var, value);
+		/*
+		 * run_rebase() also reads this option. The reason we handle it here is
+		 * that when pull.rebase is true, a fast-forward may occur without
+		 * invoking run_rebase(). We need to ensure that autostash is set even
+		 * in the fast-forward case.
+		 *
+		 * run_merge() handles merge.autostash, so we don't handle it here.
+		 */
+		config_rebase_autostash = git_config_bool(var, value);
+		return 0;
+	} else if (!strcmp(var, "pull.autostash")) {
+		config_pull_autostash = git_config_bool(var, value);
 		return 0;
 	} else if (!strcmp(var, "submodule.recurse")) {
 		recurse_submodules = git_config_bool(var, value) ?
@@ -1006,6 +1018,8 @@ int cmd_pull(int argc,
 	}
 
 	argc = parse_options(argc, argv, prefix, pull_options, pull_usage, 0);
+	if (opt_autostash == -1)
+		opt_autostash = config_pull_autostash;
 
 	if (recurse_submodules_cli != RECURSE_SUBMODULES_DEFAULT)
 		recurse_submodules = recurse_submodules_cli;
@@ -1052,7 +1066,7 @@ int cmd_pull(int argc,
 
 	if (opt_rebase) {
 		if (opt_autostash == -1)
-			opt_autostash = config_autostash;
+			opt_autostash = config_rebase_autostash;
 
 		if (is_null_oid(&orig_head) && !is_index_unborn(the_repository->index))
 			die(_("Updating an unborn branch with changes added to the index."));
diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index 63c9a8f04b..0e0019347e 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -472,6 +472,66 @@ test_expect_success 'pull --no-autostash & merge.autostash unset' '
 	test_pull_autostash_fail --no-autostash --no-rebase
 '
 
+test_expect_success 'pull succeeds with dirty working directory and pull.autostash=true' '
+	test_config pull.autostash true &&
+	test_pull_autostash 1 --rebase &&
+	test_pull_autostash 2 --no-rebase &&
+	test_pull_autostash 1 --autostash --rebase &&
+	test_pull_autostash 2 --autostash --no-rebase
+'
+
+test_expect_success 'pull fails with dirty working directory and pull.autostash=false' '
+	test_config pull.autostash false &&
+	test_pull_autostash_fail --rebase &&
+	test_pull_autostash_fail --no-rebase &&
+	test_pull_autostash_fail --no-autostash --rebase &&
+	test_pull_autostash_fail --no-autostash --no-rebase
+'
+
+test_expect_success 'pull --autostash overrides pull.autostash=false' '
+	test_config pull.autostash false &&
+	test_pull_autostash 1 --autostash --rebase &&
+	test_pull_autostash 2 --autostash --no-rebase
+'
+
+test_expect_success 'pull --no-autostash overrides pull.autostash=true' '
+	test_config pull.autostash true &&
+	test_pull_autostash_fail --no-autostash --rebase &&
+	test_pull_autostash_fail --no-autostash --no-rebase
+'
+
+test_expect_success 'pull.autostash=true overrides rebase.autostash' '
+	test_config pull.autostash true &&
+	test_config rebase.autostash true &&
+	test_pull_autostash 1 --rebase &&
+	test_config rebase.autostash false &&
+	test_pull_autostash 1 --rebase
+'
+
+test_expect_success 'pull.autostash=false overrides rebase.autostash' '
+	test_config pull.autostash false &&
+	test_config rebase.autostash true &&
+	test_pull_autostash_fail --rebase &&
+	test_config rebase.autostash false &&
+	test_pull_autostash_fail --rebase
+'
+
+test_expect_success 'pull.autostash=true overrides merge.autostash' '
+	test_config pull.autostash true &&
+	test_config merge.autostash true &&
+	test_pull_autostash 2 --no-rebase &&
+	test_config merge.autostash false &&
+	test_pull_autostash 2 --no-rebase
+'
+
+test_expect_success 'pull.autostash=false overrides merge.autostash' '
+	test_config pull.autostash false &&
+	test_config merge.autostash true &&
+	test_pull_autostash_fail --no-rebase &&
+	test_config merge.autostash false &&
+	test_pull_autostash_fail --no-rebase
+'
+
 test_expect_success 'pull.rebase' '
 	git reset --hard before-rebase &&
 	test_config pull.rebase true &&
-- 
2.39.5 (Apple Git-154)


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

* Re: [PATCH v2] pull: add pull.autoStash config option
  2025-07-20 12:43                   ` [PATCH v2] " Lidong Yan
@ 2025-07-21 22:10                     ` Junio C Hamano
  0 siblings, 0 replies; 23+ messages in thread
From: Junio C Hamano @ 2025-07-21 22:10 UTC (permalink / raw)
  To: Lidong Yan; +Cc: git, hi, j6t, sunshine

Lidong Yan <yldhome2d2@gmail.com> writes:

>  Documentation/config/pull.adoc | 16 +++++++++
>  builtin/pull.c                 | 20 ++++++++++--
>  t/t5520-pull.sh                | 60 ++++++++++++++++++++++++++++++++++
>  3 files changed, 93 insertions(+), 3 deletions(-)

Thanks, will replace.

Shall we mark it for 'next' now?

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

end of thread, other threads:[~2025-07-21 22:10 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-15  3:26 [BUG] git pull ignores pull.autostash=true configuration when used with --git-dir and --work-tree flags on a bare repository Bryan Lee
2025-07-15  4:09 ` Lidong Yan
2025-07-15  5:31   ` Bryan Lee
2025-07-15  5:31   ` Bryan Lee
2025-07-15 15:02     ` Lidong Yan
2025-07-15 20:46       ` Junio C Hamano
2025-07-16  1:39         ` Lidong Yan
2025-07-16  5:55           ` Johannes Sixt
2025-07-16 11:20             ` Lidong Yan
2025-07-16 15:16             ` Junio C Hamano
2025-07-17  3:07               ` [PATCH] pull: add pull.autoStash config option Lidong Yan
2025-07-17  3:27                 ` Eric Sunshine
2025-07-17  4:09                   ` Lidong Yan
2025-07-17  4:37                     ` Junio C Hamano
2025-07-17  5:01                       ` Lidong Yan
2025-07-17  5:48                         ` Junio C Hamano
2025-07-17  4:44                   ` Junio C Hamano
2025-07-18  3:52                 ` Lidong Yan
2025-07-18 22:13                   ` Junio C Hamano
2025-07-19  3:14                     ` Lidong Yan
2025-07-20 12:43                   ` [PATCH v2] " Lidong Yan
2025-07-21 22:10                     ` Junio C Hamano
2025-07-17 19:32               ` [BUG] git pull ignores pull.autostash=true configuration when used with --git-dir and --work-tree flags on a bare repository Ben Knoble

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 NNTP newsgroup(s).