Re: [GIT GUI PATCH v2] git-gui: tolerate major version changes when comparing the git version

2014-05-19 Thread Junio C Hamano
Jens Lehmann jens.lehm...@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.
--
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


[GIT GUI PATCH v2] git-gui: tolerate major version changes when comparing the git version

2014-05-17 Thread 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 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.pack...@gmail.com
Reported-by: Yann Dirson ydir...@free.fr
Helped-by: Pat Thoyts pattho...@users.sourceforge.net
Signed-off-by: Jens Lehmann jens.lehm...@web.de
---

Am 17.05.2014 14:23, schrieb Pat Thoyts:
 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

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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [GIT GUI PATCH v2] git-gui: tolerate major version changes when comparing the git version

2014-05-17 Thread Chris Packham
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.pack...@gmail.com
 Reported-by: Yann Dirson ydir...@free.fr
 Helped-by: Pat Thoyts pattho...@users.sourceforge.net
 Signed-off-by: Jens Lehmann jens.lehm...@web.de
 ---
 
 Am 17.05.2014 14:23, schrieb Pat Thoyts:
 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
 
 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.
--
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 v2] git-gui: tolerate major version changes when comparing the git version

2014-05-17 Thread Eric Sunshine
On Sat, May 17, 2014 at 3:49 PM, Jens Lehmann jens.lehm...@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.pack...@gmail.com
 Reported-by: Yann Dirson ydir...@free.fr
 Helped-by: Pat Thoyts pattho...@users.sourceforge.net
 Signed-off-by: Jens Lehmann jens.lehm...@web.de
 ---

 Am 17.05.2014 14:23, schrieb Pat Thoyts:
 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

 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 majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
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