Patrick Steinhardt <ps@xxxxxx> writes:
> Any packed objects indexed via git-index-pack(1) are subject to a
> collision check. This collision check has the intent to determine
> whether we already have an object with the same object ID, but different
> contents in the repository.
>
> The check whether the collision check is really needed is performed via
> `repo_has_object_file_with_flags(..., OBJECT_INFO_QUICK)`. \
>
Nit: this was a little confusing at first, until I saw the code. So what
this means is that the collision check is only performed, iff
`repo_has_object_file_with_flags(...)` returns true.
I think the confusing part was 'is performed via', perhaps:
The collision check is only performed, if
repo_has_object_file_with_flags(..., OBJECT_INFO_QUICK) returns a
truthy value.
But it is okay as is too!
> But unless
> explicitly told otherwise via `OBJECT_INFO_SKIP_FETCH_OBJECT`, this
> function will also cause us to fetch the object ID in case it is part of
> a promisor pack. As such, we may end up fetching the object only to
> check whether the fetched object and the object that we're indexing have
> the same content.
>
So us fetching the object is pointless, since we only care about the
'does it exist' part and not really what it contains. In that case,
shouldn't this be s/same content/same oid/?
> This behaviour is highly dubious and more likely than not unintended.
> Fix it by converting to `has_object()`, which knows to neither reload
> packfiles nor to fetch promisor objects by default.
>
> Signed-off-by: Patrick Steinhardt <ps@xxxxxx>
> ---
> builtin/index-pack.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/builtin/index-pack.c b/builtin/index-pack.c
> index f49431d626b..805b7aa1e28 100644
> --- a/builtin/index-pack.c
> +++ b/builtin/index-pack.c
> @@ -892,9 +892,7 @@ static void sha1_object(const void *data, struct object_entry *obj_entry,
>
> if (startup_info->have_repository) {
> read_lock();
> - collision_test_needed =
> - repo_has_object_file_with_flags(the_repository, oid,
> - OBJECT_INFO_QUICK);
> + collision_test_needed = has_object(the_repository, oid, 0);
> read_unlock();
> }
>
>
> --
> 2.49.0.901.g37484f566f.dirty
Attachment:
signature.asc
Description: PGP signature
