From: Johannes Schindelin <johannes.schindelin@xxxxxx> 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. Signed-off-by: Johannes Schindelin <johannes.schindelin@xxxxxx> --- builtin/branch.c | 12 +++++-- builtin/fast-import.c | 6 +++- builtin/fetch.c | 2 ++ builtin/log.c | 7 ++-- builtin/merge-base.c | 6 +++- builtin/pull.c | 4 +++ builtin/receive-pack.c | 6 +++- commit-reach.c | 12 ++++--- http-push.c | 5 ++- merge-ort.c | 75 ++++++++++++++++++++++++++++++++++++------ merge-recursive.c | 48 ++++++++++++++++++++++----- shallow.c | 18 ++++++---- 12 files changed, 163 insertions(+), 38 deletions(-) diff --git a/builtin/branch.c b/builtin/branch.c index e7ee9bd0f15..7f9e79237f3 100644 --- a/builtin/branch.c +++ b/builtin/branch.c @@ -161,6 +161,8 @@ static int branch_merged(int kind, const char *name, merged = reference_rev ? repo_in_merge_bases(the_repository, rev, reference_rev) : 0; + if (merged < 0) + exit(128); /* * After the safety valve is fully redefined to "check with @@ -169,9 +171,13 @@ static int branch_merged(int kind, const char *name, * any of the following code, but during the transition period, * a gentle reminder is in order. */ - if ((head_rev != reference_rev) && - (head_rev ? repo_in_merge_bases(the_repository, rev, head_rev) : 0) != merged) { - if (merged) + if (head_rev != reference_rev) { + int expect = head_rev ? repo_in_merge_bases(the_repository, rev, head_rev) : 0; + if (expect < 0) + exit(128); + if (expect == merged) + ; /* okay */ + else if (merged) warning(_("deleting branch '%s' that has been merged to\n" " '%s', but not yet merged to HEAD"), name, reference_name); diff --git a/builtin/fast-import.c b/builtin/fast-import.c index 444f41cf8ca..14c2efa88fc 100644 --- a/builtin/fast-import.c +++ b/builtin/fast-import.c @@ -1625,6 +1625,7 @@ static int update_branch(struct branch *b) oidclr(&old_oid); if (!force_update && !is_null_oid(&old_oid)) { struct commit *old_cmit, *new_cmit; + int ret; old_cmit = lookup_commit_reference_gently(the_repository, &old_oid, 0); @@ -1633,7 +1634,10 @@ static int update_branch(struct branch *b) if (!old_cmit || !new_cmit) return error("Branch %s is missing commits.", b->name); - if (!repo_in_merge_bases(the_repository, old_cmit, new_cmit)) { + ret = repo_in_merge_bases(the_repository, old_cmit, new_cmit); + if (ret < 0) + exit(128); + if (!ret) { warning("Not updating %s" " (new tip %s does not contain %s)", b->name, oid_to_hex(&b->oid), diff --git a/builtin/fetch.c b/builtin/fetch.c index fd134ba74d9..0584a1f8b64 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -978,6 +978,8 @@ static int update_local_ref(struct ref *ref, uint64_t t_before = getnanotime(); fast_forward = repo_in_merge_bases(the_repository, current, updated); + if (fast_forward < 0) + exit(128); forced_updates_ms += (getnanotime() - t_before) / 1000000; } else { fast_forward = 1; diff --git a/builtin/log.c b/builtin/log.c index ba775d7b5cf..1705da71aca 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -1623,7 +1623,7 @@ static struct commit *get_base_commit(const char *base_commit, { struct commit *base = NULL; struct commit **rev; - int i = 0, rev_nr = 0, auto_select, die_on_failure; + int i = 0, rev_nr = 0, auto_select, die_on_failure, ret; switch (auto_base) { case AUTO_BASE_NEVER: @@ -1723,7 +1723,10 @@ static struct commit *get_base_commit(const char *base_commit, rev_nr = DIV_ROUND_UP(rev_nr, 2); } - if (!repo_in_merge_bases(the_repository, base, rev[0])) { + ret = repo_in_merge_bases(the_repository, base, rev[0]); + if (ret < 0) + exit(128); + if (!ret) { if (die_on_failure) { die(_("base commit should be the ancestor of revision list")); } else { diff --git a/builtin/merge-base.c b/builtin/merge-base.c index e68b7fe45d7..0308fd73289 100644 --- a/builtin/merge-base.c +++ b/builtin/merge-base.c @@ -103,12 +103,16 @@ static int handle_octopus(int count, const char **args, int show_all) static int handle_is_ancestor(int argc, const char **argv) { struct commit *one, *two; + int ret; if (argc != 2) die("--is-ancestor takes exactly two commits"); one = get_commit_reference(argv[0]); two = get_commit_reference(argv[1]); - if (repo_in_merge_bases(the_repository, one, two)) + ret = repo_in_merge_bases(the_repository, one, two); + if (ret < 0) + exit(128); + if (ret) return 0; else return 1; diff --git a/builtin/pull.c b/builtin/pull.c index be2b2c9ebc9..e6f2942c0c5 100644 --- a/builtin/pull.c +++ b/builtin/pull.c @@ -931,6 +931,8 @@ static int get_can_ff(struct object_id *orig_head, merge_head = lookup_commit_reference(the_repository, orig_merge_head); ret = repo_is_descendant_of(the_repository, merge_head, list); free_commit_list(list); + if (ret < 0) + exit(128); return ret; } @@ -955,6 +957,8 @@ static int already_up_to_date(struct object_id *orig_head, commit_list_insert(theirs, &list); ok = repo_is_descendant_of(the_repository, ours, list); free_commit_list(list); + if (ok < 0) + exit(128); if (!ok) return 0; } diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c index 8c4f0cb90a9..956fea6293e 100644 --- a/builtin/receive-pack.c +++ b/builtin/receive-pack.c @@ -1546,6 +1546,7 @@ static const char *update(struct command *cmd, struct shallow_info *si) starts_with(name, "refs/heads/")) { struct object *old_object, *new_object; struct commit *old_commit, *new_commit; + int ret2; old_object = parse_object(the_repository, old_oid); new_object = parse_object(the_repository, new_oid); @@ -1559,7 +1560,10 @@ static const char *update(struct command *cmd, struct shallow_info *si) } old_commit = (struct commit *)old_object; new_commit = (struct commit *)new_object; - if (!repo_in_merge_bases(the_repository, old_commit, new_commit)) { + ret2 = repo_in_merge_bases(the_repository, old_commit, new_commit); + if (ret2 < 0) + exit(128); + if (!ret2) { rp_error("denying non-fast-forward %s" " (you should pull first)", name); ret = "non-fast-forward"; diff --git a/commit-reach.c b/commit-reach.c index b2b102926c9..dab32eb470d 100644 --- a/commit-reach.c +++ b/commit-reach.c @@ -463,11 +463,13 @@ int repo_is_descendant_of(struct repository *r, } else { while (with_commit) { struct commit *other; + int ret; other = with_commit->item; with_commit = with_commit->next; - if (repo_in_merge_bases_many(r, other, 1, &commit)) - return 1; + ret = repo_in_merge_bases_many(r, other, 1, &commit); + if (ret) + return ret; } return 0; } @@ -484,10 +486,10 @@ int repo_in_merge_bases_many(struct repository *r, struct commit *commit, timestamp_t generation, max_generation = GENERATION_NUMBER_ZERO; if (repo_parse_commit(r, commit)) - return ret; + return -1; for (i = 0; i < nr_reference; i++) { if (repo_parse_commit(r, reference[i])) - return ret; + return -1; generation = commit_graph_generation(reference[i]); if (generation > max_generation) @@ -596,6 +598,8 @@ int ref_newer(const struct object_id *new_oid, const struct object_id *old_oid) commit_list_insert(old_commit, &old_commit_list); ret = repo_is_descendant_of(the_repository, new_commit, old_commit_list); + if (ret < 0) + exit(128); free_commit_list(old_commit_list); return ret; } diff --git a/http-push.c b/http-push.c index a704f490fdb..85fa2f457d4 100644 --- a/http-push.c +++ b/http-push.c @@ -1576,8 +1576,11 @@ static int verify_merge_base(struct object_id *head_oid, struct ref *remote) struct commit *head = lookup_commit_or_die(head_oid, "HEAD"); struct commit *branch = lookup_commit_or_die(&remote->old_oid, remote->name); + int i = repo_in_merge_bases(the_repository, branch, head); - return repo_in_merge_bases(the_repository, branch, head); + if (i < 0) + exit(128); + return i; } static int delete_remote_branch(const char *pattern, int force) diff --git a/merge-ort.c b/merge-ort.c index 6491070d965..64e76afe89f 100644 --- a/merge-ort.c +++ b/merge-ort.c @@ -544,6 +544,7 @@ enum conflict_and_info_types { CONFLICT_SUBMODULE_HISTORY_NOT_AVAILABLE, CONFLICT_SUBMODULE_MAY_HAVE_REWINDS, CONFLICT_SUBMODULE_NULL_MERGE_BASE, + CONFLICT_SUBMODULE_CORRUPT, /* Keep this entry _last_ in the list */ NB_CONFLICT_TYPES, @@ -596,7 +597,9 @@ static const char *type_short_descriptions[] = { [CONFLICT_SUBMODULE_MAY_HAVE_REWINDS] = "CONFLICT (submodule may have rewinds)", [CONFLICT_SUBMODULE_NULL_MERGE_BASE] = - "CONFLICT (submodule lacks merge base)" + "CONFLICT (submodule lacks merge base)", + [CONFLICT_SUBMODULE_CORRUPT] = + "CONFLICT (submodule corrupt)" }; struct logical_conflict_info { @@ -1710,7 +1713,11 @@ static int find_first_merges(struct repository *repo, die("revision walk setup failed"); while ((commit = get_revision(&revs)) != NULL) { struct object *o = &(commit->object); - if (repo_in_merge_bases(repo, b, commit)) + int ret = repo_in_merge_bases(repo, b, commit); + + if (ret < 0) + return ret; + if (ret > 0) add_object_array(o, NULL, &merges); } reset_revision_walk(); @@ -1725,9 +1732,14 @@ static int find_first_merges(struct repository *repo, contains_another = 0; for (j = 0; j < merges.nr; j++) { struct commit *m2 = (struct commit *) merges.objects[j].item; - if (i != j && repo_in_merge_bases(repo, m2, m1)) { - contains_another = 1; - break; + if (i != j) { + int ret = repo_in_merge_bases(repo, m2, m1); + if (ret < 0) + return ret; + if (ret > 0) { + contains_another = 1; + break; + } } } @@ -1749,7 +1761,7 @@ static int merge_submodule(struct merge_options *opt, { struct repository subrepo; struct strbuf sb = STRBUF_INIT; - int ret = 0; + int ret = 0, ret2; struct commit *commit_o, *commit_a, *commit_b; int parent_count; struct object_array merges; @@ -1796,8 +1808,26 @@ static int merge_submodule(struct merge_options *opt, } /* check whether both changes are forward */ - if (!repo_in_merge_bases(&subrepo, commit_o, commit_a) || - !repo_in_merge_bases(&subrepo, commit_o, commit_b)) { + ret2 = repo_in_merge_bases(&subrepo, commit_o, commit_a); + if (ret2 < 1) { + path_msg(opt, CONFLICT_SUBMODULE_CORRUPT, 0, + path, NULL, NULL, NULL, + _("Failed to merge submodule %s " + "(repository corrupt)"), + path); + goto cleanup; + } + if (!ret2) + ret2 = repo_in_merge_bases(&subrepo, commit_o, commit_b); + if (ret2 < 1) { + path_msg(opt, CONFLICT_SUBMODULE_CORRUPT, 0, + path, NULL, NULL, NULL, + _("Failed to merge submodule %s " + "(repository corrupt)"), + path); + goto cleanup; + } + if (!ret2) { path_msg(opt, CONFLICT_SUBMODULE_MAY_HAVE_REWINDS, 0, path, NULL, NULL, NULL, _("Failed to merge submodule %s " @@ -1807,7 +1837,16 @@ static int merge_submodule(struct merge_options *opt, } /* Case #1: a is contained in b or vice versa */ - if (repo_in_merge_bases(&subrepo, commit_a, commit_b)) { + ret2 = repo_in_merge_bases(&subrepo, commit_a, commit_b); + if (ret2 < 0) { + path_msg(opt, CONFLICT_SUBMODULE_CORRUPT, 0, + path, NULL, NULL, NULL, + _("Failed to merge submodule %s " + "(repository corrupt)"), + path); + goto cleanup; + } + if (ret2 > 0) { oidcpy(result, b); path_msg(opt, INFO_SUBMODULE_FAST_FORWARDING, 1, path, NULL, NULL, NULL, @@ -1816,7 +1855,16 @@ static int merge_submodule(struct merge_options *opt, ret = 1; goto cleanup; } - if (repo_in_merge_bases(&subrepo, commit_b, commit_a)) { + ret2 = repo_in_merge_bases(&subrepo, commit_b, commit_a); + if (ret2 < 0) { + path_msg(opt, CONFLICT_SUBMODULE_CORRUPT, 0, + path, NULL, NULL, NULL, + _("Failed to merge submodule %s " + "(repository corrupt)"), + path); + goto cleanup; + } + if (ret2 > 0) { oidcpy(result, a); path_msg(opt, INFO_SUBMODULE_FAST_FORWARDING, 1, path, NULL, NULL, NULL, @@ -1841,6 +1889,13 @@ static int merge_submodule(struct merge_options *opt, parent_count = find_first_merges(&subrepo, path, commit_a, commit_b, &merges); switch (parent_count) { + case -1: + path_msg(opt, CONFLICT_SUBMODULE_CORRUPT, 0, + path, NULL, NULL, NULL, + _("Failed to merge submodule %s " + "(repository corrupt)"), + path); + break; case 0: path_msg(opt, CONFLICT_SUBMODULE_FAILED_TO_MERGE, 0, path, NULL, NULL, NULL, diff --git a/merge-recursive.c b/merge-recursive.c index e3beb0801b1..e3fe7803cbe 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -1144,7 +1144,10 @@ static int find_first_merges(struct repository *repo, die("revision walk setup failed"); while ((commit = get_revision(&revs)) != NULL) { struct object *o = &(commit->object); - if (repo_in_merge_bases(repo, b, commit)) + int ret = repo_in_merge_bases(repo, b, commit); + if (ret < 0) + return ret; + if (ret) add_object_array(o, NULL, &merges); } reset_revision_walk(); @@ -1159,9 +1162,14 @@ static int find_first_merges(struct repository *repo, contains_another = 0; for (j = 0; j < merges.nr; j++) { struct commit *m2 = (struct commit *) merges.objects[j].item; - if (i != j && repo_in_merge_bases(repo, m2, m1)) { - contains_another = 1; - break; + if (i != j) { + int ret = repo_in_merge_bases(repo, m2, m1); + if (ret < 0) + return ret; + if (ret > 0) { + contains_another = 1; + break; + } } } @@ -1197,7 +1205,7 @@ static int merge_submodule(struct merge_options *opt, const struct object_id *b) { struct repository subrepo; - int ret = 0; + int ret = 0, ret2; struct commit *commit_base, *commit_a, *commit_b; int parent_count; struct object_array merges; @@ -1234,14 +1242,29 @@ static int merge_submodule(struct merge_options *opt, } /* check whether both changes are forward */ - if (!repo_in_merge_bases(&subrepo, commit_base, commit_a) || - !repo_in_merge_bases(&subrepo, commit_base, commit_b)) { + ret2 = repo_in_merge_bases(&subrepo, commit_base, commit_a); + if (ret2 < 0) { + output(opt, 1, _("Failed to merge submodule %s (repository corrupt)"), path); + goto cleanup; + } + if (ret2) + ret2 = repo_in_merge_bases(&subrepo, commit_base, commit_b); + if (ret2 < 0) { + output(opt, 1, _("Failed to merge submodule %s (repository corrupt)"), path); + goto cleanup; + } + if (!ret2) { output(opt, 1, _("Failed to merge submodule %s (commits don't follow merge-base)"), path); goto cleanup; } /* Case #1: a is contained in b or vice versa */ - if (repo_in_merge_bases(&subrepo, commit_a, commit_b)) { + ret2 = repo_in_merge_bases(&subrepo, commit_a, commit_b); + if (ret2 < 0) { + output(opt, 1, _("Failed to merge submodule %s (repository corrupt)"), path); + goto cleanup; + } + if (ret2) { oidcpy(result, b); if (show(opt, 3)) { output(opt, 3, _("Fast-forwarding submodule %s to the following commit:"), path); @@ -1254,7 +1277,12 @@ static int merge_submodule(struct merge_options *opt, ret = 1; goto cleanup; } - if (repo_in_merge_bases(&subrepo, commit_b, commit_a)) { + ret2 = repo_in_merge_bases(&subrepo, commit_b, commit_a); + if (ret2 < 0) { + output(opt, 1, _("Failed to merge submodule %s (repository corrupt)"), path); + goto cleanup; + } + if (ret2) { oidcpy(result, a); if (show(opt, 3)) { output(opt, 3, _("Fast-forwarding submodule %s to the following commit:"), path); @@ -1402,6 +1430,8 @@ static int merge_mode_and_contents(struct merge_options *opt, &o->oid, &a->oid, &b->oid); + if (result->clean < 0) + return -1; } else if (S_ISLNK(a->mode)) { switch (opt->recursive_variant) { case MERGE_VARIANT_NORMAL: diff --git a/shallow.c b/shallow.c index ac728cdd778..cf4b95114b7 100644 --- a/shallow.c +++ b/shallow.c @@ -795,12 +795,16 @@ static void post_assign_shallow(struct shallow_info *info, if (!*bitmap) continue; for (j = 0; j < bitmap_nr; j++) - if (bitmap[0][j] && - /* Step 7, reachability test at commit level */ - !repo_in_merge_bases_many(the_repository, c, ca.nr, ca.commits)) { - update_refstatus(ref_status, info->ref->nr, *bitmap); - dst++; - break; + if (bitmap[0][j]) { + /* Step 7, reachability test at commit level */ + int ret = repo_in_merge_bases_many(the_repository, c, ca.nr, ca.commits); + if (ret < 0) + exit(128); + if (!ret) { + update_refstatus(ref_status, info->ref->nr, *bitmap); + dst++; + break; + } } } info->nr_ours = dst; @@ -829,6 +833,8 @@ int delayed_reachability_test(struct shallow_info *si, int c) commit, si->nr_commits, si->commits); + if (si->reachable[c] < 0) + exit(128); si->need_reachability_test[c] = 0; } return si->reachable[c]; -- gitgitgadget