Hi Johannes, On Tue, Jun 21, 2016 at 10:12 PM, Johannes Schindelin <Johannes.Schindelin@xxxxxx> wrote: > Hi Paul, > > On Tue, 21 Jun 2016, Paul Tan wrote: > >> On Tue, Jun 21, 2016 at 6:34 PM, Johannes Schindelin >> <johannes.schindelin@xxxxxx> 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 -- 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