Re: [PATCH v4 17/21] fsck: verify commit-graph

[Derrick Stolee]

On 6/6/2018 7:08 AM, Ævar Arnfjörð Bjarmason wrote:

On Mon, Jun 04 2018, Derrick Stolee wrote:


+   prepare_alt_odb();
+   for (alt = alt_odb_list; alt; alt = alt->next) {
+   verify_argv[2] = "--object-dir";
+   verify_argv[3] = alt->path;
+   if (run_command(&commit_graph_verify))
+   errors_found |= ERROR_COMMIT_GRAPH;
+   }
+   }
+

This doesn't compile under clang on master. It needs to account for
0b20903405 ("sha1_file: add repository argument to prepare_alt_odb",
2018-03-23).

 builtin/fsck.c:837:19: error: too few arguments to function call, single 
argument 'r' was not specified
 prepare_alt_odb();
 ~~~ ^

Ditto this error due to a missing resolution with 031dc927f4
("object-store: move alt_odb_list and alt_odb_tail to object store",
2018-03-23):

 builtin/fsck.c:838:14: error: use of undeclared identifier 'alt_odb_list'
 for (alt = alt_odb_list; alt; alt = alt->next) {


Thanks, Ævar. I forgot to rebase onto 'next'. Doing so now and will send 
v5 shortly.


-Stolee

Re: [PATCH v4 17/21] fsck: verify commit-graph

[Ævar Arnfjörð Bjarmason]

On Mon, Jun 04 2018, Derrick Stolee wrote:

> + prepare_alt_odb();
> + for (alt = alt_odb_list; alt; alt = alt->next) {
> + verify_argv[2] = "--object-dir";
> + verify_argv[3] = alt->path;
> + if (run_command(&commit_graph_verify))
> + errors_found |= ERROR_COMMIT_GRAPH;
> + }
> + }
> +

This doesn't compile under clang on master. It needs to account for
0b20903405 ("sha1_file: add repository argument to prepare_alt_odb",
2018-03-23).

builtin/fsck.c:837:19: error: too few arguments to function call, single 
argument 'r' was not specified
prepare_alt_odb();
~~~ ^

Ditto this error due to a missing resolution with 031dc927f4
("object-store: move alt_odb_list and alt_odb_tail to object store",
2018-03-23):

builtin/fsck.c:838:14: error: use of undeclared identifier 'alt_odb_list'
for (alt = alt_odb_list; alt; alt = alt->next) {

[PATCH v4 17/21] fsck: verify commit-graph

[Derrick Stolee]

If core.commitGraph is true, verify the contents of the commit-graph
during 'git fsck' using the 'git commit-graph verify' subcommand. Run
this check on all alternates, as well.

We use a new process for two reasons:

1. The subcommand decouples the details of loading and verifying a
   commit-graph file from the other fsck details.

2. The commit-graph verification requires the commits to be loaded
   in a specific order to guarantee we parse from the commit-graph
   file for some objects and from the object database for others.

Signed-off-by: Derrick Stolee 
---
 Documentation/git-fsck.txt |  3 +++
 builtin/fsck.c | 21 +
 t/t5318-commit-graph.sh|  8 
 3 files changed, 32 insertions(+)

diff --git a/Documentation/git-fsck.txt b/Documentation/git-fsck.txt
index b9f060e3b2..ab9a93fb9b 100644
--- a/Documentation/git-fsck.txt
+++ b/Documentation/git-fsck.txt
@@ -110,6 +110,9 @@ Any corrupt objects you will have to find in backups or 
other archives
 (i.e., you can just remove them and do an 'rsync' with some other site in
 the hopes that somebody else has the object you have corrupted).
 
+If core.commitGraph is true, the commit-graph file will also be inspected
+using 'git commit-graph verify'. See linkgit:git-commit-graph[1].
+
 Extracted Diagnostics
 -
 
diff --git a/builtin/fsck.c b/builtin/fsck.c
index ef78c6c00c..a6d5045b77 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -16,6 +16,7 @@
 #include "streaming.h"
 #include "decorate.h"
 #include "packfile.h"
+#include "run-command.h"
 
 #define REACHABLE 0x0001
 #define SEEN  0x0002
@@ -45,6 +46,7 @@ static int name_objects;
 #define ERROR_REACHABLE 02
 #define ERROR_PACK 04
 #define ERROR_REFS 010
+#define ERROR_COMMIT_GRAPH 020
 
 static const char *describe_object(struct object *obj)
 {
@@ -815,5 +817,24 @@ int cmd_fsck(int argc, const char **argv, const char 
*prefix)
}
 
check_connectivity();
+
+   if (core_commit_graph) {
+   struct child_process commit_graph_verify = CHILD_PROCESS_INIT;
+   const char *verify_argv[] = { "commit-graph", "verify", NULL, 
NULL, NULL, NULL };
+   commit_graph_verify.argv = verify_argv;
+   commit_graph_verify.git_cmd = 1;
+
+   if (run_command(&commit_graph_verify))
+   errors_found |= ERROR_COMMIT_GRAPH;
+
+   prepare_alt_odb();
+   for (alt = alt_odb_list; alt; alt = alt->next) {
+   verify_argv[2] = "--object-dir";
+   verify_argv[3] = alt->path;
+   if (run_command(&commit_graph_verify))
+   errors_found |= ERROR_COMMIT_GRAPH;
+   }
+   }
+
return errors_found;
 }
diff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh
index 2297a44e7d..44d4c71f0b 100755
--- a/t/t5318-commit-graph.sh
+++ b/t/t5318-commit-graph.sh
@@ -399,4 +399,12 @@ test_expect_success 'detect invalid checksum hash' '
"incorrect checksum"
 '
 
+test_expect_success 'git fsck (checks commit-graph)' '
+   cd "$TRASH_DIRECTORY/full" &&
+   git fsck &&
+   corrupt_graph_and_verify $GRAPH_BYTE_FOOTER "\00" \
+   "incorrect checksum" &&
+   test_must_fail git fsck
+'
+
 test_done
-- 
2.18.0.rc1