v2, 2 partspack-objects: don't loosen objects available in alternate or kept packs

> If pack-objects is called with the --unpack-unreachable option then it will
> unpack (i.e. loosen) all unreferenced objects from local not-kept packs,
> including those that also exist in packs residing in an alternate object
> database or a local kept pack.  The primary(sole?) user of this option is
> git-repack.  In this case, repack will follow the call to pack-objects with
> a call to prune-packed which will delete these newly loosened objects,
> making the act of loosening a waste of time.  The unnecessary loosening can
> be avoided by checking whether an object exists in a non-local pack or a
> local kept pack before loosening it.
>
> This fixes the 'local packed unreachable obs that exist in alternate ODB
> are not loosened' test in t7700.
>
> Signed-off-by: Brandon Casey <drafnel@gmail.com>

>  builtin-pack-objects.c |   26 +++++++++++++++++++++++++-
>  t/t7700-repack.sh      |    2 +-
>  2 files changed, 26 insertions(+), 2 deletions(-)
>
> diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
> index 6222f19..3f477c5 100644
> --- a/builtin-pack-objects.c
> +++ b/builtin-pack-objects.c
> @@ -1944,6 +1944,29 @@ static void
> add_objects_in_unpacked_packs(struct rev_info *revs)
>  	free(in_pack.array);
>  }
>
> +static int has_sha1_pack_kept_or_nonlocal(const unsigned char *sha1)
> +{
> +	static struct packed_git *last_found = (void *)1;
> +	struct packed_git *p;
> +
> +	p = (last_found == (void *)1) ? packed_git : last_found;

> +	while (p) {
> +		if ((!p->pack_local || p->pack_keep) &&
> +			find_pack_entry_one(sha1, p)) {
> +			last_found = p;
> +			return 1;
> +		}
> +		if (p == last_found)
> +			p = packed_git;
> +		else
> +			p = p->next;
> +		if (p == last_found)
> +			p = p->next;
> +	}
> +	return 0;
> +}

>  But I am not sure about one thing...
>
>> diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
>> index 6222f19..3f477c5 100644
>> --- a/builtin-pack-objects.c
>> +++ b/builtin-pack-objects.c
>> @@ -1944,6 +1944,29 @@ static void
>> add_objects_in_unpacked_packs(struct rev_info *revs)
>>       free(in_pack.array);
>>  }
>>
>> +static int has_sha1_pack_kept_or_nonlocal(const unsigned char *sha1)
>> +{
>> +     static struct packed_git *last_found = (void *)1;
>> +     struct packed_git *p;
>> +
>> +     p = (last_found == (void *)1) ? packed_git : last_found;
>
> Why (void *)1, not like:
>
>        static struct packed_git *last_found;
>        struct packed_git *p = last_found ? last_found : packed_git;
>
> Am I missing something?

> >> +static int has_sha1_pack_kept_or_nonlocal(const unsigned char *sha1)
> >> +{
> >> +     static struct packed_git *last_found = (void *)1;
> >> +     struct packed_git *p;
> >> +
> >> +     p = (last_found == (void *)1) ? packed_git : last_found;
> >
> > Why (void *)1, not like:
> >
> >        static struct packed_git *last_found;
> >        struct packed_git *p = last_found ? last_found : packed_git;
> >
> > Am I missing something?
> 
> Heh, I am missing something too.  Maybe I should have _thought_ more
> about this code that I copied from sha1_file.c: find_pack_entry() and
> I would have asked the same question about _that_ code.
> 
> Maybe Nico has some idea?

>> +static int has_sha1_pack_kept_or_nonlocal(const unsigned char *sha1)
>> +{
>> +	static struct packed_git *last_found = (void *)1;
>> +	struct packed_git *p;
>> +
>> +	p = (last_found == (void *)1) ? packed_git : last_found;
>
> Why (void *)1, not like:
>
> 	static struct packed_git *last_found;
> 	struct packed_git *p = last_found ? last_found : packed_git;
>
> Am I missing something?
>
>> +	while (p) {
>> +		if ((!p->pack_local || p->pack_keep) &&
>> +			find_pack_entry_one(sha1, p)) {
>> +			last_found = p;
>> +			return 1;
>> +		}
>> +		if (p == last_found)
>> +			p = packed_git;
>> +		else
>> +			p = p->next;
>> +		if (p == last_found)
>> +			p = p->next;
>> +	}
>> +	return 0;
>> +}

> If you allow (last_found == NULL), the loop iterates over packed_git list,
> and when it has its last element in p after discovering nothing matches
> the criteria, this happens:
> 
> 	if (p == last_found) /* false, p is the last element on the list */
>         	p = packed_git; /* not taken */
> 	else
>         	p = p->next; /* taken, p == NULL now */
> 	if (p == last_found) /* true! */
> 		p = p->next; /* OOPS */