Re: [GIT GUI PATCH] git-gui: use vcompare when comparing the git version
Junio C Hamano gits...@pobox.com writes: Junio C Hamano gits...@pobox.com writes: Jens Lehmann jens.lehm...@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 Thoytshttp://www.patthoyts.tk/ PGP fingerprint 2C 6E 98 07 2C 59 C8 97 10 CE 11 E6 04 E0 B9 DD -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [GIT GUI PATCH] git-gui: use vcompare when comparing the git version
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.pack...@gmail.com
Reported-by: Yann Dirson ydir...@free.fr
Signed-off-by: Jens Lehmann jens.lehm...@web.de
---
Looks good to me (refer to previous comment about being rusty with tcl).
More importantly
Tested-by: Chris Packham judge.pack...@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.pack...@gmail.com writes:
On Tue, Apr 29, 2014 at 2:56 PM, Chris Packham judge.pack...@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
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.pack...@gmail.com
Reported-by: Yann Dirson ydir...@free.fr
Signed-off-by: Jens Lehmann jens.lehm...@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.pack...@gmail.com writes:
On Tue, Apr 29, 2014 at 2:56 PM, Chris Packham judge.pack...@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]
--
To unsubscribe from this list: send the line unsubscribe git in
the
Re: [GIT GUI PATCH] git-gui: use vcompare when comparing the git version
Jens Lehmann jens.lehm...@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.pack...@gmail.com
Reported-by: Yann Dirson ydir...@free.fr
Signed-off-by: Jens Lehmann jens.lehm...@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.pack...@gmail.com writes:
On Tue, Apr 29, 2014 at 2:56 PM, Chris Packham judge.pack...@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]
Re: [GIT GUI PATCH] git-gui: use vcompare when comparing the git version
Junio C Hamano gits...@pobox.com writes: Jens Lehmann jens.lehm...@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. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
