Re: [PATCH v2 14/25] shallow.c: implement a generic shallow boundary finder based on rev-list

On Tue, Feb 9, 2016 at 4:09 AM, Junio C Hamano [off-list ref] wrote:

quoted

+     is_repository_shallow(); /* make sure shallows are read */
+
+     init_revisions(&revs, NULL);
+     save_commit_buffer = 0;
+     setup_revisions(ac, av, &revs, NULL);
+
+     /* Mark all reachable commits as NOT_SHALLOW */
+     if (prepare_revision_walk(&revs))
+             die("revision walk setup failed");
+     traverse_commit_list(&revs, show_commit, NULL, &not_shallow_flag);
+
+     /*
+      * mark border commits SHALLOW + NOT_SHALLOW.
+      * We cannot clear NOT_SHALLOW right now. Imagine border
+      * commit A is processed first, then commit B, whose parent is
+      * A, later. If NOT_SHALLOW on A is cleared at step 1, B
+      * itself is considered border at step 2, which is incorrect.
+      */
+     nr = get_max_object_index();
+     for (i = 0; i < nr; i++) {

I'd really like not to see a loop over 0..get_max_object_index().
Are there many codepaths that peek into the in-core entire object
store already?

You started it with check_non_tip(). At least that's how I know about
this loop. But I think that's the only code path, not counting this.

Would it work equally well to keep track of the
commits discovered in show_commit() to use as the set of commits
you need to visit in this second pass?

We can't do this in show_commit. In this loop, we check
not_shallow_flag of parent commits. If one parent commit is not
show_commit'd yet, the flag is not set and we may incorrectly think
this is a border commit. The only way to avoid going through the
entire in-core object database is keeping a new commit_list and go
through it here. Which way is preferred?

quoted

+             struct object *o = get_indexed_object(i);
+             struct commit *c = (struct commit *)o;
+
+             if (!o || o->type != OBJ_COMMIT ||
+                 !(o->flags & not_shallow_flag))
+                     continue;
+
+             if (parse_commit(c))
+                     die("unable to parse commit %s",
+                         oid_to_hex(&c->object.oid));
+
+             for (p = c->parents; p; p = p->next)
+                     if (!(p->item->object.flags & not_shallow_flag)) {
+                             o->flags |= shallow_flag;
+                             commit_list_insert(c, &result);
+                             break;
+                     }
+     }

-- 
Duy