On 10 May 2018 at 19:34, Derrick Stolee <dstolee@xxxxxxxxxxxxx> wrote:
> While running 'git commit-graph verify', verify that the object IDs
> are listed in lexicographic order and that the fanout table correctly
> navigates into that list of object IDs.
>
> Signed-off-by: Derrick Stolee <dstolee@xxxxxxxxxxxxx>
> ---
> commit-graph.c | 33 +++++++++++++++++++++++++++++++++
> 1 file changed, 33 insertions(+)
>
> diff --git a/commit-graph.c b/commit-graph.c
> index ce11af1d20..b4c146c423 100644
> --- a/commit-graph.c
> +++ b/commit-graph.c
> @@ -839,6 +839,9 @@ static int verify_commit_graph_error;
>
> int verify_commit_graph(struct commit_graph *g)
> {
> + uint32_t i, cur_fanout_pos = 0;
> + struct object_id prev_oid, cur_oid;
> +
> if (!g) {
> graph_report(_("no commit-graph file loaded"));
> return 1;
> @@ -853,5 +856,35 @@ int verify_commit_graph(struct commit_graph *g)
> if (!g->chunk_commit_data)
> graph_report(_("commit-graph is missing the Commit Data chunk"));
>
> + for (i = 0; i < g->num_commits; i++) {
> + hashcpy(cur_oid.hash, g->chunk_oid_lookup + g->hash_len * i);
> +
> + if (i > 0 && oidcmp(&prev_oid, &cur_oid) >= 0)
> + graph_report(_("commit-graph has incorrect oid order: %s then %s"),
Minor: I think our style would prefer s/i > 0/i/.
I suppose the second check should be s/>=/>/, but it's not like it
should matter. ;-)
I wonder if this is a message that would virtually never make sense to a
user, but more to a developer. Leave it untranslated to make sure any
bug reports to the list are readable to us?
> +
> + oid_to_hex(&prev_oid),
> + oid_to_hex(&cur_oid));
Hmm, these two lines do not actually achieve anything?
> + oidcpy(&prev_oid, &cur_oid);
> +
> + while (cur_oid.hash[0] > cur_fanout_pos) {
> + uint32_t fanout_value = get_be32(g->chunk_oid_fanout + cur_fanout_pos);
> + if (i != fanout_value)
> + graph_report(_("commit-graph has incorrect fanout value: fanout[%d] = %u != %u"),
> + cur_fanout_pos, fanout_value, i);
Same though re `_()`, even more so because of the more technical
notation.
> +
> + cur_fanout_pos++;
> + }
> + }
> +
> + while (cur_fanout_pos < 256) {
> + uint32_t fanout_value = get_be32(g->chunk_oid_fanout + cur_fanout_pos);
> +
> + if (g->num_commits != fanout_value)
> + graph_report(_("commit-graph has incorrect fanout value: fanout[%d] = %u != %u"),
> + cur_fanout_pos, fanout_value, i);
Same here. Or maybe these should just give a translated user-readable
basic idea of what is wrong and skip the details?
Martin
