diff mbox series

[RFC,2/4] rename all *_sha1 variables and make null_oid hash aware

Message ID 20211011121757.627-3-carenas@gmail.com (mailing list archive)
State New, archived
Headers show
Series git-gui: support SHA-256 repositories | expand

Commit Message

Carlo Marcelo Arenas Belón Oct. 11, 2021, 12:17 p.m. UTC 
Before this change, creating a branch in an SHA-256 repository would
fail because the null_sha1 used was of the wrong size.

Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
---
 git-gui.sh          | 26 +++++++++++++++-----------
 lib/blame.tcl       | 10 +++++-----
 lib/checkout_op.tcl |  4 ++--
 3 files changed, 22 insertions(+), 18 deletions(-)

Comments

Eric Sunshine Oct. 11, 2021, 8:07 p.m. UTC | #1 
On Mon, Oct 11, 2021 at 8:18 AM Carlo Marcelo Arenas Belón
<carenas@gmail.com> wrote:
> Before this change, creating a branch in an SHA-256 repository would
> fail because the null_sha1 used was of the wrong size.
>
> Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
> ---
> diff --git a/git-gui.sh b/git-gui.sh
> @@ -1820,10 +1820,14 @@ proc short_path {path} {
> +if { [get_config extensions.objectformat] eq "sha256" } {
> +       set null_oid [string repeat 0 64]
> +} else {
> +       set null_oid [string repeat 0 40]
> +}

Should this be using:

    git rev-parse --show-object-format

rather than reading the configuration directly?
Pratyush Yadav Nov. 13, 2021, 6:54 a.m. UTC | #2 
On 11/10/21 05:17AM, Carlo Marcelo Arenas Belón wrote:
> Before this change, creating a branch in an SHA-256 repository would
> fail because the null_sha1 used was of the wrong size.
> 
> Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
> ---
>  git-gui.sh          | 26 +++++++++++++++-----------
>  lib/blame.tcl       | 10 +++++-----
>  lib/checkout_op.tcl |  4 ++--
>  3 files changed, 22 insertions(+), 18 deletions(-)
> 
> diff --git a/git-gui.sh b/git-gui.sh
> index a69b0fe..c0dc8ce 100755
> --- a/git-gui.sh
> +++ b/git-gui.sh
> @@ -1820,10 +1820,14 @@ proc short_path {path} {
>  }
>  
>  set next_icon_id 0
> -set null_sha1 [string repeat 0 40]
> +if { [get_config extensions.objectformat] eq "sha256" } {

From the docs I see that this feature is experimental as of now and 
might change in the future. Can we expect this config option to stay 
stable over time? If not I think this might be too early to introduce it 
into git-gui.

Anyway, nitpick: don't add spaces after opening brace and before closing 
brace.

> +	set null_oid [string repeat 0 64]
> +} else {
> +	set null_oid [string repeat 0 40]
> +}
>  
>  proc merge_state {path new_state {head_info {}} {index_info {}}} {
> -	global file_states next_icon_id null_sha1
> +	global file_states next_icon_id null_oid
>  
>  	set s0 [string index $new_state 0]
>  	set s1 [string index $new_state 1]

Rest of the patch looks good to me. Thanks.
diff mbox series

Patch

diff --git a/git-gui.sh b/git-gui.sh
index a69b0fe..c0dc8ce 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -1820,10 +1820,14 @@  proc short_path {path} {
 }
 
 set next_icon_id 0
-set null_sha1 [string repeat 0 40]
+if { [get_config extensions.objectformat] eq "sha256" } {
+	set null_oid [string repeat 0 64]
+} else {
+	set null_oid [string repeat 0 40]
+}
 
 proc merge_state {path new_state {head_info {}} {index_info {}}} {
-	global file_states next_icon_id null_sha1
+	global file_states next_icon_id null_oid
 
 	set s0 [string index $new_state 0]
 	set s1 [string index $new_state 1]
@@ -1845,7 +1849,7 @@  proc merge_state {path new_state {head_info {}} {index_info {}}} {
 	elseif {$s1 eq {_}} {set s1 _}
 
 	if {$s0 eq {A} && $s1 eq {_} && $head_info eq {}} {
-		set head_info [list 0 $null_sha1]
+		set head_info [list 0 $null_oid]
 	} elseif {$s0 ne {_} && [string index $state 0] eq {_}
 		&& $head_info eq {}} {
 		set head_info $index_info
@@ -2179,21 +2183,21 @@  proc do_gitk {revs {is_submodule false}} {
 			cd $current_diff_path
 			if {$revs eq {--}} {
 				set s $file_states($current_diff_path)
-				set old_sha1 {}
-				set new_sha1 {}
+				set old_oid {}
+				set new_oid {}
 				switch -glob -- [lindex $s 0] {
-				M_ { set old_sha1 [lindex [lindex $s 2] 1] }
-				_M { set old_sha1 [lindex [lindex $s 3] 1] }
+				M_ { set old_oid [lindex [lindex $s 2] 1] }
+				_M { set old_oid [lindex [lindex $s 3] 1] }
 				MM {
 					if {$current_diff_side eq $ui_index} {
-						set old_sha1 [lindex [lindex $s 2] 1]
-						set new_sha1 [lindex [lindex $s 3] 1]
+						set old_oid [lindex [lindex $s 2] 1]
+						set new_oid [lindex [lindex $s 3] 1]
 					} else {
-						set old_sha1 [lindex [lindex $s 3] 1]
+						set old_oid [lindex [lindex $s 3] 1]
 					}
 				}
 				}
-				set revs $old_sha1...$new_sha1
+				set revs $old_oid...$new_oid
 			}
 			# GIT_DIR and GIT_WORK_TREE for the submodule are not the ones
 			# we've been using for the main repository, so unset them.
diff --git a/lib/blame.tcl b/lib/blame.tcl
index 6ece79d..e6d4302 100644
--- a/lib/blame.tcl
+++ b/lib/blame.tcl
@@ -1056,14 +1056,14 @@  method _format_offset_date {base offset} {
 }
 
 method _gitkcommit {} {
-	global null_sha1
+	global null_oid
 
 	set dat [_get_click_amov_info $this]
 	if {$dat ne {}} {
 		set cmit [lindex $dat 0]
 
 		# If the line belongs to the working copy, use HEAD instead
-		if {$cmit eq $null_sha1} {
+		if {$cmit eq $null_oid} {
 			if {[catch {set cmit [git rev-parse --verify HEAD]} err]} {
 				error_popup [strcat [mc "Cannot find HEAD commit:"] "\n\n$err"]
 				return;
@@ -1106,7 +1106,7 @@  method _gitkcommit {} {
 }
 
 method _blameparent {} {
-	global null_sha1
+	global null_oid
 
 	set dat [_get_click_amov_info $this]
 	if {$dat ne {}} {
@@ -1114,7 +1114,7 @@  method _blameparent {} {
 		set new_path [lindex $dat 1]
 
 		# Allow using Blame Parent on lines modified in the working copy
-		if {$cmit eq $null_sha1} {
+		if {$cmit eq $null_oid} {
 			set parent_ref "HEAD"
 		} else {
 			set parent_ref "$cmit^"
@@ -1129,7 +1129,7 @@  method _blameparent {} {
 		# Generate a diff between the commit and its parent,
 		# and use the hunks to update the line number.
 		# Request zero context to simplify calculations.
-		if {$cmit eq $null_sha1} {
+		if {$cmit eq $null_oid} {
 			set diffcmd [list diff-index --unified=0 $cparent -- $new_path]
 		} else {
 			set diffcmd [list diff-tree --unified=0 $cparent $cmit -- $new_path]
diff --git a/lib/checkout_op.tcl b/lib/checkout_op.tcl
index 21ea768..be1ebba 100644
--- a/lib/checkout_op.tcl
+++ b/lib/checkout_op.tcl
@@ -151,7 +151,7 @@  method _finish_fetch {ok} {
 }
 
 method _update_ref {} {
-	global null_sha1 current_branch repo_config
+	global null_oid current_branch repo_config
 
 	set ref $new_ref
 	set new $new_hash
@@ -177,7 +177,7 @@  method _update_ref {} {
 		}
 
 		set reflog_msg "branch: Created from $new_expr"
-		set cur $null_sha1
+		set cur $null_oid
 
 		if {($repo_config(branch.autosetupmerge) eq {true}
 			|| $repo_config(branch.autosetupmerge) eq {always})