Re: [PATCH v7 07/22] commit-graph: add 'verify' subcommand

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> +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/



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux