commit-reach(repo_in_merge_bases_many): report missing commits

Some functions in Git's source code follow the convention that returning a negative value indicates a fatal error, e.g. repository corruption. Let's use this convention in `repo_in_merge_bases()` to report when one of the specified commits is missing (i.e. when `repo_parse_commit()` reports an error). Also adjust the callers of `repo_in_merge_bases()` to handle such negative return values. Note: As of this patch, errors are returned only if any of the specified merge heads is missing. Over the course of the next patches, missing commits will also be reported by the `paint_down_to_common()` function, which is called by `repo_in_merge_bases_many()`, and those errors will be properly propagated back to the caller at that stage. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Johannes Schindelin committed Feb 28, 2024 at 09:44 UTC 24876ebf68baf90075dad5ca3acba8a305f308d4
12 files changed +173 -36
builtin/branch.c
+9 -3
@@ -158,6 +158,8 @@ static int branch_merged(int kind, const char *name,
158
159 merged = reference_rev ? repo_in_merge_bases(the_repository, rev,
160 reference_rev) : 0;
161 + if (merged < 0)
162 + exit(128);
163
164 /*
165 * After the safety valve is fully redefined to "check with
@@ -166,9 +168,13 @@ static int branch_merged(int kind, const char *name,
168 * any of the following code, but during the transition period,
169 * a gentle reminder is in order.
170 */
169 - if ((head_rev != reference_rev) &&
170 - (head_rev ? repo_in_merge_bases(the_repository, rev, head_rev) : 0) != merged) {
171 - if (merged)
171 + if (head_rev != reference_rev) {
172 + int expect = head_rev ? repo_in_merge_bases(the_repository, rev, head_rev) : 0;
173 + if (expect < 0)
174 + exit(128);
175 + if (expect == merged)
176 + ; /* okay */
177 + else if (merged)
178 warning(_("deleting branch '%s' that has been merged to\n"
179 " '%s', but not yet merged to HEAD"),
180 name, reference_name);
builtin/fast-import.c
+5 -1
@@ -1625,6 +1625,7 @@ static int update_branch(struct branch *b)
1625 oidclr(&old_oid);
1626 if (!force_update && !is_null_oid(&old_oid)) {
1627 struct commit *old_cmit, *new_cmit;
1628 + int ret;
1629
1630 old_cmit = lookup_commit_reference_gently(the_repository,
1631 &old_oid, 0);
@@ -1633,7 +1634,10 @@ static int update_branch(struct branch *b)
1634 if (!old_cmit || !new_cmit)
1635 return error("Branch %s is missing commits.", b->name);
1636
1636 - if (!repo_in_merge_bases(the_repository, old_cmit, new_cmit)) {
1637 + ret = repo_in_merge_bases(the_repository, old_cmit, new_cmit);
1638 + if (ret < 0)
1639 + exit(128);
1640 + if (!ret) {
1641 warning("Not updating %s"
1642 " (new tip %s does not contain %s)",
1643 b->name, oid_to_hex(&b->oid),
builtin/fetch.c
+2
@@ -982,6 +982,8 @@ static int update_local_ref(struct ref *ref,
982 uint64_t t_before = getnanotime();
983 fast_forward = repo_in_merge_bases(the_repository, current,
984 updated);
985 + if (fast_forward < 0)
986 + exit(128);
987 forced_updates_ms += (getnanotime() - t_before) / 1000000;
988 } else {
989 fast_forward = 1;
builtin/log.c
+5 -2
@@ -1625,7 +1625,7 @@ static struct commit *get_base_commit(const char *base_commit,
1625 {
1626 struct commit *base = NULL;
1627 struct commit **rev;
1628 - int i = 0, rev_nr = 0, auto_select, die_on_failure;
1628 + int i = 0, rev_nr = 0, auto_select, die_on_failure, ret;
1629
1630 switch (auto_base) {
1631 case AUTO_BASE_NEVER:
@@ -1725,7 +1725,10 @@ static struct commit *get_base_commit(const char *base_commit,
1725 rev_nr = DIV_ROUND_UP(rev_nr, 2);
1726 }
1727
1728 - if (!repo_in_merge_bases(the_repository, base, rev[0])) {
1728 + ret = repo_in_merge_bases(the_repository, base, rev[0]);
1729 + if (ret < 0)
1730 + exit(128);
1731 + if (!ret) {
1732 if (die_on_failure) {
1733 die(_("base commit should be the ancestor of revision list"));
1734 } else {
builtin/merge-base.c
+5 -1
@@ -100,12 +100,16 @@ static int handle_octopus(int count, const char **args, int show_all)
100 static int handle_is_ancestor(int argc, const char **argv)
101 {
102 struct commit *one, *two;
103 + int ret;
104
105 if (argc != 2)
106 die("--is-ancestor takes exactly two commits");
107 one = get_commit_reference(argv[0]);
108 two = get_commit_reference(argv[1]);
108 - if (repo_in_merge_bases(the_repository, one, two))
109 + ret = repo_in_merge_bases(the_repository, one, two);
110 + if (ret < 0)
111 + exit(128);
112 + if (ret)
113 return 0;
114 else
115 return 1;
builtin/pull.c
+4
@@ -926,6 +926,8 @@ static int get_can_ff(struct object_id *orig_head,
926 merge_head = lookup_commit_reference(the_repository, orig_merge_head);
927 ret = repo_is_descendant_of(the_repository, merge_head, list);
928 free_commit_list(list);
929 + if (ret < 0)
930 + exit(128);
931 return ret;
932 }
933
@@ -950,6 +952,8 @@ static int already_up_to_date(struct object_id *orig_head,
952 commit_list_insert(theirs, &list);
953 ok = repo_is_descendant_of(the_repository, ours, list);
954 free_commit_list(list);
955 + if (ok < 0)
956 + exit(128);
957 if (!ok)
958 return 0;
959 }
builtin/receive-pack.c
+5 -1
@@ -1526,6 +1526,7 @@ static const char *update(struct command *cmd, struct shallow_info *si)
1526 starts_with(name, "refs/heads/")) {
1527 struct object *old_object, *new_object;
1528 struct commit *old_commit, *new_commit;
1529 + int ret2;
1530
1531 old_object = parse_object(the_repository, old_oid);
1532 new_object = parse_object(the_repository, new_oid);
@@ -1539,7 +1540,10 @@ static const char *update(struct command *cmd, struct shallow_info *si)
1540 }
1541 old_commit = (struct commit *)old_object;
1542 new_commit = (struct commit *)new_object;
1542 - if (!repo_in_merge_bases(the_repository, old_commit, new_commit)) {
1543 + ret2 = repo_in_merge_bases(the_repository, old_commit, new_commit);
1544 + if (ret2 < 0)
1545 + exit(128);
1546 + if (!ret2) {
1547 rp_error("denying non-fast-forward %s"
1548 " (you should pull first)", name);
1549 ret = "non-fast-forward";
commit-reach.c
+6 -2
@@ -463,11 +463,13 @@ int repo_is_descendant_of(struct repository *r,
463 } else {
464 while (with_commit) {
465 struct commit *other;
466 + int ret;
467
468 other = with_commit->item;
469 with_commit = with_commit->next;
469 - if (repo_in_merge_bases_many(r, other, 1, &commit, 0))
470 - return 1;
470 + ret = repo_in_merge_bases_many(r, other, 1, &commit, 0);
471 + if (ret)
472 + return ret;
473 }
474 return 0;
475 }
@@ -597,6 +599,8 @@ int ref_newer(const struct object_id *new_oid, const struct object_id *old_oid)
599 commit_list_insert(old_commit, &old_commit_list);
600 ret = repo_is_descendant_of(the_repository,
601 new_commit, old_commit_list);
602 + if (ret < 0)
603 + exit(128);
604 free_commit_list(old_commit_list);
605 return ret;
606 }
http-push.c
+4 -1
@@ -1575,8 +1575,11 @@ static int verify_merge_base(struct object_id *head_oid, struct ref *remote)
1575 struct commit *head = lookup_commit_or_die(head_oid, "HEAD");
1576 struct commit *branch = lookup_commit_or_die(&remote->old_oid,
1577 remote->name);
1578 + int ret = repo_in_merge_bases(the_repository, branch, head);
1579
1579 - return repo_in_merge_bases(the_repository, branch, head);
1580 + if (ret < 0)
1581 + exit(128);
1582 + return ret;
1583 }
1584
1585 static int delete_remote_branch(const char *pattern, int force)
merge-ort.c
+71 -10
@@ -542,6 +542,7 @@ enum conflict_and_info_types {
542 CONFLICT_SUBMODULE_HISTORY_NOT_AVAILABLE,
543 CONFLICT_SUBMODULE_MAY_HAVE_REWINDS,
544 CONFLICT_SUBMODULE_NULL_MERGE_BASE,
545 + CONFLICT_SUBMODULE_CORRUPT,
546
547 /* Keep this entry _last_ in the list */
548 NB_CONFLICT_TYPES,
@@ -594,7 +595,9 @@ static const char *type_short_descriptions[] = {
595 [CONFLICT_SUBMODULE_MAY_HAVE_REWINDS] =
596 "CONFLICT (submodule may have rewinds)",
597 [CONFLICT_SUBMODULE_NULL_MERGE_BASE] =
597 - "CONFLICT (submodule lacks merge base)"
598 + "CONFLICT (submodule lacks merge base)",
599 + [CONFLICT_SUBMODULE_CORRUPT] =
600 + "CONFLICT (submodule corrupt)"
601 };
602
603 struct logical_conflict_info {
@@ -1708,7 +1711,14 @@ static int find_first_merges(struct repository *repo,
1711 die("revision walk setup failed");
1712 while ((commit = get_revision(&revs)) != NULL) {
1713 struct object *o = &(commit->object);
1711 - if (repo_in_merge_bases(repo, b, commit))
1714 + int ret = repo_in_merge_bases(repo, b, commit);
1715 +
1716 + if (ret < 0) {
1717 + object_array_clear(&merges);
1718 + release_revisions(&revs);
1719 + return ret;
1720 + }
1721 + if (ret > 0)
1722 add_object_array(o, NULL, &merges);
1723 }
1724 reset_revision_walk();
@@ -1723,9 +1733,17 @@ static int find_first_merges(struct repository *repo,
1733 contains_another = 0;
1734 for (j = 0; j < merges.nr; j++) {
1735 struct commit *m2 = (struct commit *) merges.objects[j].item;
1726 - if (i != j && repo_in_merge_bases(repo, m2, m1)) {
1727 - contains_another = 1;
1728 - break;
1736 + if (i != j) {
1737 + int ret = repo_in_merge_bases(repo, m2, m1);
1738 + if (ret < 0) {
1739 + object_array_clear(&merges);
1740 + release_revisions(&revs);
1741 + return ret;
1742 + }
1743 + if (ret > 0) {
1744 + contains_another = 1;
1745 + break;
1746 + }
1747 }
1748 }
1749
@@ -1747,7 +1765,7 @@ static int merge_submodule(struct merge_options *opt,
1765 {
1766 struct repository subrepo;
1767 struct strbuf sb = STRBUF_INIT;
1750 - int ret = 0;
1768 + int ret = 0, ret2;
1769 struct commit *commit_o, *commit_a, *commit_b;
1770 int parent_count;
1771 struct object_array merges;
@@ -1794,8 +1812,26 @@ static int merge_submodule(struct merge_options *opt,
1812 }
1813
1814 /* check whether both changes are forward */
1797 - if (!repo_in_merge_bases(&subrepo, commit_o, commit_a) ||
1798 - !repo_in_merge_bases(&subrepo, commit_o, commit_b)) {
1815 + ret2 = repo_in_merge_bases(&subrepo, commit_o, commit_a);
1816 + if (ret2 < 0) {
1817 + path_msg(opt, CONFLICT_SUBMODULE_CORRUPT, 0,
1818 + path, NULL, NULL, NULL,
1819 + _("Failed to merge submodule %s "
1820 + "(repository corrupt)"),
1821 + path);
1822 + goto cleanup;
1823 + }
1824 + if (ret2 > 0)
1825 + ret2 = repo_in_merge_bases(&subrepo, commit_o, commit_b);
1826 + if (ret2 < 0) {
1827 + path_msg(opt, CONFLICT_SUBMODULE_CORRUPT, 0,
1828 + path, NULL, NULL, NULL,
1829 + _("Failed to merge submodule %s "
1830 + "(repository corrupt)"),
1831 + path);
1832 + goto cleanup;
1833 + }
1834 + if (!ret2) {
1835 path_msg(opt, CONFLICT_SUBMODULE_MAY_HAVE_REWINDS, 0,
1836 path, NULL, NULL, NULL,
1837 _("Failed to merge submodule %s "
@@ -1805,7 +1841,16 @@ static int merge_submodule(struct merge_options *opt,
1841 }
1842
1843 /* Case #1: a is contained in b or vice versa */
1808 - if (repo_in_merge_bases(&subrepo, commit_a, commit_b)) {
1844 + ret2 = repo_in_merge_bases(&subrepo, commit_a, commit_b);
1845 + if (ret2 < 0) {
1846 + path_msg(opt, CONFLICT_SUBMODULE_CORRUPT, 0,
1847 + path, NULL, NULL, NULL,
1848 + _("Failed to merge submodule %s "
1849 + "(repository corrupt)"),
1850 + path);
1851 + goto cleanup;
1852 + }
1853 + if (ret2 > 0) {
1854 oidcpy(result, b);
1855 path_msg(opt, INFO_SUBMODULE_FAST_FORWARDING, 1,
1856 path, NULL, NULL, NULL,
@@ -1814,7 +1859,16 @@ static int merge_submodule(struct merge_options *opt,
1859 ret = 1;
1860 goto cleanup;
1861 }
1817 - if (repo_in_merge_bases(&subrepo, commit_b, commit_a)) {
1862 + ret2 = repo_in_merge_bases(&subrepo, commit_b, commit_a);
1863 + if (ret2 < 0) {
1864 + path_msg(opt, CONFLICT_SUBMODULE_CORRUPT, 0,
1865 + path, NULL, NULL, NULL,
1866 + _("Failed to merge submodule %s "
1867 + "(repository corrupt)"),
1868 + path);
1869 + goto cleanup;
1870 + }
1871 + if (ret2 > 0) {
1872 oidcpy(result, a);
1873 path_msg(opt, INFO_SUBMODULE_FAST_FORWARDING, 1,
1874 path, NULL, NULL, NULL,
@@ -1839,6 +1893,13 @@ static int merge_submodule(struct merge_options *opt,
1893 parent_count = find_first_merges(&subrepo, path, commit_a, commit_b,
1894 &merges);
1895 switch (parent_count) {
1896 + case -1:
1897 + path_msg(opt, CONFLICT_SUBMODULE_CORRUPT, 0,
1898 + path, NULL, NULL, NULL,
1899 + _("Failed to merge submodule %s "
1900 + "(repository corrupt)"),
1901 + path);
1902 + break;
1903 case 0:
1904 path_msg(opt, CONFLICT_SUBMODULE_FAILED_TO_MERGE, 0,
1905 path, NULL, NULL, NULL,
merge-recursive.c
+45 -9
@@ -1139,7 +1139,13 @@ static int find_first_merges(struct repository *repo,
1139 die("revision walk setup failed");
1140 while ((commit = get_revision(&revs)) != NULL) {
1141 struct object *o = &(commit->object);
1142 - if (repo_in_merge_bases(repo, b, commit))
1142 + int ret = repo_in_merge_bases(repo, b, commit);
1143 + if (ret < 0) {
1144 + object_array_clear(&merges);
1145 + release_revisions(&revs);
1146 + return ret;
1147 + }
1148 + if (ret)
1149 add_object_array(o, NULL, &merges);
1150 }
1151 reset_revision_walk();
@@ -1154,9 +1160,17 @@ static int find_first_merges(struct repository *repo,
1160 contains_another = 0;
1161 for (j = 0; j < merges.nr; j++) {
1162 struct commit *m2 = (struct commit *) merges.objects[j].item;
1157 - if (i != j && repo_in_merge_bases(repo, m2, m1)) {
1158 - contains_another = 1;
1159 - break;
1163 + if (i != j) {
1164 + int ret = repo_in_merge_bases(repo, m2, m1);
1165 + if (ret < 0) {
1166 + object_array_clear(&merges);
1167 + release_revisions(&revs);
1168 + return ret;
1169 + }
1170 + if (ret > 0) {
1171 + contains_another = 1;
1172 + break;
1173 + }
1174 }
1175 }
1176
@@ -1192,7 +1206,7 @@ static int merge_submodule(struct merge_options *opt,
1206 const struct object_id *b)
1207 {
1208 struct repository subrepo;
1195 - int ret = 0;
1209 + int ret = 0, ret2;
1210 struct commit *commit_base, *commit_a, *commit_b;
1211 int parent_count;
1212 struct object_array merges;
@@ -1229,14 +1243,29 @@ static int merge_submodule(struct merge_options *opt,
1243 }
1244
1245 /* check whether both changes are forward */
1232 - if (!repo_in_merge_bases(&subrepo, commit_base, commit_a) ||
1233 - !repo_in_merge_bases(&subrepo, commit_base, commit_b)) {
1246 + ret2 = repo_in_merge_bases(&subrepo, commit_base, commit_a);
1247 + if (ret2 < 0) {
1248 + output(opt, 1, _("Failed to merge submodule %s (repository corrupt)"), path);
1249 + goto cleanup;
1250 + }
1251 + if (ret2 > 0)
1252 + ret2 = repo_in_merge_bases(&subrepo, commit_base, commit_b);
1253 + if (ret2 < 0) {
1254 + output(opt, 1, _("Failed to merge submodule %s (repository corrupt)"), path);
1255 + goto cleanup;
1256 + }
1257 + if (!ret2) {
1258 output(opt, 1, _("Failed to merge submodule %s (commits don't follow merge-base)"), path);
1259 goto cleanup;
1260 }
1261
1262 /* Case #1: a is contained in b or vice versa */
1239 - if (repo_in_merge_bases(&subrepo, commit_a, commit_b)) {
1263 + ret2 = repo_in_merge_bases(&subrepo, commit_a, commit_b);
1264 + if (ret2 < 0) {
1265 + output(opt, 1, _("Failed to merge submodule %s (repository corrupt)"), path);
1266 + goto cleanup;
1267 + }
1268 + if (ret2) {
1269 oidcpy(result, b);
1270 if (show(opt, 3)) {
1271 output(opt, 3, _("Fast-forwarding submodule %s to the following commit:"), path);
@@ -1249,7 +1278,12 @@ static int merge_submodule(struct merge_options *opt,
1278 ret = 1;
1279 goto cleanup;
1280 }
1252 - if (repo_in_merge_bases(&subrepo, commit_b, commit_a)) {
1281 + ret2 = repo_in_merge_bases(&subrepo, commit_b, commit_a);
1282 + if (ret2 < 0) {
1283 + output(opt, 1, _("Failed to merge submodule %s (repository corrupt)"), path);
1284 + goto cleanup;
1285 + }
1286 + if (ret2) {
1287 oidcpy(result, a);
1288 if (show(opt, 3)) {
1289 output(opt, 3, _("Fast-forwarding submodule %s to the following commit:"), path);
@@ -1397,6 +1431,8 @@ static int merge_mode_and_contents(struct merge_options *opt,
1431 &o->oid,
1432 &a->oid,
1433 &b->oid);
1434 + if (result->clean < 0)
1435 + return -1;
1436 } else if (S_ISLNK(a->mode)) {
1437 switch (opt->recursive_variant) {
1438 case MERGE_VARIANT_NORMAL:
shallow.c
+12 -6
@@ -794,12 +794,16 @@ static void post_assign_shallow(struct shallow_info *info,
794 if (!*bitmap)
795 continue;
796 for (j = 0; j < bitmap_nr; j++)
797 - if (bitmap[0][j] &&
798 - /* Step 7, reachability test at commit level */
799 - !repo_in_merge_bases_many(the_repository, c, ca.nr, ca.commits, 1)) {
800 - update_refstatus(ref_status, info->ref->nr, *bitmap);
801 - dst++;
802 - break;
797 + if (bitmap[0][j]) {
798 + /* Step 7, reachability test at commit level */
799 + int ret = repo_in_merge_bases_many(the_repository, c, ca.nr, ca.commits, 1);
800 + if (ret < 0)
801 + exit(128);
802 + if (!ret) {
803 + update_refstatus(ref_status, info->ref->nr, *bitmap);
804 + dst++;
805 + break;
806 + }
807 }
808 }
809 info->nr_ours = dst;
@@ -829,6 +833,8 @@ int delayed_reachability_test(struct shallow_info *si, int c)
833 si->nr_commits,
834 si->commits,
835 1);
836 + if (si->reachable[c] < 0)
837 + exit(128);
838 si->need_reachability_test[c] = 0;
839 }
840 return si->reachable[c];