Re: [GIT GUI PATCH] git-gui: use vcompare when comparing the git version

> Junio, I believe this issue needs to be fixed before 2.0 final. Otherwise
> git gui will not work inside submodules anymore due to the major version
> number change from 1 to 2. I'd like to hear Pat's opinion on this; even
> though I think my patch is less risky (as it doesn't change behavior for
> pre-2 versions), he might like Chris' proposal better.

> Am 13.05.2014 23:24, schrieb Jens Lehmann:
>> Since git 2.0.0 starting git gui in a submodule using a gitfile fails with
>> the following error:
>> 
>>    No working directory ../../../<path>
>> 
>>    couldn't change working directory
>>    to "../../../<path>": no such file or
>>    directory
>> 
>> This is because "git rev-parse --show-toplevel" is only run when git gui
>> sees a git version of at least 1.7.0 (which is the version in which the
>> --show-toplevel option was introduced). But it uses vsatisfies to check
>> that, which is documented to return false when the major version changes,
>> which is not what we want here.
>> 
>> Change vsatisfies to vcompare when testing for a git version to avoid the
>> problem when the major version changes (but still use vsatisfies in those
>> places where the Tk version is checked). This is done for both the place
>> that caused the reported bug and another spot where the git version is
>> tested for another feature.
>> 
>> Reported-by: Chris Packham <judge.packham@gmail.com>
>> Reported-by: Yann Dirson <ydirson@free.fr>
>> Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
>> ---
>> 
>> Am 07.05.2014 09:49, schrieb Chris Packham:
>>> On 07/05/14 19:28, Chris Packham wrote:
>>>> On 07/05/14 00:10, Pat Thoyts wrote:
>>>>> Chris Packham <judge.packham@gmail.com> writes:
>>>>>
>>>>>> On Tue, Apr 29, 2014 at 2:56 PM, Chris Packham <judge.packham@gmail.com> wrote:
>>>>>>> Hi Pat,
>>>>>>>
>>>>>>> I'm running git 2.0.0-rc0 (haven't got round to pulling down rc1 yet)
>>>>>>> which includes gitgui-0.19.0 and I'm getting a new error when I run
>>>>>>> 'git gui' in a repository with a .git file (created by git submodule).
>>>>>>>
>>>>>>> I can send you a screencap of the error message off list if you want
>>>>>>> but the text is
>>>>>>>
>>>>>>> "No working directory ../../../<repo>
>>>>>>>
>>>>>>> couldn't change working directory to ../../../<repo>: no such file or directory"
>>>>>>
>>>>>> My tcl is a little rusty but I think the problem might be this snippet.
>>>>>>
>>>>>> # v1.7.0 introduced --show-toplevel to return the canonical work-tree
>>>>>> if {[package vsatisfies $_git_version 1.7.0]} {
>>>>>>    if { [is_Cygwin] } {
>>>>>>        catch {set _gitworktree [exec cygpath --windows [git rev-parse
>>>>>> --show-toplevel]]}
>>>>>>    } else {
>>>>>>        set _gitworktree [git rev-parse --show-toplevel]
>>>>>>    }
>>>>>> } else {
>>>>>>    # try to set work tree from environment, core.worktree or use
>>>>>>    # cdup to obtain a relative path to the top of the worktree. If
>>>>>>    # run from the top, the ./ prefix ensures normalize expands pwd.
>>>>>>    if {[catch { set _gitworktree $env(GIT_WORK_TREE) }]} {
>>>>>>        set _gitworktree [get_config core.worktree]
>>>>>>        if {$_gitworktree eq ""} {
>>>>>>            set _gitworktree [file normalize ./[git rev-parse --show-cdup]]
>>>>>>        }
>>>>>>    }
>>>>>> }
>>>>>>
>>>>>> The  vsatisfies call probably doesn't handle '2.0.0.rc0' and the
>>>>>> fallback behaviour probably needs to normalise core.worktree
>>>>>>
>>>>>
>>>>> The _git_version variable has already been trimmed to remove such
>>>>> suffixes so the version comparison here will be ok. 
>>>>
>>>> I don't think that's true 'git rev-parse --show-toplevel' does the right
>>>> thing - if it's run.
>>>
>>> We'll the trimming works but vstatisfies doesn't
>>>
>>>   puts $_git_version
>>>   puts [package vsatisfies $_git_version 1.7.0]
>>>
>>>   2.0.0
>>>   0
>> 
>> Yup, looks like vsatisfies is doing just what it is supposed to (according
>> to http://www.astro.princeton.edu/~rhl/Tcl-Tk_docs/tcl/package.n.html):
>> 
>>    package vsatisfies version1 version2
>>        Returns 1 if scripts written for version2 will work unchanged
>>        with version1 (i.e. version1 is equal to or greater than version2
>>        and they both have the same major version number), 0 otherwise.
>> 
>> The bump in the major number from 1 to 2 makes vsatisfies assume that the
>> version is not compatible anymore, I believe we should have used vcompare
>> here and in another place.
>> 
>> 
>>  git-gui.sh | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>> 
>> diff --git a/git-gui.sh b/git-gui.sh
>> index cf2209b..ed2418b 100755
>> --- a/git-gui.sh
>> +++ b/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]} {
>>  	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]} {
>>  		set ls_others [list --exclude-standard]
>>  	} else {
>>  		set ls_others [list --exclude-per-directory=.gitignore]
>>