Re: [PATCH v2 6/9] bundle: avoid using the rev_info flag leak_pending
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Dec 28, 2017, 21:13 UTC
- Message-ID
- <xmqq7et6k137.fsf@gitster.mtv.corp.google.com>
- In-Reply-To
- <5fe3b4c8-e48c-1922-f564-e75c5a354cef@web.de>
René Scharfe <l.s.r@web.de> 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.
Show 19 quoted lines
> 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 ...
Show 8 quoted lines
> 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 ...
Show 5 quoted lines
> ... > 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[]).
Show 11 quoted lines
> ...
> + 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?
Show 11 quoted lines
>
> /* 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.