Re: [PATCH 3/3] git-pull.sh: introduce --[no-]autostash and pull.autostash

From: Ramkumar Ramachandra <artagnon@gmail.com>
Date: Mar 23, 2013, 12:48 UTC
Message-ID: <CALkWK0=oOt0teGqCjpDkerR3-t1cY=qKyK-AtoLLCz9L1-+vyw@mail.gmail.com>
In-Reply-To: <7vtxo374h6.fsf@alter.siamese.dyndns.org>

Junio C Hamano wrote:

Show 16 quoted lines

> Ramkumar Ramachandra <artagnon@gmail.com> writes:
>
>> @@ -1786,6 +1786,11 @@ 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, automatically stash all changes before attempting to
>> +     merge/rebase, and pop the stash after a successful
>> +     merge/rebase.
>>  +
>>  *NOTE*: this is a possibly dangerous operation; do *not* use
>>  it unless you understand the implications (see linkgit:git-rebase[1]
>
> Is autosquash a possibly dangerous operation?

Oops. I must admit I was in a bit of a hurry with the documentation. I was eager to send out the series seeing that the tests were passing.

Show 13 quoted lines

>> @@ -196,7 +203,8 @@ test true = "$rebase" && {
>>               then
>>                       die "$(gettext "updating an unborn branch with changes added to the index")"
>>               fi
>> -     else
>> +     elif test "$autostash" = false
>> +     then
>>               require_clean_work_tree "pull with rebase" "Please commit or stash them."
>>       fi
>
> A safety net, after you run "git stash", to validate that the
> added "git stash" indeed made the working tree clean, is necessary
> below, but there does not seem to be any.

Um, isn't that part of the "git stash" testsuite?

Show 35 quoted lines

>>       oldremoteref= &&
>> @@ -213,6 +221,12 @@ test true = "$rebase" && {
>>               fi
>>       done
>>  }
>> +
>> +stash_required () {
>> +     ! (git diff-files --quiet --ignore-submodules &&
>> +     git diff-index --quiet --cached HEAD --ignore-submodules)
>> +}
>> +
>>  orig_head=$(git rev-parse -q --verify HEAD)
>>  git fetch $verbosity $progress $dry_run $recurse_submodules --update-head-ok "$@" || exit 1
>>  test -z "$dry_run" || exit 0
>> @@ -288,4 +302,12 @@ true)
>>       eval="$eval \"\$merge_name\" HEAD $merge_head"
>>       ;;
>>  esac
>> -eval "exec $eval"
>> +
>> +if test "$autostash" = true && stash_required
>> +then
>> +     git stash
>> +     eval "$eval"
>> +     test $? = 0 && git stash pop
>> +else
>> +     eval "exec $eval"
>> +fi
>
> Delaying to run "git stash" as much as possible is fine, but with
> the above, can the user tell if something was stashed and she has
> to "stash pop" once she is done helping the command to resolve the
> conflicts, or she shouldn't run "stash pop" after she is done
> (because if nothing is stashed at this point, that "pop" will pop an
> unrelated stash)?

I could easily tell, from the "git pull" output: if conflict, then stash hasn't been applied. Otherwise, yes. Should we print a message guarded by an advice variable?

> What protects the "git stash" from failing and how is its error
> handled?

Oh, this is my stupidity. I should've just &&-chained the git stash, eval $eval, and git stash pop.