Re: [PATCH v2] git-gui: Add support of SHA256 repo

From: Johannes Sixt <j6t@kdbg.org>
Date: Jul 15, 2025, 19:19 UTC
Message-ID: <c543a644-8e58-4601-8a79-247cd5e124c5@kdbg.org>
In-Reply-To: <87ecuha66p.wl-tiwai@suse.de>

Am 15.07.25 um 16:18 schrieb Takashi Iwai:

Show 16 quoted lines

> On Mon, 14 Jul 2025 18:28:13 +0200,
>> BTW, there is a case
>>
>>       if {[regexp {^[0-9a-f]{1,39}$} $head]}
>>
>> around line 3217 in git-gui.sh.
> 
> Obviously I didn't look for numbers less than 40 :)
> I'll replace it, too.
> 
> But I don't understand why it matches up to only 39, not 40 in the
> code above.
> It seems trying to get the proper hash id if it's no full length id?
> If so, the check should be rather like
> 	if {![regexp {^[0-9a-f]{40}$} $head]}
> ??  It makes the conversion a bit simpler.

Obviously, the code wants to turn abbreviated hashes to full hashes. That can be skipped if it is already the full length. I haven't analyzed why this is needed or desirable.

Show 18 quoted lines

>>> @@ -879,7 +881,7 @@ method _do_clone_full_end {ok} {
>>>  		if {[file exists [gitdir FETCH_HEAD]]} {
>>>  			set fd [open [gitdir FETCH_HEAD] r]
>>>  			while {[gets $fd line] >= 0} {
>>> -				if {[regexp "^(.{40})\t\t" $line line HEAD]} {
>>> +				if {[regexp [string map "@@ $hashlength" "^(.{@@})\t\t"] $line line HEAD]} {
>>>  					break
>>>  				}
>>>  			}
>>
>> The repository picker dialog runs before $hashlength is set. Therefore,
>> at the time that this function is executed, $hashlength is not available.
>>
>> This procedure can depend on the file format, which is to have \t\t
>> after the hash regardless of its length.
> 
> Oh that's bad.  I'll rewrite without the reference to $hashlength.
> I guess we can simply replace the above with a range check {40,64}.

Yes, let's do that.

I had something like

   set pos [string find \t\t $line]
   if {pos > 0} {
     set HEAD [string range $line 0 $pos]
     break
   }

in mind.

Show 15 quoted lines

>>> @@ -986,7 +990,7 @@ method _readtree_wait {fd} {
>>>  
>>>  	# -- Run the post-checkout hook.
>>>  	#
>>> -	set fd_ph [githook_read post-checkout [string repeat 0 40] \
>>> +	set fd_ph [githook_read post-checkout [string repeat 0 $hashlength] \
>>
>> Yet another case where $nullid can be used.
> 
> But it's also in repo picker code, so we don't have $nullid yet?
> I'll rewrite somehow without $hashlength reference here, too.
> (e.g. use the length of "git-rev-parse HEAD" output that is called
> below)
> 
>>>  		[git rev-parse HEAD] 1]

Good call, and good idea for a fix!

-- Hannes