René Scharfe <[email protected]> writes:

> The leak_pending flag is so awkward to use that multiple comments had to
> be added around each occurrence.  We use it for remembering the
> prerequisites for the bundle.  That is easy, though: We have the
> ref_list named "prerequisites" in the header for just that purpose.

Hmph.

>  int verify_bundle(struct bundle_header *header, int verbose)
>  {
>  ...
>       struct rev_info revs;
>       const char *argv[] = {NULL, "--all", NULL};
> -     struct object_array refs;
>       struct commit *commit;
>       int i, ret = 0, req_nr;
>       const char *message = _("Repository lacks these prerequisite commits:");
>  
>       init_revisions(&revs, NULL);
>       for (i = 0; i < p->nr; i++) {
>               struct ref_list_entry *e = p->list + i;
>               struct object *o = parse_object(&e->oid);
>               if (o) {
>                       o->flags |= PREREQ_MARK;
>                       add_pending_object(&revs, o, e->name);
>                       continue;
>               }

We mark the prereq objects with PREREQ_MARK and then ...

>               if (++ret == 1)
>                       error("%s", message);
>               error("%s %s", oid_to_hex(&e->oid), e->name);
>       }
>       if (revs.pending.nr != p->nr)
>               return ret;
>       req_nr = revs.pending.nr;
>       setup_revisions(2, argv, &revs, NULL);

... run "rev-list" with "--all" plus these prereq objects, and ...

> ...
>       i = req_nr;
>       while (i && (commit = get_revision(&revs)))
>               if (commit->object.flags & PREREQ_MARK)
>                       i--;

... let the traversal run until we see all the objects with PREREQ_MARK
set (i.e. those that appear in p->list[]).

> ...
> +     for (i = 0; i < p->nr; i++) {
> +             struct ref_list_entry *e = p->list + i;
> +             struct object *o = parse_object(&e->oid);
> +             assert(o); /* otherwise we'd have returned early */
> +             if (o->flags & SHOWN)
> +                     continue;
> +             if (++ret == 1)
> +                     error("%s", message);
> +             error("%s %s", oid_to_hex(&e->oid), e->name);
> +     }

And then make sure that all of those that appear in p->list[] are
already shown.

Doesn't that mean that these SHOWN and other flags are first given
to the commits at the tip of "--all" refs and propagated down to
those in p->list[]?  Would it be sufficient to clear those that can
be reached from them, like the following?

>  
>       /* Clean up objects used, as they will be reused. */
> -     clear_commit_marks_for_object_array(&refs, ALL_REV_FLAGS);
> -
> -     object_array_clear(&refs);
> +     for (i = 0; i < p->nr; i++) {
> +             struct ref_list_entry *e = p->list + i;
> +             commit = lookup_commit_reference_gently(&e->oid, 1);
> +             if (commit)
> +                     clear_commit_marks(commit, ALL_REV_FLAGS);
> +     }

I do not think the patch in question changes behaviour.  The set of
objects the code starts the clearing does not change.  So I am more
puzzled by the original than the conversion done with this change.

Reply via email to