From: Junio C Hamano <gitster@pobox•com>
To: Jens Lehmann <Jens.Lehmann@web•de>
Cc: Chris Packham <judge.packham@gmail•com>,
patthoyts@users•sourceforge.net, GIT <git@vger•kernel.org>
Subject: Re: [GIT GUI PATCH] git-gui: use vcompare when comparing the git version
Date: Wed, 14 May 2014 12:22:28 -0700 [thread overview]
Message-ID: <xmqqlhu4jhcb.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <5373200D.7020108@web.de> (Jens Lehmann's message of "Wed, 14 May 2014 09:49:33 +0200")
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]
>>
next prev parent reply other threads:[~2014-05-14 19:27 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 [this message]
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
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=xmqqlhu4jhcb.fsf@gitster.dls.corp.google.com \
--to=gitster@pobox$(echo .)com \
--cc=Jens.Lehmann@web$(echo .)de \
--cc=git@vger$(echo .)kernel.org \
--cc=judge.packham@gmail$(echo .)com \
--cc=patthoyts@users$(echo .)sourceforge.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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