# [PATCH] pull: allow dirty tree when rebase.autostash enabled 16 messages from 2015-06-02 to 2015-07-22. Participants: Kevin Daudt, Paul Tan, Junio C Hamano. Thread: https://gitlist.dev/t/39506 ## Kevin Daudt, 2015-06-02 21:55 Subject: [PATCH] pull: allow dirty tree when rebase.autostash enabled Message-ID: <1433282157-8171-1-git-send-email-me@ikke.info> URL: https://gitlist.dev/e/1433282157-8171-1-git-send-email-me%40ikke.info ``` 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 ``` ## Paul Tan, 2015-06-03 04:50 Subject: Re: [PATCH] pull: allow dirty tree when rebase.autostash enabled Message-ID: URL: https://gitlist.dev/e/CACRoPnQ%3D5sdWpnebv0OKPHPBh4iyMZ1H47cix6KFh7i5fYomNA%40mail.gmail.com In-Reply-To: <1433282157-8171-1-git-send-email-me@ikke.info> ``` Hi, Some comments which may not necessarily be correct. On Wed, Jun 3, 2015 at 5:55 AM, Kevin Daudt 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 ``` ## Kevin Daudt, 2015-06-06 21:12 Subject: [PATCH v2 1/2] t5520-pull: Simplify --rebase with dirty tree test Message-ID: <1433625145-29668-1-git-send-email-me@ikke.info> URL: https://gitlist.dev/e/1433625145-29668-1-git-send-email-me%40ikke.info In-Reply-To: <1433282157-8171-1-git-send-email-me@ikke.info> ``` Simplify the test case for testing git aborts the pull --rebase when the work tree is dirty. Signed-off-by: Kevin Daudt Helped-by: Paul Tan --- 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 ``` ## Kevin Daudt, 2015-06-06 21:12 Subject: [PATCH v2 2/2] pull: allow dirty tree when rebase.autostash enabled Message-ID: <1433625145-29668-2-git-send-email-me@ikke.info> URL: https://gitlist.dev/e/1433625145-29668-2-git-send-email-me%40ikke.info In-Reply-To: <1433625145-29668-1-git-send-email-me@ikke.info> ``` From: 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 Helped-by: Paul Tan --- 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 ``` ## Paul Tan, 2015-06-11 13:20 Subject: Re: [PATCH v2 1/2] t5520-pull: Simplify --rebase with dirty tree test Message-ID: URL: https://gitlist.dev/e/CACRoPnSnxT2_joEcgiimbiuo53sA_JobNGHj%2BZJ_1%2BmjkaG8HQ%40mail.gmail.com In-Reply-To: <1433625145-29668-1-git-send-email-me@ikke.info> ``` On Sun, Jun 7, 2015 at 5:12 AM, Kevin Daudt 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 ``` ## Paul Tan, 2015-06-11 13:34 Subject: Re: [PATCH v2 2/2] pull: allow dirty tree when rebase.autostash enabled Message-ID: URL: https://gitlist.dev/e/CACRoPnQ0%2Bh2g2cUJk%2B8zSkcHjoHjezAGC%3D5pTjn8aFsgB0GrQA%40mail.gmail.com In-Reply-To: <1433625145-29668-2-git-send-email-me@ikke.info> ``` On Sun, Jun 7, 2015 at 5:12 AM, Kevin Daudt wrote: > From: 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 Ehh? The sign-off does not match the author of the patch. > Helped-by: Paul Tan > --- > 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 ``` ## Kevin Daudt, 2015-06-17 10:40 Subject: Re: [PATCH v2 2/2] pull: allow dirty tree when rebase.autostash enabled Message-ID: <20150617104003.GA7345@vps892.directvps.nl> URL: https://gitlist.dev/e/20150617104003.GA7345%40vps892.directvps.nl In-Reply-To: ``` On Thu, Jun 11, 2015 at 09:34:08PM +0800, Paul Tan wrote: > On Sun, Jun 7, 2015 at 5:12 AM, Kevin Daudt wrote: > > From: Kevin Daudt > > > > Signed-off-by: Kevin Daudt > > 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. ``` ## Kevin Daudt, 2015-06-17 11:01 Subject: [PATCH v3] pull: allow dirty tree when rebase.autostash enabled Message-ID: <1434538880-15608-1-git-send-email-me@ikke.info> URL: https://gitlist.dev/e/1434538880-15608-1-git-send-email-me%40ikke.info In-Reply-To: <1433625145-29668-2-git-send-email-me@ikke.info> ``` 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 Helped-by: Paul Tan --- 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 ``` ## Junio C Hamano, 2015-06-17 15:36 Subject: Re: [PATCH v3] pull: allow dirty tree when rebase.autostash enabled Message-ID: URL: https://gitlist.dev/e/xmqqh9q64a9p.fsf%40gitster.dls.corp.google.com In-Reply-To: <1434538880-15608-1-git-send-email-me@ikke.info> ``` Kevin Daudt 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 > Helped-by: Paul Tan > --- > 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 && ``` ## Kevin Daudt, 2015-07-04 21:00 Subject: kd/ Message-ID: <20150704210032.GB7345@vps892.directvps.nl> URL: https://gitlist.dev/e/20150704210032.GB7345%40vps892.directvps.nl In-Reply-To: ``` On Wed, Jun 17, 2015 at 08:36:34AM -0700, Junio C Hamano wrote: > Kevin Daudt 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. ``` ## Kevin Daudt, 2015-07-04 21:42 Subject: [PATCH v4] pull: allow dirty tree when rebase.autostash enabled Message-ID: <1436046158-19426-1-git-send-email-me@ikke.info> URL: https://gitlist.dev/e/1436046158-19426-1-git-send-email-me%40ikke.info In-Reply-To: <1434538880-15608-1-git-send-email-me@ikke.info> ``` 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 Helped-by: Paul Tan --- 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 ``` ## Junio C Hamano, 2015-07-06 20:39 Subject: Re: [PATCH v4] pull: allow dirty tree when rebase.autostash enabled Message-ID: URL: https://gitlist.dev/e/xmqqd205yq98.fsf%40gitster.dls.corp.google.com In-Reply-To: <1436046158-19426-1-git-send-email-me@ikke.info> ``` Kevin Daudt 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 > Helped-by: Paul Tan > --- 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 && ``` ## Paul Tan, 2015-07-07 03:59 Subject: [PATCH v5] pull: allow dirty tree when rebase.autostash enabled Message-ID: <20150707035956.GA6105@yoshi.chippynet.com> URL: https://gitlist.dev/e/20150707035956.GA6105%40yoshi.chippynet.com In-Reply-To: ``` On Mon, Jul 06, 2015 at 01:39:47PM -0700, Junio C Hamano wrote: > Kevin Daudt 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 > > Helped-by: Paul Tan > > --- > > 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 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 Signed-off-by: Paul Tan --- 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 ``` ## Kevin Daudt, 2015-07-22 19:07 Subject: Re: [PATCH v5] pull: allow dirty tree when rebase.autostash enabled Message-ID: <20150722190724.GA11291@ikke.info> URL: https://gitlist.dev/e/20150722190724.GA11291%40ikke.info In-Reply-To: <20150707035956.GA6105@yoshi.chippynet.com> ``` 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 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 > > > Helped-by: Paul Tan > > > --- > > > > 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 > 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 > Signed-off-by: Paul Tan > --- > 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? ``` ## Junio C Hamano, 2015-07-22 19:42 Subject: Re: [PATCH v5] pull: allow dirty tree when rebase.autostash enabled Message-ID: URL: https://gitlist.dev/e/xmqqvbdcq8rq.fsf%40gitster.dls.corp.google.com In-Reply-To: <20150722190724.GA11291@ikke.info> ``` Kevin Daudt 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. ``` ## Kevin Daudt, 2015-07-22 20:48 Subject: Re: [PATCH v5] pull: allow dirty tree when rebase.autostash enabled Message-ID: <20150722204852.GB11291@ikke.info> URL: https://gitlist.dev/e/20150722204852.GB11291%40ikke.info In-Reply-To: ``` On Wed, Jul 22, 2015 at 12:42:17PM -0700, Junio C Hamano wrote: > Kevin Daudt 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. ```