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

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

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

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

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

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

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

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

On Fri, 10 Aug 2007, Jeff King wrote:

I think this is true.

And I think this is *also* true.

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

Linus Torvalds writes:

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.

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

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

Jeff King writes:

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

Paul.

Paul Mackerras <paulus@samba.org> writes:

Thanks; pulled.

Junio C Hamano writes:

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

Paul.