[PATCH] Look at both unpacked and packed objects for short name expansion

[PATCH] Look at both unpacked and packed objects for short name expansion

[Johannes Schindelin]

We used to check for a unique expansion in the unpacked objects, and if
one was found, we forgot to check in the packed objects, too.

Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>

---

 sha1_name.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

213fff36f398fbbbee9b76a9d59339ba60f1d58c
diff --git a/sha1_name.c b/sha1_name.c
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -115,7 +115,8 @@ static int get_short_sha1(const char *na
 	if (i < 4)
 		return -1;
 	if (find_short_object_filename(i, canonical, sha1))
-		return 0;
+                /* do not allow non-unique short names */
+                return find_short_packed_object(i, res, canonical)!=0?-1:0;
 	if (find_short_packed_object(i, res, sha1))
 		return 0;
 	return -1;

Re: [PATCH] Look at both unpacked and packed objects for short name expansion

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> We used to check for a unique expansion in the unpacked objects, and if
> one was found, we forgot to check in the packed objects, too.

> diff --git a/sha1_name.c b/sha1_name.c
> --- a/sha1_name.c
> +++ b/sha1_name.c
> @@ -115,7 +115,8 @@ static int get_short_sha1(const char *na
>  	if (i < 4)
>  		return -1;
>  	if (find_short_object_filename(i, canonical, sha1))
> -		return 0;
> +                /* do not allow non-unique short names */
> +                return find_short_packed_object(i, res, canonical)!=0?-1:0;
>  	if (find_short_packed_object(i, res, sha1))
>  		return 0;
>  	return -1;

Sorry, I do not understand what you are doing here.

We find an unpacked object file in the local object store, and
then if we find a unique match (otherwise we try if
short-packed-object finds unique match) we try to see if we have
the same one in one of the packs we have and refuse?  Wouldn't
this make things inconvenient for people who run 'git repack'
without running 'git prune-packed' afterwards?

And reading find_short_object_filename() I noticed it does not
do anything with alternates, and wonder if is this intentional.
Who did this part anyway?

Also find_short_packed_object() does not appear to require
unique prefix when it does its thing.

I would prefer a proper fix to:

 - find_short_object_filename() to look at alt_odb as well;

 - find_short_packed_object() to make sure it is getting a
   unique match.

 - get_short_sha1() to make sure that the input is overall
   unique.  I think with the current code structure, with or
   without your patch, would do a wrong thing if you pass a
   prefix that matches two unpacked objects and two packed
   ones.

I suspect your patch is an attempt to fix the last one, but I am
not sure..

Re: [PATCH] Look at both unpacked and packed objects for short name expansion

[Johannes Schindelin]

Hi,

On Thu, 15 Sep 2005, Junio C Hamano wrote:

> I suspect your patch is an attempt to fix the last one, but I am
> not sure..

Half correct. I was not thinking it through... I had that case where I 
called git-whatchanged with a short sha1 and I kept getting the wrong 
history until I realized that the short sha1 was only unique when looking 
at the unpacked objects only.

Maybe I find time tomorrow to do something about the three problems you 
mentioned.

Ciao,
Dscho