Re: [PATCH v14 3/3] am: support --allow-empty to record specific empty patches

["Aleen 徐沛文" <pwxu@coremail.cn>]

Dears Hamano,

    For those 6 cases, it seems `git-am` should only work when the am session
    was stopped in the middle according to an empty patch, but not when the index
    has not changed. Does that mean we should record a new state for the am state?
    If that's right, I will try to patch it via version 15, and try to add additional
    test cases for these 6 situations. I was a little busy those days and sorry for
    the late answer.

Aleen

> These two are "positive" tests, i.e. the feature does the right
> thing when used in the expected situation.
> 
> I am worried about the cases where "--allow-empty" is used and
> creates a commit that is empty, when it is reasonably expected to
> either create a non-empty commit or still fail the same way to give
> the user another chance to try.
> 
> For example, when "git am -3" fails on such an email message (there
> are two failure modes: (a) one results in an unmerged index due to
> merge conflicts, (b) the other results in a clean index due to not
> finding the blob object recorded as a preimage), what happens when
> the user runs "git am --allow-empty", after:
> 
>  (1) doing nothing since the failed "git am -3" application,
>  (2) resolving the conflicts in working tree, or
>  (3) resolving the conflicts in working tree and recording the
>      resolution in the index.
> 
> There are 6 cases in the above matrix, and "git am --allow-empty"
> should not create an empty commit in any one of them.  In 1a and 1b,
> "git am --allow-empty" should continue to fail ("git am --skip"
> would be a way to ignore and proceed).  3a and 3b should record a
> non-empty commit as the result.
> 
> Similarly, when an email message with a patch is given to "git am"
> and the application fails, and the user runs "git am --allow-empty"
> without doing anything in the working tree, what happens?  It should
> not happily create an empty commit with only the log message,
> without the change, but continue to fail.  If the user resolves and
> registers the resolution to the index before "git am --allow-empty",
> then the command should create a non-empty commit.
> 
> Having positive tests are of course important to avoid regression in
> the future, but negative tests, the new and shiny feature does not
> misbehave when it shouldn't even kick in, is even more important.
> 
> Thanks.