[BUG] git gui blame: Show History Context broken since 29e5573d

[BUG] git gui blame: Show History Context broken since 29e5573d

[Matthieu Moy]

Hi,

In "git gui blame", right-clicking on the left fringe and chosing
"Show History Context" in the context-menu doesn't work for me in the
latest git. It says:

couldn't change working directory to "": no such file or directory
couldn't change working directory to "": no such file or directory
    while executing
"cd $_gitworktree"
    (procedure "do_gitk" line 25)
    invoked from within
"do_gitk $cmdline"
    (procedure "blame::_gitkcommit" line 47)
    invoked from within
"blame::_gitkcommit ::blame::__o1::__d"
    invoked from within
".ctxm invoke active"
    ("uplevel" body line 1)
    invoked from within
"uplevel #0 [list $w invoke active]"
    (procedure "tk::MenuInvoke" line 50)
    invoked from within
"tk::MenuInvoke .ctxm 1"
    (command bound to event)

Git bisect shows me the guilty commit:

commit 29e5573d1ef67c92314c39e55d26504fee119c04
Author: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
Date:   Sat Jan 23 11:03:35 2010 +0100

    git-gui: handle bare repos correctly
    
    Refactor checking for a bare repository into its own proc, that relies
    on git rev-parse --is-bare-repository if possible. For older versions of
    git we fall back to a logic such that the repository is considered bare
    if:
     * either the core.bare setting is true
     * or the worktree is not set and the directory name ends with .git
    The error message for the case of an unhandled bare repository is also
    updated to reflect the fact that the problem is not the funny name but
    the bareness.
    
    The new refactored proc is also used to disable the menu entry to
    explore the working copy, and to skip changing to the worktree before
    the gitk invocation.
    
    Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
    Signed-off-by: Shawn O. Pearce <spearce@spearce.org>

This is in a non-bare repo.

Anyone to fix this?

Thanks,

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [BUG] git gui blame: Show History Context broken since 29e5573d

[Giuseppe Bilotta]

On Mon, Feb 22, 2010 at 9:27 AM, Matthieu Moy
<Matthieu.Moy@grenoble-inp.fr> wrote:
> Hi,
>
> In "git gui blame", right-clicking on the left fringe and chosing
> "Show History Context" in the context-menu doesn't work for me in the
> latest git. It says:
>
> couldn't change working directory to "": no such file or directory

Definitely my fault. _gitworktree was not being set up correctly when
support for bare repositories was enabled and the repo was not bare
(like in the blame case). Patch incoming, can you see if it does the
job for you? It seems to fix it here.

-- 
Giuseppe "Oblomov" Bilotta

Re: Re: [BUG] git gui blame: Show History Context broken since 29e5573d

[Heiko Voigt]

On Mon, Feb 22, 2010 at 04:18:11PM +0100, Giuseppe Bilotta wrote:
> On Mon, Feb 22, 2010 at 9:27 AM, Matthieu Moy
> <Matthieu.Moy@grenoble-inp.fr> wrote:
> > Hi,
> >
> > In "git gui blame", right-clicking on the left fringe and chosing
> > "Show History Context" in the context-menu doesn't work for me in the
> > latest git. It says:
> >
> > couldn't change working directory to "": no such file or directory
> 
> Definitely my fault. _gitworktree was not being set up correctly when
> support for bare repositories was enabled and the repo was not bare
> (like in the blame case). Patch incoming, can you see if it does the
> job for you? It seems to fix it here.

Isn't this the same bug as this one fixes:

http://article.gmane.org/gmane.comp.version-control.git/140288

cheers Heiko

Re: Re: [BUG] git gui blame: Show History Context broken since 29e5573d

[Giuseppe Bilotta]

On Mon, Feb 22, 2010 at 11:38 PM, Heiko Voigt <hvoigt@hvoigt.net> wrote:
> On Mon, Feb 22, 2010 at 04:18:11PM +0100, Giuseppe Bilotta wrote:
>> On Mon, Feb 22, 2010 at 9:27 AM, Matthieu Moy
>> <Matthieu.Moy@grenoble-inp.fr> wrote:
>> > Hi,
>> >
>> > In "git gui blame", right-clicking on the left fringe and chosing
>> > "Show History Context" in the context-menu doesn't work for me in the
>> > latest git. It says:
>> >
>> > couldn't change working directory to "": no such file or directory
>>
>> Definitely my fault. _gitworktree was not being set up correctly when
>> support for bare repositories was enabled and the repo was not bare
>> (like in the blame case). Patch incoming, can you see if it does the
>> job for you? It seems to fix it here.
>
> Isn't this the same bug as this one fixes:
>
> http://article.gmane.org/gmane.comp.version-control.git/140288
>
> cheers Heiko

Interesting, I missed that patch. However, I strongly suspect that
patch is not correct, since in that case the setup of gitworktree is
done before checking for bareness, meaning that when working in
somerepo.git (bare repo) for which the config flag is not set, it
might misdetect the situation as being in the non-bare case. I believe
my fix to be more correct in this regard.

-- 
Giuseppe "Oblomov" Bilotta

Re: [BUG] git gui blame: Show History Context broken since 29e5573d

[Matthieu Moy]

[ Adding Pat Thoyts, author of the first patch, in Cc ]

Giuseppe Bilotta <giuseppe.bilotta@gmail.com> writes:

> On Mon, Feb 22, 2010 at 11:38 PM, Heiko Voigt <hvoigt@hvoigt.net> wrote:
>> On Mon, Feb 22, 2010 at 04:18:11PM +0100, Giuseppe Bilotta wrote:
>>> On Mon, Feb 22, 2010 at 9:27 AM, Matthieu Moy
>>> <Matthieu.Moy@grenoble-inp.fr> wrote:
>>> > Hi,
>>> >
>>> > In "git gui blame", right-clicking on the left fringe and chosing
>>> > "Show History Context" in the context-menu doesn't work for me in the
>>> > latest git. It says:
>>> >
>>> > couldn't change working directory to "": no such file or directory
>>>
>>> Definitely my fault. _gitworktree was not being set up correctly when
>>> support for bare repositories was enabled and the repo was not bare
>>> (like in the blame case). Patch incoming, can you see if it does the
>>> job for you? It seems to fix it here.

[ patch available here :
  http://thread.gmane.org/gmane.comp.version-control.git/140688 ]

>> Isn't this the same bug as this one fixes:
>>
>> http://article.gmane.org/gmane.comp.version-control.git/140288

It seems it is, yes.

> I believe my fix to be more correct in this regard.

Not familiar enought with the code to say which is the right one.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: Re: Re: [BUG] git gui blame: Show History Context broken since 29e5573d

[Heiko Voigt]

On Tue, Feb 23, 2010 at 12:29:03AM +0100, Giuseppe Bilotta wrote:
> On Mon, Feb 22, 2010 at 11:38 PM, Heiko Voigt <hvoigt@hvoigt.net> wrote:
> > On Mon, Feb 22, 2010 at 04:18:11PM +0100, Giuseppe Bilotta wrote:
> >> On Mon, Feb 22, 2010 at 9:27 AM, Matthieu Moy
> >> <Matthieu.Moy@grenoble-inp.fr> wrote:
> >> > Hi,
> >> >
> >> > In "git gui blame", right-clicking on the left fringe and chosing
> >> > "Show History Context" in the context-menu doesn't work for me in the
> >> > latest git. It says:
> >> >
> >> > couldn't change working directory to "": no such file or directory
> >>
> >> Definitely my fault. _gitworktree was not being set up correctly when
> >> support for bare repositories was enabled and the repo was not bare
> >> (like in the blame case). Patch incoming, can you see if it does the
> >> job for you? It seems to fix it here.
> >
> > Isn't this the same bug as this one fixes:
> >
> > http://article.gmane.org/gmane.comp.version-control.git/140288
> >
> > cheers Heiko
> 
> Interesting, I missed that patch. However, I strongly suspect that
> patch is not correct, since in that case the setup of gitworktree is
> done before checking for bareness, meaning that when working in
> somerepo.git (bare repo) for which the config flag is not set, it
> might misdetect the situation as being in the non-bare case. I believe
> my fix to be more correct in this regard.

Good to know. I already picked that one into my private repo and did not
experience any problems with it, but that might be due to the fact that
I have never tried to open git gui on bare repos.

cheers Heiko

[PATCH] git-gui: fix worktree initialization with empty prefix

[Giuseppe Bilotta]

This is sort of a chicken-and-egg problem, because we should only
bother setting the worktree if we are not in a bare repository, but
an unset worktree is one of the conditions we check to see if we have
a bare repository.

Maintain the same sequence of checks, but swap the check for bareness
support and bareness of repository, and check again for an unset
worktree if we are not in the bare case.
---
 git-gui/git-gui.sh |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh
index 7d54511..d9e1598 100755
--- a/git-gui/git-gui.sh
+++ b/git-gui/git-gui.sh
@@ -1169,12 +1169,13 @@ if {$_prefix ne {}} {
 	}
 	set _gitworktree [pwd]
 	unset cdup
-} elseif {![is_enabled bare]} {
-	if {[is_bare]} {
+} elseif {[is_bare]} {
+	if {![is_enabled bare]} {
 		catch {wm withdraw .}
 		error_popup [strcat [mc "Cannot use bare repository:"] "\n\n$_gitdir"]
 		exit 1
 	}
+} else {
 	if {$_gitworktree eq {}} {
 		set _gitworktree [file dirname $_gitdir]
 	}
-- 
1.7.0.198.g1e558.dirty

Re: [PATCH] git-gui: fix worktree initialization with empty prefix

[Matthieu Moy]

Giuseppe Bilotta <giuseppe.bilotta@gmail.com> writes:

> This is sort of a chicken-and-egg problem, because we should only
> bother setting the worktree if we are not in a bare repository, but
> an unset worktree is one of the conditions we check to see if we have
> a bare repository.
>
> Maintain the same sequence of checks, but swap the check for bareness
> support and bareness of repository, and check again for an unset
> worktree if we are not in the bare case.

Tested-by: Matthieu Moy <Matthieu.Moy@imag.fr>

(don't forget Signed-off-by)

Thanks,

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [PATCH] git-gui: fix worktree initialization with empty prefix

[Giuseppe Bilotta]

On Mon, Feb 22, 2010 at 4:32 PM, Matthieu Moy
<Matthieu.Moy@grenoble-inp.fr> wrote:
> Giuseppe Bilotta <giuseppe.bilotta@gmail.com> writes:
>
>> This is sort of a chicken-and-egg problem, because we should only
>> bother setting the worktree if we are not in a bare repository, but
>> an unset worktree is one of the conditions we check to see if we have
>> a bare repository.
>>
>> Maintain the same sequence of checks, but swap the check for bareness
>> support and bareness of repository, and check again for an unset
>> worktree if we are not in the bare case.

Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>

>
> Tested-by: Matthieu Moy <Matthieu.Moy@imag.fr>
>
> (don't forget Signed-off-by)

Damn. Thanks. I just set format.signoff so I won't forget again.

> Thanks,

Thank you for spotting and testing this.

-- 
Giuseppe "Oblomov" Bilotta