Re: [PATCH 3/3] path-walk: create initializer for path lists

>
> From: Derrick Stolee <stolee@gmail.com>
>
> The previous change fixed a bug in 'git repack -adf --path-walk' that
> was due to an update to how path lists are initialized and missing some
> important cases when processing the pending objects.
>
> This change takes the three critical places where path lists are
> initialized and combines them into a static method. This simplifies the
> callers somewhat while also helping to avoid a missed update in the
> future.
>
> The other places where a path list (struct type_and_oid_list) is
> initialized is for the following "fixed" lists:
>
>  * Tag objects.
>  * Commit objects.
>  * Root trees.
>  * Tagged trees.
>  * Tagged blobs.
>
> These lists are created and consumed in different ways, with only the
> root trees being passed into the logic that cares about the
> "maybe_interesting" bit. It is appropriate to keep these uses separate.
>
> Signed-off-by: Derrick Stolee <stolee@gmail.com>
> ---
>  path-walk.c | 57 +++++++++++++++++++++++------------------------------
>  1 file changed, 25 insertions(+), 32 deletions(-)
>
> diff --git a/path-walk.c b/path-walk.c
> index 1215ed398f4f..f1ceed99e94c 100644
> --- a/path-walk.c
> +++ b/path-walk.c
> @@ -105,6 +105,24 @@ static void push_to_stack(struct path_walk_context *ctx,
>         prio_queue_put(&ctx->path_stack, xstrdup(path));
>  }
>
> +static void add_path_to_list(struct path_walk_context *ctx,
> +                            const char *path,
> +                            enum object_type type,
> +                            struct object_id *oid,
> +                            int interesting)
> +{
> +       struct type_and_oid_list *list = strmap_get(&ctx->paths_to_lists, path);
> +
> +       if (!list) {
> +               CALLOC_ARRAY(list, 1);
> +               list->type = type;
> +               strmap_put(&ctx->paths_to_lists, path, list);
> +       }
> +
> +       list->maybe_interesting |= interesting;
> +       oid_array_append(&list->oids, oid);
> +}
> +
>  static int add_tree_entries(struct path_walk_context *ctx,
>                             const char *base_path,
>                             struct object_id *oid)
> @@ -129,7 +147,6 @@ static int add_tree_entries(struct path_walk_context *ctx,
>
>         init_tree_desc(&desc, &tree->object.oid, tree->buffer, tree->size);
>         while (tree_entry(&desc, &entry)) {
> -               struct type_and_oid_list *list;
>                 struct object *o;
>                 /* Not actually true, but we will ignore submodules later. */
>                 enum object_type type = S_ISDIR(entry.mode) ? OBJ_TREE : OBJ_BLOB;
> @@ -190,17 +207,10 @@ static int add_tree_entries(struct path_walk_context *ctx,
>                                 continue;
>                 }
>
> -               if (!(list = strmap_get(&ctx->paths_to_lists, path.buf))) {
> -                       CALLOC_ARRAY(list, 1);
> -                       list->type = type;
> -                       strmap_put(&ctx->paths_to_lists, path.buf, list);
> -               }
> -               push_to_stack(ctx, path.buf);
> -
> -               if (!(o->flags & UNINTERESTING))
> -                       list->maybe_interesting = 1;
> +               add_path_to_list(ctx, path.buf, type, &entry.oid,
> +                                !(o->flags & UNINTERESTING));
>
> -               oid_array_append(&list->oids, &entry.oid);
> +               push_to_stack(ctx, path.buf);
>         }
>
>         free_tree_buffer(tree);
> @@ -377,16 +387,9 @@ static int setup_pending_objects(struct path_walk_info *info,
>                         if (!info->trees)
>                                 continue;
>                         if (pending->path) {
> -                               struct type_and_oid_list *list;
>                                 char *path = *pending->path ? xstrfmt("%s/", pending->path)
>                                                             : xstrdup("");
> -                               if (!(list = strmap_get(&ctx->paths_to_lists, path))) {
> -                                       CALLOC_ARRAY(list, 1);
> -                                       list->type = OBJ_TREE;
> -                                       strmap_put(&ctx->paths_to_lists, path, list);
> -                               }
> -                               list->maybe_interesting = 1;
> -                               oid_array_append(&list->oids, &obj->oid);
> +                               add_path_to_list(ctx, path, OBJ_TREE, &obj->oid, 1);
>                                 free(path);
>                         } else {
>                                 /* assume a root tree, such as a lightweight tag. */
> @@ -397,20 +400,10 @@ static int setup_pending_objects(struct path_walk_info *info,
>                 case OBJ_BLOB:
>                         if (!info->blobs)
>                                 continue;
> -                       if (pending->path) {
> -                               struct type_and_oid_list *list;
> -                               char *path = pending->path;
> -                               if (!(list = strmap_get(&ctx->paths_to_lists, path))) {
> -                                       CALLOC_ARRAY(list, 1);
> -                                       list->type = OBJ_BLOB;
> -                                       strmap_put(&ctx->paths_to_lists, path, list);
> -                               }
> -                               list->maybe_interesting = 1;
> -                               oid_array_append(&list->oids, &obj->oid);
> -                       } else {
> -                               /* assume a root tree, such as a lightweight tag. */
> +                       if (pending->path)
> +                               add_path_to_list(ctx, pending->path, OBJ_BLOB, &obj->oid, 1);
> +                       else
>                                 oid_array_append(&tagged_blobs->oids, &obj->oid);
> -                       }
>                         break;
>
>                 case OBJ_COMMIT:
> --
> gitgitgadget