On Fri, Jun 5, 2015 at 1:27 AM, Stefan Beller <sbeller@xxxxxxxxxx> wrote:
> On Thu, Jun 4, 2015 at 3:34 AM, Paul Tan <pyokagan@xxxxxxxxx> wrote:
>> Or, the current behavior of git-am.sh will print some scary errors
>> about the missing HEAD, but will then continue on to the next patch.
>> If the index is not clean, it will then error out. Should we preserve
>> this behavior? (without the errors about the missing HEAD)
>>
>> 2. am --abort
>>
>> For git am --abort, git-am.sh does something similar. It does a
>> fast-forward checkout from HEAD to ORIG_HEAD, discarding unmerged
>> entries, and then resets the index to ORIG_HEAD so that local changes
>> will be unstaged.
>>
>> if safe_to_abort
>> then
>> git read-tree --reset -u HEAD ORIG_HEAD
>> git reset ORIG_HEAD
>> fi
>> rm -fr "$dotest"
>>
>> This, however, requires a valid HEAD and ORIG_HEAD. If we don't have a
>> HEAD or ORIG_HEAD (because we were on an unborn branch when we started
>> git am), what should we do? (Note: safe_to_abort returns true if we
>> git am with no HEAD because $dotest/abort-safety will not exist)
>> Should we discard all entires in the index as well? (Since we might
>> think of the "original HEAD" as an empty tree?)
>>
>> Or, the current behavior of git-am.sh will print some scary errors
>> about the missing HEAD and ORIG_HEAD, but will not touch the index at
>> all, and still delete the rebase-apply directory. Should we preserve
>> this behavior (without the errors)?
>
> I guess so, looking at the documentation
> --abort
> Restore the original branch and abort the patching operation.
>
> a user may want to not go to the unborn branch, but rather to the previous
> HEAD?
I think we need to be consistent with the non-unborn-branch behavior
of git-am. In normal use am --abort would reset the branch back to
ORIG_HEAD, which is the HEAD when am was first run, thus losing all
the applied commits. However, since f79d4c8 (git-am: teach git-am to
apply a patch to an unborn branch, 2009-04-10) if git-am is run
without a HEAD, no ORIG_HEAD would be created.
I guess if we are to follow this normal behavior, we need to destroy
the current branch as well.
So the abort logic would be something like:
1. If we are still on an unborn branch, we clear the index.
2. If we are not on an unborn branch, but we have no ORIG_HEAD because
we started from an unborn branch, then we destroy the current branch.
3. If we are not on an unborn branch, and we have an ORIG_HEAD, then
we do the normal behavior of fast-forwarding and resetting the index
to ORIG_HEAD.
We also need to consider safe_to_abort() as well though, which was
introduced in 7b3b7e3 (am --abort: keep unrelated commits since the
last failure and warn, 2010-12-21). Specifically,
safe_to_abort () {
...
if ! test -s "$dotest/abort-safety"
then
return 0
fi
...
}
where we are allowed to reset HEAD if the abort-safety file is empty
or does not exist. If patch application failed while we are on an
unborn branch, we will have no abort-safety file. However, the user
may have made commits since then, and we should not make the user lose
those commits. As such, we should probably not reset HEAD if there is
no abort-safety file.
Thanks,
Paul
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html