Re: [PATCH] Re: Gitk --all error when there are more than 797 refs in a repository

From: Paul Mackerras <paulus@samba.org>
Date: Sep 22, 2009, 23:30 UTC
Message-ID: <19129.24056.422939.880134@cargo.ozlabs.ibm.com>
In-Reply-To: <874oqvc0n3.fsf@users.sourceforge.net>

Pat Thoyts writes:

Show 10 quoted lines

> That script gives me a repository I can test against. thanks.
> The start_rev_list function calls parseviewrevs and expands the
> arguments into a list of appropriate revision ids. In this case --all
> gets expanded to a list of 1000 sha1 ids. This is appended to any
> other view arguments and passed to git log on the command line
> yielding our error.
> git log can accept a --all argument it seems so it looks like we can
> just short-circuit the parseviewrevs function when --all is passed in
> and return --all instead of expanding the list. The following seems to
> work for me with this test repository.

What the code is trying to do here is to get git log to give us all the commits that the user asked for *except* any commits we have already received. So, when gitk is first invoked, this means all the commits that the user asked for. If the user presses F5 or does File->Update, then we do git log with some starting points removed (those that haven't changed since the last update) and some negative arguments added (to exclude the previous starting points).

To do that accurately, we need to know exactly what set of revisions we are asking git log to start from, and exactly what set of revisions we are asking git log to stop at. The problem with just passing --all to git log, as your patch does, is that the list of revs might change between when gitk expands --all and when git log expands --all (due to commits getting added, heads getting reset etc.). Then, if the user presses F5, some commits might get missed.

If git log had an argument to tell it to mark those commits that were a starting point or a finishing point, then I could simplify this logic enormously, plus we wouldn't have to pass a long parameter list to git log. It may still turn out to be necessary to add a negative argument for each previous starting point, though, when refreshing the list.

I think the simplest fix for now is to arrange to take the non-optimized path on windows when the list of revs gets too long, i.e., set $vcanopt($view) to 0 and take that path. That means that refreshing the view will be slow, but I think it's the best we can do at this point.

Paul.