* [RFC PATCH] gitk: avoid hardcoded 40 characters for oid
@ 2021-10-11 11:47 Carlo Marcelo Arenas Belón
2021-10-11 19:55 ` [PATCH] gitk: avoid hardcoded 40 character length " Carlo Marcelo Arenas Belón
0 siblings, 1 reply; 4+ messages in thread
From: Carlo Marcelo Arenas Belón @ 2021-10-11 11:47 UTC (permalink / raw)
To: git, paulus; +Cc: Carlo Marcelo Arenas Belón
Current git could return ids larger than 40 characters if using
SHA-256.
Detect if the current repository does and set the oid_size to 64
instead, but fallback to 40 for backward compatibility, as well
as matching ids in a hash agnostic way.
Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
---
With this patch, I was able to open gitk in a SHA-256 repository
without an error, and still do the same in a regular one.
Sending it as an RFC, since I am not a gitk user and haven't done
much testing of it, and because it is likely incomplete, as they
are still variables and other labels referring to SHA1 which would
need to be updated as well.
There is also a related set of changes needed for git-gui that
will be send independently.
gitk | 45 +++++++++++++++++++++++++++++----------------
1 file changed, 29 insertions(+), 16 deletions(-)
diff --git a/gitk b/gitk
index 23d9dd1..5087543 100755
--- a/gitk
+++ b/gitk
@@ -1418,6 +1418,7 @@ proc getcommitlines {fd inst view updating} {
global parents children curview hlview
global idpending ordertok
global varccommits varcid varctok vtokmod vfilelimit vshortids
+ global oid_size
set stuff [read $fd 500000]
# git log doesn't terminate the last commit with a null...
@@ -1500,7 +1501,7 @@ proc getcommitlines {fd inst view updating} {
}
set ok 1
foreach id $ids {
- if {[string length $id] != 40} {
+ if {[string length $id] != $oid_size} {
set ok 0
break
}
@@ -3925,7 +3926,7 @@ proc stopblaming {} {
}
proc read_line_source {fd inst} {
- global blamestuff curview commfd blameinst nullid nullid2
+ global blamestuff curview commfd blameinst nullid nullid2 oid_size
while {[gets $fd line] >= 0} {
lappend blamestuff($inst) $line
@@ -3946,7 +3947,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] == $oid_size && [string is xdigit $id] &&
[string is digit -strict $lnum]} {
# look for "filename" line
foreach l $blamestuff($inst) {
@@ -5094,7 +5095,7 @@ proc shortids {ids} {
foreach id $ids {
if {[llength $id] > 1} {
lappend res [shortids $id]
- } elseif {[regexp {^[0-9a-f]{40}$} $id]} {
+ } elseif {[regexp {^[0-9a-f]{40}(?:[0-9a-f]{24})?$} $id]} {
lappend res [string range $id 0 7]
} else {
lappend res $id
@@ -5269,13 +5270,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 oid_size
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] == $oid_size && [string is xdigit $line]} {
set id $line
}
set viewmainheadid($view) $id
@@ -7070,13 +7072,14 @@ proc appendwithlinks {text tags} {
proc setlink {id lk} {
global curview ctext pendinglinks
global linkfgcolor
+ global oid_size
if {[string range $id 0 1] eq "-g"} {
set id [string range $id 2 end]
}
set known 0
- if {[string length $id] < 40} {
+ if {[string length $id] < $oid_size} {
set matches [longid $id]
if {[llength $matches] > 0} {
if {[llength $matches] > 1} return
@@ -8715,8 +8718,8 @@ proc incrfont {inc} {
}
proc clearsha1 {} {
- global sha1entry sha1string
- if {[string length $sha1string] == 40} {
+ global sha1entry sha1string oid_size
+ if {[string length $sha1string] == $oid_size} {
$sha1entry delete 0 end
}
}
@@ -9445,11 +9448,11 @@ proc mktaggo {} {
}
proc copyreference {} {
- global rowmenuid autosellen
+ global rowmenuid autosellen oid_size
set format "%h (\"%s\", %ad)"
set cmd [list git show -s --pretty=format:$format --date=short]
- if {$autosellen < 40} {
+ if {$autosellen < $oid_size} {
lappend cmd --abbrev=$autosellen
}
set reference [eval exec $cmd $rowmenuid]
@@ -9459,7 +9462,7 @@ proc copyreference {} {
}
proc writecommit {} {
- global rowmenuid wrcomtop commitinfo wrcomcmd NS
+ global rowmenuid wrcomtop commitinfo wrcomcmd NS oid_size
set top .writecommit
set wrcomtop $top
@@ -9469,7 +9472,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 $oid_size
$top.sha1 insert 0 $rowmenuid
$top.sha1 conf -state readonly
grid $top.id $top.sha1 -sticky w
@@ -9548,7 +9551,7 @@ proc mvbranch {} {
}
proc branchdia {top valvar uivar} {
- global NS commitinfo
+ global NS commitinfo oid_size
upvar $valvar val $uivar ui
catch {destroy $top}
@@ -9557,7 +9560,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 $oid_size
$top.sha1 insert 0 $val(id)
$top.sha1 conf -state readonly
grid $top.id $top.sha1 -sticky w
@@ -12565,8 +12568,18 @@ if {$i >= [llength $argv] && $revtreeargs ne {}} {
}
}
-set nullid "0000000000000000000000000000000000000000"
-set nullid2 "0000000000000000000000000000000000000001"
+if {[catch {exec git config --get extensions.objectformat} hash]} {
+ set oid_size 40
+} else {
+ if { $hash eq "sha256" } {
+ set oid_size 64
+ } else {
+ set oid_size 40
+ }
+}
+
+set nullid %-0*s $oid_size 0
+set nullid2 %-0*s $oid_size 1
set nullfile "/dev/null"
set have_tk85 [expr {[package vcompare $tk_version "8.5"] >= 0}]
--
2.33.0.1081.g099423f5b7
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH] gitk: avoid hardcoded 40 character length for oid
2021-10-11 11:47 [RFC PATCH] gitk: avoid hardcoded 40 characters for oid Carlo Marcelo Arenas Belón
@ 2021-10-11 19:55 ` Carlo Marcelo Arenas Belón
2021-10-11 20:03 ` Eric Sunshine
0 siblings, 1 reply; 4+ messages in thread
From: Carlo Marcelo Arenas Belón @ 2021-10-11 19:55 UTC (permalink / raw)
To: git, paulus; +Cc: avarab, Carlo Marcelo Arenas Belón, Rostislav Krasny
Current git could return ids larger than 40 characters if using
SHA-256.
Detect if the current repository does and set the oid_size to 64
instead, but fallback to 40 for backward compatibility.
Helped-by: Rostislav Krasny <rosti.bsd@gmail.com>
Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
---
gitk | 91 ++++++++++++++++++++++++++++++++++++------------------------
1 file changed, 54 insertions(+), 37 deletions(-)
diff --git a/gitk b/gitk
index 23d9dd1..22b3cd5 100755
--- a/gitk
+++ b/gitk
@@ -289,7 +289,7 @@ proc parseviewargs {n arglist} {
}
proc parseviewrevs {view revs} {
- global vposids vnegids
+ global vposids vnegids oid_size
if {$revs eq {}} {
set revs HEAD
@@ -303,7 +303,8 @@ 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] == $oid_size
+ && [string is xdigit $line])} {
if {[string match "fatal:*" $line]} {
if {[string match "fatal: ambiguous argument*" $line]
&& $badrev ne {}} {
@@ -507,6 +508,7 @@ proc updatecommits {} {
global hasworktree
global varcid vposids vnegids vflags vrevs
global show_notes
+ global oid_size
set hasworktree [hasworktree]
rereadrefs
@@ -540,7 +542,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] == $oid_size} {
if {[lsearch -exact $oldpos $rev] < 0
&& ![info exists varcid($view,$rev)]} {
lappend newrevs $rev
@@ -1418,6 +1420,7 @@ proc getcommitlines {fd inst view updating} {
global parents children curview hlview
global idpending ordertok
global varccommits varcid varctok vtokmod vfilelimit vshortids
+ global oid_size
set stuff [read $fd 500000]
# git log doesn't terminate the last commit with a null...
@@ -1500,7 +1503,7 @@ proc getcommitlines {fd inst view updating} {
}
set ok 1
foreach id $ids {
- if {[string length $id] != 40} {
+ if {[string length $id] != $oid_size} {
set ok 0
break
}
@@ -1780,6 +1783,7 @@ proc readrefs {} {
global selecthead selectheadid
global hideremotes
global tclencoding
+ global oid_size
foreach v {tagids idtags headids idheads otherrefids idotherrefs} {
unset -nocomplain $v
@@ -1789,9 +1793,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 $oid_size] ne " "} continue
+ set id [string range $line 0 [expr {$oid_size - 1}]]
+ set ref [string range $line [expr {$oid_size + 1}] end]
if {![string match "refs/*" $ref]} continue
set name [string range $ref 5 end]
if {[string match "remotes/*" $name]} {
@@ -2082,6 +2086,7 @@ proc makewindow {} {
global have_tk85 use_ttk NS
global git_version
global worddiff
+ global oid_size
# The "mc" arguments here are purely so that xgettext
# sees the following string as needing to be translated
@@ -2203,11 +2208,12 @@ proc makewindow {} {
set sha1entry .tf.bar.sha1
set entries $sha1entry
set sha1but .tf.bar.sha1label
- button $sha1but -text "[mc "SHA1 ID:"] " -state disabled -relief flat \
- -command gotocommit -width 8
+ button $sha1but -text "[mc "SHA ID:"] " -state disabled -relief flat \
+ -command gotocommit -width 7
$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 $oid_size -font textfont \
+ -textvariable sha1string
trace add variable sha1string write sha1change
pack $sha1entry -side left -pady 2
@@ -3925,7 +3931,7 @@ proc stopblaming {} {
}
proc read_line_source {fd inst} {
- global blamestuff curview commfd blameinst nullid nullid2
+ global blamestuff curview commfd blameinst nullid nullid2 oid_size
while {[gets $fd line] >= 0} {
lappend blamestuff($inst) $line
@@ -3946,7 +3952,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] == $oid_size && [string is xdigit $id] &&
[string is digit -strict $lnum]} {
# look for "filename" line
foreach l $blamestuff($inst) {
@@ -5094,7 +5100,7 @@ proc shortids {ids} {
foreach id $ids {
if {[llength $id] > 1} {
lappend res [shortids $id]
- } elseif {[regexp {^[0-9a-f]{40}$} $id]} {
+ } elseif {[regexp {^[0-9a-f]{40,}$} $id]} {
lappend res [string range $id 0 7]
} else {
lappend res $id
@@ -5269,13 +5275,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 oid_size
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] == $oid_size && [string is xdigit $line]} {
set id $line
}
set viewmainheadid($view) $id
@@ -7034,15 +7041,16 @@ proc commit_descriptor {p} {
return "$p ($l)\n"
}
-# append some text to the ctext widget, and make any SHA1 ID
+# append some text to the ctext widget, and make any SHA ID
# that we know about be a clickable link.
# Also look for URLs of the form "http[s]://..." and make them web links.
proc appendwithlinks {text tags} {
- global ctext linknum curview
+ global ctext linknum curview oid_size
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 "@@ $oid_size" {(?:\m|-g)[0-9a-f]{6,@@}\M}] $text]
foreach l $links {
set s [lindex $l 0]
set e [lindex $l 1]
@@ -7070,13 +7078,14 @@ proc appendwithlinks {text tags} {
proc setlink {id lk} {
global curview ctext pendinglinks
global linkfgcolor
+ global oid_size
if {[string range $id 0 1] eq "-g"} {
set id [string range $id 2 end]
}
set known 0
- if {[string length $id] < 40} {
+ if {[string length $id] < $oid_size} {
set matches [longid $id]
if {[llength $matches] > 0} {
if {[llength $matches] > 1} return
@@ -8715,8 +8724,8 @@ proc incrfont {inc} {
}
proc clearsha1 {} {
- global sha1entry sha1string
- if {[string length $sha1string] == 40} {
+ global sha1entry sha1string oid_size
+ if {[string length $sha1string] == $oid_size} {
$sha1entry delete 0 end
}
}
@@ -8733,7 +8742,7 @@ proc sha1change {n1 n2 op} {
if {$state == "normal"} {
$sha1but conf -state normal -relief raised -text "[mc "Goto:"] "
} else {
- $sha1but conf -state disabled -relief flat -text "[mc "SHA1 ID:"] "
+ $sha1but conf -state disabled -relief flat -text "[mc "SHA ID:"] "
}
}
@@ -8748,11 +8757,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,}$} $id]} {
set matches [longid $id]
if {$matches ne {}} {
if {[llength $matches] > 1} {
- error_popup [mc "Short SHA1 id %s is ambiguous" $id]
+ error_popup [mc "Short SHA id %s is ambiguous" $id]
return
}
set id [lindex $matches 0]
@@ -8769,7 +8778,7 @@ proc gotocommit {} {
return
}
if {[regexp {^[0-9a-fA-F]{4,}$} $sha1string]} {
- set msg [mc "SHA1 id %s is not known" $sha1string]
+ set msg [mc "SHA id %s is not known" $sha1string]
} else {
set msg [mc "Revision %s is not in the current view" $sha1string]
}
@@ -9445,11 +9454,11 @@ proc mktaggo {} {
}
proc copyreference {} {
- global rowmenuid autosellen
+ global rowmenuid autosellen oid_size
set format "%h (\"%s\", %ad)"
set cmd [list git show -s --pretty=format:$format --date=short]
- if {$autosellen < 40} {
+ if {$autosellen < $oid_size} {
lappend cmd --abbrev=$autosellen
}
set reference [eval exec $cmd $rowmenuid]
@@ -9459,7 +9468,7 @@ proc copyreference {} {
}
proc writecommit {} {
- global rowmenuid wrcomtop commitinfo wrcomcmd NS
+ global rowmenuid wrcomtop commitinfo wrcomcmd NS oid_size
set top .writecommit
set wrcomtop $top
@@ -9469,7 +9478,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 $oid_size
$top.sha1 insert 0 $rowmenuid
$top.sha1 conf -state readonly
grid $top.id $top.sha1 -sticky w
@@ -9548,7 +9557,7 @@ proc mvbranch {} {
}
proc branchdia {top valvar uivar} {
- global NS commitinfo
+ global NS commitinfo oid_size
upvar $valvar val $uivar ui
catch {destroy $top}
@@ -9557,7 +9566,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 $oid_size
$top.sha1 insert 0 $val(id)
$top.sha1 conf -state readonly
grid $top.id $top.sha1 -sticky w
@@ -11565,9 +11574,9 @@ proc prefspage_general {notebook} {
${NS}::checkbutton $page.showlocal -text [mc "Show local changes"] \
-variable showlocalchanges
grid x $page.showlocal -sticky w
- ${NS}::checkbutton $page.autoselect -text [mc "Auto-select SHA1 (length)"] \
+ ${NS}::checkbutton $page.autoselect -text [mc "Auto-select SHA (length)"] \
-variable autoselect
- spinbox $page.autosellen -from 1 -to 40 -width 4 -textvariable autosellen
+ spinbox $page.autosellen -from 1 -to 64 -width 4 -textvariable autosellen
grid x $page.autoselect $page.autosellen -sticky w
${NS}::checkbutton $page.hideremotes -text [mc "Hide remote refs"] \
-variable hideremotes
@@ -12338,6 +12347,18 @@ catch {
set log_showroot [exec git config --bool --get log.showroot]
}
+if {![catch {exec git config --get extensions.objectformat} oid_type]
+ && $oid_type eq "sha256" } {
+ set oid_size 64
+} else {
+ set oid_type "sha1"
+ set oid_size 40
+}
+
+set nullid [format "%0*s" $oid_size 0]
+set nullid2 [format "%0*s" $oid_size 1]
+set nullfile "/dev/null"
+
if {[tk windowingsystem] eq "aqua"} {
set mainfont {{Lucida Grande} 9}
set textfont {Monaco 9}
@@ -12372,7 +12393,7 @@ set showlocalchanges 1
set limitdiffs 1
set datetimeformat "%Y-%m-%d %H:%M:%S"
set autoselect 1
-set autosellen 40
+set autosellen $oid_size
set perfile_attrs 0
set want_ttk 1
@@ -12565,10 +12586,6 @@ if {$i >= [llength $argv] && $revtreeargs ne {}} {
}
}
-set nullid "0000000000000000000000000000000000000000"
-set nullid2 "0000000000000000000000000000000000000001"
-set nullfile "/dev/null"
-
set have_tk85 [expr {[package vcompare $tk_version "8.5"] >= 0}]
if {![info exists have_ttk]} {
set have_ttk [llength [info commands ::ttk::style]]
--
2.33.0.1081.g099423f5b7
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] gitk: avoid hardcoded 40 character length for oid
2021-10-11 19:55 ` [PATCH] gitk: avoid hardcoded 40 character length " Carlo Marcelo Arenas Belón
@ 2021-10-11 20:03 ` Eric Sunshine
2021-10-11 20:37 ` Carlo Arenas
0 siblings, 1 reply; 4+ messages in thread
From: Eric Sunshine @ 2021-10-11 20:03 UTC (permalink / raw)
To: Carlo Marcelo Arenas Belón
Cc: Git List, Paul Mackerras, Ævar Arnfjörð Bjarmason,
Rostislav Krasny
On Mon, Oct 11, 2021 at 3:56 PM Carlo Marcelo Arenas Belón
<carenas@gmail.com> wrote:
> Current git could return ids larger than 40 characters if using
> SHA-256.
>
> Detect if the current repository does and set the oid_size to 64
> instead, but fallback to 40 for backward compatibility.
>
> Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
> ---
> diff --git a/gitk b/gitk
> @@ -12338,6 +12347,18 @@ catch {
> +if {![catch {exec git config --get extensions.objectformat} oid_type]
> + && $oid_type eq "sha256" } {
> + set oid_size 64
> +} else {
> + set oid_type "sha1"
> + set oid_size 40
> +}
Should this be using:
git rev-parse --show-object-format
rather than `git-config`?
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] gitk: avoid hardcoded 40 character length for oid
2021-10-11 20:03 ` Eric Sunshine
@ 2021-10-11 20:37 ` Carlo Arenas
0 siblings, 0 replies; 4+ messages in thread
From: Carlo Arenas @ 2021-10-11 20:37 UTC (permalink / raw)
To: Eric Sunshine
Cc: Git List, Paul Mackerras, Ævar Arnfjörð Bjarmason,
Rostislav Krasny
On Mon, Oct 11, 2021 at 1:04 PM Eric Sunshine <sunshine@sunshineco.com> wrote:
>
> On Mon, Oct 11, 2021 at 3:56 PM Carlo Marcelo Arenas Belón
> > diff --git a/gitk b/gitk
> > @@ -12338,6 +12347,18 @@ catch {
> > +if {![catch {exec git config --get extensions.objectformat} oid_type]
> > + && $oid_type eq "sha256" } {
> > + set oid_size 64
> > +} else {
> > + set oid_type "sha1"
> > + set oid_size 40
> > +}
>
> Should this be using:
>
> git rev-parse --show-object-format
>
> rather than `git-config`?
definitely a better fit, will be included in the next version.
Carlo
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-10-11 20:37 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-11 11:47 [RFC PATCH] gitk: avoid hardcoded 40 characters for oid Carlo Marcelo Arenas Belón
2021-10-11 19:55 ` [PATCH] gitk: avoid hardcoded 40 character length " Carlo Marcelo Arenas Belón
2021-10-11 20:03 ` Eric Sunshine
2021-10-11 20:37 ` Carlo Arenas
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).