Git fork

commit-graph: stop passing in redundant repository

Many of the commit-graph related functions take in both a repository and
the object database source (directly or via `struct commit_graph`) for
which we are supposed to load such a commit-graph. In the best case this
information is simply redundant as the source already contains a
reference to its owning object database, which in turn has a reference
to its repository. In the worst case this information could even
mismatch when passing in a source that doesn't belong to the same
repository.

Refactor the code so that we only pass in the object database source in
those cases.

There is one exception though, namely `load_commit_graph_chain_fd_st()`,
which is responsible for loading a commit-graph chain. It is expected
that parts of the commit-graph chain aren't located in the same object
source as the chain file itself, but in a different one. Consequently,
this function doesn't work on the source level but on the database level
instead.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

authored by

Patrick Steinhardt and committed by
Junio C Hamano
7be9e410 ddacfc74

+59 -81
+3 -3
builtin/commit-graph.c
··· 122 if (opened == OPENED_NONE) 123 return 0; 124 else if (opened == OPENED_GRAPH) 125 - graph = load_commit_graph_one_fd_st(the_repository, fd, &st, source); 126 else 127 - graph = load_commit_graph_chain_fd_st(the_repository, fd, &st, 128 &incomplete_chain); 129 130 if (!graph) 131 return 1; 132 133 - ret = verify_commit_graph(the_repository, graph, flags); 134 free_commit_graph(graph); 135 136 if (incomplete_chain) {
··· 122 if (opened == OPENED_NONE) 123 return 0; 124 else if (opened == OPENED_GRAPH) 125 + graph = load_commit_graph_one_fd_st(source, fd, &st); 126 else 127 + graph = load_commit_graph_chain_fd_st(the_repository->objects, fd, &st, 128 &incomplete_chain); 129 130 if (!graph) 131 return 1; 132 133 + ret = verify_commit_graph(graph, flags); 134 free_commit_graph(graph); 135 136 if (incomplete_chain) {
+50 -70
commit-graph.c
··· 253 return 1; 254 } 255 256 - struct commit_graph *load_commit_graph_one_fd_st(struct repository *r, 257 - int fd, struct stat *st, 258 - struct odb_source *source) 259 { 260 void *graph_map; 261 size_t graph_size; ··· 263 264 graph_size = xsize_t(st->st_size); 265 266 - if (graph_size < graph_min_size(r->hash_algo)) { 267 close(fd); 268 error(_("commit-graph file is too small")); 269 return NULL; ··· 271 graph_map = xmmap(NULL, graph_size, PROT_READ, MAP_PRIVATE, fd, 0); 272 close(fd); 273 274 - ret = parse_commit_graph(r, graph_map, graph_size); 275 if (ret) 276 ret->odb_source = source; 277 else ··· 491 return NULL; 492 } 493 494 - static struct commit_graph *load_commit_graph_one(struct repository *r, 495 - const char *graph_file, 496 - struct odb_source *source) 497 { 498 - 499 struct stat st; 500 int fd; 501 struct commit_graph *g; ··· 504 if (!open_ok) 505 return NULL; 506 507 - g = load_commit_graph_one_fd_st(r, fd, &st, source); 508 - 509 if (g) 510 g->filename = xstrdup(graph_file); 511 512 return g; 513 } 514 515 - static struct commit_graph *load_commit_graph_v1(struct repository *r, 516 - struct odb_source *source) 517 { 518 char *graph_name = get_commit_graph_filename(source); 519 - struct commit_graph *g = load_commit_graph_one(r, graph_name, source); 520 free(graph_name); 521 522 return g; ··· 643 return 1; 644 } 645 646 - struct commit_graph *load_commit_graph_chain_fd_st(struct repository *r, 647 int fd, struct stat *st, 648 int *incomplete_chain) 649 { ··· 653 int i = 0, valid = 1, count; 654 FILE *fp = xfdopen(fd, "r"); 655 656 - count = st->st_size / (r->hash_algo->hexsz + 1); 657 CALLOC_ARRAY(oids, count); 658 659 - odb_prepare_alternates(r->objects); 660 661 for (i = 0; i < count; i++) { 662 struct odb_source *source; ··· 664 if (strbuf_getline_lf(&line, fp) == EOF) 665 break; 666 667 - if (get_oid_hex_algop(line.buf, &oids[i], r->hash_algo)) { 668 warning(_("invalid commit-graph chain: line '%s' not a hash"), 669 line.buf); 670 valid = 0; ··· 672 } 673 674 valid = 0; 675 - for (source = r->objects->sources; source; source = source->next) { 676 char *graph_name = get_split_graph_filename(source, line.buf); 677 - struct commit_graph *g = load_commit_graph_one(r, graph_name, source); 678 679 free(graph_name); 680 ··· 707 return graph_chain; 708 } 709 710 - static struct commit_graph *load_commit_graph_chain(struct repository *r, 711 - struct odb_source *source) 712 { 713 char *chain_file = get_commit_graph_chain_filename(source); 714 struct stat st; 715 int fd; 716 struct commit_graph *g = NULL; 717 718 - if (open_commit_graph_chain(chain_file, &fd, &st, r->hash_algo)) { 719 int incomplete; 720 /* ownership of fd is taken over by load function */ 721 - g = load_commit_graph_chain_fd_st(r, fd, &st, &incomplete); 722 } 723 724 free(chain_file); 725 return g; 726 } 727 728 - struct commit_graph *read_commit_graph_one(struct repository *r, 729 - struct odb_source *source) 730 { 731 - struct commit_graph *g = load_commit_graph_v1(r, source); 732 733 if (!g) 734 - g = load_commit_graph_chain(r, source); 735 736 return g; 737 } 738 739 - static void prepare_commit_graph_one(struct repository *r, 740 - struct odb_source *source) 741 - { 742 - 743 - if (r->objects->commit_graph) 744 - return; 745 - 746 - r->objects->commit_graph = read_commit_graph_one(r, source); 747 - } 748 - 749 /* 750 * Return 1 if commit_graph is non-NULL, and 0 otherwise. 751 * ··· 786 return 0; 787 788 odb_prepare_alternates(r->objects); 789 - for (source = r->objects->sources; 790 - !r->objects->commit_graph && source; 791 - source = source->next) 792 - prepare_commit_graph_one(r, source); 793 return !!r->objects->commit_graph; 794 } 795 ··· 874 g->hash_algo); 875 } 876 877 - static struct commit_list **insert_parent_or_die(struct repository *r, 878 - struct commit_graph *g, 879 uint32_t pos, 880 struct commit_list **pptr) 881 { ··· 886 die("invalid parent position %"PRIu32, pos); 887 888 load_oid_from_graph(g, pos, &oid); 889 - c = lookup_commit(r, &oid); 890 if (!c) 891 die(_("could not find commit %s"), oid_to_hex(&oid)); 892 commit_graph_data_at(c)->graph_pos = pos; ··· 942 c->maybe_tree = t; 943 } 944 945 - static int fill_commit_in_graph(struct repository *r, 946 - struct commit *item, 947 struct commit_graph *g, uint32_t pos) 948 { 949 uint32_t edge_value; ··· 969 edge_value = get_be32(commit_data + g->hash_algo->rawsz); 970 if (edge_value == GRAPH_PARENT_NONE) 971 return 1; 972 - pptr = insert_parent_or_die(r, g, edge_value, pptr); 973 974 edge_value = get_be32(commit_data + g->hash_algo->rawsz + 4); 975 if (edge_value == GRAPH_PARENT_NONE) 976 return 1; 977 if (!(edge_value & GRAPH_EXTRA_EDGES_NEEDED)) { 978 - pptr = insert_parent_or_die(r, g, edge_value, pptr); 979 return 1; 980 } 981 ··· 990 } 991 edge_value = get_be32(g->chunk_extra_edges + 992 sizeof(uint32_t) * parent_data_pos); 993 - pptr = insert_parent_or_die(r, g, 994 edge_value & GRAPH_EDGE_LAST_MASK, 995 pptr); 996 parent_data_pos++; ··· 1056 if (commit->object.parsed) 1057 return commit; 1058 1059 - if (!fill_commit_in_graph(repo, commit, repo->objects->commit_graph, pos)) 1060 return NULL; 1061 1062 return commit; 1063 } 1064 1065 - static int parse_commit_in_graph_one(struct repository *r, 1066 - struct commit_graph *g, 1067 struct commit *item) 1068 { 1069 uint32_t pos; ··· 1072 return 1; 1073 1074 if (find_commit_pos_in_graph(item, g, &pos)) 1075 - return fill_commit_in_graph(r, item, g, pos); 1076 1077 return 0; 1078 } ··· 1089 1090 if (!prepare_commit_graph(r)) 1091 return 0; 1092 - return parse_commit_in_graph_one(r, r->objects->commit_graph, item); 1093 } 1094 1095 void load_commit_graph_info(struct repository *r, struct commit *item) ··· 1099 fill_commit_graph_info(item, r->objects->commit_graph, pos); 1100 } 1101 1102 - static struct tree *load_tree_for_commit(struct repository *r, 1103 - struct commit_graph *g, 1104 struct commit *c) 1105 { 1106 struct object_id oid; ··· 1115 graph_pos - g->num_commits_in_base); 1116 1117 oidread(&oid, commit_data, g->hash_algo); 1118 - set_commit_tree(c, lookup_tree(r, &oid)); 1119 1120 return c->maybe_tree; 1121 } 1122 1123 - static struct tree *get_commit_tree_in_graph_one(struct repository *r, 1124 - struct commit_graph *g, 1125 const struct commit *c) 1126 { 1127 if (c->maybe_tree) ··· 1129 if (commit_graph_position(c) == COMMIT_NOT_FROM_GRAPH) 1130 BUG("get_commit_tree_in_graph_one called from non-commit-graph commit"); 1131 1132 - return load_tree_for_commit(r, g, (struct commit *)c); 1133 } 1134 1135 struct tree *get_commit_tree_in_graph(struct repository *r, const struct commit *c) 1136 { 1137 - return get_commit_tree_in_graph_one(r, r->objects->commit_graph, c); 1138 } 1139 1140 struct packed_commit_list { ··· 2741 g->data, g->data_len); 2742 } 2743 2744 - static int verify_one_commit_graph(struct repository *r, 2745 - struct commit_graph *g, 2746 struct progress *progress, 2747 uint64_t *seen) 2748 { 2749 uint32_t i, cur_fanout_pos = 0; 2750 struct object_id prev_oid, cur_oid; 2751 struct commit *seen_gen_zero = NULL; ··· 2779 } 2780 2781 graph_commit = lookup_commit(r, &cur_oid); 2782 - if (!parse_commit_in_graph_one(r, g, graph_commit)) 2783 graph_report(_("failed to parse commit %s from commit-graph"), 2784 oid_to_hex(&cur_oid)); 2785 } ··· 2815 continue; 2816 } 2817 2818 - if (!oideq(&get_commit_tree_in_graph_one(r, g, graph_commit)->object.oid, 2819 get_commit_tree_oid(odb_commit))) 2820 graph_report(_("root tree OID for commit %s in commit-graph is %s != %s"), 2821 oid_to_hex(&cur_oid), ··· 2833 } 2834 2835 /* parse parent in case it is in a base graph */ 2836 - parse_commit_in_graph_one(r, g, graph_parents->item); 2837 2838 if (!oideq(&graph_parents->item->object.oid, &odb_parents->item->object.oid)) 2839 graph_report(_("commit-graph parent for %s is %s != %s"), ··· 2893 return verify_commit_graph_error; 2894 } 2895 2896 - int verify_commit_graph(struct repository *r, struct commit_graph *g, int flags) 2897 { 2898 struct progress *progress = NULL; 2899 int local_error = 0; ··· 2909 if (!(flags & COMMIT_GRAPH_VERIFY_SHALLOW)) 2910 total += g->num_commits_in_base; 2911 2912 - progress = start_progress(r, 2913 _("Verifying commits in commit graph"), 2914 total); 2915 } 2916 2917 for (; g; g = g->base_graph) { 2918 - local_error |= verify_one_commit_graph(r, g, progress, &seen); 2919 if (flags & COMMIT_GRAPH_VERIFY_SHALLOW) 2920 break; 2921 }
··· 253 return 1; 254 } 255 256 + struct commit_graph *load_commit_graph_one_fd_st(struct odb_source *source, 257 + int fd, struct stat *st) 258 { 259 void *graph_map; 260 size_t graph_size; ··· 262 263 graph_size = xsize_t(st->st_size); 264 265 + if (graph_size < graph_min_size(source->odb->repo->hash_algo)) { 266 close(fd); 267 error(_("commit-graph file is too small")); 268 return NULL; ··· 270 graph_map = xmmap(NULL, graph_size, PROT_READ, MAP_PRIVATE, fd, 0); 271 close(fd); 272 273 + ret = parse_commit_graph(source->odb->repo, graph_map, graph_size); 274 if (ret) 275 ret->odb_source = source; 276 else ··· 490 return NULL; 491 } 492 493 + static struct commit_graph *load_commit_graph_one(struct odb_source *source, 494 + const char *graph_file) 495 { 496 struct stat st; 497 int fd; 498 struct commit_graph *g; ··· 501 if (!open_ok) 502 return NULL; 503 504 + g = load_commit_graph_one_fd_st(source, fd, &st); 505 if (g) 506 g->filename = xstrdup(graph_file); 507 508 return g; 509 } 510 511 + static struct commit_graph *load_commit_graph_v1(struct odb_source *source) 512 { 513 char *graph_name = get_commit_graph_filename(source); 514 + struct commit_graph *g = load_commit_graph_one(source, graph_name); 515 free(graph_name); 516 517 return g; ··· 638 return 1; 639 } 640 641 + struct commit_graph *load_commit_graph_chain_fd_st(struct object_database *odb, 642 int fd, struct stat *st, 643 int *incomplete_chain) 644 { ··· 648 int i = 0, valid = 1, count; 649 FILE *fp = xfdopen(fd, "r"); 650 651 + count = st->st_size / (odb->repo->hash_algo->hexsz + 1); 652 CALLOC_ARRAY(oids, count); 653 654 + odb_prepare_alternates(odb); 655 656 for (i = 0; i < count; i++) { 657 struct odb_source *source; ··· 659 if (strbuf_getline_lf(&line, fp) == EOF) 660 break; 661 662 + if (get_oid_hex_algop(line.buf, &oids[i], odb->repo->hash_algo)) { 663 warning(_("invalid commit-graph chain: line '%s' not a hash"), 664 line.buf); 665 valid = 0; ··· 667 } 668 669 valid = 0; 670 + for (source = odb->sources; source; source = source->next) { 671 char *graph_name = get_split_graph_filename(source, line.buf); 672 + struct commit_graph *g = load_commit_graph_one(source, graph_name); 673 674 free(graph_name); 675 ··· 702 return graph_chain; 703 } 704 705 + static struct commit_graph *load_commit_graph_chain(struct odb_source *source) 706 { 707 char *chain_file = get_commit_graph_chain_filename(source); 708 struct stat st; 709 int fd; 710 struct commit_graph *g = NULL; 711 712 + if (open_commit_graph_chain(chain_file, &fd, &st, source->odb->repo->hash_algo)) { 713 int incomplete; 714 /* ownership of fd is taken over by load function */ 715 + g = load_commit_graph_chain_fd_st(source->odb, fd, &st, &incomplete); 716 } 717 718 free(chain_file); 719 return g; 720 } 721 722 + struct commit_graph *read_commit_graph_one(struct odb_source *source) 723 { 724 + struct commit_graph *g = load_commit_graph_v1(source); 725 726 if (!g) 727 + g = load_commit_graph_chain(source); 728 729 return g; 730 } 731 732 /* 733 * Return 1 if commit_graph is non-NULL, and 0 otherwise. 734 * ··· 769 return 0; 770 771 odb_prepare_alternates(r->objects); 772 + for (source = r->objects->sources; source; source = source->next) { 773 + r->objects->commit_graph = read_commit_graph_one(source); 774 + if (r->objects->commit_graph) 775 + break; 776 + } 777 + 778 return !!r->objects->commit_graph; 779 } 780 ··· 859 g->hash_algo); 860 } 861 862 + static struct commit_list **insert_parent_or_die(struct commit_graph *g, 863 uint32_t pos, 864 struct commit_list **pptr) 865 { ··· 870 die("invalid parent position %"PRIu32, pos); 871 872 load_oid_from_graph(g, pos, &oid); 873 + c = lookup_commit(g->odb_source->odb->repo, &oid); 874 if (!c) 875 die(_("could not find commit %s"), oid_to_hex(&oid)); 876 commit_graph_data_at(c)->graph_pos = pos; ··· 926 c->maybe_tree = t; 927 } 928 929 + static int fill_commit_in_graph(struct commit *item, 930 struct commit_graph *g, uint32_t pos) 931 { 932 uint32_t edge_value; ··· 952 edge_value = get_be32(commit_data + g->hash_algo->rawsz); 953 if (edge_value == GRAPH_PARENT_NONE) 954 return 1; 955 + pptr = insert_parent_or_die(g, edge_value, pptr); 956 957 edge_value = get_be32(commit_data + g->hash_algo->rawsz + 4); 958 if (edge_value == GRAPH_PARENT_NONE) 959 return 1; 960 if (!(edge_value & GRAPH_EXTRA_EDGES_NEEDED)) { 961 + pptr = insert_parent_or_die(g, edge_value, pptr); 962 return 1; 963 } 964 ··· 973 } 974 edge_value = get_be32(g->chunk_extra_edges + 975 sizeof(uint32_t) * parent_data_pos); 976 + pptr = insert_parent_or_die(g, 977 edge_value & GRAPH_EDGE_LAST_MASK, 978 pptr); 979 parent_data_pos++; ··· 1039 if (commit->object.parsed) 1040 return commit; 1041 1042 + if (!fill_commit_in_graph(commit, repo->objects->commit_graph, pos)) 1043 return NULL; 1044 1045 return commit; 1046 } 1047 1048 + static int parse_commit_in_graph_one(struct commit_graph *g, 1049 struct commit *item) 1050 { 1051 uint32_t pos; ··· 1054 return 1; 1055 1056 if (find_commit_pos_in_graph(item, g, &pos)) 1057 + return fill_commit_in_graph(item, g, pos); 1058 1059 return 0; 1060 } ··· 1071 1072 if (!prepare_commit_graph(r)) 1073 return 0; 1074 + return parse_commit_in_graph_one(r->objects->commit_graph, item); 1075 } 1076 1077 void load_commit_graph_info(struct repository *r, struct commit *item) ··· 1081 fill_commit_graph_info(item, r->objects->commit_graph, pos); 1082 } 1083 1084 + static struct tree *load_tree_for_commit(struct commit_graph *g, 1085 struct commit *c) 1086 { 1087 struct object_id oid; ··· 1096 graph_pos - g->num_commits_in_base); 1097 1098 oidread(&oid, commit_data, g->hash_algo); 1099 + set_commit_tree(c, lookup_tree(g->odb_source->odb->repo, &oid)); 1100 1101 return c->maybe_tree; 1102 } 1103 1104 + static struct tree *get_commit_tree_in_graph_one(struct commit_graph *g, 1105 const struct commit *c) 1106 { 1107 if (c->maybe_tree) ··· 1109 if (commit_graph_position(c) == COMMIT_NOT_FROM_GRAPH) 1110 BUG("get_commit_tree_in_graph_one called from non-commit-graph commit"); 1111 1112 + return load_tree_for_commit(g, (struct commit *)c); 1113 } 1114 1115 struct tree *get_commit_tree_in_graph(struct repository *r, const struct commit *c) 1116 { 1117 + return get_commit_tree_in_graph_one(r->objects->commit_graph, c); 1118 } 1119 1120 struct packed_commit_list { ··· 2721 g->data, g->data_len); 2722 } 2723 2724 + static int verify_one_commit_graph(struct commit_graph *g, 2725 struct progress *progress, 2726 uint64_t *seen) 2727 { 2728 + struct repository *r = g->odb_source->odb->repo; 2729 uint32_t i, cur_fanout_pos = 0; 2730 struct object_id prev_oid, cur_oid; 2731 struct commit *seen_gen_zero = NULL; ··· 2759 } 2760 2761 graph_commit = lookup_commit(r, &cur_oid); 2762 + if (!parse_commit_in_graph_one(g, graph_commit)) 2763 graph_report(_("failed to parse commit %s from commit-graph"), 2764 oid_to_hex(&cur_oid)); 2765 } ··· 2795 continue; 2796 } 2797 2798 + if (!oideq(&get_commit_tree_in_graph_one(g, graph_commit)->object.oid, 2799 get_commit_tree_oid(odb_commit))) 2800 graph_report(_("root tree OID for commit %s in commit-graph is %s != %s"), 2801 oid_to_hex(&cur_oid), ··· 2813 } 2814 2815 /* parse parent in case it is in a base graph */ 2816 + parse_commit_in_graph_one(g, graph_parents->item); 2817 2818 if (!oideq(&graph_parents->item->object.oid, &odb_parents->item->object.oid)) 2819 graph_report(_("commit-graph parent for %s is %s != %s"), ··· 2873 return verify_commit_graph_error; 2874 } 2875 2876 + int verify_commit_graph(struct commit_graph *g, int flags) 2877 { 2878 struct progress *progress = NULL; 2879 int local_error = 0; ··· 2889 if (!(flags & COMMIT_GRAPH_VERIFY_SHALLOW)) 2890 total += g->num_commits_in_base; 2891 2892 + progress = start_progress(g->odb_source->odb->repo, 2893 _("Verifying commits in commit graph"), 2894 total); 2895 } 2896 2897 for (; g; g = g->base_graph) { 2898 + local_error |= verify_one_commit_graph(g, progress, &seen); 2899 if (flags & COMMIT_GRAPH_VERIFY_SHALLOW) 2900 break; 2901 }
+5 -7
commit-graph.h
··· 114 struct bloom_filter_settings *bloom_filter_settings; 115 }; 116 117 - struct commit_graph *load_commit_graph_one_fd_st(struct repository *r, 118 - int fd, struct stat *st, 119 - struct odb_source *source); 120 - struct commit_graph *load_commit_graph_chain_fd_st(struct repository *r, 121 int fd, struct stat *st, 122 int *incomplete_chain); 123 - struct commit_graph *read_commit_graph_one(struct repository *r, 124 - struct odb_source *source); 125 126 struct repo_settings; 127 ··· 185 186 #define COMMIT_GRAPH_VERIFY_SHALLOW (1 << 0) 187 188 - int verify_commit_graph(struct repository *r, struct commit_graph *g, int flags); 189 190 void close_commit_graph(struct object_database *); 191 void free_commit_graph(struct commit_graph *);
··· 114 struct bloom_filter_settings *bloom_filter_settings; 115 }; 116 117 + struct commit_graph *load_commit_graph_one_fd_st(struct odb_source *source, 118 + int fd, struct stat *st); 119 + struct commit_graph *load_commit_graph_chain_fd_st(struct object_database *odb, 120 int fd, struct stat *st, 121 int *incomplete_chain); 122 + struct commit_graph *read_commit_graph_one(struct odb_source *source); 123 124 struct repo_settings; 125 ··· 183 184 #define COMMIT_GRAPH_VERIFY_SHALLOW (1 << 0) 185 186 + int verify_commit_graph(struct commit_graph *g, int flags); 187 188 void close_commit_graph(struct object_database *); 189 void free_commit_graph(struct commit_graph *);
+1 -1
t/helper/test-read-graph.c
··· 81 82 prepare_repo_settings(the_repository); 83 84 - graph = read_commit_graph_one(the_repository, source); 85 if (!graph) { 86 ret = 1; 87 goto done;
··· 81 82 prepare_repo_settings(the_repository); 83 84 + graph = read_commit_graph_one(source); 85 if (!graph) { 86 ret = 1; 87 goto done;