Re: [PATCH] fetch-pack: fix object_id of exact sha1

On Sat, Feb 27, 2016 at 7:12 PM, Jeff King [off-list ref] wrote:

On Sat, Feb 27, 2016 at 05:32:54PM -0300, Gabriel Souza Franco wrote:

quoted

Commit 58f2ed0 (remote-curl: pass ref SHA-1 to fetch-pack as well,
2013-12-05) added support for specifying a SHA-1 as well as a ref name.
Add support for specifying just a SHA-1 and set the ref name to the same
value in this case.

Signed-off-by: Gabriel Souza Franco <redacted>
---
 builtin/fetch-pack.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
index 79a611f..d7e439a 100644
--- a/builtin/fetch-pack.c
+++ b/builtin/fetch-pack.c

@@ -16,10 +16,10 @@ static void add_sought_entry(struct ref ***sought, int *nr, int *alloc,
      struct ref *ref;
      struct object_id oid;

-     if (!get_oid_hex(name, &oid) && name[GIT_SHA1_HEXSZ] == ' ')
-             name += GIT_SHA1_HEXSZ + 1;
-     else
+     if (get_oid_hex(name, &oid))
              oidclr(&oid);
+     else if (name[GIT_SHA1_HEXSZ] == ' ')
+             name += GIT_SHA1_HEXSZ + 1;

This makes sense to me. I wonder if we should be more particular about
the pure-sha1 case consuming the whole string, though. E.g., if we get:

  1234567890123456789012345678901234567890-bananas

that should probably not have sha1 1234...

I don't think it should ever really happen in practice, but it might be
worth noticing and complaining when name[GIT_SHA1_HEXSZ] is neither
space nor '\0'.

Right. What kind of complaining? Is doing oidclr(&oid) and letting it
fail elsewhere enough?
Also, it already fails precisely because of that check I wanted to
remove earlier.

-Peff