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

From: Brandon Williams <bmwill@google.com>
Date: Jun 2, 2017, 18:55 UTC
Message-ID: <20170602185528.GC57260@google.com>
In-Reply-To: <xmqqpoen2p2a.fsf@gitster.mtv.corp.google.com>

On 06/02, Junio C Hamano wrote:

Show 31 quoted lines

> Brandon Williams <bmwill@google.com> writes:
> 
> > Signed-off-by: Brandon Williams <bmwill@google.com>
> > ---
> >  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.

Show 12 quoted lines

> 
> > -	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).

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

-- 
Brandon Williams