On 6/10/2019 5:47 PM, Junio C Hamano wrote:
> "Derrick Stolee via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes:
>> + if (get_oid_hex(line.buf, &oids[i])) {
>> + warning(_("invalid commit-graph chain: line '%s' not a hash"),
>> + line.buf);
>> + valid = 0;
>> + break;
>> + }
>> +
>> + graph_name = get_split_graph_filename(obj_dir, line.buf);
>> + g = load_commit_graph_one(graph_name);
>> + free(graph_name);
>> +
>> + if (g && add_graph_to_chain(g, graph_chain, oids, i))
>> + graph_chain = g;
>> + else
>> + valid = 0;
>> + }
>
> At this point, if 'i' is smaller than 'count', that would indicate
> that the file was corrupt (we hit one of the 'break' in the loop).
This is correct.
> How would we handle such an error? It appears that the strategy
> taken in this loop is to "read as many as possible without an error
> and then give up upon the first error---keep whatever we have read
> so far", so from that point of view, the only thing that is missing
> may be a warning() after the loop.
Based on previous experience with the commit-graph feature, I am
deliberately trying to ensure the commit-graph feature does not lead
to a failure case whenever possible. If we can continue with the data
at hand (and maybe lose some performance benefits if we cannot read
a graph file) then we should keep going.
I believe that with this "only warning" case, the next write would
fix the issue. The chain builds only from the graphs that successfully
load. That way, this should be a "self-healing" state.
Thanks,
-Stolee
