{"thread":{"id":"17631","subject":"[PATCH (GIT-GUI BUG) v2] git-gui: Avoid an infinite rescan loop in handle_empty_diff.","startedAt":"2009-02-07T16:24:01Z","lastAt":"2009-02-07T16:24:01Z","messageCount":1,"participants":["Alexander Gavrilov"],"isPatch":true,"patchVersion":2,"patchTotal":null},"messages":[{"id":"103634","messageId":"200902071924.02419.angavrilov@gmail.com","threadId":"17631","inReplyTo":null,"subject":"[PATCH (GIT-GUI BUG) v2] git-gui: Avoid an infinite rescan loop in handle_empty_diff.","fromName":"Alexander Gavrilov","fromEmail":"angavrilov@gmail.com","sentAt":"2009-02-07T16:24:01Z","receivedAt":"2009-02-07T16:24:01Z","isPatch":true,"sender":{"key":"angavrilov@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42666?v=4"},"body":"If the index update machinery and git diff happen to disagree\non whether a particular file is modified, it may cause git-gui\nto enter an infinite index rescan loop, where an empty diff\nstarts a rescan, which finds the same set of files modified,\nand tries to display the diff for the first one, which happens\nto be the empty one. A current example of a possible disagreement\npoint is the autocrlf filter.\n\nThis patch breaks the loop by using a global counter to track\nthe auto-rescans. The variable is reset whenever a non-empty\ndiff is displayed.\n\nAnother suggested approach, which is based on giving the\n--exit-code argument to git diff, cannot be used, because\ndiff-files seems to trust the timestamps in the index, and\nreturns a non-zero code even if the file is actually\nunchanged, which essentially defeats the purpose of the\nauto-rescan logic.\n\nSigned-off-by: Alexander Gavrilov <angavrilov@gmail.com>\n---\n\n\tApparently git-diff also has a small crlf-related bug.\n\tIt can be reproduced using the following commands:\n\n\n\t$ rm -rf .git *\n\t$ git init\n\t$ echo > foo\n\t$ git add foo && git commit -m init\n\t$ git diff-files -p --exit-code && echo unchanged\n\tunchanged\n\t$ git diff --exit-code && echo unchanged\n\tunchanged\n\t$ touch foo\n\t$ git diff-files -p --exit-code && echo unchanged\n\tdiff --git a/foo b/foo\n\t$ git diff --exit-code && echo unchanged\n\tunchanged\n\n\t    Without autocrlf git-diff recognizes that\n\t    the file is unchanged.\n\n\n\t$ rm -rf .git *\n\t$ git init\n\t$ git config core.autocrlf true\n\t$ echo > foo\n\t$ unix2dos -o foo\n\t$ git add foo && git commit -m init\n\t$ git diff-files -p --exit-code && echo unchanged\n\tunchanged\n\t$ git diff --exit-code && echo unchanged\n\tunchanged\n\t$ touch foo\n\t$ git diff-files -p --exit-code && echo unchanged\n\tdiff --git a/foo b/foo\n\t$ git diff --exit-code && echo unchanged\n\tdiff --git a/foo b/foo\n\n\t    With crlf it prints an empty diff and says\n\t    that there are some changes.\n\n\n\tI don't think that this behavior should depend\n\ton the autocrlf setting.\n\n\t-- Alexander\n\n lib/diff.tcl |    9 +++++++++\n 1 files changed, 9 insertions(+), 0 deletions(-)\n\ndiff --git a/lib/diff.tcl b/lib/diff.tcl\nindex bbbf15c..925b3f5 100644\n--- a/lib/diff.tcl\n+++ b/lib/diff.tcl\n@@ -51,11 +51,16 @@ proc force_diff_encoding {enc} {\n \n proc handle_empty_diff {} {\n \tglobal current_diff_path file_states file_lists\n+\tglobal diff_empty_count\n \n \tset path $current_diff_path\n \tset s $file_states($path)\n \tif {[lindex $s 0] ne {_M}} return\n \n+\t# Prevent infinite rescan loops\n+\tincr diff_empty_count\n+\tif {$diff_empty_count > 1} return\n+\n \tinfo_popup [mc \"No differences detected.\n \n %s has no changes.\n@@ -310,6 +315,7 @@ proc read_diff {fd cont_info} {\n \tglobal ui_diff diff_active\n \tglobal is_3way_diff is_conflict_diff current_diff_header\n \tglobal current_diff_queue\n+\tglobal diff_empty_count\n \n \t$ui_diff conf -state normal\n \twhile {[gets $fd line] >= 0} {\n@@ -415,7 +421,10 @@ proc read_diff {fd cont_info} {\n \n \t\tif {[$ui_diff index end] eq {2.0}} {\n \t\t\thandle_empty_diff\n+\t\t} else {\n+\t\t\tset diff_empty_count 0\n \t\t}\n+\n \t\tset callback [lindex $cont_info 1]\n \t\tif {$callback ne {}} {\n \t\t\teval $callback\n-- \n1.6.1.63.g950db\n"}]}