On 7/10/2018 1:31 PM, Jonathan Tan wrote:
+static void test_get_commit_tree_in_graph(const char *gitdir,
+ const char *worktree,
+ const struct object_id *commit_oid)
+{
+ struct repository r;
+ struct commit *c;
+ struct tree *tree;
+
+ /*
+ * Create a commit independent of any repository.
+ */
+ c = lookup_commit(commit_oid);
Would this be more accurate to say we are creating a commit object
stored in the object cache of the_repository? How would you expect this
to work if/when lookup_commit() takes an arbitrary repository? You want
to provide &r, right (after initializing)?
Yes, you're right - Stefan too mentioned that this will need to be moved
below lookup_commit(). I'm not sure what the best way is to handle this
- maybe move this, and add a "needswork" stating that we need to pass r
to lookup_commit once it supports taking in a repository argument, as an
aid to the person who performs the merge. I'll do that if a reroll is
needed.
Also, this will conflict with sb/object-store-lookup, won't it? I'm
guessing this is why you didn't touch the "git commit-graph
[write|verify]"code paths.
It will conflict because of the change to lookup_commit(), but the only
new code I'm writing is in t/helper/test-repository.c, so hopefully the
merge won't be too tedious. The main reason why I didn't touch the
writing/verifying part is to reduce the size of this patch set, and
because that change is not needed to update parse_commit() and others.
I guess my main complaint is that this won't be an actual "merge"
conflict, but the result will not compile. Since Stefan already has a
series out that changes this method, I recommend basing your series on
it (in addition to basing it on ds/commit-graph-fsck).
Thanks,
-Stolee
