Re: [PATCH] git-gui.sh: support Tcl 8.4

From: Kyle J. McKay <mackyle@gmail.com>
Date: Jan 6, 2015, 22:47 UTC
Message-ID: <82A625FF-768E-4D7E-8248-B14005464EAE@gmail.com>
In-Reply-To: <xmqqvbkjofvw.fsf@gitster.dls.corp.google.com>

On Jan 6, 2015, at 11:42, Junio C Hamano wrote:

Show 20 quoted lines

> "Kyle J. McKay" <mackyle@gmail.com> writes:
>
>> Tcl 8.5 introduced an extended vsatisfies syntax that is not
>> supported by Tcl 8.4.
>
> Interesting.  We discussed this exact thing just before 2.0 in
>
>    http://thread.gmane.org/gmane.comp.version-control.git/247511/focus=248858
>
> and nobody seems to have noticed that giving the new range notation
> to vsatisfies is too new back then.
>
>> Since only Tcl 8.4 is required this presents a problem.
>
> Indeed.
>
>> However, what we really want is just a basic version compare,
>> so use vcompare instead to restore compatibility with Tcl 8.4.
>
> My Tcl is not just rusty but corroded, so help me out here.

My Tcl is barely operational, but I'll give it a shot.  :)

> * Your version that compares the sign of the result looks more
>   correct than $gmane/248858; was the patch proposed back then but
>   did not get applied wrong?  This question is out of mere
>   curiosity.

Thanks for the reference.  That patch proposed this type of change:

- if {[package vsatisfies $::_git_version 1.6.3]} { + if {[package vcompare $::_git_version 1.6.3]} {

But that's wrong because vsatisfies returns a boolean but vcompare returns an integer (think strcmp result) so the proposed change is testing whether the version is not 1.6.3 rather than being 1.6.3 or greater. But Jens mentions this in $gmane/249491 (that the original patch was missing the ">= 0" part).

I can't find anything in that thread about why vsatisfies was preferred over vcompare other than the obvious that the vsatisfies version is only a 1-character change. And that would be more than enough except that Tcl 8.4 doesn't support the trailing '-' vsatisfies syntax. There are complaints about this problem with git-gui [1] by folks who have Tcl 8.4 on their system and have upgraded to Git 2.0 or later.

> * Would it be a good idea to update the places $gmane/248895 points
>   out?  It is clearly outside the scope of this fix, but we may
>   want to do so while our mind is on the "how do we check required
>   version?" in a separate patch.

Makes sense to me, but my Tcl knowledge isn't up to making those changes as the code's a bit different. I have to paraphrase Chris's message here by saying that I guess those checks are correct if not consistent with the others.

[1] http://stackoverflow.com/questions/24315854/git-gui-cannot-start-because-of-bad-version-number

-Kyle