Re: [PATCH v2 4/6] path-walk: allow consumer to specify object types

From: Taylor Blau <me@ttaylorr.com>
Date: Nov 21, 2024, 22:44 UTC
Message-ID: <Zz+3whunQkfLX0A1@nand.local>
In-Reply-To: <83b746f569df932452d8d102dc29dda0859c4ed2.1731181273.git.gitgitgadget@gmail.com>

On Sat, Nov 09, 2024 at 07:41:10PM +0000, Derrick Stolee via GitGitGadget wrote:

Show 12 quoted lines

> diff --git a/path-walk.c b/path-walk.c
> index 24cf04c1e7d..2ca08402367 100644
> --- a/path-walk.c
> +++ b/path-walk.c
> @@ -98,6 +98,10 @@ static int add_children(struct path_walk_context *ctx,
>  		if (S_ISGITLINK(entry.mode))
>  			continue;
>
> +		/* If the caller doesn't want blobs, then don't bother. */
> +		if (!ctx->info->blobs && type == OBJ_BLOB)
> +			continue;
> +

I was going to ask why we're not reusing the existing property of the rev_info struct to specify what object type(s) we do/don't want, but the answer is obvious: we don't want that to change the behavior of the commit-level walk which is used to seed the actual path walk based on its results.

However, it would be kind of nice to have a single place to specify how you want to traverse objects, and using the rev_info struct seems like a good choice there since you can naively ask Git to parse command-line arguments like --blobs, --trees, --objects, etc. and set the appropriate bits.

I wonder if it might make sense to do something like:

    unsigned int tmp_blobs = revs->blob_objects;
    unsigned int tmp_trees = revs->tree_objects;
    unsigned int tmp_tags = revs->tag_objects;

    if (prepare_revision_walk(revs))
        die(_("failed to setup revision walk"));

    /* commit-level walk */

    revs->blob_objects = tmp_blobs;
    revs->tree_objects = tmp_trees;
    revs->tag_objects = tmp_tags;

    /* path-level walk */

I don't have strong feelings about it, but it feels like this approach could be cleaner from a caller's perspective. But I can see an argument to the contrary that it does introduce some awkwardness with the dual-use of those fields.

Thanks, Taylor