From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: git@vger.kernel.org
Cc: "Junio C Hamano" <gitster@pobox.com>,
"Derrick Stolee" <stolee@gmail.com>,
"SZEDER Gábor" <szeder.dev@gmail.com>,
"Eric Sunshine" <sunshine@sunshineco.com>,
"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Subject: [PATCH v2 0/8] commit-graph: segfault & other fixes for broken graphs
Date: Thu, 14 Mar 2019 22:47:32 +0100 [thread overview]
Message-ID: <20190314214740.23360-1-avarab@gmail.com> (raw)
In-Reply-To: <20190221223753.20070-1-avarab@gmail.com>
See the v1 cover letter for details:
https://public-inbox.org/git/20190221223753.20070-1-avarab@gmail.com/
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@gmail.com>
@@ -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
next prev parent reply other threads:[~2019-03-14 21:47 UTC|newest]
Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-02-21 22:37 [PATCH 0/8] commit-graph: segfault & other fixes for broken graphs Ævar Arnfjörð Bjarmason
2019-02-21 22:37 ` [PATCH 1/8] commit-graph tests: split up corrupt_graph_and_verify() Ævar Arnfjörð Bjarmason
2019-02-21 22:37 ` [PATCH 2/8] commit-graph tests: test a graph that's too small Ævar Arnfjörð Bjarmason
2019-02-21 22:37 ` [PATCH 3/8] commit-graph: fix segfault on e.g. "git status" Ævar Arnfjörð Bjarmason
2019-02-21 22:37 ` [PATCH 4/8] commit-graph: don't early exit(1) " Ævar Arnfjörð Bjarmason
2019-02-21 22:37 ` [PATCH 5/8] commit-graph: don't pass filename to load_commit_graph_one_fd_st() Ævar Arnfjörð Bjarmason
2019-02-21 22:37 ` [PATCH 6/8] commit-graph verify: detect inability to read the graph Ævar Arnfjörð Bjarmason
2019-02-21 23:15 ` SZEDER Gábor
2019-02-21 22:37 ` [PATCH 7/8] commit-graph write: don't die if the existing graph is corrupt Ævar Arnfjörð Bjarmason
2019-02-22 23:13 ` Junio C Hamano
2019-02-23 9:29 ` Eric Sunshine
2019-02-21 22:37 ` [PATCH 8/8] commit-graph: improve & i18n error messages Ævar Arnfjörð Bjarmason
2019-03-14 21:47 ` Ævar Arnfjörð Bjarmason [this message]
2019-03-15 7:47 ` [PATCH v2 0/8] commit-graph: segfault & other fixes for broken graphs Eric Sunshine
2019-03-15 10:39 ` Ævar Arnfjörð Bjarmason
2019-03-25 12:08 ` [PATCH v3 " Ævar Arnfjörð Bjarmason
2019-03-25 12:08 ` [PATCH v3 1/8] commit-graph tests: split up corrupt_graph_and_verify() Ævar Arnfjörð Bjarmason
2019-03-25 12:08 ` [PATCH v3 2/8] commit-graph tests: test a graph that's too small Ævar Arnfjörð Bjarmason
2019-03-25 12:08 ` [PATCH v3 3/8] commit-graph: fix segfault on e.g. "git status" Ævar Arnfjörð Bjarmason
2019-03-25 12:08 ` [PATCH v3 4/8] commit-graph: don't early exit(1) " Ævar Arnfjörð Bjarmason
2019-04-27 13:06 ` Ævar Arnfjörð Bjarmason
2019-04-29 12:48 ` Derrick Stolee
2019-04-29 14:30 ` Ævar Arnfjörð Bjarmason
2019-05-01 18:31 ` Jeff King
2019-03-25 12:08 ` [PATCH v3 5/8] commit-graph: don't pass filename to load_commit_graph_one_fd_st() Ævar Arnfjörð Bjarmason
2019-03-25 12:08 ` [PATCH v3 6/8] commit-graph verify: detect inability to read the graph Ævar Arnfjörð Bjarmason
2019-03-25 12:08 ` [PATCH v3 7/8] commit-graph write: don't die if the existing graph is corrupt Ævar Arnfjörð Bjarmason
2019-03-25 12:08 ` [PATCH v3 8/8] commit-graph: improve & i18n error messages Ævar Arnfjörð Bjarmason
2019-03-14 21:47 ` [PATCH v2 1/8] commit-graph tests: split up corrupt_graph_and_verify() Ævar Arnfjörð Bjarmason
2019-03-14 21:47 ` [PATCH v2 2/8] commit-graph tests: test a graph that's too small Ævar Arnfjörð Bjarmason
2019-03-14 21:47 ` [PATCH v2 3/8] commit-graph: fix segfault on e.g. "git status" Ævar Arnfjörð Bjarmason
2019-03-14 21:47 ` [PATCH v2 4/8] commit-graph: don't early exit(1) " Ævar Arnfjörð Bjarmason
2019-03-14 21:47 ` [PATCH v2 5/8] commit-graph: don't pass filename to load_commit_graph_one_fd_st() Ævar Arnfjörð Bjarmason
2019-03-14 21:47 ` [PATCH v2 6/8] commit-graph verify: detect inability to read the graph Ævar Arnfjörð Bjarmason
2019-03-14 21:47 ` [PATCH v2 7/8] commit-graph write: don't die if the existing graph is corrupt Ævar Arnfjörð Bjarmason
2019-03-14 21:47 ` [PATCH v2 8/8] commit-graph: improve & i18n error messages Ævar Arnfjörð Bjarmason
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190314214740.23360-1-avarab@gmail.com \
--to=avarab@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=stolee@gmail.com \
--cc=sunshine@sunshineco.com \
--cc=szeder.dev@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).