All of lore.kernel.org
 help / color / mirror / Atom feed
From: Pat Thoyts <patthoyts@users.sourceforge.net>
To: Jens Lehmann <Jens.Lehmann@web.de>
Cc: Junio C Hamano <gitster@pobox.com>,
	"Kyle J. McKay" <mackyle@gmail.com>,
	Git mailing list <git@vger.kernel.org>
Subject: Re: [PATCH] git-gui.sh: support Tcl 8.4
Date: Tue, 13 Jan 2015 01:12:19 +0000	[thread overview]
Message-ID: <87iogbmqks.fsf@red.patthoyts.tk> (raw)
In-Reply-To: <54ACE1C4.4030502@web.de> (Jens Lehmann's message of "Wed, 07 Jan 2015 08:35:32 +0100")

Jens Lehmann <Jens.Lehmann@web.de> writes:

>Am 07.01.2015 um 01:02 schrieb Junio C Hamano:
>> ^"Kyle J. McKay" <mackyle@gmail.com> writes:
>>> 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.
>>
>> Yeah, I fully agree with that observation.
>
>Having rather corroded TCL-knowledge myself it was Pat's comment in
>
>   http://thread.gmane.org/gmane.comp.version-control.git/247511/focus=249464
>
>that made me change the patch to use the smaller change of adding
>the trailing '-' after vsatisfies instead of using vcompare with
>a trailing ">= 0" in v2.
>
>>>> * 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.
>
>When I looked at it back then I was convinced these checks are ok
>and should stay as they are to support ancient Git versions (and
>they do not use vsatisfies either).
>
>> OK, let's ask Pat (cc'ed) to apply your version as-is without
>> touching these 1.5.3 references.  I do not take patches to git-gui
>> directly to my tree.
>
>It's an ack from me on the change below as that was what I came up
>with and tested successfully before Pat suggested to just add the '-'.
>
>> -- >8 --
>> From: "Kyle J. McKay" <mackyle@gmail.com>
>> Date: Tue,  6 Jan 2015 02:41:21 -0800
>>
>> Tcl 8.5 introduced an extended vsatisfies syntax that is not
>> supported by Tcl 8.4.
>>
>> Since only Tcl 8.4 is required this presents a problem.
>>
>> The extended syntax was used starting with Git 2.0.0 in commit
>> b3f0c5c0 (git-gui: tolerate major version changes when comparing the
>> git version, 2014-05-17), so that a major version change would still
>> satisfy the condition.
>>
>> However, what we really want is just a basic version compare, so use
>> vcompare instead to restore compatibility with Tcl 8.4.
>>
>> Signed-off-by: Kyle J. McKay <mackyle@gmail.com>
>> Signed-off-by: Junio C Hamano <gitster@pobox.com>
>> ---
>>   git-gui.sh | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/git-gui.sh b/git-gui.sh
>> index b186329d..a1a23b56 100755
>> --- a/git-gui/git-gui.sh
>> +++ b/git-gui/git-gui.sh
>> @@ -1283,7 +1283,7 @@ load_config 0
>>   apply_config
>>
>>   # v1.7.0 introduced --show-toplevel to return the canonical work-tree
>> -if {[package vsatisfies $_git_version 1.7.0-]} {
>> +if {[package vcompare $_git_version 1.7.0] >= 0} {
>>   	if { [is_Cygwin] } {
>>   		catch {set _gitworktree [exec cygpath --windows [git rev-parse --show-toplevel]]}
>>   	} else {
>> @@ -1539,7 +1539,7 @@ proc rescan_stage2 {fd after} {
>>   		close $fd
>>   	}
>>
>> -	if {[package vsatisfies $::_git_version 1.6.3-]} {
>> +	if {[package vcompare $::_git_version 1.6.3] >= 0} {
>>   		set ls_others [list --exclude-standard]
>>   	} else {
>>   		set ls_others [list --exclude-per-directory=.gitignore]
>>
>

This look good and tested ok with 8.4.19.

vsatisfies is the smarter command but vcompare will work just fine as it
is used here given the git version string gets pre-processed before any
comparisons are performed. The vcompare test will be ok with increasing
major version numbers in the future.

Applied.

-- 
Pat Thoyts                            http://www.patthoyts.tk/
PGP fingerprint 2C 6E 98 07 2C 59 C8 97  10 CE 11 E6 04 E0 B9 DD

      reply	other threads:[~2015-01-13  1:12 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-01-06 10:41 [PATCH] git-gui.sh: support Tcl 8.4 Kyle J. McKay
2015-01-06 19:42 ` Junio C Hamano
2015-01-06 22:47   ` Kyle J. McKay
2015-01-07  0:02     ` Junio C Hamano
2015-01-07  7:35       ` Jens Lehmann
2015-01-13  1:12         ` Pat Thoyts [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87iogbmqks.fsf@red.patthoyts.tk \
    --to=patthoyts@users.sourceforge.net \
    --cc=Jens.Lehmann@web.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=mackyle@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.