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

[Junio C Hamano <gitster@pobox.com>]

Junio C Hamano <gitster@pobox.com> writes:

> Pat Thoyts <patthoyts@users.sourceforge.net> writes:
>
>> commit 7f289ca8370e5e2f9622a4fbc30b934eb97b984f
>> Author: Pat Thoyts <patthoyts@users.sourceforge.net>
>> Date:   Tue Sep 22 00:55:50 2009 +0100
>>
>>     Avoid expanding --all when passing arguments to git log.
>>     There is no need to expand --all into a list of all revisions as
>>     git log can accept --all as an argument. This avoids any
>>     command-line
>>     length limitations caused by expanding --all into a list of all
>>     revision ids.
>>
>>     Signed-off-by: Pat Thoyts <patthoyts@users.sourceforge.net>
>>
>> diff --git a/gitk b/gitk
>> index a0214b7..635b97e 100755
>> --- a/gitk
>> +++ b/gitk
>> @@ -241,6 +241,8 @@ proc parseviewrevs {view revs} {
>>
>>      if {$revs eq {}} {
>>         set revs HEAD
>> +    } elseif {$revs eq "--all"} {
>> +        return $revs
>>      }
>
> That looks like an ugly hack (aka sweeping the issue under the rug).
>
> What if there are many tags and the user used --tags?  Don't you have
> exactly the same problem?  Likewise, what if $revs were "..master"?

Sorry, I meant "--all --not master" to grab all the topics not merged to
master yet.

But my point still stands.

I do not understand what computed values storedin vposids() and vnegids()
arrays are being used in the other parts of the program that rely on this
function to do what it was asked to do, but if this patch can ever be
correct, a much simpler solution to make this function almost no-op and
always return {} (empty array ret is initialized to) would be an equally
valid fix, no?  And my gut feeling tells me that such a change to make
this function a no-op  _can't_ be a valid fix.

> The right approach would be to understand what limit it is busting (it is
> not likely to be the command line length limit for this particular "exec",
> as it only gets "git" "rev-parse" "--all") first, and then fix that.
>
>>      if {[catch {set ids [eval exec git rev-parse $revs]} err]} {
>>         # we get stdout followed by stderr in $err