[PATCH] pull: allow dirty tree when rebase.autostash enabled

[PATCH] pull: allow dirty tree when rebase.autostash enabled

[Kevin Daudt]

rebase learned to stash changes when it encounters a dirty work tree, but
git pull --rebase does not.

Only verify if the working tree is dirty when rebase.autostash is not
enabled.
---
 git-pull.sh     |  5 ++++-
 t/t5520-pull.sh | 17 +++++++++++++++++
 2 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/git-pull.sh b/git-pull.sh
index 0917d0d..6b9e8a3 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -239,7 +239,10 @@ test true = "$rebase" && {
 			die "$(gettext "updating an unborn branch with changes added to the index")"
 		fi
 	else
-		require_clean_work_tree "pull with rebase" "Please commit or stash them."
+		if [ $(git config --bool --get rebase.autostash || echo false) = "false" ]
+		then
+			require_clean_work_tree "pull with rebase" "Please commit or stash them."
+		fi
 	fi
 	oldremoteref= &&
 	test -n "$curr_branch" &&
diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index 7efd45b..d849a19 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -297,6 +297,23 @@ test_expect_success 'pull --rebase dies early with dirty working directory' '
 
 '
 
+test_expect_success 'pull --rebase succeeds with dirty working directory and rebase.autostash set' '
+
+	test_when_finished "git rm -f file4" && 
+	git checkout to-rebase &&
+	git update-ref refs/remotes/me/copy copy^ &&
+	COPY=$(git rev-parse --verify me/copy) &&
+	git rebase --onto $COPY copy &&
+	test_config branch.to-rebase.remote me &&
+	test_config branch.to-rebase.merge refs/heads/copy &&
+	test_config branch.to-rebase.rebase true &&
+	test_config rebase.autostash true &&
+	echo dirty >> file4 &&
+	git add file4 &&
+	git pull
+
+'
+
 test_expect_success 'pull --rebase works on branch yet to be born' '
 	git rev-parse master >expect &&
 	mkdir empty_repo &&
-- 
2.4.2

Re: [PATCH] pull: allow dirty tree when rebase.autostash enabled

[Paul Tan]

Hi,

Some comments which may not necessarily be correct.

On Wed, Jun 3, 2015 at 5:55 AM, Kevin Daudt <me@ikke.info> wrote:
> rebase learned to stash changes when it encounters a dirty work tree, but
> git pull --rebase does not.
>
> Only verify if the working tree is dirty when rebase.autostash is not
> enabled.
> ---

Missing sign-off.

>  git-pull.sh     |  5 ++++-
>  t/t5520-pull.sh | 17 +++++++++++++++++
>  2 files changed, 21 insertions(+), 1 deletion(-)
>
> diff --git a/git-pull.sh b/git-pull.sh
> index 0917d0d..6b9e8a3 100755
> --- a/git-pull.sh
> +++ b/git-pull.sh
> @@ -239,7 +239,10 @@ test true = "$rebase" && {
>                         die "$(gettext "updating an unborn branch with changes added to the index")"
>                 fi
>         else
> -               require_clean_work_tree "pull with rebase" "Please commit or stash them."
> +               if [ $(git config --bool --get rebase.autostash || echo false) = "false" ]

"false" doesn't need to be quoted.

> +               then
> +                       require_clean_work_tree "pull with rebase" "Please commit or stash them."
> +               fi
>         fi
>         oldremoteref= &&
>         test -n "$curr_branch" &&
> diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
> index 7efd45b..d849a19 100755
> --- a/t/t5520-pull.sh
> +++ b/t/t5520-pull.sh
> @@ -297,6 +297,23 @@ test_expect_success 'pull --rebase dies early with dirty working directory' '
>
>  '
>
> +test_expect_success 'pull --rebase succeeds with dirty working directory and rebase.autostash set' '
> +

I know the surrounding old tests use a newline, but I think that all
new tests should use the modern style of not having a newline, since
t5520 already consists of a mix of old and modern styles anyway.

> +       test_when_finished "git rm -f file4" &&

There is trailing whitespace here.

Furthermore, git rm -f will fail if "file4" does not exist in the
index. Perhaps it should be moved below the "git add" below.

> +       git checkout to-rebase &&
> +       git update-ref refs/remotes/me/copy copy^ &&
> +       COPY=$(git rev-parse --verify me/copy) &&

$COPY is not used anywhere in the test.

> +       git rebase --onto $COPY copy &&
> +       test_config branch.to-rebase.remote me &&
> +       test_config branch.to-rebase.merge refs/heads/copy &&
> +       test_config branch.to-rebase.rebase true &&
> +       test_config rebase.autostash true &&
> +       echo dirty >> file4 &&

file4 does not exist, so we don't need to append to it. I know the
above few tests do not adhere to it, but CodingGuidelines says that
redirection operators do not have a space after

> +       git add file4 &&
> +       git pull

I think we should check for file contents to ensure that
git-pull/git-stash/git-rebase is doing its job properly.

> +

Same as above, no need the newline.

> +'
> +

With all that said, I wonder if this test, and the test above ("pull
--rebase dies early with dirty working directory") could be vastly
simplified, since we are not testing if we can handle a rebased
upstream.

E.g., my simplified version for the above test would be something like:

    git checkout -f to-rebase &&
    git rebase --onto copy^ copy &&
    test_config rebase.autostash true &&
    echo dirty >file4 &&
    git add file4 &&
    test_when_finished "git rm -f file4" &&
    git pull --rebase . me/copy &&
    test "$(cat file4)" = dirty &&
    test "$(cat file2)" = file

It's still confusing though, because we cannot take advantage of the
'before-rebase' tag introduced in the above tests. I would much prefer
if this test and the ("pull --rebase dies with dirty working
directory") test could be moved to the --rebase tests at lines 214+.
Also, this section in the t5520 test suite always gives me a headache
trying to decipher what it is trying to do ><

Thanks,
Paul

[PATCH v2 1/2] t5520-pull: Simplify --rebase with dirty tree test

[Kevin Daudt]

Simplify the test case for testing git aborts the pull --rebase when the
work tree is dirty.

Signed-off-by: Kevin Daudt <me@ikke.info>
Helped-by: Paul Tan <pyokagan@gmail.com>
---
This is a preparation for the next pathch.

Changes since v1:
- Moved the tests just belof the first --rebase test
- Simplified both tests to only test if the rebase either succeded for
  failed


 t/t5520-pull.sh | 32 +++++++++++++-------------------
 1 file changed, 13 insertions(+), 19 deletions(-)

diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index 7efd45b..925ad49 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -122,6 +122,19 @@ test_expect_success '--rebase' '
 	test $(git rev-parse HEAD^) = $(git rev-parse copy) &&
 	test new = $(git show HEAD:file2)
 '
+
+test_expect_success 'pull --rebase dies early with dirty working directory' '
+	git reset --hard before-rebase &&
+	before=$(git rev-parse --verify before-rebase) &&
+	test_config branch.to-rebase.rebase true &&
+	echo dirty >>file &&
+	cp file expect &&
+	git add file &&
+	test_must_fail git pull . copy &&
+	test $(git rev-parse --verify to-rebase) = $before &&
+	test_cmp file expect
+'
+
 test_expect_success 'pull.rebase' '
 	git reset --hard before-rebase &&
 	test_config pull.rebase true &&
@@ -278,25 +291,6 @@ test_expect_success 'rebased upstream + fetch + pull --rebase' '
 
 '
 
-test_expect_success 'pull --rebase dies early with dirty working directory' '
-
-	git checkout to-rebase &&
-	git update-ref refs/remotes/me/copy copy^ &&
-	COPY=$(git rev-parse --verify me/copy) &&
-	git rebase --onto $COPY copy &&
-	test_config branch.to-rebase.remote me &&
-	test_config branch.to-rebase.merge refs/heads/copy &&
-	test_config branch.to-rebase.rebase true &&
-	echo dirty >> file &&
-	git add file &&
-	test_must_fail git pull &&
-	test $COPY = $(git rev-parse --verify me/copy) &&
-	git checkout HEAD -- file &&
-	git pull &&
-	test $COPY != $(git rev-parse --verify me/copy)
-
-'
-
 test_expect_success 'pull --rebase works on branch yet to be born' '
 	git rev-parse master >expect &&
 	mkdir empty_repo &&
-- 
2.4.2

Re: [PATCH v2 1/2] t5520-pull: Simplify --rebase with dirty tree test

[Paul Tan]

On Sun, Jun 7, 2015 at 5:12 AM, Kevin Daudt <me@ikke.info> wrote:
> @@ -278,25 +291,6 @@ test_expect_success 'rebased upstream + fetch + pull --rebase' '
>
>  '
>
> -test_expect_success 'pull --rebase dies early with dirty working directory' '
> -
> -       git checkout to-rebase &&
> -       git update-ref refs/remotes/me/copy copy^ &&
> -       COPY=$(git rev-parse --verify me/copy) &&
> -       git rebase --onto $COPY copy &&
> -       test_config branch.to-rebase.remote me &&
> -       test_config branch.to-rebase.merge refs/heads/copy &&
> -       test_config branch.to-rebase.rebase true &&
> -       echo dirty >> file &&
> -       git add file &&
> -       test_must_fail git pull &&
> -       test $COPY = $(git rev-parse --verify me/copy) &&
> -       git checkout HEAD -- file &&
> -       git pull &&
> -       test $COPY != $(git rev-parse --verify me/copy)
> -
> -'

Eh whoops, I don't think we should touch this test. It comes from
f9189cf, which states that:

    When rebasing fails during "pull --rebase", you cannot just clean up
    the working directory and call "pull --rebase" again, since the
    remote branch was already fetched.

Which makes me believe that "die-ing early with dirty working
directory" has something to do with the rebased upstream handling
feature of git-pull, and so this test is correct in testing that, and
thus we should not touch it.

The location of the test in the other patch is fine though.

Thanks,
Paul

[PATCH v2 2/2] pull: allow dirty tree when rebase.autostash enabled

[Kevin Daudt]

From: Kevin Daudt <compufreak@gmail.com>

rebase learned to stash changes when it encounters a dirty work tree, but
git pull --rebase does not.

Only verify if the working tree is dirty when rebase.autostash is not
enabled.

Signed-off-by: Kevin Daudt <me@ikke.info>
Helped-by: Paul Tan <pyokagan@gmail.com>
---
 git-pull.sh     |  5 ++++-
 t/t5520-pull.sh | 12 ++++++++++++
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/git-pull.sh b/git-pull.sh
index 0917d0d..f0a3b6e 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -239,7 +239,10 @@ test true = "$rebase" && {
 			die "$(gettext "updating an unborn branch with changes added to the index")"
 		fi
 	else
-		require_clean_work_tree "pull with rebase" "Please commit or stash them."
+		if [ $(git config --bool --get rebase.autostash || echo false) = false ]
+		then
+			require_clean_work_tree "pull with rebase" "Please commit or stash them."
+		fi
 	fi
 	oldremoteref= &&
 	test -n "$curr_branch" &&
diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index 925ad49..d06119f 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -135,6 +135,18 @@ test_expect_success 'pull --rebase dies early with dirty working directory' '
 	test_cmp file expect
 '
 
+test_expect_success 'pull --rebase succeeds with dirty working directory and rebase.autostash set' '
+	test_config branch.to-rebase.rebase true &&
+	test_config rebase.autostash true &&
+	git checkout HEAD -- file &&
+	echo dirty > new_file &&
+	git add new_file &&
+	git pull . copy &&
+	test $(git rev-parse HEAD^) = $(git rev-parse copy) &&
+	test $(cat new_file) = dirty &&
+	test "$(cat file)" = "modified again"
+'
+
 test_expect_success 'pull.rebase' '
 	git reset --hard before-rebase &&
 	test_config pull.rebase true &&
-- 
2.4.2

Re: [PATCH v2 2/2] pull: allow dirty tree when rebase.autostash enabled

[Paul Tan]

On Sun, Jun 7, 2015 at 5:12 AM, Kevin Daudt <me@ikke.info> wrote:
> From: Kevin Daudt <compufreak@gmail.com>
>
> rebase learned to stash changes when it encounters a dirty work tree, but
> git pull --rebase does not.
>
> Only verify if the working tree is dirty when rebase.autostash is not
> enabled.
>
> Signed-off-by: Kevin Daudt <me@ikke.info>

Ehh? The sign-off does not match the author of the patch.

> Helped-by: Paul Tan <pyokagan@gmail.com>
> ---
>  git-pull.sh     |  5 ++++-
>  t/t5520-pull.sh | 12 ++++++++++++
>  2 files changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/git-pull.sh b/git-pull.sh
> index 0917d0d..f0a3b6e 100755
> --- a/git-pull.sh
> +++ b/git-pull.sh
> @@ -239,7 +239,10 @@ test true = "$rebase" && {
>                         die "$(gettext "updating an unborn branch with changes added to the index")"
>                 fi
>         else
> -               require_clean_work_tree "pull with rebase" "Please commit or stash them."
> +               if [ $(git config --bool --get rebase.autostash || echo false) = false ]
> +               then
> +                       require_clean_work_tree "pull with rebase" "Please commit or stash them."
> +               fi
>         fi
>         oldremoteref= &&
>         test -n "$curr_branch" &&
> diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
> index 925ad49..d06119f 100755
> --- a/t/t5520-pull.sh
> +++ b/t/t5520-pull.sh
> @@ -135,6 +135,18 @@ test_expect_success 'pull --rebase dies early with dirty working directory' '
>         test_cmp file expect
>  '
>
> +test_expect_success 'pull --rebase succeeds with dirty working directory and rebase.autostash set' '
> +       test_config branch.to-rebase.rebase true &&

Ok, though I wonder why not just a git pull --rebase...

> +       test_config rebase.autostash true &&
> +       git checkout HEAD -- file &&

Why not git reset --hard before-rebase? If we don't reset HEAD, then
how would we know if we actually did a rebase?

> +       echo dirty > new_file &&

style: echo dirty >new_file &&

> +       git add new_file &&
> +       git pull . copy &&
> +       test $(git rev-parse HEAD^) = $(git rev-parse copy) &&

Okay, although it would be better to use "test_cmp_rev HEAD^ copy"
because it prints out the hashes if they are different.

> +       test $(cat new_file) = dirty &&

"$(cat new_file)" should be quoted to prevent field splitting.

> +       test "$(cat file)" = "modified again"
> +'
> +
>  test_expect_success 'pull.rebase' '
>         git reset --hard before-rebase &&
>         test_config pull.rebase true &&
> --
> 2.4.2

Thanks,
Paul

Re: [PATCH v2 2/2] pull: allow dirty tree when rebase.autostash enabled

[Kevin Daudt]

On Thu, Jun 11, 2015 at 09:34:08PM +0800, Paul Tan wrote:
> On Sun, Jun 7, 2015 at 5:12 AM, Kevin Daudt <me@ikke.info> wrote:
> > From: Kevin Daudt <compufreak@gmail.com>
> >
> > Signed-off-by: Kevin Daudt <me@ikke.info>
> 
> Ehh? The sign-off does not match the author of the patch.

I changed it, but aparently forgot to reset the author for that commit

> 
> >  '
> >
> > +test_expect_success 'pull --rebase succeeds with dirty working directory and rebase.autostash set' '
> > +       test_config branch.to-rebase.rebase true &&
> 
> Ok, though I wonder why not just a git pull --rebase...

Copied that from another test, but was doubting whether to use it or
not.

> 
> > +       test_config rebase.autostash true &&
> > +       git checkout HEAD -- file &&
> 
> Why not git reset --hard before-rebase? If we don't reset HEAD, then
> how would we know if we actually did a rebase?
> 

Good tip, thanks.

> > +       echo dirty > new_file &&
> 
> style: echo dirty >new_file &&
> 

Fixed

> > +       git add new_file &&
> > +       git pull . copy &&
> > +       test $(git rev-parse HEAD^) = $(git rev-parse copy) &&
> 
> Okay, although it would be better to use "test_cmp_rev HEAD^ copy"
> because it prints out the hashes if they are different.
> 

Didn't know about that, and aparently, also not documented. Thanks.

> > +       test $(cat new_file) = dirty &&
> 
> "$(cat new_file)" should be quoted to prevent field splitting.
> 

Fixed

New patch is coming.

[PATCH v3] pull: allow dirty tree when rebase.autostash enabled

[Kevin Daudt]

rebase learned to stash changes when it encounters a dirty work tree, but
git pull --rebase does not.

Only verify if the working tree is dirty when rebase.autostash is not
enabled.

Signed-off-by: Kevin Daudt <me@ikke.info>
Helped-by: Paul Tan <pyokagan@gmail.com>
---
Changes to v2:
 - Dropped the change of the existing --rebase test
 - Improvements to the test.

Verified that the test fails before the change, and succeeds after the change.

 git-pull.sh     |  5 ++++-
 t/t5520-pull.sh | 11 +++++++++++
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/git-pull.sh b/git-pull.sh
index 0917d0d..f0a3b6e 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -239,7 +239,10 @@ test true = "$rebase" && {
 			die "$(gettext "updating an unborn branch with changes added to the index")"
 		fi
 	else
-		require_clean_work_tree "pull with rebase" "Please commit or stash them."
+		if [ $(git config --bool --get rebase.autostash || echo false) = false ]
+		then
+			require_clean_work_tree "pull with rebase" "Please commit or stash them."
+		fi
 	fi
 	oldremoteref= &&
 	test -n "$curr_branch" &&
diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index af31f04..aa247ec 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -233,6 +233,17 @@ test_expect_success '--rebase fails with multiple branches' '
 	test modified = "$(git show HEAD:file)"
 '
 
+test_expect_success 'pull --rebase succeeds with dirty working directory and rebase.autostash set' '
+	test_config rebase.autostash true &&
+	git reset --hard before-rebase &&
+	echo dirty >new_file &&
+	git add new_file &&
+	git pull --rebase . copy &&
+	test_cmp_rev HEAD^ copy &&
+	test "$(cat new_file)" = dirty &&
+	test "$(cat file)" = "modified again"
+'
+
 test_expect_success 'pull.rebase' '
 	git reset --hard before-rebase &&
 	test_config pull.rebase true &&
-- 
2.4.3

Re: [PATCH v3] pull: allow dirty tree when rebase.autostash enabled

[Junio C Hamano]

Kevin Daudt <me@ikke.info> writes:

> rebase learned to stash changes when it encounters a dirty work tree, but
> git pull --rebase does not.
>
> Only verify if the working tree is dirty when rebase.autostash is not
> enabled.
>
> Signed-off-by: Kevin Daudt <me@ikke.info>
> Helped-by: Paul Tan <pyokagan@gmail.com>
> ---
> Changes to v2:
>  - Dropped the change of the existing --rebase test
>  - Improvements to the test.
>
> Verified that the test fails before the change, and succeeds after the change.
>
>  git-pull.sh     |  5 ++++-
>  t/t5520-pull.sh | 11 +++++++++++
>  2 files changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/git-pull.sh b/git-pull.sh
> index 0917d0d..f0a3b6e 100755
> --- a/git-pull.sh
> +++ b/git-pull.sh
> @@ -239,7 +239,10 @@ test true = "$rebase" && {
>  			die "$(gettext "updating an unborn branch with changes added to the index")"
>  		fi
>  	else
> -		require_clean_work_tree "pull with rebase" "Please commit or stash them."
> +		if [ $(git config --bool --get rebase.autostash || echo false) = false ]

Style (use of []).

Shouldn't you be doing

	if ...
	then        	
		on an unborn
	elif we are not doing autostash
		require clean work tree
	fi

which does not need unnecessarily deep nesting?

> +		then
> +			require_clean_work_tree "pull with rebase" "Please commit or stash them."
> +		fi
>  	fi
>  	oldremoteref= &&
>  	test -n "$curr_branch" &&
> diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
> index af31f04..aa247ec 100755
> --- a/t/t5520-pull.sh
> +++ b/t/t5520-pull.sh
> @@ -233,6 +233,17 @@ test_expect_success '--rebase fails with multiple branches' '
>  	test modified = "$(git show HEAD:file)"
>  '
>  
> +test_expect_success 'pull --rebase succeeds with dirty working directory and rebase.autostash set' '
> +	test_config rebase.autostash true &&
> +	git reset --hard before-rebase &&
> +	echo dirty >new_file &&
> +	git add new_file &&
> +	git pull --rebase . copy &&
> +	test_cmp_rev HEAD^ copy &&
> +	test "$(cat new_file)" = dirty &&
> +	test "$(cat file)" = "modified again"
> +'
> +
>  test_expect_success 'pull.rebase' '
>  	git reset --hard before-rebase &&
>  	test_config pull.rebase true &&

kd/

[Kevin Daudt]

On Wed, Jun 17, 2015 at 08:36:34AM -0700, Junio C Hamano wrote:
> Kevin Daudt <me@ikke.info> writes:
> 
> > -		require_clean_work_tree "pull with rebase" "Please commit or stash them."
> > +		if [ $(git config --bool --get rebase.autostash || echo false) = false ]
> 
> Style (use of []).

Fixed it.

> 
> Shouldn't you be doing
> 
> 	if ...
> 	then        	
> 		on an unborn
> 	elif we are not doing autostash
> 		require clean work tree
> 	fi
> 
> which does not need unnecessarily deep nesting?
> 

You are right, much simpler. New patch is underway.

[PATCH v4] pull: allow dirty tree when rebase.autostash enabled

[Kevin Daudt]

rebase learned to stash changes when it encounters a dirty work tree, but
git pull --rebase does not.

Only verify if the working tree is dirty when rebase.autostash is not
enabled.

Signed-off-by: Kevin Daudt <me@ikke.info>
Helped-by: Paul Tan <pyokagan@gmail.com>
---
 git-pull.sh     |  3 ++-
 t/t5520-pull.sh | 11 +++++++++++
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/git-pull.sh b/git-pull.sh
index a814bf6..ff28d3f 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -284,7 +284,8 @@ test true = "$rebase" && {
 		then
 			die "$(gettext "updating an unborn branch with changes added to the index")"
 		fi
-	else
+	elif test $(git config --bool --get rebase.autostash || echo false) = false
+	then
 		require_clean_work_tree "pull with rebase" "Please commit or stash them."
 	fi
 	oldremoteref= &&
diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index f4a7193..a0013ee 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -245,6 +245,17 @@ test_expect_success '--rebase fails with multiple branches' '
 	test modified = "$(git show HEAD:file)"
 '
 
+test_expect_success 'pull --rebase succeeds with dirty working directory and rebase.autostash set' '
+	test_config rebase.autostash true &&
+	git reset --hard before-rebase &&
+	echo dirty >new_file &&
+	git add new_file &&
+	git pull --rebase . copy &&
+	test_cmp_rev HEAD^ copy &&
+	test "$(cat new_file)" = dirty &&
+	test "$(cat file)" = "modified again"
+'
+
 test_expect_success 'pull.rebase' '
 	git reset --hard before-rebase &&
 	test_config pull.rebase true &&
-- 
2.4.5

Re: [PATCH v4] pull: allow dirty tree when rebase.autostash enabled

[Junio C Hamano]

Kevin Daudt <me@ikke.info> writes:

> rebase learned to stash changes when it encounters a dirty work tree, but
> git pull --rebase does not.
>
> Only verify if the working tree is dirty when rebase.autostash is not
> enabled.
>
> Signed-off-by: Kevin Daudt <me@ikke.info>
> Helped-by: Paul Tan <pyokagan@gmail.com>
> ---

I applied it, tried to run today's integration cycle, and then ended
up ejecting it from my tree for now, as this seemed to break 5520
when merged to 'pu' X-<.

Well, that is partly expected, as Paul's builtin/pull.c does not
know about it (yet).

>  git-pull.sh     |  3 ++-
>  t/t5520-pull.sh | 11 +++++++++++
>  2 files changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/git-pull.sh b/git-pull.sh
> index a814bf6..ff28d3f 100755
> --- a/git-pull.sh
> +++ b/git-pull.sh
> @@ -284,7 +284,8 @@ test true = "$rebase" && {
>  		then
>  			die "$(gettext "updating an unborn branch with changes added to the index")"
>  		fi
> -	else
> +	elif test $(git config --bool --get rebase.autostash || echo false) = false
> +	then
>  		require_clean_work_tree "pull with rebase" "Please commit or stash them."
>  	fi
>  	oldremoteref= &&
> diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
> index f4a7193..a0013ee 100755
> --- a/t/t5520-pull.sh
> +++ b/t/t5520-pull.sh
> @@ -245,6 +245,17 @@ test_expect_success '--rebase fails with multiple branches' '
>  	test modified = "$(git show HEAD:file)"
>  '
>  
> +test_expect_success 'pull --rebase succeeds with dirty working directory and rebase.autostash set' '
> +	test_config rebase.autostash true &&
> +	git reset --hard before-rebase &&
> +	echo dirty >new_file &&
> +	git add new_file &&
> +	git pull --rebase . copy &&
> +	test_cmp_rev HEAD^ copy &&
> +	test "$(cat new_file)" = dirty &&
> +	test "$(cat file)" = "modified again"
> +'
> +
>  test_expect_success 'pull.rebase' '
>  	git reset --hard before-rebase &&
>  	test_config pull.rebase true &&

[PATCH v5] pull: allow dirty tree when rebase.autostash enabled

[Paul Tan]

On Mon, Jul 06, 2015 at 01:39:47PM -0700, Junio C Hamano wrote:
> Kevin Daudt <me@ikke.info> writes:
> 
> > rebase learned to stash changes when it encounters a dirty work tree, but
> > git pull --rebase does not.
> >
> > Only verify if the working tree is dirty when rebase.autostash is not
> > enabled.
> >
> > Signed-off-by: Kevin Daudt <me@ikke.info>
> > Helped-by: Paul Tan <pyokagan@gmail.com>
> > ---
> 
> I applied it, tried to run today's integration cycle, and then ended
> up ejecting it from my tree for now, as this seemed to break 5520
> when merged to 'pu' X-<.
> 
> Well, that is partly expected, as Paul's builtin/pull.c does not
> know about it (yet).

Yeah, sorry about that.

Here's a modified patch for the C code.

Regards,
Paul

--- >8 ---
From: Kevin Daudt <me@ikke.info>
Date: Sat, 4 Jul 2015 23:42:38 +0200

rebase learned to stash changes when it encounters a dirty work tree,
but git pull --rebase does not.

Only verify if the working tree is dirty when rebase.autostash is not
enabled.

Signed-off-by: Kevin Daudt <me@ikke.info>
Signed-off-by: Paul Tan <pyokagan@gmail.com>
---
 builtin/pull.c  |  6 +++++-
 t/t5520-pull.sh | 11 +++++++++++
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/builtin/pull.c b/builtin/pull.c
index 722a83c..b7bc1ff 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -823,10 +823,14 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
 		hashclr(orig_head);
 
 	if (opt_rebase) {
+		int autostash = 0;
+
 		if (is_null_sha1(orig_head) && !is_cache_unborn())
 			die(_("Updating an unborn branch with changes added to the index."));
 
-		die_on_unclean_work_tree(prefix);
+		git_config_get_bool("rebase.autostash", &autostash);
+		if (!autostash)
+			die_on_unclean_work_tree(prefix);
 
 		if (get_rebase_fork_point(rebase_fork_point, repo, *refspecs))
 			hashclr(rebase_fork_point);
diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index f4a7193..a0013ee 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -245,6 +245,17 @@ test_expect_success '--rebase fails with multiple branches' '
 	test modified = "$(git show HEAD:file)"
 '
 
+test_expect_success 'pull --rebase succeeds with dirty working directory and rebase.autostash set' '
+	test_config rebase.autostash true &&
+	git reset --hard before-rebase &&
+	echo dirty >new_file &&
+	git add new_file &&
+	git pull --rebase . copy &&
+	test_cmp_rev HEAD^ copy &&
+	test "$(cat new_file)" = dirty &&
+	test "$(cat file)" = "modified again"
+'
+
 test_expect_success 'pull.rebase' '
 	git reset --hard before-rebase &&
 	test_config pull.rebase true &&
-- 
2.5.0.rc1.21.gbd65f2d.dirty

Re: [PATCH v5] pull: allow dirty tree when rebase.autostash enabled

[Kevin Daudt]

On Tue, Jul 07, 2015 at 11:59:56AM +0800, Paul Tan wrote:
> On Mon, Jul 06, 2015 at 01:39:47PM -0700, Junio C Hamano wrote:
> > Kevin Daudt <me@ikke.info> writes:
> > 
> > > rebase learned to stash changes when it encounters a dirty work tree, but
> > > git pull --rebase does not.
> > >
> > > Only verify if the working tree is dirty when rebase.autostash is not
> > > enabled.
> > >
> > > Signed-off-by: Kevin Daudt <me@ikke.info>
> > > Helped-by: Paul Tan <pyokagan@gmail.com>
> > > ---
> > 
> > I applied it, tried to run today's integration cycle, and then ended
> > up ejecting it from my tree for now, as this seemed to break 5520
> > when merged to 'pu' X-<.
> > 
> > Well, that is partly expected, as Paul's builtin/pull.c does not
> > know about it (yet).
> 
> Yeah, sorry about that.
> 
> Here's a modified patch for the C code.
> 
> Regards,
> Paul
> 
> --- >8 ---
> From: Kevin Daudt <me@ikke.info>
> Date: Sat, 4 Jul 2015 23:42:38 +0200
> 
> rebase learned to stash changes when it encounters a dirty work tree,
> but git pull --rebase does not.
> 
> Only verify if the working tree is dirty when rebase.autostash is not
> enabled.
> 
> Signed-off-by: Kevin Daudt <me@ikke.info>
> Signed-off-by: Paul Tan <pyokagan@gmail.com>
> ---
>  builtin/pull.c  |  6 +++++-
>  t/t5520-pull.sh | 11 +++++++++++
>  2 files changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/builtin/pull.c b/builtin/pull.c
> index 722a83c..b7bc1ff 100644
> --- a/builtin/pull.c
> +++ b/builtin/pull.c
> @@ -823,10 +823,14 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
>  		hashclr(orig_head);
>  
>  	if (opt_rebase) {
> +		int autostash = 0;
> +
>  		if (is_null_sha1(orig_head) && !is_cache_unborn())
>  			die(_("Updating an unborn branch with changes added to the index."));
>  
> -		die_on_unclean_work_tree(prefix);
> +		git_config_get_bool("rebase.autostash", &autostash);
> +		if (!autostash)
> +			die_on_unclean_work_tree(prefix);
>  
>  		if (get_rebase_fork_point(rebase_fork_point, repo, *refspecs))
>  			hashclr(rebase_fork_point);
> diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
> index f4a7193..a0013ee 100755
> --- a/t/t5520-pull.sh
> +++ b/t/t5520-pull.sh
> @@ -245,6 +245,17 @@ test_expect_success '--rebase fails with multiple branches' '
>  	test modified = "$(git show HEAD:file)"
>  '
>  
> +test_expect_success 'pull --rebase succeeds with dirty working directory and rebase.autostash set' '
> +	test_config rebase.autostash true &&
> +	git reset --hard before-rebase &&
> +	echo dirty >new_file &&
> +	git add new_file &&
> +	git pull --rebase . copy &&
> +	test_cmp_rev HEAD^ copy &&
> +	test "$(cat new_file)" = dirty &&
> +	test "$(cat file)" = "modified again"
> +'
> +
>  test_expect_success 'pull.rebase' '
>  	git reset --hard before-rebase &&
>  	test_config pull.rebase true &&
> -- 
> 2.5.0.rc1.21.gbd65f2d.dirty
> 

Any news about this? Is it still waiting for something?

Re: [PATCH v5] pull: allow dirty tree when rebase.autostash enabled

[Junio C Hamano]

Kevin Daudt <me@ikke.info> writes:

> On Tue, Jul 07, 2015 at 11:59:56AM +0800, Paul Tan wrote:
>
> Any news about this? Is it still waiting for something?

Paul's patch was buried in the noise and I didn't notice it.

I'd prefer to see a new feature like this, that did not exist in the
original, be done on top of the "rewrite pull in C" topic, which
will need a bit more time to mature and be merged to 'master'.

Thanks.

Re: [PATCH v5] pull: allow dirty tree when rebase.autostash enabled

[Kevin Daudt]

On Wed, Jul 22, 2015 at 12:42:17PM -0700, Junio C Hamano wrote:
> Kevin Daudt <me@ikke.info> writes:
> 
> > On Tue, Jul 07, 2015 at 11:59:56AM +0800, Paul Tan wrote:
> >
> > Any news about this? Is it still waiting for something?
> 
> Paul's patch was buried in the noise and I didn't notice it.
> 
> I'd prefer to see a new feature like this, that did not exist in the
> original, be done on top of the "rewrite pull in C" topic, which
> will need a bit more time to mature and be merged to 'master'.
> 
> Thanks.

Ok, no problem.