Re: [WIP PATCH] revision-walking: allow iterating revisions multiple times

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 24, 2011, 21:44 UTC
Message-ID: <7vhb56o56h.fsf@alter.siamese.dyndns.org>
In-Reply-To: <20110824211431.GH45292@book.hvoigt.net>

Heiko Voigt <hvoigt@hvoigt.net> writes:

Show 16 quoted lines

> +void reset_revision_walk(struct rev_info *revs)
> +{
> +	int nr = revs->walked.nr;
> +	struct object_array_entry *e = revs->walked.objects;
> +
> +	/* reset the seen flags set by prepare_revision_walk */
> +	while (--nr >= 0) {
> +		struct object *o = e->item;
> +		o->flags &= ~(ALL_REV_FLAGS);
> +		e++;
> +	}
> +	free(revs->walked.objects);
> +	revs->walked.nr = 0;
> +	revs->walked.alloc = 0;
> +	revs->walked.objects = NULL;
> +}

I am afraid that this is not good enough for general purpose. The object you walk in the middle of doing something may have been marked for reasons other than your extra walking before you started your walk. Imagine

 * The command takes arguments like rev-list does;

 * It calls setup_revisions(), which marks commits given from the command
   line with marks like UNINTERESTING, and then prepare_revision_walk();

 * It walks the commit graph and does interesting things on commits that
   it discovers, by repeatedly calling get_revision(), e.g.:

   	while ((commit = get_revision()) != NULL) {
		do_something_interesting(commit);
        }

Now, you add a new caller that walks the commit graph for a different reason from the primary revision walking done by the command somewhere down in the callchain of do_something_interesting()---obviously you cannot use the above reset_revision_walk() to clean things up, as it will break the outer revision walk.

If on the other hand you will _never_ have more than one revision walk going on, it may amount to the same thing to iterate over the object array and clear all the flags.

Traditionally the way to do nested revision walk that can potentially be done more than once (but never having such a sub-walk in parallel) was to remember the start points of the subwalk, use private marks that are not used in the outer walk during the subwalk, and call clear_commit_marks() on these start points when a subwalk is done to clear only the marks the subwalk used.