Re: [PATCH 10/14] treewide: drop uses of `for_each_{loose,packed}_object()`

[Justin Tobler <jltobler@gmail.com>]

On 26/01/15 12:04PM, Patrick Steinhardt wrote:
> We're using `for_each_loose_object()` and `for_each_packed_object()` at
> a couple of callsites to enumerate all loose and packed objects,
> respectively. These functions will be removed in a subsequent commit in
> favor of the newly introduced `odb_source_loose_for_each_object()` and
> `packfile_store_for_each_object()` replacements.
> 
> Prepare for this by refactoring the sites accordingly.
> 
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  builtin/cat-file.c | 28 ++++++++++++++++++++++------
>  commit-graph.c     | 44 +++++++++++++++++++++++++++++++-------------
>  2 files changed, 53 insertions(+), 19 deletions(-)
> 
> diff --git a/builtin/cat-file.c b/builtin/cat-file.c
> index 6964a5a52c..7d16fbc1b8 100644
> --- a/builtin/cat-file.c
> +++ b/builtin/cat-file.c
> @@ -806,11 +806,14 @@ struct for_each_object_payload {
>  	void *payload;
>  };
>  
> -static int batch_one_object_loose(const struct object_id *oid,
> -				  const char *path UNUSED,
> -				  void *_payload)
> +static int batch_one_object_oi(const struct object_id *oid,
> +			       struct object_info *oi,
> +			       void *_payload)
>  {
>  	struct for_each_object_payload *payload = _payload;
> +	if (oi && oi->whence == OI_PACKED)
> +		return payload->callback(oid, oi->u.packed.pack, oi->u.packed.offset,
> +					 payload->payload);
>  	return payload->callback(oid, NULL, 0, payload->payload);
>  }
>  
> @@ -846,8 +849,15 @@ static void batch_each_object(struct batch_options *opt,
>  		.payload = _payload,
>  	};
>  	struct bitmap_index *bitmap = prepare_bitmap_git(the_repository);
> +	struct odb_source *source;
>  
> -	for_each_loose_object(the_repository->objects, batch_one_object_loose, &payload, 0);
> +	odb_prepare_alternates(the_repository->objects);
> +	for (source = the_repository->objects->sources; source; source = source->next) {
> +		int ret = odb_source_loose_for_each_object(source, NULL, batch_one_object_oi,
> +							   &payload, flags);
> +		if (ret)
> +			break;
> +	}
>  
>  	if (bitmap && !for_each_bitmapped_object(bitmap, &opt->objects_filter,
>  						 batch_one_object_bitmapped, &payload)) {
> @@ -861,8 +871,14 @@ static void batch_each_object(struct batch_options *opt,
>  						&payload, flags);
>  		}
>  	} else {
> -		for_each_packed_object(the_repository, batch_one_object_packed,
> -				       &payload, flags);
> +		struct object_info oi = { 0 };
> +
> +		for (source = the_repository->objects->sources; source; source = source->next) {
> +			int ret = packfile_store_for_each_object(source->packfiles, &oi,
> +								 batch_one_object_oi, &payload, flags);
> +			if (ret)
> +				break;
> +		}

Huh, I was a bit surprised to see that we are still handling object
iteration in a backend specific banner here. I would assume ideally we
would want to transparently iterate across objects wherever possible. I
assume the reason here has something to do with how iteration is handled
with bitmaps?

-Justin