merge-recursive: switch to returning errors instead of dying

The recursive merge machinery is supposed to be a library function, i.e. it should return an error when it fails. Originally the functions were part of the builtin "merge-recursive", though, where it was simpler to call die() and be done with error handling. The existing callers were already prepared to detect negative return values to indicate errors and to behave as previously: exit with code 128 (which is the same thing that die() does, after printing the message). Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Johannes Schindelin committed Jul 26, 2016 at 18:06 UTC 6003303a1e5086a9f28f18469a8a72ad22f75c86
1 file changed +35 -27
merge-recursive.c
+35 -27
@@ -275,8 +275,10 @@ struct tree *write_tree_from_memory(struct merge_options *o)
275 active_cache_tree = cache_tree();
276
277 if (!cache_tree_fully_valid(active_cache_tree) &&
278 - cache_tree_update(&the_index, 0) < 0)
279 - die(_("error building trees"));
278 + cache_tree_update(&the_index, 0) < 0) {
279 + error(_("error building trees"));
280 + return NULL;
281 + }
282
283 result = lookup_tree(active_cache_tree->sha1);
284
@@ -716,12 +718,10 @@ static int make_room_for_path(struct merge_options *o, const char *path)
718 /* Make sure leading directories are created */
719 status = safe_create_leading_directories_const(path);
720 if (status) {
719 - if (status == SCLD_EXISTS) {
721 + if (status == SCLD_EXISTS)
722 /* something else exists */
721 - error(msg, path, _(": perhaps a D/F conflict?"));
722 - return -1;
723 - }
724 - die(msg, path, "");
723 + return error(msg, path, _(": perhaps a D/F conflict?"));
724 + return error(msg, path, "");
725 }
726
727 /*
@@ -749,6 +749,8 @@ static int update_file_flags(struct merge_options *o,
749 int update_cache,
750 int update_wd)
751 {
752 + int ret = 0;
753 +
754 if (o->call_depth)
755 update_wd = 0;
756
@@ -769,9 +771,11 @@ static int update_file_flags(struct merge_options *o,
771
772 buf = read_sha1_file(oid->hash, &type, &size);
773 if (!buf)
772 - die(_("cannot read object %s '%s'"), oid_to_hex(oid), path);
773 - if (type != OBJ_BLOB)
774 - die(_("blob expected for %s '%s'"), oid_to_hex(oid), path);
774 + return error(_("cannot read object %s '%s'"), oid_to_hex(oid), path);
775 + if (type != OBJ_BLOB) {
776 + ret = error(_("blob expected for %s '%s'"), oid_to_hex(oid), path);
777 + goto free_buf;
778 + }
779 if (S_ISREG(mode)) {
780 struct strbuf strbuf = STRBUF_INIT;
781 if (convert_to_working_tree(path, buf, size, &strbuf)) {
@@ -792,8 +796,11 @@ static int update_file_flags(struct merge_options *o,
796 else
797 mode = 0666;
798 fd = open(path, O_WRONLY | O_TRUNC | O_CREAT, mode);
795 - if (fd < 0)
796 - die_errno(_("failed to open '%s'"), path);
799 + if (fd < 0) {
800 + ret = error_errno(_("failed to open '%s'"),
801 + path);
802 + goto free_buf;
803 + }
804 write_in_full(fd, buf, size);
805 close(fd);
806 } else if (S_ISLNK(mode)) {
@@ -801,18 +808,18 @@ static int update_file_flags(struct merge_options *o,
808 safe_create_leading_directories_const(path);
809 unlink(path);
810 if (symlink(lnk, path))
804 - die_errno(_("failed to symlink '%s'"), path);
811 + ret = error_errno(_("failed to symlink '%s'"), path);
812 free(lnk);
813 } else
807 - die(_("do not know what to do with %06o %s '%s'"),
808 - mode, oid_to_hex(oid), path);
814 + ret = error(_("do not know what to do with %06o %s '%s'"),
815 + mode, oid_to_hex(oid), path);
816 free_buf:
817 free(buf);
818 }
819 update_index:
813 - if (update_cache)
820 + if (!ret && update_cache)
821 add_cacheinfo(mode, oid, path, 0, update_wd, ADD_CACHE_OK_TO_ADD);
815 - return 0;
822 + return ret;
823 }
824
825 static int update_file(struct merge_options *o,
@@ -938,20 +945,22 @@ static int merge_file_1(struct merge_options *o,
945 oidcpy(&result->oid, &a->oid);
946 else if (S_ISREG(a->mode)) {
947 mmbuffer_t result_buf;
941 - int merge_status;
948 + int ret = 0, merge_status;
949
950 merge_status = merge_3way(o, &result_buf, one, a, b,
951 branch1, branch2);
952
953 if ((merge_status < 0) || !result_buf.ptr)
947 - die(_("Failed to execute internal merge"));
954 + ret = error(_("Failed to execute internal merge"));
955
949 - if (write_sha1_file(result_buf.ptr, result_buf.size,
950 - blob_type, result->oid.hash))
951 - die(_("Unable to add %s to database"),
952 - a->path);
956 + if (!ret && write_sha1_file(result_buf.ptr, result_buf.size,
957 + blob_type, result->oid.hash))
958 + ret = error(_("Unable to add %s to database"),
959 + a->path);
960
961 free(result_buf.ptr);
962 + if (ret)
963 + return ret;
964 result->clean = (merge_status == 0);
965 } else if (S_ISGITLINK(a->mode)) {
966 result->clean = merge_submodule(result->oid.hash,
@@ -1885,11 +1894,10 @@ int merge_trees(struct merge_options *o,
1894
1895 if (code != 0) {
1896 if (show(o, 4) || o->call_depth)
1888 - die(_("merging of trees %s and %s failed"),
1897 + error(_("merging of trees %s and %s failed"),
1898 oid_to_hex(&head->object.oid),
1899 oid_to_hex(&merge->object.oid));
1891 - else
1892 - exit(128);
1900 + return -1;
1901 }
1902
1903 if (unmerged_cache()) {
@@ -2021,7 +2029,7 @@ int merge_recursive(struct merge_options *o,
2029 o->call_depth--;
2030
2031 if (!merged_common_ancestors)
2024 - die(_("merge returned no commit"));
2032 + return error(_("merge returned no commit"));
2033 }
2034
2035 discard_cache();