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

From: Paul Tan <pyokagan@gmail.com>
Date: Jun 11, 2015, 13:34 UTC
Message-ID: <CACRoPnQ0+h2g2cUJk+8zSkcHjoHjezAGC=5pTjn8aFsgB0GrQA@mail.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 <me@ikke.info> wrote:

Show 9 quoted lines

> 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.

Show 32 quoted lines

> 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.

Show 8 quoted lines

> +       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