Heiko Voigt <hvoigt@xxxxxxxxxx> writes:
> Previously it was not possible to iterate revisions twice using the
> revision walking api. We add a reset_revision_walk() which clears the
> used flags. This allows us to do multiple sequencial revision walks.
>
> Signed-off-by: Heiko Voigt <hvoigt@xxxxxxxxxx>
I am kind of surprised that this is already its 5th round.
> diff --git a/object.c b/object.c
> index 6b06297..6291ce9 100644
> --- a/object.c
> +++ b/object.c
> @@ -275,3 +275,14 @@ void object_array_remove_duplicates(struct object_array *array)
> array->nr = dst;
> }
> }
> +
> +void clear_object_flags(unsigned flags)
> +{
> + int i;
> + struct object *obj;
> +
> + for (i=0; i < obj_hash_size; i++) {
> + if ((obj = obj_hash[i]) && obj->flags & flags)
> + obj->flags &= ~flags;
> + }
> +}
Minimally,
void clear_object_flags(unsigned flags)
{
int i;
for (i = 0; i < obj_hash_size; i++) {
struct object *obj = obj_hash[i];
if (obj && (obj->flags & flags))
obj->flags &= ~flags;
}
}
I am not sure if the "If there is any bit set we care about, drop them"
buys us anything, though.
> diff --git a/revision.c b/revision.c
> index c97d834..77ce6bd 100644
> --- a/revision.c
> +++ b/revision.c
> @@ -2061,6 +2061,11 @@ static void set_children(struct rev_info *revs)
> }
> }
>
> +void reset_revision_walk()
void reset_revision_walk(void)
> +{
> + clear_object_flags(SEEN | ADDED | SHOWN);
> +}
But is this really the right API? After a particular program finishes
using the revision walker, wouldn't it want to clear both the set of these
standard flag bits used by the traversal machinery, as well as whatever
program specific bits it used to mark the objects with?
> diff --git a/revision.h b/revision.h
> index b8e9223..3535733 100644
> --- a/revision.h
> +++ b/revision.h
> @@ -192,6 +192,7 @@ extern void parse_revision_opt(struct rev_info *revs, struct parse_opt_ctx_t *ct
> ...
> +extern void reset_revision_walk();
Likewise, "extern void reset_revision_walk(void);".
> diff --git a/submodule.c b/submodule.c
> index 9a28060..645ff5d 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -404,6 +404,7 @@ int check_submodule_needs_pushing(unsigned char new_sha1[20], const char *remote
> while ((commit = get_revision(&rev)) && !needs_pushing)
> commit_need_pushing(commit, &needs_pushing);
>
> + reset_revision_walk();
> free(sha1_copy);
> strbuf_release(&remotes_arg);
>
> @@ -741,6 +742,7 @@ static int find_first_merges(struct object_array *result, const char *path,
> if (in_merge_bases(b, &commit, 1))
> add_object_array(o, NULL, &merges);
> }
> + reset_revision_walk();
>
> /* Now we've got all merges that contain a and b. Prune all
> * merges that contain another found merge and save them in
These two hunk look like a *BUGFIX* to me (certainly it does not look like
this is an addition of any new feature).
What bug does this fix, and how is the current submodule code broken
without this patch? Can you describe the problem in the log message, and
add a test to demonstrate the existing breakage?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html