• Ævar Arnfjörð Bjarmason's avatar
    commit-graph write: don't die if the existing graph is corrupt · 43d35618
    Ævar Arnfjörð Bjarmason authored
    When the commit-graph is written we end up calling
    parse_commit(). This will in turn invoke code that'll consult the
    existing commit-graph about the commit, if the graph is corrupted we
    die.
    
    We thus get into a state where a failing "commit-graph verify" can't
    be followed-up with a "commit-graph write" if core.commitGraph=true is
    set, the graph either needs to be manually removed to proceed, or
    core.commitGraph needs to be set to "false".
    
    Change the "commit-graph write" codepath to use a new
    parse_commit_no_graph() helper instead of parse_commit() to avoid
    this. The latter will call repo_parse_commit_internal() with
    use_commit_graph=1 as seen in 177722b3 ("commit: integrate commit
    graph with commit parsing", 2018-04-10).
    
    Not using the old graph at all slows down the writing of the new graph
    by some small amount, but is a sensible way to prevent an error in the
    existing commit-graph from spreading.
    
    Just fixing the current issue would be likely to result in code that's
    inadvertently broken in the future. New code might use the
    commit-graph at a distance. To detect such cases introduce a
    "GIT_TEST_COMMIT_GRAPH_DIE_ON_LOAD" setting used when we do our
    corruption tests, and test that a "write/verify" combo works after
    every one of our current test cases where we now detect commit-graph
    corruption.
    
    Some of the code changes here might be strictly unnecessary, e.g. I
    was unable to find cases where the parse_commit() called from
    write_graph_chunk_data() didn't exit early due to
    "item->object.parsed" being true in
    repo_parse_commit_internal() (before the use_commit_graph=1 has any
    effect). But let's also convert those cases for good measure, we do
    not have exhaustive tests for all possible types of commit-graph
    corruption.
    
    This might need to be re-visited if we learn to write the commit-graph
    incrementally, but probably not. Hopefully we'll just start by finding
    out what commits we have in total, then read the old graph(s) to see
    what they cover, and finally write a new graph file with everything
    that's missing. In that case the new graph writing code just needs to
    continue to use e.g. a parse_commit() that doesn't consult the
    existing commit-graphs.
    Signed-off-by: Ævar Arnfjörð Bjarmason's avatarÆvar Arnfjörð Bjarmason <avarab@gmail.com>
    Signed-off-by: 's avatarJunio C Hamano <gitster@pobox.com>
    43d35618
commit-graph.h 2.23 KB