Re: [PATCH v2 6/9] bundle: avoid using the rev_info flag leak_pending

[Junio C Hamano <gitster@pobox.com>]

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.

>  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.