Re: [PATCH v5 16/16] merge-recursive: flush output buffer even when erroring out
Johannes Schindelin writes: >> This is of course a good change, but we need to assume that no >> further output is made from the remainder of the function for the >> change in the next hunk to remove the existing flush to be correct. > ... > But you made me realize that I cannot simply *move* the flush_output() > call here, in case that code in between will eventually add output. Yup, that removal of the original one was the only thing I was pointing out. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 16/16] merge-recursive: flush output buffer even when erroring out
Hi Junio, On Wed, 27 Jul 2016, Junio C Hamano wrote: > Johannes Schindelin writes: > > > diff --git a/merge-recursive.c b/merge-recursive.c > > index a16b150..66e93e0 100644 > > --- a/merge-recursive.c > > +++ b/merge-recursive.c > > @@ -2069,6 +2069,7 @@ int merge_recursive(struct merge_options *o, > > o->ancestor = "merged common ancestors"; > > clean = merge_trees(o, h1->tree, h2->tree, > > merged_common_ancestors->tree, > > &mrtree); > > + flush_output(o); > > if (clean < 0) > > return clean; > > This is of course a good change, but we need to assume that no > further output is made from the remainder of the function for the > change in the next hunk to remove the existing flush to be correct. Please note that nothing prevents the code further down from adding more output. All we do here is flushing the output *so far*, in case we return an error. And of course nothing gets flushed if buffer_output == 2, because that value states that the caller wants to take care of displaying the output herself. But you made me realize that I cannot simply *move* the flush_output() call here, in case that code in between will eventually add output. Thanks, Dscho -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 16/16] merge-recursive: flush output buffer even when erroring out
Johannes Schindelin writes: > diff --git a/merge-recursive.c b/merge-recursive.c > index a16b150..66e93e0 100644 > --- a/merge-recursive.c > +++ b/merge-recursive.c > @@ -2069,6 +2069,7 @@ int merge_recursive(struct merge_options *o, > o->ancestor = "merged common ancestors"; > clean = merge_trees(o, h1->tree, h2->tree, > merged_common_ancestors->tree, > &mrtree); > + flush_output(o); > if (clean < 0) > return clean; This is of course a good change, but we need to assume that no further output is made from the remainder of the function for the change in the next hunk to remove the existing flush to be correct. And once we assume that, then the "we no longer need this buffer, so release it" added in 15/16 can also move here, right? I am wondering if there is a low-impact way to make sure that assumption will not be broken. > @@ -2077,7 +2078,6 @@ int merge_recursive(struct merge_options *o, > commit_list_insert(h1, &(*result)->parents); > commit_list_insert(h2, &(*result)->parents->next); > } > - flush_output(o); > if (o->buffer_output < 2) > strbuf_release(&o->obuf); > if (show(o, 2)) -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 16/16] merge-recursive: flush output buffer even when erroring out
Ever since 66a155b (Enable output buffering in merge-recursive., 2007-01-14), we had a problem: When the merge failed in a fatal way, all regular output was swallowed because we called die() and did not get a chance to drain the output buffers. To fix this, several modifications were necessary: - we needed to stop die()ing, to give callers a chance to do something when an error occurred (in this case, flush the output buffers), - we needed to delay printing the error message so that the caller can print the buffered output before that, and - we needed to make sure that the output buffers are flushed even when the return value indicates an error. The first two changes were introduced through earlier commits in this patch series, and this commit addresses the third one. Signed-off-by: Johannes Schindelin --- merge-recursive.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/merge-recursive.c b/merge-recursive.c index a16b150..66e93e0 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -2069,6 +2069,7 @@ int merge_recursive(struct merge_options *o, o->ancestor = "merged common ancestors"; clean = merge_trees(o, h1->tree, h2->tree, merged_common_ancestors->tree, &mrtree); + flush_output(o); if (clean < 0) return clean; @@ -2077,7 +2078,6 @@ int merge_recursive(struct merge_options *o, commit_list_insert(h1, &(*result)->parents); commit_list_insert(h2, &(*result)->parents->next); } - flush_output(o); if (o->buffer_output < 2) strbuf_release(&o->obuf); if (show(o, 2)) -- 2.9.0.281.g286a8d9 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
