{"thread":{"id":"38291","subject":"[PATCH] git-gui.sh: support Tcl 8.4","startedAt":"2015-01-06T10:41:21Z","lastAt":"2015-01-13T01:12:19Z","messageCount":6,"participants":["Kyle J. McKay","Junio C Hamano","Jens Lehmann","Pat Thoyts"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"254326","messageId":"97e448e7908a1f959a7294e389553b5@74d39fa044aa309eaea14b9f57fe79c","threadId":"38291","inReplyTo":null,"subject":"[PATCH] git-gui.sh: support Tcl 8.4","fromName":"Kyle J. McKay","fromEmail":"mackyle@gmail.com","sentAt":"2015-01-06T10:41:21Z","receivedAt":"2015-01-06T10:41:21Z","isPatch":true,"sender":{"key":"mackyle@gmail.com","avatar":"https://avatars.githubusercontent.com/u/813346?v=4"},"body":"Tcl 8.5 introduced an extended vsatisfies syntax that is not\nsupported by Tcl 8.4.\n\nSince only Tcl 8.4 is required this presents a problem.\n\nThe extended syntax was used starting with Git 2.0.0 in\ncommit b3f0c5c0 so that a major version change would still\nsatisfy the condition.\n\nHowever, what we really want is just a basic version compare,\nso use vcompare instead to restore compatibility with Tcl 8.4.\n\nSigned-off-by: Kyle J. McKay\n---\n git-gui/git-gui.sh | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh\nindex b186329d..a1a23b56 100755\n--- a/git-gui/git-gui.sh\n+++ b/git-gui/git-gui.sh\n@@ -1283,7 +1283,7 @@ load_config 0\n apply_config\n \n # v1.7.0 introduced --show-toplevel to return the canonical work-tree\n-if {[package vsatisfies $_git_version 1.7.0-]} {\n+if {[package vcompare $_git_version 1.7.0] >= 0} {\n \tif { [is_Cygwin] } {\n \t\tcatch {set _gitworktree [exec cygpath --windows [git rev-parse --show-toplevel]]}\n \t} else {\n@@ -1539,7 +1539,7 @@ proc rescan_stage2 {fd after} {\n \t\tclose $fd\n \t}\n \n-\tif {[package vsatisfies $::_git_version 1.6.3-]} {\n+\tif {[package vcompare $::_git_version 1.6.3] >= 0} {\n \t\tset ls_others [list --exclude-standard]\n \t} else {\n \t\tset ls_others [list --exclude-per-directory=.gitignore]\n-- \n2.1.4\n"},{"id":"254366","messageId":"xmqqvbkjofvw.fsf@gitster.dls.corp.google.com","threadId":"38291","inReplyTo":"97e448e7908a1f959a7294e389553b5@74d39fa044aa309eaea14b9f57fe79c","subject":"Re: [PATCH] git-gui.sh: support Tcl 8.4","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-01-06T19:42:11Z","receivedAt":"2015-01-06T19:42:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Kyle J. McKay\" writes:\n\n> Tcl 8.5 introduced an extended vsatisfies syntax that is not\n> supported by Tcl 8.4.\n\nInteresting. We discussed this exact thing just before 2.0 in\n\n http://thread.gmane.org/gmane.comp.version-control.git/247511/focus=248858\n\nand nobody seems to have noticed that giving the new range notation\nto vsatisfies is too new back then.\n\n> Since only Tcl 8.4 is required this presents a problem.\n\nIndeed.\n\n> However, what we really want is just a basic version compare,\n> so use vcompare instead to restore compatibility with Tcl 8.4.\n\nMy Tcl is not just rusty but corroded, so help me out here.\n\n * Your version that compares the sign of the result looks more\n correct than $gmane/248858; was the patch proposed back then but\n did not get applied wrong? This question is out of mere\n curiosity.\n\n * Would it be a good idea to update the places $gmane/248895 points\n out? It is clearly outside the scope of this fix, but we may\n want to do so while our mind is on the \"how do we check required\n version?\" in a separate patch.\n\nThanks.\n\n> Signed-off-by: Kyle J. McKay\n> ---\n> git-gui/git-gui.sh | 4 ++--\n> 1 file changed, 2 insertions(+), 2 deletions(-)\n>\n> diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh\n> index b186329d..a1a23b56 100755\n> --- a/git-gui/git-gui.sh\n> +++ b/git-gui/git-gui.sh\n> @@ -1283,7 +1283,7 @@ load_config 0\n> apply_config\n> \n> # v1.7.0 introduced --show-toplevel to return the canonical work-tree\n> -if {[package vsatisfies $_git_version 1.7.0-]} {\n> +if {[package vcompare $_git_version 1.7.0] >= 0} {\n> \tif { [is_Cygwin] } {\n> \t\tcatch {set _gitworktree [exec cygpath --windows [git rev-parse --show-toplevel]]}\n> \t} else {\n> @@ -1539,7 +1539,7 @@ proc rescan_stage2 {fd after} {\n> \t\tclose $fd\n> \t}\n> \n> -\tif {[package vsatisfies $::_git_version 1.6.3-]} {\n> +\tif {[package vcompare $::_git_version 1.6.3] >= 0} {\n> \t\tset ls_others [list --exclude-standard]\n> \t} else {\n> \t\tset ls_others [list --exclude-per-directory=.gitignore]\n"},{"id":"254374","messageId":"82A625FF-768E-4D7E-8248-B14005464EAE@gmail.com","threadId":"38291","inReplyTo":"xmqqvbkjofvw.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] git-gui.sh: support Tcl 8.4","fromName":"Kyle J. McKay","fromEmail":"mackyle@gmail.com","sentAt":"2015-01-06T22:47:32Z","receivedAt":"2015-01-06T22:47:32Z","isPatch":true,"sender":{"key":"mackyle@gmail.com","avatar":"https://avatars.githubusercontent.com/u/813346?v=4"},"body":"On Jan 6, 2015, at 11:42, Junio C Hamano wrote:\n\n> \"Kyle J. McKay\" writes:\n>\n>> Tcl 8.5 introduced an extended vsatisfies syntax that is not\n>> supported by Tcl 8.4.\n>\n> Interesting. We discussed this exact thing just before 2.0 in\n>\n> http://thread.gmane.org/gmane.comp.version-control.git/247511/focus=248858\n>\n> and nobody seems to have noticed that giving the new range notation\n> to vsatisfies is too new back then.\n>\n>> Since only Tcl 8.4 is required this presents a problem.\n>\n> Indeed.\n>\n>> However, what we really want is just a basic version compare,\n>> so use vcompare instead to restore compatibility with Tcl 8.4.\n>\n> My Tcl is not just rusty but corroded, so help me out here.\n\nMy Tcl is barely operational, but I'll give it a shot. :)\n\n> * Your version that compares the sign of the result looks more\n> correct than $gmane/248858; was the patch proposed back then but\n> did not get applied wrong? This question is out of mere\n> curiosity.\n\nThanks for the reference. That patch proposed this type of change:\n\n-\tif {[package vsatisfies $::_git_version 1.6.3]} {\n+\tif {[package vcompare $::_git_version 1.6.3]} {\n\nBut that's wrong because vsatisfies returns a boolean but vcompare \nreturns an integer (think strcmp result) so the proposed change is \ntesting whether the version is not 1.6.3 rather than being 1.6.3 or \ngreater. But Jens mentions this in $gmane/249491 (that the original \npatch was missing the \">= 0\" part).\n\nI can't find anything in that thread about why vsatisfies was \npreferred over vcompare other than the obvious that the vsatisfies \nversion is only a 1-character change. And that would be more than \nenough except that Tcl 8.4 doesn't support the trailing '-' vsatisfies \nsyntax. There are complaints about this problem with git-gui [1] by \nfolks who have Tcl 8.4 on their system and have upgraded to Git 2.0 or \nlater.\n\n> * Would it be a good idea to update the places $gmane/248895 points\n> out? It is clearly outside the scope of this fix, but we may\n> want to do so while our mind is on the \"how do we check required\n> version?\" in a separate patch.\n\nMakes sense to me, but my Tcl knowledge isn't up to making those \nchanges as the code's a bit different. I have to paraphrase Chris's \nmessage here by saying that I guess those checks are correct if not \nconsistent with the others.\n\n[1] http://stackoverflow.com/questions/24315854/git-gui-cannot-start-because-of-bad-version-number\n\n-Kyle\n"},{"id":"254384","messageId":"xmqqk30zmp9q.fsf@gitster.dls.corp.google.com","threadId":"38291","inReplyTo":"82A625FF-768E-4D7E-8248-B14005464EAE@gmail.com","subject":"Re: [PATCH] git-gui.sh: support Tcl 8.4","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-01-07T00:02:25Z","receivedAt":"2015-01-07T00:02:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"^\"Kyle J. McKay\" writes:\n\n> greater. But Jens mentions this in $gmane/249491 (that the original\n> patch was missing the \">= 0\" part).\n\nAh, that is what I missed. Thanks.\n\n> I can't find anything in that thread about why vsatisfies was\n> preferred over vcompare other than the obvious that the vsatisfies\n> version is only a 1-character change. And that would be more than\n> enough except that Tcl 8.4 doesn't support the trailing '-' vsatisfies\n> syntax.\n\nYeah, I fully agree with that observation.\n\n>> * Would it be a good idea to update the places $gmane/248895 points\n>> out? It is clearly outside the scope of this fix, but we may\n>> want to do so while our mind is on the \"how do we check required\n>> version?\" in a separate patch.\n>\n> Makes sense to me, but my Tcl knowledge isn't up to making those\n> changes as the code's a bit different. I have to paraphrase Chris's\n> message here by saying that I guess those checks are correct if not\n> consistent with the others.\n\nOK, let's ask Pat (cc'ed) to apply your version as-is without\ntouching these 1.5.3 references. I do not take patches to git-gui\ndirectly to my tree.\n\nThanks.\n\n-- >8 --\nFrom: \"Kyle J. McKay\" \nDate: Tue, 6 Jan 2015 02:41:21 -0800 \n\nTcl 8.5 introduced an extended vsatisfies syntax that is not\nsupported by Tcl 8.4.\n\nSince only Tcl 8.4 is required this presents a problem.\n\nThe extended syntax was used starting with Git 2.0.0 in commit\nb3f0c5c0 (git-gui: tolerate major version changes when comparing the\ngit version, 2014-05-17), so that a major version change would still\nsatisfy the condition.\n\nHowever, what we really want is just a basic version compare, so use\nvcompare instead to restore compatibility with Tcl 8.4.\n\nSigned-off-by: Kyle J. McKay \nSigned-off-by: Junio C Hamano \n---\n git-gui.sh | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/git-gui.sh b/git-gui.sh\nindex b186329d..a1a23b56 100755\n--- a/git-gui/git-gui.sh\n+++ b/git-gui/git-gui.sh\n@@ -1283,7 +1283,7 @@ load_config 0\n apply_config\n \n # v1.7.0 introduced --show-toplevel to return the canonical work-tree\n-if {[package vsatisfies $_git_version 1.7.0-]} {\n+if {[package vcompare $_git_version 1.7.0] >= 0} {\n \tif { [is_Cygwin] } {\n \t\tcatch {set _gitworktree [exec cygpath --windows [git rev-parse --show-toplevel]]}\n \t} else {\n@@ -1539,7 +1539,7 @@ proc rescan_stage2 {fd after} {\n \t\tclose $fd\n \t}\n \n-\tif {[package vsatisfies $::_git_version 1.6.3-]} {\n+\tif {[package vcompare $::_git_version 1.6.3] >= 0} {\n \t\tset ls_others [list --exclude-standard]\n \t} else {\n \t\tset ls_others [list --exclude-per-directory=.gitignore]\n-- \n2.1.4\n"},{"id":"254396","messageId":"54ACE1C4.4030502@web.de","threadId":"38291","inReplyTo":"xmqqk30zmp9q.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] git-gui.sh: support Tcl 8.4","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2015-01-07T07:35:32Z","receivedAt":"2015-01-07T07:35:32Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Am 07.01.2015 um 01:02 schrieb Junio C Hamano:\n> ^\"Kyle J. McKay\" writes:\n>> I can't find anything in that thread about why vsatisfies was\n>> preferred over vcompare other than the obvious that the vsatisfies\n>> version is only a 1-character change. And that would be more than\n>> enough except that Tcl 8.4 doesn't support the trailing '-' vsatisfies\n>> syntax.\n>\n> Yeah, I fully agree with that observation.\n\nHaving rather corroded TCL-knowledge myself it was Pat's comment in\n\n http://thread.gmane.org/gmane.comp.version-control.git/247511/focus=249464\n\nthat made me change the patch to use the smaller change of adding\nthe trailing '-' after vsatisfies instead of using vcompare with\na trailing \">= 0\" in v2.\n\n>>> * Would it be a good idea to update the places $gmane/248895 points\n>>> out? It is clearly outside the scope of this fix, but we may\n>>> want to do so while our mind is on the \"how do we check required\n>>> version?\" in a separate patch.\n>>\n>> Makes sense to me, but my Tcl knowledge isn't up to making those\n>> changes as the code's a bit different. I have to paraphrase Chris's\n>> message here by saying that I guess those checks are correct if not\n>> consistent with the others.\n\nWhen I looked at it back then I was convinced these checks are ok\nand should stay as they are to support ancient Git versions (and\nthey do not use vsatisfies either).\n\n> OK, let's ask Pat (cc'ed) to apply your version as-is without\n> touching these 1.5.3 references. I do not take patches to git-gui\n> directly to my tree.\n\nIt's an ack from me on the change below as that was what I came up\nwith and tested successfully before Pat suggested to just add the '-'.\n\n> -- >8 --\n> From: \"Kyle J. McKay\" \n> Date: Tue, 6 Jan 2015 02:41:21 -0800\n>\n> Tcl 8.5 introduced an extended vsatisfies syntax that is not\n> supported by Tcl 8.4.\n>\n> Since only Tcl 8.4 is required this presents a problem.\n>\n> The extended syntax was used starting with Git 2.0.0 in commit\n> b3f0c5c0 (git-gui: tolerate major version changes when comparing the\n> git version, 2014-05-17), so that a major version change would still\n> satisfy the condition.\n>\n> However, what we really want is just a basic version compare, so use\n> vcompare instead to restore compatibility with Tcl 8.4.\n>\n> Signed-off-by: Kyle J. McKay \n> Signed-off-by: Junio C Hamano \n> ---\n> git-gui.sh | 4 ++--\n> 1 file changed, 2 insertions(+), 2 deletions(-)\n>\n> diff --git a/git-gui.sh b/git-gui.sh\n> index b186329d..a1a23b56 100755\n> --- a/git-gui/git-gui.sh\n> +++ b/git-gui/git-gui.sh\n> @@ -1283,7 +1283,7 @@ load_config 0\n> apply_config\n>\n> # v1.7.0 introduced --show-toplevel to return the canonical work-tree\n> -if {[package vsatisfies $_git_version 1.7.0-]} {\n> +if {[package vcompare $_git_version 1.7.0] >= 0} {\n> \tif { [is_Cygwin] } {\n> \t\tcatch {set _gitworktree [exec cygpath --windows [git rev-parse --show-toplevel]]}\n> \t} else {\n> @@ -1539,7 +1539,7 @@ proc rescan_stage2 {fd after} {\n> \t\tclose $fd\n> \t}\n>\n> -\tif {[package vsatisfies $::_git_version 1.6.3-]} {\n> +\tif {[package vcompare $::_git_version 1.6.3] >= 0} {\n> \t\tset ls_others [list --exclude-standard]\n> \t} else {\n> \t\tset ls_others [list --exclude-per-directory=.gitignore]\n>\n"},{"id":"254579","messageId":"87iogbmqks.fsf@red.patthoyts.tk","threadId":"38291","inReplyTo":"54ACE1C4.4030502@web.de","subject":"Re: [PATCH] git-gui.sh: support Tcl 8.4","fromName":"Pat Thoyts","fromEmail":"patthoyts@users.sourceforge.net","sentAt":"2015-01-13T01:12:19Z","receivedAt":"2015-01-13T01:12:19Z","isPatch":true,"sender":{"key":"patthoyts@users.sourceforge.net","avatar":"https://avatars.githubusercontent.com/u/30739?v=4"},"body":"Jens Lehmann writes:\n\n>Am 07.01.2015 um 01:02 schrieb Junio C Hamano:\n>> ^\"Kyle J. McKay\" writes:\n>>> I can't find anything in that thread about why vsatisfies was\n>>> preferred over vcompare other than the obvious that the vsatisfies\n>>> version is only a 1-character change. And that would be more than\n>>> enough except that Tcl 8.4 doesn't support the trailing '-' vsatisfies\n>>> syntax.\n>>\n>> Yeah, I fully agree with that observation.\n>\n>Having rather corroded TCL-knowledge myself it was Pat's comment in\n>\n> http://thread.gmane.org/gmane.comp.version-control.git/247511/focus=249464\n>\n>that made me change the patch to use the smaller change of adding\n>the trailing '-' after vsatisfies instead of using vcompare with\n>a trailing \">= 0\" in v2.\n>\n>>>> * Would it be a good idea to update the places $gmane/248895 points\n>>>> out? It is clearly outside the scope of this fix, but we may\n>>>> want to do so while our mind is on the \"how do we check required\n>>>> version?\" in a separate patch.\n>>>\n>>> Makes sense to me, but my Tcl knowledge isn't up to making those\n>>> changes as the code's a bit different. I have to paraphrase Chris's\n>>> message here by saying that I guess those checks are correct if not\n>>> consistent with the others.\n>\n>When I looked at it back then I was convinced these checks are ok\n>and should stay as they are to support ancient Git versions (and\n>they do not use vsatisfies either).\n>\n>> OK, let's ask Pat (cc'ed) to apply your version as-is without\n>> touching these 1.5.3 references. I do not take patches to git-gui\n>> directly to my tree.\n>\n>It's an ack from me on the change below as that was what I came up\n>with and tested successfully before Pat suggested to just add the '-'.\n>\n>> -- >8 --\n>> From: \"Kyle J. McKay\" \n>> Date: Tue, 6 Jan 2015 02:41:21 -0800\n>>\n>> Tcl 8.5 introduced an extended vsatisfies syntax that is not\n>> supported by Tcl 8.4.\n>>\n>> Since only Tcl 8.4 is required this presents a problem.\n>>\n>> The extended syntax was used starting with Git 2.0.0 in commit\n>> b3f0c5c0 (git-gui: tolerate major version changes when comparing the\n>> git version, 2014-05-17), so that a major version change would still\n>> satisfy the condition.\n>>\n>> However, what we really want is just a basic version compare, so use\n>> vcompare instead to restore compatibility with Tcl 8.4.\n>>\n>> Signed-off-by: Kyle J. McKay \n>> Signed-off-by: Junio C Hamano \n>> ---\n>> git-gui.sh | 4 ++--\n>> 1 file changed, 2 insertions(+), 2 deletions(-)\n>>\n>> diff --git a/git-gui.sh b/git-gui.sh\n>> index b186329d..a1a23b56 100755\n>> --- a/git-gui/git-gui.sh\n>> +++ b/git-gui/git-gui.sh\n>> @@ -1283,7 +1283,7 @@ load_config 0\n>> apply_config\n>>\n>> # v1.7.0 introduced --show-toplevel to return the canonical work-tree\n>> -if {[package vsatisfies $_git_version 1.7.0-]} {\n>> +if {[package vcompare $_git_version 1.7.0] >= 0} {\n>> \tif { [is_Cygwin] } {\n>> \t\tcatch {set _gitworktree [exec cygpath --windows [git rev-parse --show-toplevel]]}\n>> \t} else {\n>> @@ -1539,7 +1539,7 @@ proc rescan_stage2 {fd after} {\n>> \t\tclose $fd\n>> \t}\n>>\n>> -\tif {[package vsatisfies $::_git_version 1.6.3-]} {\n>> +\tif {[package vcompare $::_git_version 1.6.3] >= 0} {\n>> \t\tset ls_others [list --exclude-standard]\n>> \t} else {\n>> \t\tset ls_others [list --exclude-per-directory=.gitignore]\n>>\n>\n\nThis look good and tested ok with 8.4.19.\n\nvsatisfies is the smarter command but vcompare will work just fine as it\nis used here given the git version string gets pre-processed before any\ncomparisons are performed. The vcompare test will be ok with increasing\nmajor version numbers in the future.\n\nApplied.\n\n-- \nPat Thoyts http://www.patthoyts.tk/\nPGP fingerprint 2C 6E 98 07 2C 59 C8 97 10 CE 11 E6 04 E0 B9 DD\n"}]}