merge-recursive: flush output buffer before printing error messages
The data structure passed to the recursive merge machinery has a feature where the caller can ask for the output to be buffered into a strbuf, by setting the field 'buffer_output'. Previously, we died without flushing, losing accumulated output. With this patch, we show the output first, and only then print the error message. Currently, the only user of that buffering is merge_recursive() itself, to avoid the progress output to interfere. In the next patches, we will introduce a new buffer_output mode that forces merge_recursive() to retain the output buffer for further processing by the caller. If the caller asked for that, we will then also write the error messages into the output buffer. This is necessary to give the caller more control not only how to react in case of errors but also control how/if to display the error messages. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Johannes Schindelin committed
Aug 1, 2016 at 13:44 UTC
bc9204d4ef6e0672389fdfb0d398fa9a39dba3d5
1 file changed
+68
-48
merge-recursive.c
+68
-48
@@ -23,6 +23,28 @@
23
#include "dir.h"
24
#include "submodule.h"
25
26
+static void flush_output(struct merge_options *o)
27
+{
28
+ if (o->obuf.len) {
29
+ fputs(o->obuf.buf, stdout);
30
+ strbuf_reset(&o->obuf);
31
+ }
32
+}
33
+
34
+static int err(struct merge_options *o, const char *err, ...)
35
+{
36
+ va_list params;
37
+
38
+ flush_output(o);
39
+ va_start(params, err);
40
+ strbuf_vaddf(&o->obuf, err, params);
41
+ va_end(params);
42
+ error("%s", o->obuf.buf);
43
+ strbuf_reset(&o->obuf);
44
+
45
+ return -1;
46
+}
47
+
48
static struct tree *shift_tree_object(struct tree *one, struct tree *two,
49
const char *subtree_shift)
50
{
@@ -148,14 +170,6 @@ static int show(struct merge_options *o, int v)
170
return (!o->call_depth && o->verbosity >= v) || o->verbosity >= 5;
171
}
172
151
-static void flush_output(struct merge_options *o)
152
-{
153
- if (o->obuf.len) {
154
- fputs(o->obuf.buf, stdout);
155
- strbuf_reset(&o->obuf);
156
- }
157
-}
158
-
173
__attribute__((format (printf, 3, 4)))
174
static void output(struct merge_options *o, int v, const char *fmt, ...)
175
{
@@ -198,7 +212,8 @@ static void output_commit_title(struct merge_options *o, struct commit *commit)
212
}
213
}
214
201
-static int add_cacheinfo(unsigned int mode, const struct object_id *oid,
215
+static int add_cacheinfo(struct merge_options *o,
216
+ unsigned int mode, const struct object_id *oid,
217
const char *path, int stage, int refresh, int options)
218
{
219
struct cache_entry *ce;
@@ -206,7 +221,7 @@ static int add_cacheinfo(unsigned int mode, const struct object_id *oid,
221
222
ce = make_cache_entry(mode, oid ? oid->hash : null_sha1, path, stage, 0);
223
if (!ce)
209
- return error(_("addinfo_cache failed for path '%s'"), path);
224
+ return err(o, _("addinfo_cache failed for path '%s'"), path);
225
226
ret = add_cache_entry(ce, options);
227
if (refresh) {
@@ -276,7 +291,7 @@ struct tree *write_tree_from_memory(struct merge_options *o)
291
292
if (!cache_tree_fully_valid(active_cache_tree) &&
293
cache_tree_update(&the_index, 0) < 0) {
279
- error(_("error building trees"));
294
+ err(o, _("error building trees"));
295
return NULL;
296
}
297
@@ -544,7 +559,8 @@ static struct string_list *get_renames(struct merge_options *o,
559
return renames;
560
}
561
547
-static int update_stages(const char *path, const struct diff_filespec *o,
562
+static int update_stages(struct merge_options *opt, const char *path,
563
+ const struct diff_filespec *o,
564
const struct diff_filespec *a,
565
const struct diff_filespec *b)
566
{
@@ -563,13 +579,13 @@ static int update_stages(const char *path, const struct diff_filespec *o,
579
if (remove_file_from_cache(path))
580
return -1;
581
if (o)
566
- if (add_cacheinfo(o->mode, &o->oid, path, 1, 0, options))
582
+ if (add_cacheinfo(opt, o->mode, &o->oid, path, 1, 0, options))
583
return -1;
584
if (a)
569
- if (add_cacheinfo(a->mode, &a->oid, path, 2, 0, options))
585
+ if (add_cacheinfo(opt, a->mode, &a->oid, path, 2, 0, options))
586
return -1;
587
if (b)
572
- if (add_cacheinfo(b->mode, &b->oid, path, 3, 0, options))
588
+ if (add_cacheinfo(opt, b->mode, &b->oid, path, 3, 0, options))
589
return -1;
590
return 0;
591
}
@@ -720,8 +736,8 @@ static int make_room_for_path(struct merge_options *o, const char *path)
736
if (status) {
737
if (status == SCLD_EXISTS)
738
/* something else exists */
723
- return error(msg, path, _(": perhaps a D/F conflict?"));
724
- return error(msg, path, "");
739
+ return err(o, msg, path, _(": perhaps a D/F conflict?"));
740
+ return err(o, msg, path, "");
741
}
742
743
/*
@@ -729,7 +745,7 @@ static int make_room_for_path(struct merge_options *o, const char *path)
745
* tracking it.
746
*/
747
if (would_lose_untracked(path))
732
- return error(_("refusing to lose untracked file at '%s'"),
748
+ return err(o, _("refusing to lose untracked file at '%s'"),
749
path);
750
751
/* Successful unlink is good.. */
@@ -739,7 +755,7 @@ static int make_room_for_path(struct merge_options *o, const char *path)
755
if (errno == ENOENT)
756
return 0;
757
/* .. but not some other error (who really cares what?) */
742
- return error(msg, path, _(": perhaps a D/F conflict?"));
758
+ return err(o, msg, path, _(": perhaps a D/F conflict?"));
759
}
760
761
static int update_file_flags(struct merge_options *o,
@@ -771,9 +787,9 @@ static int update_file_flags(struct merge_options *o,
787
788
buf = read_sha1_file(oid->hash, &type, &size);
789
if (!buf)
774
- return error(_("cannot read object %s '%s'"), oid_to_hex(oid), path);
790
+ return err(o, _("cannot read object %s '%s'"), oid_to_hex(oid), path);
791
if (type != OBJ_BLOB) {
776
- ret = error(_("blob expected for %s '%s'"), oid_to_hex(oid), path);
792
+ ret = err(o, _("blob expected for %s '%s'"), oid_to_hex(oid), path);
793
goto free_buf;
794
}
795
if (S_ISREG(mode)) {
@@ -797,8 +813,8 @@ static int update_file_flags(struct merge_options *o,
813
mode = 0666;
814
fd = open(path, O_WRONLY | O_TRUNC | O_CREAT, mode);
815
if (fd < 0) {
800
- ret = error_errno(_("failed to open '%s'"),
801
- path);
816
+ ret = err(o, _("failed to open '%s': %s"),
817
+ path, strerror(errno));
818
goto free_buf;
819
}
820
write_in_full(fd, buf, size);
@@ -808,17 +824,19 @@ static int update_file_flags(struct merge_options *o,
824
safe_create_leading_directories_const(path);
825
unlink(path);
826
if (symlink(lnk, path))
811
- ret = error_errno(_("failed to symlink '%s'"), path);
827
+ ret = err(o, _("failed to symlink '%s': %s"),
828
+ path, strerror(errno));
829
free(lnk);
830
} else
814
- ret = error(_("do not know what to do with %06o %s '%s'"),
815
- mode, oid_to_hex(oid), path);
831
+ ret = err(o,
832
+ _("do not know what to do with %06o %s '%s'"),
833
+ mode, oid_to_hex(oid), path);
834
free_buf:
835
free(buf);
836
}
837
update_index:
838
if (!ret && update_cache)
821
- add_cacheinfo(mode, oid, path, 0, update_wd, ADD_CACHE_OK_TO_ADD);
839
+ add_cacheinfo(o, mode, oid, path, 0, update_wd, ADD_CACHE_OK_TO_ADD);
840
return ret;
841
}
842
@@ -951,12 +969,12 @@ static int merge_file_1(struct merge_options *o,
969
branch1, branch2);
970
971
if ((merge_status < 0) || !result_buf.ptr)
954
- ret = error(_("Failed to execute internal merge"));
972
+ ret = err(o, _("Failed to execute internal merge"));
973
974
if (!ret && write_sha1_file(result_buf.ptr, result_buf.size,
975
blob_type, result->oid.hash))
958
- ret = error(_("Unable to add %s to database"),
959
- a->path);
976
+ ret = err(o, _("Unable to add %s to database"),
977
+ a->path);
978
979
free(result_buf.ptr);
980
if (ret)
@@ -1122,7 +1140,7 @@ static int conflict_rename_delete(struct merge_options *o,
1140
if (o->call_depth)
1141
return remove_file_from_cache(dest->path);
1142
else
1125
- return update_stages(dest->path, NULL,
1143
+ return update_stages(o, dest->path, NULL,
1144
rename_branch == o->branch1 ? dest : NULL,
1145
rename_branch == o->branch1 ? NULL : dest);
1146
}
@@ -1180,9 +1198,9 @@ static int handle_file(struct merge_options *o,
1198
if ((ret = update_file(o, 0, &rename->oid, rename->mode, dst_name)))
1199
; /* fall through, do allow dst_name to be released */
1200
else if (stage == 2)
1183
- ret = update_stages(rename->path, NULL, rename, add);
1201
+ ret = update_stages(o, rename->path, NULL, rename, add);
1202
else
1185
- ret = update_stages(rename->path, NULL, add, rename);
1203
+ ret = update_stages(o, rename->path, NULL, add, rename);
1204
1205
if (dst_name != rename->path)
1206
free(dst_name);
@@ -1575,23 +1593,25 @@ static struct object_id *stage_oid(const struct object_id *oid, unsigned mode)
1593
return (is_null_oid(oid) || mode == 0) ? NULL: (struct object_id *)oid;
1594
}
1595
1578
-static int read_oid_strbuf(const struct object_id *oid, struct strbuf *dst)
1596
+static int read_oid_strbuf(struct merge_options *o,
1597
+ const struct object_id *oid, struct strbuf *dst)
1598
{
1599
void *buf;
1600
enum object_type type;
1601
unsigned long size;
1602
buf = read_sha1_file(oid->hash, &type, &size);
1603
if (!buf)
1585
- return error(_("cannot read object %s"), oid_to_hex(oid));
1604
+ return err(o, _("cannot read object %s"), oid_to_hex(oid));
1605
if (type != OBJ_BLOB) {
1606
free(buf);
1588
- return error(_("object %s is not a blob"), oid_to_hex(oid));
1607
+ return err(o, _("object %s is not a blob"), oid_to_hex(oid));
1608
}
1609
strbuf_attach(dst, buf, size, size + 1);
1610
return 0;
1611
}
1612
1594
-static int blob_unchanged(const struct object_id *o_oid,
1613
+static int blob_unchanged(struct merge_options *opt,
1614
+ const struct object_id *o_oid,
1615
unsigned o_mode,
1616
const struct object_id *a_oid,
1617
unsigned a_mode,
@@ -1609,7 +1629,7 @@ static int blob_unchanged(const struct object_id *o_oid,
1629
return 0;
1630
1631
assert(o_oid && a_oid);
1612
- if (read_oid_strbuf(o_oid, &o) || read_oid_strbuf(a_oid, &a))
1632
+ if (read_oid_strbuf(opt, o_oid, &o) || read_oid_strbuf(opt, a_oid, &a))
1633
goto error_return;
1634
/*
1635
* Note: binary | is used so that both renormalizations are
@@ -1698,7 +1718,7 @@ static int merge_content(struct merge_options *o,
1718
*/
1719
path_renamed_outside_HEAD = !path2 || !strcmp(path, path2);
1720
if (!path_renamed_outside_HEAD) {
1701
- add_cacheinfo(mfi.mode, &mfi.oid, path,
1721
+ add_cacheinfo(o, mfi.mode, &mfi.oid, path,
1722
0, (!o->call_depth), 0);
1723
return mfi.clean;
1724
}
@@ -1711,7 +1731,7 @@ static int merge_content(struct merge_options *o,
1731
output(o, 1, _("CONFLICT (%s): Merge conflict in %s"),
1732
reason, path);
1733
if (rename_conflict_info && !df_conflict_remains)
1714
- if (update_stages(path, &one, &a, &b))
1734
+ if (update_stages(o, path, &one, &a, &b))
1735
return -1;
1736
}
1737
@@ -1721,7 +1741,7 @@ static int merge_content(struct merge_options *o,
1741
remove_file_from_cache(path);
1742
} else {
1743
if (!mfi.clean) {
1724
- if (update_stages(path, &one, &a, &b))
1744
+ if (update_stages(o, path, &one, &a, &b))
1745
return -1;
1746
} else {
1747
int file_from_stage2 = was_tracked(path);
@@ -1729,7 +1749,7 @@ static int merge_content(struct merge_options *o,
1749
oidcpy(&merged.oid, &mfi.oid);
1750
merged.mode = mfi.mode;
1751
1732
- if (update_stages(path, NULL,
1752
+ if (update_stages(o, path, NULL,
1753
file_from_stage2 ? &merged : NULL,
1754
file_from_stage2 ? NULL : &merged))
1755
return -1;
@@ -1797,8 +1817,8 @@ static int process_entry(struct merge_options *o,
1817
} else if (o_oid && (!a_oid || !b_oid)) {
1818
/* Case A: Deleted in one */
1819
if ((!a_oid && !b_oid) ||
1800
- (!b_oid && blob_unchanged(o_oid, o_mode, a_oid, a_mode, normalize, path)) ||
1801
- (!a_oid && blob_unchanged(o_oid, o_mode, b_oid, b_mode, normalize, path))) {
1820
+ (!b_oid && blob_unchanged(o, o_oid, o_mode, a_oid, a_mode, normalize, path)) ||
1821
+ (!a_oid && blob_unchanged(o, o_oid, o_mode, b_oid, b_mode, normalize, path))) {
1822
/* Deleted in both or deleted in one and
1823
* unchanged in the other */
1824
if (a_oid)
@@ -1894,7 +1914,7 @@ int merge_trees(struct merge_options *o,
1914
1915
if (code != 0) {
1916
if (show(o, 4) || o->call_depth)
1897
- error(_("merging of trees %s and %s failed"),
1917
+ err(o, _("merging of trees %s and %s failed"),
1918
oid_to_hex(&head->object.oid),
1919
oid_to_hex(&merge->object.oid));
1920
return -1;
@@ -2029,7 +2049,7 @@ int merge_recursive(struct merge_options *o,
2049
o->call_depth--;
2050
2051
if (!merged_common_ancestors)
2032
- return error(_("merge returned no commit"));
2052
+ return err(o, _("merge returned no commit"));
2053
}
2054
2055
discard_cache();
@@ -2088,7 +2108,7 @@ int merge_recursive_generic(struct merge_options *o,
2108
for (i = 0; i < num_base_list; ++i) {
2109
struct commit *base;
2110
if (!(base = get_ref(base_list[i], oid_to_hex(base_list[i]))))
2091
- return error(_("Could not parse object '%s'"),
2111
+ return err(o, _("Could not parse object '%s'"),
2112
oid_to_hex(base_list[i]));
2113
commit_list_insert(base, &ca);
2114
}
@@ -2102,7 +2122,7 @@ int merge_recursive_generic(struct merge_options *o,
2122
2123
if (active_cache_changed &&
2124
write_locked_index(&the_index, lock, COMMIT_LOCK))
2105
- return error(_("Unable to write index."));
2125
+ return err(o, _("Unable to write index."));
2126
2127
return clean ? 0 : 1;
2128
}