git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jens Lehmann <Jens.Lehmann@web.de>
To: patthoyts@users.sourceforge.net, Junio C Hamano <gitster@pobox.com>
Cc: Chris Packham <judge.packham@gmail.com>, GIT <git@vger.kernel.org>
Subject: [GIT GUI PATCH v2] git-gui: tolerate major version changes when comparing the git version
Date: Sat, 17 May 2014 21:49:05 +0200	[thread overview]
Message-ID: <5377BD31.8040004@web.de> (raw)
In-Reply-To: <87k39kbnmg.fsf@fox.patthoyts.tk>

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

  reply	other threads:[~2014-05-17 19:49 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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                   ` Jens Lehmann [this message]
2014-05-18  0:31                     ` [GIT GUI PATCH v2] git-gui: tolerate major version changes " Chris Packham
2014-05-18  3:01                     ` Eric Sunshine
2014-05-19 17:16                     ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5377BD31.8040004@web.de \
    --to=jens.lehmann@web.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=judge.packham@gmail.com \
    --cc=patthoyts@users.sourceforge.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).