On 6/27/2018 5:59 PM, Jonathan Tan wrote:
+int verify_commit_graph(struct repository *r, struct commit_graph *g)I haven't had the time to review this patch set, but I did rebase my object store refactoring [1] on this and wrote a test: static void test_verify_commit_graph(const char *gitdir, const char *worktree) { struct repository r; char *graph_name; struct commit_graph *graph;repo_init(&r, gitdir, worktree); graph_name = get_commit_graph_filename(r.objects->objectdir);graph = load_commit_graph_one(graph_name);printf("verification returned %d\n", verify_commit_graph(&r, graph)); repo_clear(&r);} However, it doesn't work because verify_commit_graph() invokes parse_commit_internal(), which tries to look up replace refs in the_repository. I think that verify_commit_graph() should not take a repository argument for now. To minimize churn on the review of this patch set, and to minimize diffs when we migrate parse_commit_internal() (and likely other functions) to take in a repository argument, I would be OK with something like the following instead: int verify_commit_graph(struct commit_graph *g) { /* * NEEDSWORK: Make r into a parameter when all functions * invoked by this function are not hardcoded to operate on * the_repository. */ struct repository *r = the_repository; /* ... */ As for my rebased refactoring, I'll send the patches to the mailing list once Junio updates ds/commit-graph-fsck with these latest changes, so that I can rebase once again on that and ensure that everything still works. [1] https://public-inbox.org/git/cover.1529616356.git.jonathantanmy@xxxxxxxxxx/
Thanks for looking into this, Jonathan. At some point I took my series and moved it on top of Stefan's lookup_object() series, but then moved off of it since those commits were not in 'pu'. This 'struct repository *r' didn't get removed in that process.
Thanks, -Stolee
