git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* git gui error with relocated repository
@ 2014-04-29  2:56 Chris Packham
  2014-04-29  4:23 ` Chris Packham
  0 siblings, 1 reply; 16+ messages in thread
From: Chris Packham @ 2014-04-29  2:56 UTC (permalink / raw)
  To: GIT, patthoyts

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"

Here's some other info that might help

  $ git --version
  git version 2.0.0.rc0

  $ cat .git
  gitdir: ../.git/modules/<repo>

  $ git rev-parse --git-dir
  /home/chris/src/<super>/.git/modules/<repo>

  $ git config core.worktree
  ../../../<repo>

Thanks,
Chris

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: git gui error with relocated repository
  2014-04-29  2:56 git gui error with relocated repository Chris Packham
@ 2014-04-29  4:23 ` Chris Packham
  2014-04-29 10:58   ` [GIT GUI PATCH] git-gui: unconditionally use rev-parse --show-toplevel Chris Packham
  2014-05-06 12:10   ` git gui error with relocated repository Pat Thoyts
  0 siblings, 2 replies; 16+ messages in thread
From: Chris Packham @ 2014-04-29  4:23 UTC (permalink / raw)
  To: GIT, patthoyts

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

>
> Here's some other info that might help
>
>   $ git --version
>   git version 2.0.0.rc0
>
>   $ cat .git
>   gitdir: ../.git/modules/<repo>
>
>   $ git rev-parse --git-dir
>   /home/chris/src/<super>/.git/modules/<repo>
>
>   $ git config core.worktree
>   ../../../<repo>
>
> Thanks,
> Chris

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [GIT GUI PATCH] git-gui: unconditionally use rev-parse --show-toplevel
  2014-04-29  4:23 ` Chris Packham
@ 2014-04-29 10:58   ` Chris Packham
  2014-05-06 12:10   ` git gui error with relocated repository Pat Thoyts
  1 sibling, 0 replies; 16+ messages in thread
From: Chris Packham @ 2014-04-29 10:58 UTC (permalink / raw)
  To: patthoyts, git; +Cc: Chris Packham

Previously 'git rev-parse --show-toplevel' was used to determine the
canonical work-tree only when the installed git version was detected to
be 1.7.0 or better. The fall-back logic used the core.worktree config
variable which in the case of a submodule is a relative path from the
submodule's $GIT_DIR. Unfortunately vsatisfies doesn't handle versions
like v2.0.0.rc0 so the fall-back logic is triggered.

Given the fact that git 1.7.0 was released over 4 years ago rather than
fixing the fall-back logic it seems reasonable to drop the version
check.

Signed-off-by: Chris Packham <judge.packham@gmail.com>
---
So I'm not sure if vsatisfies is failing because the version has .rc0 or
because it thinks v2.0.0 < 1.7.0. Regardless I think it's reasonably safe to
assume that people who are using the newer versions of git-gui are running new
enough versions of git.

There is also a similar section in rescan_stage2 that is checking for the
version being 1.6.3 or newer. Again I think it's probably safe to assume that
no-one is running a version of git that old (or at least no-one that wants to
run this version of git-gui). I'm not quite sure how to excercise that bit of
code so I haven't attempted to fix that.

 git-gui.sh | 20 ++++----------------
 1 file changed, 4 insertions(+), 16 deletions(-)

diff --git a/git-gui.sh b/git-gui.sh
index cf2209b..9ded5b9 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -1282,23 +1282,11 @@ if {![file isdirectory $_gitdir]} {
 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 { [is_Cygwin] } {
-		catch {set _gitworktree [exec cygpath --windows [git rev-parse --show-toplevel]]}
-	} else {
-		set _gitworktree [git rev-parse --show-toplevel]
-	}
+# Determine the canonical work-tree
+if { [is_Cygwin] } {
+	catch {set _gitworktree [exec cygpath --windows [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]]
-		}
-	}
+	set _gitworktree [git rev-parse --show-toplevel]
 }
 
 if {$_prefix ne {}} {
-- 
1.8.2.rc1.4.g27db5a0

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: git gui error with relocated repository
  2014-04-29  4:23 ` Chris Packham
  2014-04-29 10:58   ` [GIT GUI PATCH] git-gui: unconditionally use rev-parse --show-toplevel Chris Packham
@ 2014-05-06 12:10   ` Pat Thoyts
  2014-05-07  7:28     ` Chris Packham
  1 sibling, 1 reply; 16+ messages in thread
From: Pat Thoyts @ 2014-05-06 12:10 UTC (permalink / raw)
  To: Chris Packham; +Cc: GIT

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. It looks more likely
to be something to do with the .git being a file with a link being
mishandled. How did you setup this test repository with its link to a
parent?

>>
>> Here's some other info that might help
>>
>>   $ git --version
>>   git version 2.0.0.rc0
>>
>>   $ cat .git
>>   gitdir: ../.git/modules/<repo>
>>
>>   $ git rev-parse --git-dir
>>   /home/chris/src/<super>/.git/modules/<repo>
>>
>>   $ git config core.worktree
>>   ../../../<repo>
>>
>> Thanks,
>> Chris
>

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: git gui error with relocated repository
  2014-05-06 12:10   ` git gui error with relocated repository Pat Thoyts
@ 2014-05-07  7:28     ` Chris Packham
  2014-05-07  7:49       ` Chris Packham
  0 siblings, 1 reply; 16+ messages in thread
From: Chris Packham @ 2014-05-07  7:28 UTC (permalink / raw)
  To: patthoyts; +Cc: GIT

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.

> It looks more likely
> to be something to do with the .git being a file with a link being
> mishandled. How did you setup this test repository with its link to a
> parent?
>

It's just a git clone of the parent and a git submodule init. A subtle
thing to notice is that config.worktree is relative to $GIT_DIR. All the
configuration was done by git without any intervention from me.

>>>
>>> Here's some other info that might help
>>>
>>>   $ git --version
>>>   git version 2.0.0.rc0
>>>
>>>   $ cat .git
>>>   gitdir: ../.git/modules/<repo>
>>>
>>>   $ git rev-parse --git-dir
>>>   /home/chris/src/<super>/.git/modules/<repo>
>>>
>>>   $ git config core.worktree
>>>   ../../../<repo>
>>>
>>> Thanks,
>>> Chris
>>
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: git gui error with relocated repository
  2014-05-07  7:28     ` Chris Packham
@ 2014-05-07  7:49       ` Chris Packham
  2014-05-13 21:24         ` [GIT GUI PATCH] git-gui: use vcompare when comparing the git version Jens Lehmann
  0 siblings, 1 reply; 16+ messages in thread
From: Chris Packham @ 2014-05-07  7:49 UTC (permalink / raw)
  To: patthoyts; +Cc: GIT

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

> 
>> It looks more likely
>> to be something to do with the .git being a file with a link being
>> mishandled. How did you setup this test repository with its link to a
>> parent?
>>
> 
> It's just a git clone of the parent and a git submodule init. A subtle
> thing to notice is that config.worktree is relative to $GIT_DIR. All the
> configuration was done by git without any intervention from me.
> 
>>>>
>>>> Here's some other info that might help
>>>>
>>>>   $ git --version
>>>>   git version 2.0.0.rc0
>>>>
>>>>   $ cat .git
>>>>   gitdir: ../.git/modules/<repo>
>>>>
>>>>   $ git rev-parse --git-dir
>>>>   /home/chris/src/<super>/.git/modules/<repo>
>>>>
>>>>   $ git config core.worktree
>>>>   ../../../<repo>
>>>>
>>>> Thanks,
>>>> Chris
>>>
>>
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [GIT GUI PATCH] git-gui: use vcompare when comparing the git version
  2014-05-07  7:49       ` Chris Packham
@ 2014-05-13 21:24         ` Jens Lehmann
  2014-05-14  7:46           ` Chris Packham
  2014-05-14  7:49           ` Jens Lehmann
  0 siblings, 2 replies; 16+ messages in thread
From: Jens Lehmann @ 2014-05-13 21:24 UTC (permalink / raw)
  To: Chris Packham, patthoyts; +Cc: GIT

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]
-- 
2.0.0.rc3.2.g998f840

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [GIT GUI PATCH] git-gui: use vcompare when comparing the git version
  2014-05-13 21:24         ` [GIT GUI PATCH] git-gui: use vcompare when comparing the git version Jens Lehmann
@ 2014-05-14  7:46           ` Chris Packham
  2014-05-14  7:49           ` Jens Lehmann
  1 sibling, 0 replies; 16+ messages in thread
From: Chris Packham @ 2014-05-14  7:46 UTC (permalink / raw)
  To: Jens Lehmann, patthoyts; +Cc: GIT

On 14/05/14 09:24, Jens Lehmann wrote:
> 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>
> ---

Looks good to me (refer to previous comment about being rusty with tcl).
More importantly

 Tested-by: Chris Packham <judge.packham@gmail.com>

There is the alternative I posted which just does away with the version
checks (actually that only addressed one of the vcompare call sites).

There are also these checks that I guess are correct (if not
inconsistent) but again who is still using git 1.5.3?

git-gui.sh:1098:        >= 1.5.3 {
lib/blame.tcl:800:                      if {[git-version >= 1.5.3]} {
lib/blame.tcl:849:      if {[git-version >= 1.5.3]} {
lib/checkout_op.tcl:513:        >= 1.5.3 {

> 
> 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]
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [GIT GUI PATCH] git-gui: use vcompare when comparing the git version
  2014-05-13 21:24         ` [GIT GUI PATCH] git-gui: use vcompare when comparing the git version Jens Lehmann
  2014-05-14  7:46           ` Chris Packham
@ 2014-05-14  7:49           ` Jens Lehmann
  2014-05-14 19:22             ` Junio C Hamano
  1 sibling, 1 reply; 16+ messages in thread
From: Jens Lehmann @ 2014-05-14  7:49 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Chris Packham, patthoyts, GIT

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]
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [GIT GUI PATCH] git-gui: use vcompare when comparing the git version
  2014-05-14  7:49           ` Jens Lehmann
@ 2014-05-14 19:22             ` Junio C Hamano
  2014-05-14 21:49               ` Junio C Hamano
  0 siblings, 1 reply; 16+ messages in thread
From: Junio C Hamano @ 2014-05-14 19:22 UTC (permalink / raw)
  To: Jens Lehmann; +Cc: Chris Packham, patthoyts, GIT

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

> 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.

Thanks; I share the same feeling.


> 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]
>> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [GIT GUI PATCH] git-gui: use vcompare when comparing the git version
  2014-05-14 19:22             ` Junio C Hamano
@ 2014-05-14 21:49               ` Junio C Hamano
  2014-05-17 12:23                 ` Pat Thoyts
  0 siblings, 1 reply; 16+ messages in thread
From: Junio C Hamano @ 2014-05-14 21:49 UTC (permalink / raw)
  To: Jens Lehmann; +Cc: Chris Packham, patthoyts, GIT

Junio C Hamano <gitster@pobox.com> writes:

> Jens Lehmann <Jens.Lehmann@web.de> writes:
>
>> 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.
>
> Thanks; I share the same feeling.

So after checking git://repo.or.cz/git-gui.git/ and seeing that I am
not missing any commit from there, I tentatively created a fork of
it, applied your patch and merged it somewhere on 'pu' that is close
to 'next'.  We may want to fast-track it to 2.0 without waiting for
an Ack from Pat but let's give him one more day to respond.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [GIT GUI PATCH] git-gui: use vcompare when comparing the git version
  2014-05-14 21:49               ` Junio C Hamano
@ 2014-05-17 12:23                 ` Pat Thoyts
  2014-05-17 19:49                   ` [GIT GUI PATCH v2] git-gui: tolerate major version changes " Jens Lehmann
  0 siblings, 1 reply; 16+ messages in thread
From: Pat Thoyts @ 2014-05-17 12:23 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jens Lehmann, Chris Packham, GIT

Junio C Hamano <gitster@pobox.com> writes:

>Junio C Hamano <gitster@pobox.com> writes:
>
>> Jens Lehmann <Jens.Lehmann@web.de> writes:
>>
>>> 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.
>>
>> Thanks; I share the same feeling.
>
>So after checking git://repo.or.cz/git-gui.git/ and seeing that I am
>not missing any commit from there, I tentatively created a fork of
>it, applied your patch and merged it somewhere on 'pu' that is close
>to 'next'.  We may want to fast-track it to 2.0 without waiting for
>an Ack from Pat but let's give him one more day to respond.
>

The analysis about the major version number being significant is
correct. By default vsatisfies assumes that a major version number
change means all lesser versions are incompatible. However, you can
prevent that assumption using an unlimited check by appending a - (minus
sign) to the version to yield an open ended range. Or by giving another
range. So the only change required is to append a minus.

  package vsatisfies $::_git_version 1.7.0-

will suffice.

  package vsatisfies $::_git_version 1.7.0 2.0.0

would work but would cause failures when we arrive at git 3.0

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [GIT GUI PATCH v2] git-gui: tolerate major version changes when comparing the git version
  2014-05-17 12:23                 ` Pat Thoyts
@ 2014-05-17 19:49                   ` Jens Lehmann
  2014-05-18  0:31                     ` Chris Packham
                                       ` (2 more replies)
  0 siblings, 3 replies; 16+ messages in thread
From: Jens Lehmann @ 2014-05-17 19:49 UTC (permalink / raw)
  To: patthoyts, Junio C Hamano; +Cc: Chris Packham, GIT

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 "package vsatisfies" returns
false when the major version changes, which is not what we want here.

Fix that for both places where the git version is checked using vsatifies
by appending a '-' to the version number. This tells vsatisfies that a
change of the major version is not considered to be a problem, as long as
the new major version is larger. 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>
Helped-by: Pat Thoyts <patthoyts@users.sourceforge.net>
Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
---

Am 17.05.2014 14:23, schrieb Pat Thoyts:
> Junio C Hamano <gitster@pobox.com> writes:
> 
>> Junio C Hamano <gitster@pobox.com> writes:
>>
>>> Jens Lehmann <Jens.Lehmann@web.de> writes:
>>>
>>>> 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.
>>>
>>> Thanks; I share the same feeling.
>>
>> So after checking git://repo.or.cz/git-gui.git/ and seeing that I am
>> not missing any commit from there, I tentatively created a fork of
>> it, applied your patch and merged it somewhere on 'pu' that is close
>> to 'next'.  We may want to fast-track it to 2.0 without waiting for
>> an Ack from Pat but let's give him one more day to respond.
>>
> 
> The analysis about the major version number being significant is
> correct. By default vsatisfies assumes that a major version number
> change means all lesser versions are incompatible. However, you can
> prevent that assumption using an unlimited check by appending a - (minus
> sign) to the version to yield an open ended range. Or by giving another
> range. So the only change required is to append a minus.
> 
>   package vsatisfies $::_git_version 1.7.0-
> 
> will suffice.
> 
>   package vsatisfies $::_git_version 1.7.0 2.0.0
> 
> would work but would cause failures when we arrive at git 3.0

Thanks for the review! In this version I added the '-' to the version
passed to vsatisfies and updated the commit message accordingly. I
tested the result and it fixes the regression.

Junio, please replace my old version with this. In the first version
I forgot to add a ">= 0" after the vcompare, which results in all
versions that are /different/ than the one checked against pass the
test. While that fixes the 2.0.0 regression, it will fail for git
versions older than the version that is tested for. So my first
attempt wasn't /that/ different from Chris' proposal ... :-/


 git-gui/git-gui.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh
index cf2209b..6a8907e 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 vsatisfies $_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 vsatisfies $::_git_version 1.6.3-]} {
 		set ls_others [list --exclude-standard]
 	} else {
 		set ls_others [list --exclude-per-directory=.gitignore]
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [GIT GUI PATCH v2] git-gui: tolerate major version changes when comparing the git version
  2014-05-17 19:49                   ` [GIT GUI PATCH v2] git-gui: tolerate major version changes " Jens Lehmann
@ 2014-05-18  0:31                     ` Chris Packham
  2014-05-18  3:01                     ` Eric Sunshine
  2014-05-19 17:16                     ` Junio C Hamano
  2 siblings, 0 replies; 16+ messages in thread
From: Chris Packham @ 2014-05-18  0:31 UTC (permalink / raw)
  To: Jens Lehmann, patthoyts, Junio C Hamano; +Cc: GIT

On 18/05/14 07:49, Jens Lehmann wrote:
> 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 "package vsatisfies" returns
> false when the major version changes, which is not what we want here.
> 
> Fix that for both places where the git version is checked using vsatifies
> by appending a '-' to the version number. This tells vsatisfies that a
> change of the major version is not considered to be a problem, as long as
> the new major version is larger. 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>
> Helped-by: Pat Thoyts <patthoyts@users.sourceforge.net>
> Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
> ---
> 
> Am 17.05.2014 14:23, schrieb Pat Thoyts:
>> Junio C Hamano <gitster@pobox.com> writes:
>>
>>> Junio C Hamano <gitster@pobox.com> writes:
>>>
>>>> Jens Lehmann <Jens.Lehmann@web.de> writes:
>>>>
>>>>> 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.
>>>>
>>>> Thanks; I share the same feeling.
>>>
>>> So after checking git://repo.or.cz/git-gui.git/ and seeing that I am
>>> not missing any commit from there, I tentatively created a fork of
>>> it, applied your patch and merged it somewhere on 'pu' that is close
>>> to 'next'.  We may want to fast-track it to 2.0 without waiting for
>>> an Ack from Pat but let's give him one more day to respond.
>>>
>>
>> The analysis about the major version number being significant is
>> correct. By default vsatisfies assumes that a major version number
>> change means all lesser versions are incompatible. However, you can
>> prevent that assumption using an unlimited check by appending a - (minus
>> sign) to the version to yield an open ended range. Or by giving another
>> range. So the only change required is to append a minus.
>>
>>   package vsatisfies $::_git_version 1.7.0-
>>
>> will suffice.
>>
>>   package vsatisfies $::_git_version 1.7.0 2.0.0
>>
>> would work but would cause failures when we arrive at git 3.0
> 
> Thanks for the review! In this version I added the '-' to the version
> passed to vsatisfies and updated the commit message accordingly. I
> tested the result and it fixes the regression.
> 
> Junio, please replace my old version with this. In the first version
> I forgot to add a ">= 0" after the vcompare, which results in all
> versions that are /different/ than the one checked against pass the
> test. While that fixes the 2.0.0 regression, it will fail for git
> versions older than the version that is tested for. So my first
> attempt wasn't /that/ different from Chris' proposal ... :-/
> 
> 
>  git-gui/git-gui.sh | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh
> index cf2209b..6a8907e 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 vsatisfies $_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 vsatisfies $::_git_version 1.6.3-]} {
>  		set ls_others [list --exclude-standard]
>  	} else {
>  		set ls_others [list --exclude-per-directory=.gitignore]
> 

Works for me.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [GIT GUI PATCH v2] git-gui: tolerate major version changes when comparing the git version
  2014-05-17 19:49                   ` [GIT GUI PATCH v2] git-gui: tolerate major version changes " Jens Lehmann
  2014-05-18  0:31                     ` Chris Packham
@ 2014-05-18  3:01                     ` Eric Sunshine
  2014-05-19 17:16                     ` Junio C Hamano
  2 siblings, 0 replies; 16+ messages in thread
From: Eric Sunshine @ 2014-05-18  3:01 UTC (permalink / raw)
  To: Jens Lehmann; +Cc: patthoyts, Junio C Hamano, Chris Packham, GIT

On Sat, May 17, 2014 at 3:49 PM, Jens Lehmann <Jens.Lehmann@web.de> wrote:
> 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 "package vsatisfies" returns
> false when the major version changes, which is not what we want here.
>
> Fix that for both places where the git version is checked using vsatifies

s/vsatifies/vsatisfies/

> by appending a '-' to the version number. This tells vsatisfies that a
> change of the major version is not considered to be a problem, as long as
> the new major version is larger. 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>
> Helped-by: Pat Thoyts <patthoyts@users.sourceforge.net>
> Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
> ---
>
> Am 17.05.2014 14:23, schrieb Pat Thoyts:
>> Junio C Hamano <gitster@pobox.com> writes:
>>
>>> Junio C Hamano <gitster@pobox.com> writes:
>>>
>>>> Jens Lehmann <Jens.Lehmann@web.de> writes:
>>>>
>>>>> 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.
>>>>
>>>> Thanks; I share the same feeling.
>>>
>>> So after checking git://repo.or.cz/git-gui.git/ and seeing that I am
>>> not missing any commit from there, I tentatively created a fork of
>>> it, applied your patch and merged it somewhere on 'pu' that is close
>>> to 'next'.  We may want to fast-track it to 2.0 without waiting for
>>> an Ack from Pat but let's give him one more day to respond.
>>>
>>
>> The analysis about the major version number being significant is
>> correct. By default vsatisfies assumes that a major version number
>> change means all lesser versions are incompatible. However, you can
>> prevent that assumption using an unlimited check by appending a - (minus
>> sign) to the version to yield an open ended range. Or by giving another
>> range. So the only change required is to append a minus.
>>
>>   package vsatisfies $::_git_version 1.7.0-
>>
>> will suffice.
>>
>>   package vsatisfies $::_git_version 1.7.0 2.0.0
>>
>> would work but would cause failures when we arrive at git 3.0
>
> Thanks for the review! In this version I added the '-' to the version
> passed to vsatisfies and updated the commit message accordingly. I
> tested the result and it fixes the regression.
>
> Junio, please replace my old version with this. In the first version
> I forgot to add a ">= 0" after the vcompare, which results in all
> versions that are /different/ than the one checked against pass the
> test. While that fixes the 2.0.0 regression, it will fail for git
> versions older than the version that is tested for. So my first
> attempt wasn't /that/ different from Chris' proposal ... :-/
>
>
>  git-gui/git-gui.sh | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh
> index cf2209b..6a8907e 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 vsatisfies $_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 vsatisfies $::_git_version 1.6.3-]} {
>                 set ls_others [list --exclude-standard]
>         } else {
>                 set ls_others [list --exclude-per-directory=.gitignore]
> --
> 1.8.3.1
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [GIT GUI PATCH v2] git-gui: tolerate major version changes when comparing the git version
  2014-05-17 19:49                   ` [GIT GUI PATCH v2] git-gui: tolerate major version changes " Jens Lehmann
  2014-05-18  0:31                     ` Chris Packham
  2014-05-18  3:01                     ` Eric Sunshine
@ 2014-05-19 17:16                     ` Junio C Hamano
  2 siblings, 0 replies; 16+ messages in thread
From: Junio C Hamano @ 2014-05-19 17:16 UTC (permalink / raw)
  To: Jens Lehmann; +Cc: patthoyts, Chris Packham, GIT

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

> Am 17.05.2014 14:23, schrieb Pat Thoyts:
>>
>> The analysis about the major version number being significant is
>> correct. ...
>> 
>>   package vsatisfies $::_git_version 1.7.0-
>> 
>> will suffice.
>
> Junio, please replace my old version with this.

Thanks, both.  Will replace.

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2014-05-19 17:16 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-04-29  2:56 git gui error with relocated repository Chris Packham
2014-04-29  4:23 ` Chris Packham
2014-04-29 10:58   ` [GIT GUI PATCH] git-gui: unconditionally use rev-parse --show-toplevel Chris Packham
2014-05-06 12:10   ` git gui error with relocated repository Pat Thoyts
2014-05-07  7:28     ` Chris Packham
2014-05-07  7:49       ` Chris Packham
2014-05-13 21:24         ` [GIT GUI PATCH] git-gui: use vcompare when comparing the git version Jens Lehmann
2014-05-14  7:46           ` Chris Packham
2014-05-14  7:49           ` Jens Lehmann
2014-05-14 19:22             ` Junio C Hamano
2014-05-14 21:49               ` Junio C Hamano
2014-05-17 12:23                 ` Pat Thoyts
2014-05-17 19:49                   ` [GIT GUI PATCH v2] git-gui: tolerate major version changes " Jens Lehmann
2014-05-18  0:31                     ` Chris Packham
2014-05-18  3:01                     ` Eric Sunshine
2014-05-19 17:16                     ` Junio C Hamano

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).