Bug in gitk: can't unset "idinlist(...) ..."

Bug in gitk: can't unset "idinlist(...) ..."

[Brian Hetro]

Hi,
I have a problem with gitk not being able to show one of my
repositories (git version 1.5.3.rc4.41.g7efe).  I get this error while
gitk starts:

can't unset "idinlist(f1d795add789ec43d3ccf1d35f3c39fb464f6e72)": no
such element in array
can't unset "idinlist(f1d795add789ec43d3ccf1d35f3c39fb464f6e72)": no
such element in array
    while executing
"unset idinlist($id)"
    (procedure "layouttail" line 11)
    invoked from within
"layouttail"
    (procedure "layoutmore" line 35)
    invoked from within
"layoutmore $tlimit $allread"
    (procedure "chewcommits" line 9)
    invoked from within
"chewcommits 1"
    ("eval" body line 1)
    invoked from within
"eval $script"
    (procedure "dorunq" line 9)
    invoked from within
"dorunq"
    ("after" script)

The repository log is partially displayed.  gitk does work in git
version 1.5.2.4, and qgit and giggle also work.  git-fsck --full
--strict indicates no problems.

I performed a bisect and commit
1ed84157a21a3e868228b15588e4aadfbe5a030b appears to be the culprit
(Revert 88494423 (removal of duplicate parents in the output
codepath)).

Brian

Re: Bug in gitk: can't unset "idinlist(...) ..."

[Jeff King]

On Fri, Aug 10, 2007 at 11:41:08AM -0400, Brian Hetro wrote:

> Hi,
> I have a problem with gitk not being able to show one of my
> repositories (git version 1.5.3.rc4.41.g7efe).  I get this error while
> gitk starts:
> 
> can't unset "idinlist(f1d795add789ec43d3ccf1d35f3c39fb464f6e72)": no
> such element in array
> [...]
> 
> I performed a bisect and commit
> 1ed84157a21a3e868228b15588e4aadfbe5a030b appears to be the culprit
> (Revert 88494423 (removal of duplicate parents in the output
> codepath)).

This was fixed in e1abc69b, which is in 1.5.3-rc3, covered in this
thread:

http://thread.gmane.org/gmane.comp.version-control.git/53126

Can you confirm that it is still a problem in 1.5.3-rc4?

-Peff

Re: Bug in gitk: can't unset "idinlist(...) ..."

[Jeff King]

On Fri, Aug 10, 2007 at 12:11:23PM -0400, Jeff King wrote:

> http://thread.gmane.org/gmane.comp.version-control.git/53126
> 
> Can you confirm that it is still a problem in 1.5.3-rc4?

I just tried to trigger the original bug (by running "gitk arch/i386" in
the linux-2.6 repo) using 1.5.3.rc4.41.g7efe, and I couldn't reproduce
it.

Can you make your test repo available?

Note that the fix _isn't_ in gitk, but rather in git itself. Is it
possible that gitk is calling another, older version of git-rev-list
that you have installed somewhere?

-Peff

Re: Bug in gitk: can't unset "idinlist(...) ..."

[Jeff King]

On Fri, Aug 10, 2007 at 11:41:08AM -0400, Brian Hetro wrote:

> I have a problem with gitk not being able to show one of my
> repositories (git version 1.5.3.rc4.41.g7efe).  I get this error while
> gitk starts:
> 
> can't unset "idinlist(f1d795add789ec43d3ccf1d35f3c39fb464f6e72)": no
> such element in array

Junio,

I did a little digging on this bug, since it was so similar to the
duplicate parent errors we were getting a few weeks ago. As it turns
out, Brian's repository, generated by hg2git, actually has commit
objects with duplicate parents in them. Older versions of git used to
remove duplicate parents for all commits, but due to your commits
11d65967 and 1ed84157, we now do it just for history simplification.
Which is why he was able to bisect it to 1ed84157, even though it is
perhaps not a git bug.

So maybe the right attitude is "hg2git should not be generating such
broken commits" (or "gitk should not barf on such broken commits" :) ),
but I thought I would mention it as an additional data point for those
changes. Should git handle duplicate parents of this fashion more
robustly? Or should we just assume that they should never have been
generated in the first place?

-Peff

Re: Bug in gitk: can't unset "idinlist(...) ..."

[Linus Torvalds]

On Fri, 10 Aug 2007, Jeff King wrote:
> 
> So maybe the right attitude is "hg2git should not be generating such
> broken commits"

I think this is true.

> (or "gitk should not barf on such broken commits" :) ),

And I think this is *also* true.

> but I thought I would mention it as an additional data point for those
> changes. Should git handle duplicate parents of this fashion more
> robustly? Or should we just assume that they should never have been
> generated in the first place?

I think git itself is quite robust in the face of duplicate parents, and 
it really is a gitk bug that it has problems with them. That said, I don't 
think we should *assume* they don't happen, and while we should consider 
it a bug in hg2git that they did, it is not a "serious" bug per se. It's 
only gitk that reacts this violently to it.

I guess we could prune duplicate parents even for commits that didn't get 
rewritten, but I don't see why we really should even have to. I think Paul 
already said that he should look into it:

			    "I see from the following messages that the
    bug turned out to be elsewhere in git, but it looks like gitk should 
    be more robust and do something sensible rather than just throwing a 
    Tcl error.  I'll look at it."

so I think we should fix gitk regardless, and then *maybe* also consider 
doing parent simplification universally.

			Linus

Re: Bug in gitk: can't unset "idinlist(...) ..."

[Paul Mackerras]

Linus Torvalds writes:

> I think git itself is quite robust in the face of duplicate parents, and 
> it really is a gitk bug that it has problems with them. That said, I don't 
> think we should *assume* they don't happen, and while we should consider 
> it a bug in hg2git that they did, it is not a "serious" bug per se. It's 
> only gitk that reacts this violently to it.

There has been a commit with duplicate parents in the kernel git tree
for ages (13e65280), and gitk handles it just fine, so I don't think
it's simply a problem with duplicate parents.

I know gitk currently barfs if there are any dangling parents in the
git log --parents --boundary output (i.e. any ids which are listed in
the second or subsequent position on the commit lines but never in the
first position).  It shouldn't, and I'll fix it.

It would be nice to know if it is actually dangling parents that is
causing the problem, though.

Paul.

Re: Bug in gitk: can't unset "idinlist(...) ..."

[Jeff King]

On Sat, Aug 11, 2007 at 04:23:28PM +1000, Paul Mackerras wrote:

> There has been a commit with duplicate parents in the kernel git tree
> for ages (13e65280), and gitk handles it just fine, so I don't think
> it's simply a problem with duplicate parents.

gitk fails with this test script, which creates two commits, the second
one referencing the first as a duplicate:

mkdir repo &&
cd repo &&
git-init &&
echo one >file &&
git-add file &&
git-commit -m one &&
echo two >file &&
git-add file &&
tree=$(git-write-tree) &&
parent=$(git-rev-parse HEAD) &&
commit=$(git-commit-tree $tree -p $parent -p $parent <file) &&
git-update-ref HEAD $commit &&
gitk

To run it, you will actually need to disable git-commit-tree's checking
for duplicate parents. You can do this by commenting out the call to
new_parent in builtin-commit-tree.c:114.

-Peff

Re: Bug in gitk: can't unset "idinlist(...) ..."

[Paul Mackerras]

Jeff King writes:

> gitk fails with this test script, which creates two commits, the second
> one referencing the first as a duplicate:

Thanks, that shows the problem nicely.  I've pushed out a fix.

Paul.

Re: Bug in gitk: can't unset "idinlist(...) ..."

[Junio C Hamano]

Paul Mackerras <paulus@samba.org> writes:

> Jeff King writes:
>
>> gitk fails with this test script, which creates two commits, the second
>> one referencing the first as a duplicate:
>
> Thanks, that shows the problem nicely.  I've pushed out a fix.

Thanks; pulled.

Re: Bug in gitk: can't unset "idinlist(...) ..."

[Paul Mackerras]

Junio C Hamano writes:

> Thanks; pulled.

Unfortunately the fix I pushed out had a bug.  Please do another pull
to get a better fix.

Paul.