See the v1 cover letter for details: https://public-inbox.org/git/20190221223753.20070-1-avarab@xxxxxxxxx/ I'd forgotten this after 2.21 was released. This addresses all the comments on v1 and rebases it. A range-diff is below. I also improved 7/8's commit message a bit. I fixed a test to avoid the pattern a0a630192d (t/check-non-portable-shell: detect "FOO=bar shell_func", 2018-07-13) tests for. The new pattern is more obvious. As an aside I don't get how that doesn't work as intended from the commit message of that commit & its series. $ cat /tmp/x.sh sayit() { echo "saying '$SAY'"; }; SAY=hi sayit; sayit; $ sh /tmp/x.sh saying 'hi' saying '' I get the same thing on bash, dash, NetBSD ksh, Solaris's shell & AIX's. I.e. it's explicitly not assigning $SAY for the duration of the shell as this would do: $ cat /tmp/y.sh sayit() { echo "saying '$SAY'"; }; SAY=hi; sayit; sayit; $ sh /tmp/y.sh saying 'hi' saying 'hi' Which is the impression I get from the commit message. Ævar Arnfjörð Bjarmason (8): commit-graph tests: split up corrupt_graph_and_verify() commit-graph tests: test a graph that's too small commit-graph: fix segfault on e.g. "git status" commit-graph: don't early exit(1) on e.g. "git status" commit-graph: don't pass filename to load_commit_graph_one_fd_st() commit-graph verify: detect inability to read the graph commit-graph write: don't die if the existing graph is corrupt commit-graph: improve & i18n error messages builtin/commit-graph.c | 23 +++++-- commit-graph.c | 132 +++++++++++++++++++++++++++------------- commit-graph.h | 4 ++ commit.h | 6 ++ t/t5318-commit-graph.sh | 42 +++++++++++-- 5 files changed, 154 insertions(+), 53 deletions(-) Range-diff: 1: 9d318d5106 ! 1: 2f8ba0adf8 commit-graph tests: split up corrupt_graph_and_verify() @@ -49,7 +49,7 @@ - test_when_finished mv commit-graph-backup $objdir/info/commit-graph && - cp $objdir/info/commit-graph commit-graph-backup && printf "$data" | dd of="$objdir/info/commit-graph" bs=1 seek="$pos" conv=notrunc && - dd of="$objdir/info/commit-graph" bs=1 seek="$zero_pos" count=0 && + dd of="$objdir/info/commit-graph" bs=1 seek="$zero_pos" if=/dev/null && generate_zero_bytes $(($orig_size - $zero_pos)) >>"$objdir/info/commit-graph" && - test_must_fail git commit-graph verify 2>test_err && - grep -v "^+" test_err >err && 2: 73849add5e = 2: 800b17edde commit-graph tests: test a graph that's too small 3: 6bfce758e1 = 3: 7083ab81c7 commit-graph: fix segfault on e.g. "git status" 4: ac07ff415e = 4: d00564ae89 commit-graph: don't early exit(1) on e.g. "git status" 5: b2dd394cc7 = 5: 25ee185bf7 commit-graph: don't pass filename to load_commit_graph_one_fd_st() 6: 9987149e5c ! 6: 7619b46987 commit-graph verify: detect inability to read the graph @@ -37,16 +37,10 @@ } -+test_expect_success 'detect permission problem' ' ++test_expect_success POSIXPERM,SANITY 'detect permission problem' ' + corrupt_graph_setup && + chmod 000 $objdir/info/commit-graph && -+ -+ # Skip as root, or in other cases (odd fs or OS) where a -+ # "chmod 000 file" does not yield EACCES on e.g. "cat file" -+ if ! test -r $objdir/info/commit-graph -+ then -+ corrupt_graph_verify "Could not open" -+ fi ++ corrupt_graph_verify "Could not open" +' + test_expect_success 'detect too small' ' 7: 0e35b12a1a ! 7: 17ee4fc050 commit-graph write: don't die if the existing graph is corrupt @@ -18,6 +18,10 @@ use_commit_graph=1 as seen in 177722b344 ("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 @@ -36,7 +40,12 @@ corruption. This might need to be re-visited if we learn to write the commit-graph - incrementally. + 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 <avarab@xxxxxxxxx> @@ -119,7 +128,7 @@ grep -v "^+" test_err >err && test_i18ngrep "$grepstr" err && - git status --short -+ if test -z "$NO_WRITE_TEST_BACKUP" ++ if test "$2" != "no-copy" + then + cp $objdir/info/commit-graph commit-graph-pre-write-test + fi && @@ -130,14 +139,14 @@ # usage: corrupt_graph_and_verify <position> <data> <string> [<zero_pos>] @@ - # "chmod 000 file" does not yield EACCES on e.g. "cat file" - if ! test -r $objdir/info/commit-graph - then -- corrupt_graph_verify "Could not open" -+ NO_WRITE_TEST_BACKUP=1 corrupt_graph_verify "Could not open" - fi + test_expect_success POSIXPERM,SANITY 'detect permission problem' ' + corrupt_graph_setup && + chmod 000 $objdir/info/commit-graph && +- corrupt_graph_verify "Could not open" ++ corrupt_graph_verify "Could not open" "no-copy" ' + test_expect_success 'detect too small' ' @@ git fsck && corrupt_graph_and_verify $GRAPH_BYTE_FOOTER "\00" \ 8: a74d0f0f6f = 8: 29ab2895b7 commit-graph: improve & i18n error messages -- 2.21.0.360.g471c308f928
