Re: [PATCH v3 0/9] Let log-tree and friends respect diffopt's `file` field

[Paul Tan <pyokagan@gmail.com>]

Hi Johannes,

On Tue, Jun 21, 2016 at 10:12 PM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> Hi Paul,
>
> On Tue, 21 Jun 2016, Paul Tan wrote:
>
>> On Tue, Jun 21, 2016 at 6:34 PM, Johannes Schindelin
>> <johannes.schindelin@gmx.de> wrote:
>> > - this uncovered a problem with builtin am, where it asked the diff
>> >   machinery to close the file stream, but actually called the log_tree
>> >   machinery (which might mean that this patch series inadvertently fixes
>> >   a bug where `git am --rebasing` would write the commit message to
>> >   stdout instead of the `patch` file when erroring out)
>>
>> Please correct me if I'm wrong: looking at log-tree.c, the commit
>> message will not be printed when no_commit_id = 1, isn't it?
>> This is because we do not hit the code paths that write to stdout since
>> show_log() is not called.
>
> Why does builtin/am.c use log_tree_commit(), then? Why not simply run
> things through the diff machinery?

It's because git-am.sh called "git diff-tree", which in turn calls
log_tree_commit(), so to be safe I used the same function to ensure
that there were no unintended behavioral changes. e.g. what happened
with 3ecc704 (am --skip/--abort: merge HEAD/ORIG_HEAD tree into index,
2015-08-19)

Of course, it may be true that the diff machinery should be called
directly, although the code is quite involved so I can't really tell
the impact the change will have.

Thanks,
Paul