threads / bug / 22752

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

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

## tl;dr

9 messages between Feb 22, 2010 and Feb 23, 2010.

replies: 8people: 3as markdown or json

Matthieu Moy· Feb 22, 2010, 08:27 UTC · lore
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/
Giuseppe Bilotta· Feb 22, 2010, 15:18 UTC · re: Matthieu Moy · lore

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

On Mon, Feb 22, 2010 at 9:27 AM, Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> wrote:

Show 7 quoted lines
> 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
Heiko Voigt· Feb 22, 2010, 22:38 UTC · re: Giuseppe Bilotta · lore

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

On Mon, Feb 22, 2010 at 04:18:11PM +0100, Giuseppe Bilotta wrote:
Show 14 quoted lines
> 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
Giuseppe Bilotta· Feb 22, 2010, 23:29 UTC · re: Heiko Voigt · lore

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

On Mon, Feb 22, 2010 at 11:38 PM, Heiko Voigt <hvoigt@hvoigt.net> wrote:
Show 21 quoted lines
> 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
Matthieu Moy· Feb 23, 2010, 06:40 UTC · re: Giuseppe Bilotta · lore

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

[ Adding Pat Thoyts, author of the first patch, in Cc ]
Giuseppe Bilotta <giuseppe.bilotta@gmail.com> writes:
Show 16 quoted lines
> 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/
Heiko Voigt· Feb 23, 2010, 21:30 UTC · re: Giuseppe Bilotta · lore

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

On Tue, Feb 23, 2010 at 12:29:03AM +0100, Giuseppe Bilotta wrote:
Show 29 quoted lines
> 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
Giuseppe Bilotta· Feb 22, 2010, 15:19 UTC · re: Matthieu Moy · lore

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

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
Matthieu Moy· Feb 22, 2010, 15:32 UTC · re: Giuseppe Bilotta · lore

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

Giuseppe Bilotta <giuseppe.bilotta@gmail.com> writes:
Show 8 quoted lines
> 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/
Giuseppe Bilotta· Feb 22, 2010, 15:39 UTC · re: Matthieu Moy · lore

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

On Mon, Feb 22, 2010 at 4:32 PM, Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> wrote:

Show 10 quoted lines
> 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

← back to recent threads