[PATCH 0/2] gitk: Support of SHA256 repos

[PATCH 0/2] gitk: Support of SHA256 repos

[Takashi Iwai]

Hi,

I recently stumbled on the problem of gitk with sha256 repo (while
testing the new openSUSE package git workflow that enforces SHA256);
gitk aborts immediately with a message "Can't parse git log output:
{commit xxx..}".

After skimming over the net, I found the patch [*] posted in 4 years
ago to add the support for sha256, but nothing happened since then,
unfortunately.

So here is the revived patch for gitk to add sha256 support, with
cleanups and corrections, in addition to the enhancement patch for the
missing auto-select length config for sha256.

Only lightly tested on a few sha256 repos.


Takashi

[*] https://patchwork.kernel.org/project/git/patch/pull.979.git.1623687519832.gitgitgadget@gmail.com

===

Rostislav Krasny (1):
  gitk: Add a basic support of SHA256 repositories into Gitk

Takashi Iwai (1):
  gitk: Add auto-select length preference for SHA256

 gitk-git/gitk | 96 +++++++++++++++++++++++++++++++++++++--------------
 1 file changed, 71 insertions(+), 25 deletions(-)

-- 
2.49.0

[PATCH 1/2] gitk: Add a basic support of SHA256 repositories into Gitk

[Takashi Iwai]

From: Rostislav Krasny <rosti.bsd@gmail.com>

This PR makes Gitk working on both SHA256 and SHA1 repositories without
errors/crashes. I made it by changing and testing the gitk script of Git
for Windows [https://gitforwindows.org/] version 2.32.0.windows.1 that
is a little bit different than the mainstream 2.32.0 version.

Still not fixed functionality: [1] There is the "Auto-select SHA1
(length)" configuration preference that affects "Copy commit reference"
on both SHA1 and SHA256 repositories.

A new "Auto-select SHA256 (length)" configuration preference should be
added and used on SHA256 repositories instead of the old one. Since I'm
not familiar with Tcl/Tk and this issue isn't critical I didn't
implement it.

[ Changes from the original patch:
  * Discard the changes for generic words (e.g. "Commit ID"), so that
    translations can be still applied after this patch
  * Simplify the regexp check in gotocommit as suggested in the
    previous review
  -- tiwai ]

Signed-off-by: Rostislav Krasny <rosti.bsd@gmail.com>
Link: https://patchwork.kernel.org/project/git/patch/pull.979.git.1623687519832.gitgitgadget@gmail.com
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 gitk-git/gitk | 59 +++++++++++++++++++++++++++++++++++++--------------
 1 file changed, 43 insertions(+), 16 deletions(-)

diff --git a/gitk-git/gitk b/gitk-git/gitk
index bc9efa18566f..1e85cfef2ee3 100755
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -425,6 +425,7 @@ proc parseviewargs {n arglist} {
 
 proc parseviewrevs {view revs} {
     global vposids vnegids
+    global hashlength
 
     if {$revs eq {}} {
         set revs HEAD
@@ -438,7 +439,7 @@ proc parseviewrevs {view revs} {
         set badrev {}
         for {set l 0} {$l < [llength $errlines]} {incr l} {
             set line [lindex $errlines $l]
-            if {!([string length $line] == 40 && [string is xdigit $line])} {
+            if {!([string length $line] == $hashlength && [string is xdigit $line])} {
                 if {[string match "fatal:*" $line]} {
                     if {[string match "fatal: ambiguous argument*" $line]
                         && $badrev ne {}} {
@@ -655,6 +656,7 @@ proc updatecommits {} {
     global hasworktree
     global varcid vposids vnegids vflags vrevs
     global show_notes
+    global hashlength
 
     set hasworktree [hasworktree]
     rereadrefs
@@ -688,7 +690,7 @@ proc updatecommits {} {
             # take out positive refs that we asked for before or
             # that we have already seen
             foreach rev $revs {
-                if {[string length $rev] == 40} {
+                if {[string length $rev] == $hashlength} {
                     if {[lsearch -exact $oldpos $rev] < 0
                         && ![info exists varcid($view,$rev)]} {
                         lappend newrevs $rev
@@ -1573,6 +1575,7 @@ proc getcommitlines {fd inst view updating}  {
     global parents children curview hlview
     global idpending ordertok
     global varccommits varcid varctok vtokmod vfilelimit vshortids
+    global hashlength
 
     set stuff [read $fd 500000]
     # git log doesn't terminate the last commit with a null...
@@ -1655,7 +1658,7 @@ proc getcommitlines {fd inst view updating}  {
             }
             set ok 1
             foreach id $ids {
-                if {[string length $id] != 40} {
+                if {[string length $id] != $hashlength} {
                     set ok 0
                     break
                 }
@@ -1935,6 +1938,7 @@ proc readrefs {} {
     global selecthead selectheadid
     global hideremotes
     global tclencoding
+    global hashlength
 
     foreach v {tagids idtags headids idheads otherrefids idotherrefs} {
         unset -nocomplain $v
@@ -1944,9 +1948,9 @@ proc readrefs {} {
         fconfigure $refd -encoding $tclencoding
     }
     while {[gets $refd line] >= 0} {
-        if {[string index $line 40] ne " "} continue
-        set id [string range $line 0 39]
-        set ref [string range $line 41 end]
+        if {[string index $line $hashlength] ne " "} continue
+        set id [string range $line 0 [expr {$hashlength - 1}]]
+        set ref [string range $line [expr {$hashlength + 1}] end]
         if {![string match "refs/*" $ref]} continue
         set name [string range $ref 5 end]
         if {[string match "remotes/*" $name]} {
@@ -2241,6 +2245,7 @@ proc makewindow {} {
     global have_tk85 have_tk86 use_ttk NS
     global git_version
     global worddiff
+    global hashlength
 
     # The "mc" arguments here are purely so that xgettext
     # sees the following string as needing to be translated
@@ -2366,7 +2371,7 @@ proc makewindow {} {
         -command gotocommit -width 8
     $sha1but conf -disabledforeground [$sha1but cget -foreground]
     pack .tf.bar.sha1label -side left
-    ${NS}::entry $sha1entry -width 40 -font textfont -textvariable sha1string
+    ${NS}::entry $sha1entry -width $hashlength -font textfont -textvariable sha1string
     trace add variable sha1string write sha1change
     pack $sha1entry -side left -pady 2
 
@@ -4093,6 +4098,7 @@ proc stopblaming {} {
 
 proc read_line_source {fd inst} {
     global blamestuff curview commfd blameinst nullid nullid2
+    global hashlength
 
     while {[gets $fd line] >= 0} {
         lappend blamestuff($inst) $line
@@ -4113,7 +4119,7 @@ proc read_line_source {fd inst} {
     set line [split [lindex $blamestuff($inst) 0] " "]
     set id [lindex $line 0]
     set lnum [lindex $line 1]
-    if {[string length $id] == 40 && [string is xdigit $id] &&
+    if {[string length $id] == $hashlength && [string is xdigit $id] &&
         [string is digit -strict $lnum]} {
         # look for "filename" line
         foreach l $blamestuff($inst) {
@@ -5436,13 +5442,14 @@ proc get_viewmainhead {view} {
 # git rev-list should give us just 1 line to use as viewmainheadid($view)
 proc getviewhead {fd inst view} {
     global viewmainheadid commfd curview viewinstances showlocalchanges
+    global hashlength
 
     set id {}
     if {[gets $fd line] < 0} {
         if {![eof $fd]} {
             return 1
         }
-    } elseif {[string length $line] == 40 && [string is xdigit $line]} {
+    } elseif {[string length $line] == $hashlength && [string is xdigit $line]} {
         set id $line
     }
     set viewmainheadid($view) $id
@@ -7206,10 +7213,11 @@ proc commit_descriptor {p} {
 # Also look for URLs of the form "http[s]://..." and make them web links.
 proc appendwithlinks {text tags} {
     global ctext linknum curview
+    global hashlength
 
     set start [$ctext index "end - 1c"]
     $ctext insert end $text $tags
-    set links [regexp -indices -all -inline {(?:\m|-g)[0-9a-f]{6,40}\M} $text]
+    set links [regexp -indices -all -inline [string map "@@ $hashlength" {(?:\m|-g)[0-9a-f]{6,@@}\M}] $text]
     foreach l $links {
         set s [lindex $l 0]
         set e [lindex $l 1]
@@ -8888,13 +8896,16 @@ proc incrfont {inc} {
 
 proc clearsha1 {} {
     global sha1entry sha1string
-    if {[string length $sha1string] == 40} {
+    global hashlength
+
+    if {[string length $sha1string] == $hashlength} {
         $sha1entry delete 0 end
     }
 }
 
 proc sha1change {n1 n2 op} {
     global sha1string currentid sha1but
+
     if {$sha1string == {}
         || ([info exists currentid] && $sha1string == $currentid)} {
         set state disabled
@@ -8911,6 +8922,7 @@ proc sha1change {n1 n2 op} {
 
 proc gotocommit {} {
     global sha1string tagids headids curview varcid
+    global hashlength
 
     if {$sha1string == {}
         || ([info exists currentid] && $sha1string == $currentid)} return
@@ -8920,11 +8932,11 @@ proc gotocommit {} {
         set id $headids($sha1string)
     } else {
         set id [string tolower $sha1string]
-        if {[regexp {^[0-9a-f]{4,39}$} $id]} {
+	if {[regexp {^[0-9a-f]{4,63}$} $id]} {
             set matches [longid $id]
             if {$matches ne {}} {
                 if {[llength $matches] > 1} {
-                    error_popup [mc "Short commit ID %s is ambiguous" $id]
+                    error_popup [mc "Short commit id %s is ambiguous" $id]
                     return
                 }
                 set id [lindex $matches 0]
@@ -9618,10 +9630,11 @@ proc mktaggo {} {
 
 proc copyreference {} {
     global rowmenuid autosellen
+    global hashlength
 
     set format "%h (\"%s\", %ad)"
     set cmd [list git show -s --pretty=format:$format --date=short]
-    if {$autosellen < 40} {
+    if {$autosellen < $hashlength} {
         lappend cmd --abbrev=$autosellen
     }
     set reference [eval exec $cmd $rowmenuid]
@@ -9632,6 +9645,7 @@ proc copyreference {} {
 
 proc writecommit {} {
     global rowmenuid wrcomtop commitinfo wrcomcmd NS
+    global hashlength
 
     set top .writecommit
     set wrcomtop $top
@@ -9641,7 +9655,7 @@ proc writecommit {} {
     ${NS}::label $top.title -text [mc "Write commit to file"]
     grid $top.title - -pady 10
     ${NS}::label $top.id -text [mc "ID:"]
-    ${NS}::entry $top.sha1 -width 40
+    ${NS}::entry $top.sha1 -width $hashlength
     $top.sha1 insert 0 $rowmenuid
     $top.sha1 conf -state readonly
     grid $top.id $top.sha1 -sticky w
@@ -9721,6 +9735,7 @@ proc mvbranch {} {
 
 proc branchdia {top valvar uivar} {
     global NS commitinfo
+    global hashlength
     upvar $valvar val $uivar ui
 
     catch {destroy $top}
@@ -9729,7 +9744,7 @@ proc branchdia {top valvar uivar} {
     ${NS}::label $top.title -text $ui(title)
     grid $top.title - -pady 10
     ${NS}::label $top.id -text [mc "ID:"]
-    ${NS}::entry $top.sha1 -width 40
+    ${NS}::entry $top.sha1 -width $hashlength
     $top.sha1 insert 0 $val(id)
     $top.sha1 conf -state readonly
     grid $top.id $top.sha1 -sticky w
@@ -12524,6 +12539,18 @@ if {$tclencoding == {}} {
     puts stderr "Warning: encoding $gitencoding is not supported by Tcl/Tk"
 }
 
+set objformat [exec git rev-parse --show-object-format]
+if {$objformat eq "sha1"} {
+    set hashlength 40
+} elseif {$objformat eq "sha256"} {
+    set hashlength 64
+} else {
+    error_popup "[mc "Not supported hash algorithm:"] {$objformat}"
+    exit 1
+}
+set hashalgorithm [string toupper $objformat]
+unset objformat
+
 set gui_encoding [encoding system]
 catch {
     set enc [exec git config --get gui.encoding]
-- 
2.49.0

[PATCH 2/2] gitk: Add auto-select length preference for SHA256

[Takashi Iwai]

This implements the missing preference setup of the auto select length
for SHA256. The variable set via the preference menu is switched
depending on the hash algorithm.

The default auto-select length is set to 64 for SHA256, and
saved/restored as "autosellensha256" in the config.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 gitk-git/gitk | 39 +++++++++++++++++++++++++++++----------
 1 file changed, 29 insertions(+), 10 deletions(-)

diff --git a/gitk-git/gitk b/gitk-git/gitk
index 1e85cfef2ee3..b364d9e7dc93 100755
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -7488,6 +7488,16 @@ proc make_idmark {id} {
     $canv raise $t
 }
 
+proc get_autosellen {} {
+    global hashalgorithm autosellen autosellensha256
+
+    if {$hashalgorithm == "SHA256"} {
+        return $autosellensha256
+    } else {
+        return $autosellen
+    }
+}
+
 proc selectline {l isnew {desired_loc {}} {switch_to_patch 0}} {
     global canv ctext commitinfo selectedline
     global canvy0 linespc parents children curview
@@ -7496,7 +7506,7 @@ proc selectline {l isnew {desired_loc {}} {switch_to_patch 0}} {
     global mergemax numcommits pending_select
     global cmitmode showneartags allcommits
     global targetrow targetid lastscrollrows
-    global autocopy autoselect autosellen jump_to_here
+    global autocopy autoselect jump_to_here
     global vinlinediff
 
     unset -nocomplain pending_select
@@ -7563,11 +7573,11 @@ proc selectline {l isnew {desired_loc {}} {switch_to_patch 0}} {
     $sha1entry delete 0 end
     $sha1entry insert 0 $id
     if {$autoselect && [haveselectionclipboard]} {
-        $sha1entry selection range 0 $autosellen
+        $sha1entry selection range 0 [get_autosellen]
     }
     if {$autocopy} {
         clipboard clear
-        clipboard append [string range $id 0 [expr $autosellen - 1]]
+        clipboard append [string range $id 0 [expr [get_autosellen] - 1]]
     }
     rhighlight_sel $id
 
@@ -9629,13 +9639,14 @@ proc mktaggo {} {
 }
 
 proc copyreference {} {
-    global rowmenuid autosellen
+    global rowmenuid
     global hashlength
 
     set format "%h (\"%s\", %ad)"
     set cmd [list git show -s --pretty=format:$format --date=short]
-    if {$autosellen < $hashlength} {
-        lappend cmd --abbrev=$autosellen
+    set alen [get_autosellen]
+    if {$alen < $hashlength} {
+        lappend cmd --abbrev=$alen
     }
     set reference [eval exec $cmd $rowmenuid]
 
@@ -11741,8 +11752,9 @@ proc create_prefs_page {w} {
 proc prefspage_general {notebook} {
     global NS maxwidth maxgraphpct showneartags showlocalchanges
     global tabstop wrapcomment wrapdefault limitdiffs
-    global autocopy autoselect autosellen extdifftool perfile_attrs
+    global autocopy autoselect extdifftool perfile_attrs
     global hideremotes want_ttk have_ttk maxrefs web_browser
+    global hashalgorithm hashlength
 
     set page [create_prefs_page $notebook.general]
 
@@ -11771,7 +11783,13 @@ proc prefspage_general {notebook} {
             -variable autoselect
         grid x $page.autoselect -sticky w
     }
-    spinbox $page.autosellen -from 1 -to 40 -width 4 -textvariable autosellen
+
+    if {$hashalgorithm == "SHA256"} {
+        set autolenvar "autosellensha256"
+    } else {
+        set autolenvar "autosellen"
+    }
+    spinbox $page.autosellen -from 1 -to $hashlength -width 4 -textvariable $autolenvar
     ${NS}::label $page.autosellenl -text [mc "Length of commit ID to copy"]
     grid x $page.autosellenl $page.autosellen -sticky w
 
@@ -11908,7 +11926,7 @@ proc doprefs {} {
     global maxwidth maxgraphpct use_ttk NS
     global oldprefs prefstop showneartags showlocalchanges
     global uicolor bgcolor fgcolor ctext diffcolors selectbgcolor markbgcolor
-    global tabstop limitdiffs autoselect autosellen extdifftool perfile_attrs
+    global tabstop limitdiffs autoselect extdifftool perfile_attrs
     global hideremotes want_ttk have_ttk wrapcomment wrapdefault
 
     set top .gitkprefs
@@ -12606,6 +12624,7 @@ set datetimeformat "%Y-%m-%d %H:%M:%S"
 set autocopy 0
 set autoselect 1
 set autosellen 40
+set autosellensha256 64
 set perfile_attrs 0
 set want_ttk 1
 
@@ -12702,7 +12721,7 @@ config_check_tmp_exists 50
 set config_variables {
     mainfont textfont uifont tabstop findmergefiles maxgraphpct maxwidth
     cmitmode wrapcomment wrapdefault autocopy autoselect autosellen
-    showneartags maxrefs visiblerefs
+    autosellensha256 showneartags maxrefs visiblerefs
     hideremotes showlocalchanges datetimeformat limitdiffs uicolor want_ttk
     bgcolor fgcolor uifgcolor uifgdisabledcolor colors diffcolors mergecolors
     markbgcolor diffcontext selectbgcolor foundbgcolor currentsearchhitbgcolor
-- 
2.49.0

Re: [PATCH 1/2] gitk: Add a basic support of SHA256 repositories into Gitk

[Johannes Sixt]

Am 20.03.25 um 16:41 schrieb Takashi Iwai:
> From: Rostislav Krasny <rosti.bsd@gmail.com>
> 
> This PR makes Gitk working on both SHA256 and SHA1 repositories without
> errors/crashes. I made it by changing and testing the gitk script of Git
> for Windows [https://gitforwindows.org/] version 2.32.0.windows.1 that
> is a little bit different than the mainstream 2.32.0 version.
> 
> Still not fixed functionality: [1] There is the "Auto-select SHA1
> (length)" configuration preference that affects "Copy commit reference"
> on both SHA1 and SHA256 repositories.
> 
> A new "Auto-select SHA256 (length)" configuration preference should be
> added and used on SHA256 repositories instead of the old one. Since I'm
> not familiar with Tcl/Tk and this issue isn't critical I didn't
> implement it.
> 
> [ Changes from the original patch:
>   * Discard the changes for generic words (e.g. "Commit ID"), so that
>     translations can be still applied after this patch
>   * Simplify the regexp check in gotocommit as suggested in the
>     previous review
>   -- tiwai ]

The message should be updated to not mention the evolution of the change
and what is not relevant anymore or not relevant in this patch.

> 
> Signed-off-by: Rostislav Krasny <rosti.bsd@gmail.com>
> Link: https://patchwork.kernel.org/project/git/patch/pull.979.git.1623687519832.gitgitgadget@gmail.com
> Signed-off-by: Takashi Iwai <tiwai@suse.de>
> ---

> @@ -8920,11 +8932,11 @@ proc gotocommit {} {
>          set id $headids($sha1string)
>      } else {
>          set id [string tolower $sha1string]
> -        if {[regexp {^[0-9a-f]{4,39}$} $id]} {
> +	if {[regexp {^[0-9a-f]{4,63}$} $id]} {

This doesn't use $hashlength. Should it?

Also watch out space vs. TAB.

> @@ -12524,6 +12539,18 @@ if {$tclencoding == {}} {
>      puts stderr "Warning: encoding $gitencoding is not supported by Tcl/Tk"
>  }
>  
> +set objformat [exec git rev-parse --show-object-format]
> +if {$objformat eq "sha1"} {
> +    set hashlength 40
> +} elseif {$objformat eq "sha256"} {
> +    set hashlength 64
> +} else {
> +    error_popup "[mc "Not supported hash algorithm:"] {$objformat}"

This looks strange. Where is the $objformat substituted?

> +    exit 1
> +}
> +set hashalgorithm [string toupper $objformat]
> +unset objformat

Why not set hashalgorithm right away, without using a temporary
objformat? Why set it at all here? It's unused.

> +
>  set gui_encoding [encoding system]
>  catch {
>      set enc [exec git config --get gui.encoding]

Re: [PATCH 2/2] gitk: Add auto-select length preference for SHA256

[Johannes Sixt]

Am 20.03.25 um 16:41 schrieb Takashi Iwai:
> This implements the missing preference setup of the auto select length
> for SHA256. The variable set via the preference menu is switched
> depending on the hash algorithm.
> 
> The default auto-select length is set to 64 for SHA256, and
> saved/restored as "autosellensha256" in the config.

I think the purpose of this change is to offer different settings for
the selected length depending on the hash algorithm. If that is the
case, the commit message could do a better job describing that: it says
only what happens implementationwise, but not what the user sees.

I do not think this is necessary. If I had set the option to, say, 12, I
would not want it to be set to something else when I go to a repository
that has a different hash algorithm. (But I do not know for certain,
because I do not have any SHA256 repositories, yet.)

> 
> Signed-off-by: Takashi Iwai <tiwai@suse.de>
> ---
>  gitk-git/gitk | 39 +++++++++++++++++++++++++++++----------
>  1 file changed, 29 insertions(+), 10 deletions(-)
> 
> diff --git a/gitk-git/gitk b/gitk-git/gitk
> index 1e85cfef2ee3..b364d9e7dc93 100755
> --- a/gitk-git/gitk
> +++ b/gitk-git/gitk
> @@ -7488,6 +7488,16 @@ proc make_idmark {id} {
>      $canv raise $t
>  }
>  
> +proc get_autosellen {} {
> +    global hashalgorithm autosellen autosellensha256
> +
> +    if {$hashalgorithm == "SHA256"} {
> +        return $autosellensha256
> +    } else {
> +        return $autosellen
> +    }
> +}
> +
>  proc selectline {l isnew {desired_loc {}} {switch_to_patch 0}} {
>      global canv ctext commitinfo selectedline
>      global canvy0 linespc parents children curview
> @@ -7496,7 +7506,7 @@ proc selectline {l isnew {desired_loc {}} {switch_to_patch 0}} {
>      global mergemax numcommits pending_select
>      global cmitmode showneartags allcommits
>      global targetrow targetid lastscrollrows
> -    global autocopy autoselect autosellen jump_to_here
> +    global autocopy autoselect jump_to_here
>      global vinlinediff
>  
>      unset -nocomplain pending_select
> @@ -7563,11 +7573,11 @@ proc selectline {l isnew {desired_loc {}} {switch_to_patch 0}} {
>      $sha1entry delete 0 end
>      $sha1entry insert 0 $id
>      if {$autoselect && [haveselectionclipboard]} {
> -        $sha1entry selection range 0 $autosellen
> +        $sha1entry selection range 0 [get_autosellen]
>      }
>      if {$autocopy} {
>          clipboard clear
> -        clipboard append [string range $id 0 [expr $autosellen - 1]]
> +        clipboard append [string range $id 0 [expr [get_autosellen] - 1]]
>      }
>      rhighlight_sel $id
>  
> @@ -9629,13 +9639,14 @@ proc mktaggo {} {
>  }
>  
>  proc copyreference {} {
> -    global rowmenuid autosellen
> +    global rowmenuid
>      global hashlength
>  
>      set format "%h (\"%s\", %ad)"
>      set cmd [list git show -s --pretty=format:$format --date=short]
> -    if {$autosellen < $hashlength} {
> -        lappend cmd --abbrev=$autosellen
> +    set alen [get_autosellen]
> +    if {$alen < $hashlength} {
> +        lappend cmd --abbrev=$alen
>      }
>      set reference [eval exec $cmd $rowmenuid]
>  
> @@ -11741,8 +11752,9 @@ proc create_prefs_page {w} {
>  proc prefspage_general {notebook} {
>      global NS maxwidth maxgraphpct showneartags showlocalchanges
>      global tabstop wrapcomment wrapdefault limitdiffs
> -    global autocopy autoselect autosellen extdifftool perfile_attrs
> +    global autocopy autoselect extdifftool perfile_attrs
>      global hideremotes want_ttk have_ttk maxrefs web_browser
> +    global hashalgorithm hashlength
>  
>      set page [create_prefs_page $notebook.general]
>  
> @@ -11771,7 +11783,13 @@ proc prefspage_general {notebook} {
>              -variable autoselect
>          grid x $page.autoselect -sticky w
>      }
> -    spinbox $page.autosellen -from 1 -to 40 -width 4 -textvariable autosellen
> +
> +    if {$hashalgorithm == "SHA256"} {
> +        set autolenvar "autosellensha256"
> +    } else {
> +        set autolenvar "autosellen"
> +    }
> +    spinbox $page.autosellen -from 1 -to $hashlength -width 4 -textvariable $autolenvar
>      ${NS}::label $page.autosellenl -text [mc "Length of commit ID to copy"]
>      grid x $page.autosellenl $page.autosellen -sticky w
>  
> @@ -11908,7 +11926,7 @@ proc doprefs {} {
>      global maxwidth maxgraphpct use_ttk NS
>      global oldprefs prefstop showneartags showlocalchanges
>      global uicolor bgcolor fgcolor ctext diffcolors selectbgcolor markbgcolor
> -    global tabstop limitdiffs autoselect autosellen extdifftool perfile_attrs
> +    global tabstop limitdiffs autoselect extdifftool perfile_attrs
>      global hideremotes want_ttk have_ttk wrapcomment wrapdefault
>  
>      set top .gitkprefs
> @@ -12606,6 +12624,7 @@ set datetimeformat "%Y-%m-%d %H:%M:%S"
>  set autocopy 0
>  set autoselect 1
>  set autosellen 40
> +set autosellensha256 64
>  set perfile_attrs 0
>  set want_ttk 1
>  
> @@ -12702,7 +12721,7 @@ config_check_tmp_exists 50
>  set config_variables {
>      mainfont textfont uifont tabstop findmergefiles maxgraphpct maxwidth
>      cmitmode wrapcomment wrapdefault autocopy autoselect autosellen
> -    showneartags maxrefs visiblerefs
> +    autosellensha256 showneartags maxrefs visiblerefs
>      hideremotes showlocalchanges datetimeformat limitdiffs uicolor want_ttk
>      bgcolor fgcolor uifgcolor uifgdisabledcolor colors diffcolors mergecolors
>      markbgcolor diffcontext selectbgcolor foundbgcolor currentsearchhitbgcolor

Re: [PATCH 0/2] gitk: Support of SHA256 repos

[Johannes Sixt]

Am 20.03.25 um 16:41 schrieb Takashi Iwai:
> Hi,
> 
> I recently stumbled on the problem of gitk with sha256 repo (while
> testing the new openSUSE package git workflow that enforces SHA256);
> gitk aborts immediately with a message "Can't parse git log output:
> {commit xxx..}".
> 
> After skimming over the net, I found the patch [*] posted in 4 years
> ago to add the support for sha256, but nothing happened since then,
> unfortunately.
> 
> So here is the revived patch for gitk to add sha256 support, with
> cleanups and corrections, in addition to the enhancement patch for the
> missing auto-select length config for sha256.
> 
> Only lightly tested on a few sha256 repos.
> 
> 
> Takashi
> 
> [*] https://patchwork.kernel.org/project/git/patch/pull.979.git.1623687519832.gitgitgadget@gmail.com
> 
> ===
> 
> Rostislav Krasny (1):
>   gitk: Add a basic support of SHA256 repositories into Gitk
> 
> Takashi Iwai (1):
>   gitk: Add auto-select length preference for SHA256
> 
>  gitk-git/gitk | 96 +++++++++++++++++++++++++++++++++++++--------------
>  1 file changed, 71 insertions(+), 25 deletions(-)
> 

Thank you.

After these patches, I still see a  few mentions of "40" that refer to
object id lengths and are not converted.

- a comment above proc longid
- a regexp in proc shortids
- in proc setlink
- in proc mkpatch
- in proc mktag

They should be converted, too, I think.

-- Hannes

Re: [PATCH 1/2] gitk: Add a basic support of SHA256 repositories into Gitk

[Takashi Iwai]

On Thu, 08 May 2025 08:20:40 +0200,
Johannes Sixt wrote:
> 
> Am 20.03.25 um 16:41 schrieb Takashi Iwai:
> > From: Rostislav Krasny <rosti.bsd@gmail.com>
> > 
> > This PR makes Gitk working on both SHA256 and SHA1 repositories without
> > errors/crashes. I made it by changing and testing the gitk script of Git
> > for Windows [https://gitforwindows.org/] version 2.32.0.windows.1 that
> > is a little bit different than the mainstream 2.32.0 version.
> > 
> > Still not fixed functionality: [1] There is the "Auto-select SHA1
> > (length)" configuration preference that affects "Copy commit reference"
> > on both SHA1 and SHA256 repositories.
> > 
> > A new "Auto-select SHA256 (length)" configuration preference should be
> > added and used on SHA256 repositories instead of the old one. Since I'm
> > not familiar with Tcl/Tk and this issue isn't critical I didn't
> > implement it.
> > 
> > [ Changes from the original patch:
> >   * Discard the changes for generic words (e.g. "Commit ID"), so that
> >     translations can be still applied after this patch
> >   * Simplify the regexp check in gotocommit as suggested in the
> >     previous review
> >   -- tiwai ]
> 
> The message should be updated to not mention the evolution of the change
> and what is not relevant anymore or not relevant in this patch.
> 
> > 
> > Signed-off-by: Rostislav Krasny <rosti.bsd@gmail.com>
> > Link: https://patchwork.kernel.org/project/git/patch/pull.979.git.1623687519832.gitgitgadget@gmail.com
> > Signed-off-by: Takashi Iwai <tiwai@suse.de>
> > ---
> 
> > @@ -8920,11 +8932,11 @@ proc gotocommit {} {
> >          set id $headids($sha1string)
> >      } else {
> >          set id [string tolower $sha1string]
> > -        if {[regexp {^[0-9a-f]{4,39}$} $id]} {
> > +	if {[regexp {^[0-9a-f]{4,63}$} $id]} {
> 
> This doesn't use $hashlength. Should it?

Not needed.  It's a range match, and can work in a shorter string,
too.  And, that's what suggested in previous reviews (years ago!).

> Also watch out space vs. TAB.

OK.

> > @@ -12524,6 +12539,18 @@ if {$tclencoding == {}} {
> >      puts stderr "Warning: encoding $gitencoding is not supported by Tcl/Tk"
> >  }
> >  
> > +set objformat [exec git rev-parse --show-object-format]
> > +if {$objformat eq "sha1"} {
> > +    set hashlength 40
> > +} elseif {$objformat eq "sha256"} {
> > +    set hashlength 64
> > +} else {
> > +    error_popup "[mc "Not supported hash algorithm:"] {$objformat}"
> 
> This looks strange. Where is the $objformat substituted?

Sorry, I don't understand your question here.  Isn't it what you see
in your quoted line...?

> > +    exit 1
> > +}
> > +set hashalgorithm [string toupper $objformat]
> > +unset objformat
> 
> Why not set hashalgorithm right away, without using a temporary
> objformat? Why set it at all here? It's unused.

The objformat is what git-rev-parse gives, and it's also referred to
show in the error message.  You shouldn't convert it to the upper
letters blindly there.

The hashalgorithm is with upper letters for SHA1 and SHA256 to be
shown correctly in other places.  It could be "SHA-1" or other
strings, but it's done in that way because of simpleness.


thanks,

Takashi

Re: [PATCH 2/2] gitk: Add auto-select length preference for SHA256

[Takashi Iwai]

On Thu, 08 May 2025 08:20:57 +0200,
Johannes Sixt wrote:
> 
> Am 20.03.25 um 16:41 schrieb Takashi Iwai:
> > This implements the missing preference setup of the auto select length
> > for SHA256. The variable set via the preference menu is switched
> > depending on the hash algorithm.
> > 
> > The default auto-select length is set to 64 for SHA256, and
> > saved/restored as "autosellensha256" in the config.
> 
> I think the purpose of this change is to offer different settings for
> the selected length depending on the hash algorithm. If that is the
> case, the commit message could do a better job describing that: it says
> only what happens implementationwise, but not what the user sees.

Sure, I can rephrase.  Could you suggest a better text example?

> I do not think this is necessary. If I had set the option to, say, 12, I
> would not want it to be set to something else when I go to a repository
> that has a different hash algorithm.

The problem is that a value 40 is used always as default even for
SHA256.  So, without this change, the selection looks always shorter
than the full ID unless you explicitly change this option in the
configuration menu.  That sucks.

> (But I do not know for certain,
> because I do not have any SHA256 repositories, yet.)

You can find SHA256 git repos in src.opensuse.org, where we started
hitting the problems with gitk :)
For example, one of my repos below is very small:
  https://src.opensuse.org/kernel-firmware/kernel-firmware-all


thanks,

Takashi

Re: [PATCH 0/2] gitk: Support of SHA256 repos

[Takashi Iwai]

On Thu, 08 May 2025 08:21:10 +0200,
Johannes Sixt wrote:
> 
> Am 20.03.25 um 16:41 schrieb Takashi Iwai:
> > Hi,
> > 
> > I recently stumbled on the problem of gitk with sha256 repo (while
> > testing the new openSUSE package git workflow that enforces SHA256);
> > gitk aborts immediately with a message "Can't parse git log output:
> > {commit xxx..}".
> > 
> > After skimming over the net, I found the patch [*] posted in 4 years
> > ago to add the support for sha256, but nothing happened since then,
> > unfortunately.
> > 
> > So here is the revived patch for gitk to add sha256 support, with
> > cleanups and corrections, in addition to the enhancement patch for the
> > missing auto-select length config for sha256.
> > 
> > Only lightly tested on a few sha256 repos.
> > 
> > 
> > Takashi
> > 
> > [*] https://patchwork.kernel.org/project/git/patch/pull.979.git.1623687519832.gitgitgadget@gmail.com
> > 
> > ===
> > 
> > Rostislav Krasny (1):
> >   gitk: Add a basic support of SHA256 repositories into Gitk
> > 
> > Takashi Iwai (1):
> >   gitk: Add auto-select length preference for SHA256
> > 
> >  gitk-git/gitk | 96 +++++++++++++++++++++++++++++++++++++--------------
> >  1 file changed, 71 insertions(+), 25 deletions(-)
> > 
> 
> Thank you.
> 
> After these patches, I still see a  few mentions of "40" that refer to
> object id lengths and are not converted.
> 
> - a comment above proc longid
> - a regexp in proc shortids
> - in proc setlink
> - in proc mkpatch
> - in proc mktag
> 
> They should be converted, too, I think.

OK, let me try those, too.  I guess it shouldn't be too difficult.
(I didn't know of those features :)


thanks,

Takashi