On 11/28/2018 5:18 PM, Ævar Arnfjörð Bjarmason wrote:
This is really interesting. I tested this with: diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c index 124b1bafc4..5c7615f06c 100644 --- a/builtin/pack-objects.c +++ b/builtin/pack-objects.c @@ -3143 +3143 @@ static void get_object_list(int ac, const char **av) - mark_edges_uninteresting(&revs, show_edge, sparse); + mark_edges_uninteresting(&revs, show_edge, 1); To emulate having a GIT_TEST_* mode for this, which seems like a good idea since it turned up a lot of segfaults in pack-objects. I wasn't able to get a backtrace for that since it always happens indirectly, and I didn't dig enough to see how to manually invoke it the right way.
Thanks for double-checking this. I had run a similar test in my prototype implementation, but over-simplified some code when rewriting it for submission (and then forgot to re-run that test).
Specifically, these null checks are important: diff --git a/list-objects.c b/list-objects.c index 9bb93d1640..7e864b4db8 100644 --- a/list-objects.c +++ b/list-objects.c @@ -232,6 +232,10 @@ static void add_edge_parents(struct commit *commit, for (parents = commit->parents; parents; parents = parents->next) { struct commit *parent = parents->item; struct tree *tree = get_commit_tree(parent); + + if (!tree) + continue; + oidset_insert(set, &tree->object.oid); if (!(parent->object.flags & UNINTERESTING)) @@ -261,6 +265,8 @@ void mark_edges_uninteresting(struct rev_info *revs, if (sparse) { struct tree *tree = get_commit_tree(commit); + if (!tree) + continue; if (commit->object.flags & UNINTERESTING) tree->object.flags |= UNINTERESTING; I will definitely include a GIT_TEST_* variable in v2. Thanks, -Stolee