Re: [PATCH 25/33] notes-merge: convert verify_notes_filepair to struct object_id

Brandon Williams [off-list ref] writes:

It's no brainer to do s/GIT_SHA1_HEXSZ/GIT_MAX_HEXSZ/ for all of the
above, but ...

-	if (*path || i != 40)
+	if (*path || i != GIT_SHA1_HEXSZ)
 		return -1;

... this one is tricky.  

What's in our envisioned future?  Are we expecing to see object
names, named with two or more hash functions, in a same repository?
If so, and one is 20 bytes and another one is 32 bytes, then this
should check 'i' against 40 and 64 and pass if 'i' is one of these
expected lengths?

-	return get_sha1_hex(hex_sha1, sha1);
+	return get_oid_hex(hex_oid, oid);
 }

On 06/02, Junio C Hamano wrote:

Brandon Williams [off-list ref] writes:

quoted

Signed-off-by: Brandon Williams <redacted>
---
 notes-merge.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/notes-merge.c b/notes-merge.c
index 55dbb3659..962e9b1bc 100644
--- a/notes-merge.c
+++ b/notes-merge.c

@@ -22,21 +22,21 @@ void init_notes_merge_options(struct notes_merge_options *o)
 	o->verbosity = NOTES_MERGE_VERBOSITY_DEFAULT;
 }
 
-static int path_to_sha1(const char *path, unsigned char *sha1)
+static int path_to_oid(const char *path, struct object_id *oid)
 {
-	char hex_sha1[40];
+	char hex_oid[GIT_SHA1_HEXSZ];
 	int i = 0;
-	while (*path && i < 40) {
+	while (*path && i < GIT_SHA1_HEXSZ) {
 		if (*path != '/')
-			hex_sha1[i++] = *path;
+			hex_oid[i++] = *path;
 		path++;
 	}

It's no brainer to do s/GIT_SHA1_HEXSZ/GIT_MAX_HEXSZ/ for all of the
above, but ...

I'll fix this.

quoted

-	if (*path || i != 40)
+	if (*path || i != GIT_SHA1_HEXSZ)
 		return -1;

... this one is tricky.  

What's in our envisioned future?  Are we expecing to see object
names, named with two or more hash functions, in a same repository?
If so, and one is 20 bytes and another one is 32 bytes, then this
should check 'i' against 40 and 64 and pass if 'i' is one of these
expected lengths?

That's a good question, and at this point in time do we have an
envisioned future?  From some of our conversations I seem to remember
that we don't want a single repository to have objects based on two
different hash functions, but rather some translation layer to convert
between two hashing functions (for compatibility with other
non-converted repos).  Though nothing has been settled upon yet so I
don't know what the future would look like (and the best way to prepare
for it).

quoted

-	return get_sha1_hex(hex_sha1, sha1);
+	return get_oid_hex(hex_oid, oid);
 }

-- 
Brandon Williams