git gui error with relocated repository

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

Quoted 6 lines · Chris Packham

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

Quoted 6 lines

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

Quoted 17 lines

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

Quoted 36 lines · Chris Packham

Quoted 6 lines · Chris Packham

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

Quoted 6 lines

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

Quoted 3 lines

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?

Quoted 18 lines

Quoted 17 lines

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

Quoted 42 lines · Pat Thoyts

Quoted 39 lines · Chris Packham

Quoted 6 lines · Chris Packham

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

Quoted 6 lines

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

Quoted 2 lines

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.

Quoted 6 lines

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.

Quoted 19 lines

Quoted 18 lines

Quoted 17 lines

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

Quoted 43 lines · Pat Thoyts

Quoted 42 lines

Quoted 39 lines · Chris Packham

Quoted 6 lines · Chris Packham

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

Quoted 6 lines

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

Quoted 5 lines

Quoted 2 lines

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

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 <[email protected]>

Reported-by Yann Dirson <[email protected]>

Signed-off-by Jens Lehmann <[email protected]>

---

Am 07.05.2014 09:49, schrieb Chris Packham:

Quoted 50 lines · Chris Packham

Quoted 43 lines · Pat Thoyts

Quoted 42 lines

Quoted 39 lines · Chris Packham

Quoted 6 lines · Chris Packham

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

Quoted 6 lines

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

Quoted 5 lines

Quoted 2 lines

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.

Quoted 7 lines

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(-)

git-gui.sh +2 -2

index cf2209b..ed2418b 100755
@@ -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]

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 <[email protected]>

Reported-by Yann Dirson <[email protected]>

Signed-off-by Jens Lehmann <[email protected]>

---

Am 07.05.2014 09:49, schrieb Chris Packham:

Quoted 50 lines · Chris Packham

Quoted 43 lines · Pat Thoyts

Quoted 42 lines

Quoted 39 lines · Chris Packham

Quoted 6 lines · Chris Packham

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

Quoted 6 lines

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

Quoted 5 lines

Quoted 2 lines

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.

Quoted 7 lines

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(-)

git-gui.sh +2 -2

index cf2209b..ed2418b 100755
@@ -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]

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:

Quoted 124 lines

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 <[email protected]>

Reported-by Yann Dirson <[email protected]>

Signed-off-by Jens Lehmann <[email protected]>

---

Am 07.05.2014 09:49, schrieb Chris Packham:

Quoted 50 lines · Chris Packham

Quoted 43 lines · Pat Thoyts

Quoted 42 lines

Quoted 39 lines · Chris Packham

Quoted 6 lines · Chris Packham

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

Quoted 6 lines

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

Quoted 5 lines

Quoted 2 lines

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.

Quoted 7 lines

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(-)

git-gui.sh +2 -2

index cf2209b..ed2418b 100755
@@ -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]

Quoted 5 lines · Jens Lehmann

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.

Quoted 8 lines · Junio C Hamano

Quoted 5 lines · Jens Lehmann

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.

Quoted 1 line

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.

Quoted 10 lines · Junio C Hamano

Quoted 8 lines · Junio C Hamano

Quoted 5 lines · Jens Lehmann

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.

Quoted 8 lines

Quoted 1 line

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

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 <[email protected]>

Reported-by Yann Dirson <[email protected]>

Helped-by Pat Thoyts <[email protected]>

Signed-off-by Jens Lehmann <[email protected]>

---

Am 17.05.2014 14:23, schrieb Pat Thoyts:

Quoted 28 lines

Quoted 10 lines · Junio C Hamano

Quoted 8 lines · Junio C Hamano

Quoted 5 lines · Jens Lehmann

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.

Quoted 8 lines

Quoted 1 line

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.

Quoted 7 lines

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(-)

git-gui/git-gui.sh +1 -1

index cf2209b..6a8907e 100755
@@ -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 {

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 <[email protected]>

Reported-by Yann Dirson <[email protected]>

Helped-by Pat Thoyts <[email protected]>

Signed-off-by Jens Lehmann <[email protected]>

---

Am 17.05.2014 14:23, schrieb Pat Thoyts:

Quoted 28 lines

Quoted 10 lines · Junio C Hamano

Quoted 8 lines · Junio C Hamano

Quoted 5 lines · Jens Lehmann

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.

Quoted 8 lines

Quoted 1 line

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.

Quoted 7 lines

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(-)

git-gui/git-gui.sh +2 -3

index cf2209b..6a8907e 100755
@@ -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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Am 17.05.2014 14:23, schrieb Pat Thoyts:

Quoted 7 lines

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.