On 5/10/2018 2:29 PM, Martin Ågren wrote:
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. ;-)
It shouldn't, but a duplicate commit is still an incorrect oid order.
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?
Yeah, I'll make all of the errors untranslated since they are for
developer debugging, not end-user information.
+
+ oid_to_hex(&prev_oid),
+ oid_to_hex(&cur_oid));
Hmm, these two lines do not actually achieve anything?
It's a formatting bug: They complete the statement starting with
'graph_report(' above.
+ 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?
As someone who is responsible for probably inserting these problems, I
think the details are important.
Thanks,
-Stolee
