On Wed, Aug 20, 2025 at 11:40 AM Derrick Stolee via GitGitGadget
<gitgitgadget@xxxxxxxxx> wrote:
>
> From: Derrick Stolee <stolee@xxxxxxxxx>
>
> 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@xxxxxxxxx>
> ---
> 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
Looks like a straightforward collapsing of common code into a new function.
