{"thread":{"id":"20992","subject":"Gitk --all error when there are more than 797 refs in a repository","startedAt":"2009-09-17T19:07:33Z","lastAt":"2009-11-03T14:59:18Z","messageCount":19,"participants":["Murphy, John","Pat Thoyts","Johannes Sixt","Paul Mackerras","Junio C Hamano","Alex Riesen"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"123461","messageId":"6F87406399731F489FBACE5C5FFA04584BFA53@ex2k.bankofamerica.com","threadId":"20992","inReplyTo":null,"subject":"Gitk --all error when there are more than 797 refs in a repository","fromName":"Murphy, John","fromEmail":"john.murphy@bankofamerica.com","sentAt":"2009-09-17T19:07:33Z","receivedAt":"2009-09-17T19:07:33Z","isPatch":false,"sender":{"key":"john.murphy@bankofamerica.com","avatar":null},"body":"There is a error when running  gitk --all when there are more than 797 refs in a repository.\nWe get an error message:\n\nError reading commits: fatal ambiguous argument '3': unknown revision or path not in the working tree.\nUse '--' to separate paths from revisions.\n\nI believe issue is with this line of the code in proc parseviewrevs:\n\n if {[catch {set ids [eval exec git rev-parse \"$revs\"]} err]}\n\nWhen there are more than 797 refs the output of git rev-parse is too large to fit into the string, ids.\n\n797 refs = 32,677 bytes.\n798 refs = 32,718 bytes my guess is a little too close for comfort to 32,768 bytes.\n\nAs I was deleting refs locally the error message would change from '3' to any char [A-Z,0-9].\n\nI am a novice tcl programmer but is seems like ids could be an array.\nThere are also many other areas in the code where git rev-parse is called and using array may also be necessary.\n\nWe were using:\ngit 1.6.3.2.314.ge3519\n\nand then I upgraded to test if there was a change:\ngit 1.6.5.rc1.18.g401ce7\n\nWe are also using:\ntcl 8.4.1\ncygwin 1.5.25-7\nWindows XP Pro SP3\n"},{"id":"123492","messageId":"878wgcbb52.fsf@users.sourceforge.net","threadId":"20992","inReplyTo":"6F87406399731F489FBACE5C5FFA04584BFA53@ex2k.bankofamerica.com","subject":"[PATCH] Re: Gitk --all error when there are more than 797 refs in a repository","fromName":"Pat Thoyts","fromEmail":"patthoyts@users.sourceforge.net","sentAt":"2009-09-18T14:06:17Z","receivedAt":"2009-09-18T14:06:17Z","isPatch":true,"sender":{"key":"patthoyts@users.sourceforge.net","avatar":"https://avatars.githubusercontent.com/u/30739?v=4"},"body":"\"Murphy, John\" writes:\n\n>There is a error when running  gitk --all when there are more than 797 refs in a repository.\n>We get an error message:\n>\n>Error reading commits: fatal ambiguous argument '3': unknown revision or path not in the working tree.\n>Use '--' to separate paths from revisions.\n>\n>I believe issue is with this line of the code in proc parseviewrevs:\n>\n> if {[catch {set ids [eval exec git rev-parse \"$revs\"]} err]}\n>\n>When there are more than 797 refs the output of git rev-parse is too large to fit into the string, ids.\n>\n>797 refs = 32,677 bytes.\n>798 refs = 32,718 bytes my guess is a little too close for comfort to 32,768 bytes.\n>\n>As I was deleting refs locally the error message would change from '3' to any char [A-Z,0-9].\n>\n>I am a novice tcl programmer but is seems like ids could be an array.\n>There are also many other areas in the code where git rev-parse is called and using array may also be necessary.\n>\n\nTcl strings can eat all your memory. However, there is a limit to the\nsize of the command line argument passed to CreateProcess. MSDN says\nof the lpCommandLine parameter:\n \"The maximum length of this string is 32K characters.\"\nA solution for this case will be to use a pipe to read the responses\ninstead of having it all returned to the caller.\nThe following patch might be sufficient:\n\n--- patch begins -----\n\n[PATCH] Avoid command-line limits when executing git rev-parse on windows.\n\nThis patch solves the problem handling large numbers of references\nreported by John Murphy that is due to limits in executing processes\nin Windows by reading the rev-parse result over a pipe.\n\nSigned-off-by: Pat Thoyts \n---\n gitk | 14 ++++++++++++--\n 1 files changed, 12 insertions(+), 2 deletions(-)\n\ndiff --git a/gitk b/gitk\nindex 1306178..1bd7d65 100755\n--- a/gitk\n+++ b/gitk\n@@ -236,13 +236,23 @@ proc parseviewargs {n arglist} {\n return $allknown\n }\n \n+proc git-rev-parse {args} {\n+ set ids {}\n+ set pipe [open |[linsert $args 0 git rev-parse] r]\n+ while {[gets $pipe line] != -1} {\n+ lappend ids $line\n+ }\n+ close $pipe\n+ return $ids\n+}\n+ \n proc parseviewrevs {view revs} {\n global vposids vnegids\n \n if {$revs eq {}} {\n \tset revs HEAD\n }\n- if {[catch {set ids [eval exec git rev-parse $revs]} err]} {\n+ if {[catch {set ids [git-rev-parse $revs]} err]} {\n \t# we get stdout followed by stderr in $err\n \t# for an unknown rev, git rev-parse echoes it and then errors out\n \tset errlines [split $err \"\\n\"]\n@@ -273,7 +283,7 @@ proc parseviewrevs {view revs} {\n set pos {}\n set neg {}\n set sdm 0\n- foreach id [split $ids \"\\n\"] {\n+ foreach id $ids {\n \tif {$id eq \"--gitk-symmetric-diff-marker\"} {\n \t set sdm 4\n \t} elseif {[string match \"^*\" $id]} {\n-- \n1.6.4.msysgit.0\n\n\n\n-- \nPat Thoyts http://www.patthoyts.tk/\nPGP fingerprint 2C 6E 98 07 2C 59 C8 97 10 CE 11 E6 04 E0 B9 DD\n"},{"id":"123494","messageId":"4AB3A458.5070100@viscovery.net","threadId":"20992","inReplyTo":"878wgcbb52.fsf@users.sourceforge.net","subject":"Re: [PATCH] Re: Gitk --all error when there are more than 797 refs in a repository","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2009-09-18T15:16:40Z","receivedAt":"2009-09-18T15:16:40Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Pat Thoyts schrieb:\n> \"Murphy, John\" writes:\n>> There is a error when running gitk --all when there are more than 797 refs in a repository.\n>> We get an error message:\n>>\n>> Error reading commits: fatal ambiguous argument '3': unknown revision or path not in the working tree.\n>> Use '--' to separate paths from revisions.\n>>\n>> I believe issue is with this line of the code in proc parseviewrevs:\n>>\n>> if {[catch {set ids [eval exec git rev-parse \"$revs\"]} err]}\n>>\n>> When there are more than 797 refs the output of git rev-parse is too large to fit into the string, ids.\n>>\n>> 797 refs = 32,677 bytes.\n>> 798 refs = 32,718 bytes my guess is a little too close for comfort to 32,768 bytes.\n>>\n>> As I was deleting refs locally the error message would change from '3' to any char [A-Z,0-9].\n\nI cannot reproduce the error. I have a repository with 100 commits in a\nlinear history and 5000 refs (50 refs per commit). They are named\nrefs/heads/branch-XXXX. I don't see any problems with 'gitk --all'.\n\n> +proc git-rev-parse {args} {\n> + set ids {}\n> + set pipe [open |[linsert $args 0 git rev-parse] r]\n> + while {[gets $pipe line] != -1} {\n> + lappend ids $line\n> + }\n> + close $pipe\n> + return $ids\n> +}\n> + \n> proc parseviewrevs {view revs} {\n> global vposids vnegids\n> \n> if {$revs eq {}} {\n> \tset revs HEAD\n> }\n> - if {[catch {set ids [eval exec git rev-parse $revs]} err]} {\n> + if {[catch {set ids [git-rev-parse $revs]} err]} {\n\nSorry, but you are changing the wrong end of git rev-parse. The limit is\non the command line, but if you run 'gitk --all', then $revs is simply\n\"--all\" - no limit is exceeded. You changed the output of rev-parse, but\nthere is no limit on how much Tcl can eat of rev-parse's output.\n\nThe error must be in some other git invocation.\n\n-- Hannes\n"},{"id":"123510","messageId":"19124.8378.975976.347711@cargo.ozlabs.ibm.com","threadId":"20992","inReplyTo":"878wgcbb52.fsf@users.sourceforge.net","subject":"[PATCH] Re: Gitk --all error when there are more than 797 refs in a repository","fromName":"Paul Mackerras","fromEmail":"paulus@samba.org","sentAt":"2009-09-19T00:07:22Z","receivedAt":"2009-09-19T00:07:22Z","isPatch":true,"sender":{"key":"paulus@samba.org","avatar":"https://avatars.githubusercontent.com/u/1606439?v=4"},"body":"Pat Thoyts writes:\n\n> Tcl strings can eat all your memory. However, there is a limit to the\n> size of the command line argument passed to CreateProcess. MSDN says\n> of the lpCommandLine parameter:\n> \"The maximum length of this string is 32K characters.\"\n> A solution for this case will be to use a pipe to read the responses\n> instead of having it all returned to the caller.\n> The following patch might be sufficient:\n\nI knew about the 32k command-line limit under windows, but I don't see\nhow that applies in this case unless it is $revs that is too long (and\nif that is the case then I don't see how your patch helps). Is there\nalso a 32k limit on the size of data returned by a command executed\nwith [exec]?\n\nPaul.\n"},{"id":"123579","messageId":"6F87406399731F489FBACE5C5FFA0458518DE8@ex2k.bankofamerica.com","threadId":"20992","inReplyTo":"19124.8378.975976.347711@cargo.ozlabs.ibm.com","subject":"RE: [PATCH] Re: Gitk --all error when there are more than 797 refs in a repository","fromName":"Murphy, John","fromEmail":"john.murphy@bankofamerica.com","sentAt":"2009-09-21T14:02:51Z","receivedAt":"2009-09-21T14:02:51Z","isPatch":true,"sender":{"key":"john.murphy@bankofamerica.com","avatar":null},"body":"Johannes Sixt writes:\n\n> I cannot reproduce the error. I have a repository with 100 commits in\na\n> linear history and 5000 refs (50 refs per commit). They are named\n> refs/heads/branch-XXXX. I don't see any problems with 'gitk --all'.\n\nThat still leave you with only 100 unique refs.\nYou need over 797 unique refs.\n\n> The error must be in some other git invocation.\n\nI put many debug pop-ups around the code and I believe that this the\ncall that is dying on.\nThis is the only part of the code that has the error text \"fatal:\nambiguous argument\".\n\n\nPaul Mackerras writes:\n\n> I knew about the 32k command-line limit under windows, but I don't see\n> how that applies in this case unless it is $revs that is too long (and\n> if that is the case then I don't see how your patch helps). Is there\n> also a 32k limit on the size of data returned by a command executed\n> with [exec]?\n\nIn this case $revs is \"--all\"\n\nI believe what I am experiencing is a 32K limit with [exec]\n\nAdditional info:\nMy repo has: 17,737 commits\n"},{"id":"123580","messageId":"4AB78910.7010402@viscovery.net","threadId":"20992","inReplyTo":"6F87406399731F489FBACE5C5FFA0458518DE8@ex2k.bankofamerica.com","subject":"Re: [PATCH] Re: Gitk --all error when there are more than 797 refs in a repository","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2009-09-21T14:09:20Z","receivedAt":"2009-09-21T14:09:20Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Murphy, John schrieb:\n> Paul Mackerras writes:\n> \n>> I knew about the 32k command-line limit under windows, but I don't see\n>> how that applies in this case unless it is $revs that is too long (and\n>> if that is the case then I don't see how your patch helps). Is there\n>> also a 32k limit on the size of data returned by a command executed\n>> with [exec]?\n> \n> In this case $revs is \"--all\"\n> \n> I believe what I am experiencing is a 32K limit with [exec]\n\nBut in order to have a $revs that exceeds 32K, you would already have to\ninvoke gitk with a huge command line that exceeds the limit (but this is\nnot possible), no?\n\nHow do you run gitk?\n\n-- Hannes\n"},{"id":"123582","messageId":"6F87406399731F489FBACE5C5FFA0458518E11@ex2k.bankofamerica.com","threadId":"20992","inReplyTo":"4AB78910.7010402@viscovery.net","subject":"RE: [PATCH] Re: Gitk --all error when there are more than 797 refs in a repository","fromName":"Murphy, John","fromEmail":"john.murphy@bankofamerica.com","sentAt":"2009-09-21T14:11:51Z","receivedAt":"2009-09-21T14:11:51Z","isPatch":true,"sender":{"key":"john.murphy@bankofamerica.com","avatar":null},"body":"Johannes Sixt writes:\n\n> But in order to have a $revs that exceeds 32K, you would already have\nto\n> invoke gitk with a huge command line that exceeds the limit (but this\nis\n> not possible), no?\n\n>How do you run gitk?\n\ngitk --all\n"},{"id":"123586","messageId":"4AB7A2E7.5000601@viscovery.net","threadId":"20992","inReplyTo":"6F87406399731F489FBACE5C5FFA0458518E11@ex2k.bankofamerica.com","subject":"Re: [PATCH] Re: Gitk --all error when there are more than 797 refs in a repository","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2009-09-21T15:59:35Z","receivedAt":"2009-09-21T15:59:35Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Murphy, John schrieb:\n> Johannes Sixt writes:\n> \n>> But in order to have a $revs that exceeds 32K, you would already have\n> to\n>> invoke gitk with a huge command line that exceeds the limit (but this\n> is\n>> not possible), no?\n> \n>> How do you run gitk?\n> \n> gitk --all\n\nI see it. Here is a bash script that creates a repository that reproduces\nthe error. It is important that refs which sort alphabetically earlier\nalso point to earlier commits.\n\n-- snip --\n#!/bin/bash\ngit init\necho initial > file && git add file && git commit -m initial\nfor ((i = 0; i < 1000; i++))\ndo\n\techo $i > file &&\n\tgit commit -m $i file > /dev/null &&\n\tprintf -v l \"branch-%04d\" $i &&\n\tgit update-ref refs/heads/$l HEAD\ndone\ngit gc\n-- snip --\n\nOn Windows, 'gitk --all' starts with branch-0797, on Linux it starts with\nbranch-0999 aka master.\n\nI'm just throwing this out to interested parties; I'll not look into it at\nthis time.\n\nThanks,\n-- Hannes\n"},{"id":"123606","messageId":"874oqvc0n3.fsf@users.sourceforge.net","threadId":"20992","inReplyTo":"4AB7A2E7.5000601@viscovery.net","subject":"Re: [PATCH] Re: Gitk --all error when there are more than 797 refs in a repository","fromName":"Pat Thoyts","fromEmail":"patthoyts@users.sourceforge.net","sentAt":"2009-09-21T23:56:48Z","receivedAt":"2009-09-21T23:56:48Z","isPatch":true,"sender":{"key":"patthoyts@users.sourceforge.net","avatar":"https://avatars.githubusercontent.com/u/30739?v=4"},"body":"Johannes Sixt writes:\n\n>Murphy, John schrieb:\n>\n>On Windows, 'gitk --all' starts with branch-0797, on Linux it starts with\n>branch-0999 aka master.\n\nThat script gives me a repository I can test against. thanks.\nThe start_rev_list function calls parseviewrevs and expands the\narguments into a list of appropriate revision ids. In this case --all\ngets expanded to a list of 1000 sha1 ids. This is appended to any\nother view arguments and passed to git log on the command line\nyielding our error.\ngit log can accept a --all argument it seems so it looks like we can\njust short-circuit the parseviewrevs function when --all is passed in\nand return --all instead of expanding the list. The following seems to\nwork for me with this test repository.\nJohn, if this works for you can you also check that editing and\ncreating new gitk views on your real repository continues to work ok.\n\ncommit 7f289ca8370e5e2f9622a4fbc30b934eb97b984f\nAuthor: Pat Thoyts \nDate: Tue Sep 22 00:55:50 2009 +0100\n\n Avoid expanding --all when passing arguments to git log.\n There is no need to expand --all into a list of all revisions as\n git log can accept --all as an argument. This avoids any\n command-line\n length limitations caused by expanding --all into a list of all\n revision ids.\n\n Signed-off-by: Pat Thoyts \n\ndiff --git a/gitk b/gitk\nindex a0214b7..635b97e 100755\n--- a/gitk\n+++ b/gitk\n@@ -241,6 +241,8 @@ proc parseviewrevs {view revs} {\n\n if {$revs eq {}} {\n set revs HEAD\n+ } elseif {$revs eq \"--all\"} {\n+ return $revs\n }\n if {[catch {set ids [eval exec git rev-parse $revs]} err]} {\n # we get stdout followed by stderr in $err\n\n-- \nPat Thoyts http://www.patthoyts.tk/\nPGP fingerprint 2C 6E 98 07 2C 59 C8 97 10 CE 11 E6 04 E0 B9 DD\n"},{"id":"123608","messageId":"6F87406399731F489FBACE5C5FFA0458519639@ex2k.bankofamerica.com","threadId":"20992","inReplyTo":"874oqvc0n3.fsf@users.sourceforge.net","subject":"RE: [PATCH] Re: Gitk --all error when there are more than 797 refs in a repository","fromName":"Murphy, John","fromEmail":"john.murphy@bankofamerica.com","sentAt":"2009-09-22T01:23:26Z","receivedAt":"2009-09-22T01:23:26Z","isPatch":true,"sender":{"key":"john.murphy@bankofamerica.com","avatar":null},"body":"Pat Thoyts writes:\n\n> John, if this works for you can you also check that editing and\n> creating new gitk views on your real repository continues to work ok.\n\nWorks like a charm.\nI look forward to it coming in a new version of git.\nThank you very much.\n"},{"id":"123609","messageId":"7v1vlzvjtg.fsf@alter.siamese.dyndns.org","threadId":"20992","inReplyTo":"874oqvc0n3.fsf@users.sourceforge.net","subject":"Re: [PATCH] Re: Gitk --all error when there are more than 797 refs in a repository","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-09-22T01:39:55Z","receivedAt":"2009-09-22T01:39:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Pat Thoyts writes:\n\n> commit 7f289ca8370e5e2f9622a4fbc30b934eb97b984f\n> Author: Pat Thoyts \n> Date: Tue Sep 22 00:55:50 2009 +0100\n>\n> Avoid expanding --all when passing arguments to git log.\n> There is no need to expand --all into a list of all revisions as\n> git log can accept --all as an argument. This avoids any\n> command-line\n> length limitations caused by expanding --all into a list of all\n> revision ids.\n>\n> Signed-off-by: Pat Thoyts \n>\n> diff --git a/gitk b/gitk\n> index a0214b7..635b97e 100755\n> --- a/gitk\n> +++ b/gitk\n> @@ -241,6 +241,8 @@ proc parseviewrevs {view revs} {\n>\n> if {$revs eq {}} {\n> set revs HEAD\n> + } elseif {$revs eq \"--all\"} {\n> + return $revs\n> }\n\nThat looks like an ugly hack (aka sweeping the issue under the rug).\n\nWhat if there are many tags and the user used --tags? Don't you have\nexactly the same problem? Likewise, what if $revs were \"..master\"?\n\nThe right approach would be to understand what limit it is busting (it is\nnot likely to be the command line length limit for this particular \"exec\",\nas it only gets \"git\" \"rev-parse\" \"--all\") first, and then fix that.\n\n> if {[catch {set ids [eval exec git rev-parse $revs]} err]} {\n> # we get stdout followed by stderr in $err\n"},{"id":"123610","messageId":"7vws3ru4w8.fsf@alter.siamese.dyndns.org","threadId":"20992","inReplyTo":"7v1vlzvjtg.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] Re: Gitk --all error when there are more than 797 refs in a repository","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-09-22T01:47:35Z","receivedAt":"2009-09-22T01:47:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Pat Thoyts writes:\n>\n>> commit 7f289ca8370e5e2f9622a4fbc30b934eb97b984f\n>> Author: Pat Thoyts \n>> Date: Tue Sep 22 00:55:50 2009 +0100\n>>\n>> Avoid expanding --all when passing arguments to git log.\n>> There is no need to expand --all into a list of all revisions as\n>> git log can accept --all as an argument. This avoids any\n>> command-line\n>> length limitations caused by expanding --all into a list of all\n>> revision ids.\n>>\n>> Signed-off-by: Pat Thoyts \n>>\n>> diff --git a/gitk b/gitk\n>> index a0214b7..635b97e 100755\n>> --- a/gitk\n>> +++ b/gitk\n>> @@ -241,6 +241,8 @@ proc parseviewrevs {view revs} {\n>>\n>> if {$revs eq {}} {\n>> set revs HEAD\n>> + } elseif {$revs eq \"--all\"} {\n>> + return $revs\n>> }\n>\n> That looks like an ugly hack (aka sweeping the issue under the rug).\n>\n> What if there are many tags and the user used --tags? Don't you have\n> exactly the same problem? Likewise, what if $revs were \"..master\"?\n\nSorry, I meant \"--all --not master\" to grab all the topics not merged to\nmaster yet.\n\nBut my point still stands.\n\nI do not understand what computed values storedin vposids() and vnegids()\narrays are being used in the other parts of the program that rely on this\nfunction to do what it was asked to do, but if this patch can ever be\ncorrect, a much simpler solution to make this function almost no-op and\nalways return {} (empty array ret is initialized to) would be an equally\nvalid fix, no? And my gut feeling tells me that such a change to make\nthis function a no-op _can't_ be a valid fix.\n\n> The right approach would be to understand what limit it is busting (it is\n> not likely to be the command line length limit for this particular \"exec\",\n> as it only gets \"git\" \"rev-parse\" \"--all\") first, and then fix that.\n>\n>> if {[catch {set ids [eval exec git rev-parse $revs]} err]} {\n>> # we get stdout followed by stderr in $err\n"},{"id":"123652","messageId":"87y6o6a944.fsf@users.sourceforge.net","threadId":"20992","inReplyTo":"7vws3ru4w8.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] Re: Gitk --all error when there are more than 797 refs in a repository","fromName":"Pat Thoyts","fromEmail":"patthoyts@users.sourceforge.net","sentAt":"2009-09-22T22:48:59Z","receivedAt":"2009-09-22T22:48:59Z","isPatch":true,"sender":{"key":"patthoyts@users.sourceforge.net","avatar":"https://avatars.githubusercontent.com/u/30739?v=4"},"body":"(nobody) writes:\n\n>Junio C Hamano writes:\n>\n>> Pat Thoyts writes:\n>>\n>> That looks like an ugly hack (aka sweeping the issue under the rug).\n>>\n>> What if there are many tags and the user used --tags? Don't you have\n>> exactly the same problem? Likewise, what if $revs were \"..master\"?\n>\n>Sorry, I meant \"--all --not master\" to grab all the topics not merged to\n>master yet.\n>\n>But my point still stands.\n\nNot exactly. The problem is that the call to parseviewrevs will expand\n--all into a tcl list containing all the revision ids. We can do some\ntesting if we dig into this with the tcl console:\n % llength [set revs [parseviewrevs {} --all]]\n 1001\n % string length $revs\n 41040\nIn start_rev-list this list gets added to the command line for git-log\nin the $args variable. This is always going to exceed windows'\ncommandline limit (32k).\n\nSome testing shows that a number of rev-parse arguments do not get\nexpanded into a list of ids. All these can be ignored. But --all,\n--tags and --branches do. Maybe --remotes as well.\nThese arguments are accetable to git-log so it looks to me like they\ncan be left as-is.\n\nThe vposids and vnegids arrays are getting used for something\nthough. So the patch is not complete. They appear to be caching the\nset of revisions present in the current view for use in updatecommits\nto do something.\n\nSo - needs more work.\n\n-- \nPat Thoyts http://www.patthoyts.tk/\nPGP fingerprint 2C 6E 98 07 2C 59 C8 97 10 CE 11 E6 04 E0 B9 DD\n"},{"id":"123653","messageId":"19129.24056.422939.880134@cargo.ozlabs.ibm.com","threadId":"20992","inReplyTo":"874oqvc0n3.fsf@users.sourceforge.net","subject":"Re: [PATCH] Re: Gitk --all error when there are more than 797 refs in a repository","fromName":"Paul Mackerras","fromEmail":"paulus@samba.org","sentAt":"2009-09-22T23:30:00Z","receivedAt":"2009-09-22T23:30:00Z","isPatch":true,"sender":{"key":"paulus@samba.org","avatar":"https://avatars.githubusercontent.com/u/1606439?v=4"},"body":"Pat Thoyts writes:\n\n> That script gives me a repository I can test against. thanks.\n> The start_rev_list function calls parseviewrevs and expands the\n> arguments into a list of appropriate revision ids. In this case --all\n> gets expanded to a list of 1000 sha1 ids. This is appended to any\n> other view arguments and passed to git log on the command line\n> yielding our error.\n> git log can accept a --all argument it seems so it looks like we can\n> just short-circuit the parseviewrevs function when --all is passed in\n> and return --all instead of expanding the list. The following seems to\n> work for me with this test repository.\n\nWhat the code is trying to do here is to get git log to give us all\nthe commits that the user asked for *except* any commits we have\nalready received. So, when gitk is first invoked, this means all the\ncommits that the user asked for. If the user presses F5 or does\nFile->Update, then we do git log with some starting points removed\n(those that haven't changed since the last update) and some negative\narguments added (to exclude the previous starting points).\n\nTo do that accurately, we need to know exactly what set of revisions\nwe are asking git log to start from, and exactly what set of revisions\nwe are asking git log to stop at. The problem with just passing --all\nto git log, as your patch does, is that the list of revs might change\nbetween when gitk expands --all and when git log expands --all (due to\ncommits getting added, heads getting reset etc.). Then, if the user\npresses F5, some commits might get missed.\n\nIf git log had an argument to tell it to mark those commits that were\na starting point or a finishing point, then I could simplify this\nlogic enormously, plus we wouldn't have to pass a long parameter list\nto git log. It may still turn out to be necessary to add a negative\nargument for each previous starting point, though, when refreshing the\nlist.\n\nI think the simplest fix for now is to arrange to take the\nnon-optimized path on windows when the list of revs gets too long,\ni.e., set $vcanopt($view) to 0 and take that path. That means that\nrefreshing the view will be slow, but I think it's the best we can do\nat this point.\n\nPaul.\n"},{"id":"123655","messageId":"7vd45io7da.fsf@alter.siamese.dyndns.org","threadId":"20992","inReplyTo":"19129.24056.422939.880134@cargo.ozlabs.ibm.com","subject":"Re: [PATCH] Re: Gitk --all error when there are more than 797 refs in a repository","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-09-23T00:02:57Z","receivedAt":"2009-09-23T00:02:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Paul Mackerras writes:\n\n> If git log had an argument to tell it to mark those commits that were\n> a starting point or a finishing point, then I could simplify this\n> logic enormously, plus we wouldn't have to pass a long parameter list\n> to git log. It may still turn out to be necessary to add a negative\n> argument for each previous starting point, though, when refreshing the\n> list.\n>\n> I think the simplest fix for now is to arrange to take the\n> non-optimized path on windows when the list of revs gets too long,\n> i.e., set $vcanopt($view) to 0 and take that path. That means that\n> refreshing the view will be slow, but I think it's the best we can do\n> at this point.\n\nHmph.\n\nThe negative ones you can learn by giving --boundary, but I do not think\nthe set of starting points are something you can get out of log output.\n\nEven if you could, you would have the same issue giving them from the\ncommand line anyway. The right solution would likely to be to give the\nsame --stdin option as rev-list to \"git log\", I think.\n"},{"id":"126636","messageId":"19183.64129.695745.269570@cargo.ozlabs.ibm.com","threadId":"20992","inReplyTo":"7vd45io7da.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] Re: Gitk --all error when there are more than 797 refs in a repository","fromName":"Paul Mackerras","fromEmail":"paulus@samba.org","sentAt":"2009-11-03T09:40:17Z","receivedAt":"2009-11-03T09:40:17Z","isPatch":true,"sender":{"key":"paulus@samba.org","avatar":"https://avatars.githubusercontent.com/u/1606439?v=4"},"body":"Junio C Hamano writes:\n\n> Paul Mackerras writes:\n> \n> > If git log had an argument to tell it to mark those commits that were\n> > a starting point or a finishing point, then I could simplify this\n> > logic enormously, plus we wouldn't have to pass a long parameter list\n> > to git log. It may still turn out to be necessary to add a negative\n> > argument for each previous starting point, though, when refreshing the\n> > list.\n> >\n> > I think the simplest fix for now is to arrange to take the\n> > non-optimized path on windows when the list of revs gets too long,\n> > i.e., set $vcanopt($view) to 0 and take that path. That means that\n> > refreshing the view will be slow, but I think it's the best we can do\n> > at this point.\n> \n> Hmph.\n> \n> The negative ones you can learn by giving --boundary, but I do not think\n> the set of starting points are something you can get out of log output.\n> \n> Even if you could, you would have the same issue giving them from the\n> command line anyway. The right solution would likely to be to give the\n> same --stdin option as rev-list to \"git log\", I think.\n\nA --stdin option to git log would be great, but it doesn't seem to be\nimplemented yet. How hard would it be to add?\n\nPaul.\n"},{"id":"126638","messageId":"81b0412b0911030204v46adf54gb9ed65e78ce2b6df@mail.gmail.com","threadId":"20992","inReplyTo":"7v1vlzvjtg.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] Re: Gitk --all error when there are more than 797 refs in a repository","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2009-11-03T10:04:44Z","receivedAt":"2009-11-03T10:04:44Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"On Tue, Sep 22, 2009 at 02:39, Junio C Hamano wrote:\n> Pat Thoyts writes:\n>>      if {$revs eq {}} {\n>>         set revs HEAD\n>> +    } elseif {$revs eq \"--all\"} {\n>> +        return $revs\n>>      }\n>\n> That looks like an ugly hack (aka sweeping the issue under the rug).\n>\n\nAnd it is a race condition. By the time git log has got --all list of references\nit may look completely different to what gitk has.\n"},{"id":"126642","messageId":"19184.2253.656355.506185@cargo.ozlabs.ibm.com","threadId":"20992","inReplyTo":"81b0412b0911030204v46adf54gb9ed65e78ce2b6df@mail.gmail.com","subject":"Re: [PATCH] Re: Gitk --all error when there are more than 797 refs in a repository","fromName":"Paul Mackerras","fromEmail":"paulus@samba.org","sentAt":"2009-11-03T10:41:17Z","receivedAt":"2009-11-03T10:41:17Z","isPatch":true,"sender":{"key":"paulus@samba.org","avatar":"https://avatars.githubusercontent.com/u/1606439?v=4"},"body":"Alex Riesen writes:\n\n> On Tue, Sep 22, 2009 at 02:39, Junio C Hamano wrote:\n> > Pat Thoyts writes:\n> >>      if {$revs eq {}} {\n> >>         set revs HEAD\n> >> +    } elseif {$revs eq \"--all\"} {\n> >> +        return $revs\n> >>      }\n> >\n> > That looks like an ugly hack (aka sweeping the issue under the rug).\n> >\n> \n> And it is a race condition. By the time git log has got --all list of references\n> it may look completely different to what gitk has.\n\nYes, exactly. Until git log understands --stdin, I think the only\nreal solution is to disable the view update optimization on windows.\n\nPaul.\n"},{"id":"126650","messageId":"7v3a4vu01l.fsf@alter.siamese.dyndns.org","threadId":"20992","inReplyTo":"19183.64129.695745.269570@cargo.ozlabs.ibm.com","subject":"Re: [PATCH] Re: Gitk --all error when there are more than 797 refs in a repository","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-11-03T14:59:18Z","receivedAt":"2009-11-03T14:59:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Paul Mackerras writes:\n\n> Junio C Hamano writes:\n>\n>> Even if you could, you would have the same issue giving them from the\n>> command line anyway. The right solution would likely to be to give the\n>> same --stdin option as rev-list to \"git log\", I think.\n>\n> A --stdin option to git log would be great, but it doesn't seem to be\n> implemented yet. How hard would it be to add?\n\nFairly trivial. Only lightly tested with things like:\n\n $ (echo ^master; echo next) | ./git log --stdin\n $ (echo ^master; echo jc/log-tz) | ./git format-patch --stdin --stdout\n\nI am not signing this off because...\n\n - I do not want to think about what would happen if you give \"-p\" option,\n nor if our \"pager\" infrastructure is set up to allow us to do a\n sensible thing; and\n\n - This disables --stdin for blame as it wants to read contents from its\n standard input when given \"-\" as the file argument. I suspect there\n probably are similar commands that use setup_revisions() and do not use\n a \"--stdin\" option but still read from the standard input, and they\n need to be fixed similarly, but I want somebody else to do the audit.\n\nAt least, not yet.\n\nThat is, making it work for \"log\" is trivial---making sure it won't break\nunsuspecting bystanders is much more time-consuming work.\n\n-- >8 --\nSubject: teach --stdin to \"log\" family\n\nMove the logic to read revs from standard input that rev-list knows about\nfrom it to revision machinery, so that all the users of setup_revisions()\ncan feed the list of revs from the standard input when \"--stdin\" is used\non the command line.\n\nAllow some users of the revision machinery that want different semantics\nfrom the \"--stdin\" option to disable it by setting an option in the\nrev_info structure.\n\nThis also cleans up the kludge made to bundle.c via cut and paste.\n\n---\n builtin-blame.c | 1 +\n builtin-diff-tree.c | 1 +\n builtin-rev-list.c | 7 -------\n bundle.c | 7 -------\n revision.c | 15 +++++++++++++--\n revision.h | 4 ++--\n 6 files changed, 17 insertions(+), 18 deletions(-)\n\ndiff --git a/builtin-blame.c b/builtin-blame.c\nindex 7512773..b0aa530 100644\n--- a/builtin-blame.c\n+++ b/builtin-blame.c\n@@ -2352,6 +2352,7 @@ parse_done:\n \t\t\tdie_errno(\"cannot stat path '%s'\", path);\n \t}\n \n+\trevs.disable_stdin = 1;\n \tsetup_revisions(argc, argv, &revs, NULL);\n \tmemset(&sb, 0, sizeof(sb));\n \ndiff --git a/builtin-diff-tree.c b/builtin-diff-tree.c\nindex 79cedb7..2380c21 100644\n--- a/builtin-diff-tree.c\n+++ b/builtin-diff-tree.c\n@@ -104,6 +104,7 @@ int cmd_diff_tree(int argc, const char **argv, const char *prefix)\n \tgit_config(git_diff_basic_config, NULL); /* no \"diff\" UI options */\n \topt->abbrev = 0;\n \topt->diff = 1;\n+\topt->disable_stdin = 1;\n \targc = setup_revisions(argc, argv, opt, NULL);\n \n \twhile (--argc > 0) {\ndiff --git a/builtin-rev-list.c b/builtin-rev-list.c\nindex 42cc8d8..f6a56f3 100644\n--- a/builtin-rev-list.c\n+++ b/builtin-rev-list.c\n@@ -306,7 +306,6 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)\n \tstruct rev_info revs;\n \tstruct rev_list_info info;\n \tint i;\n-\tint read_from_stdin = 0;\n \tint bisect_list = 0;\n \tint bisect_show_vars = 0;\n \tint bisect_find_all = 0;\n@@ -349,12 +348,6 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)\n \t\t\tbisect_show_vars = 1;\n \t\t\tcontinue;\n \t\t}\n-\t\tif (!strcmp(arg, \"--stdin\")) {\n-\t\t\tif (read_from_stdin++)\n-\t\t\t\tdie(\"--stdin given twice?\");\n-\t\t\tread_revisions_from_stdin(&revs);\n-\t\t\tcontinue;\n-\t\t}\n \t\tusage(rev_list_usage);\n \n \t}\ndiff --git a/bundle.c b/bundle.c\nindex df95e15..a7c9987 100644\n--- a/bundle.c\n+++ b/bundle.c\n@@ -204,7 +204,6 @@ int create_bundle(struct bundle_header *header, const char *path,\n \tint i, ref_count = 0;\n \tchar buffer[1024];\n \tstruct rev_info revs;\n-\tint read_from_stdin = 0;\n \tstruct child_process rls;\n \tFILE *rls_fout;\n \n@@ -257,12 +256,6 @@ int create_bundle(struct bundle_header *header, const char *path,\n \targc = setup_revisions(argc, argv, &revs, NULL);\n \n \tfor (i = 1; i < argc; i++) {\n-\t\tif (!strcmp(argv[i], \"--stdin\")) {\n-\t\t\tif (read_from_stdin++)\n-\t\t\t\tdie(\"--stdin given twice?\");\n-\t\t\tread_revisions_from_stdin(&revs);\n-\t\t\tcontinue;\n-\t\t}\n \t\treturn error(\"unrecognized argument: %s'\", argv[i]);\n \t}\n \ndiff --git a/revision.c b/revision.c\nindex 9fc4e8d..45c5de8 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -953,7 +953,7 @@ int handle_revision_arg(const char *arg, struct rev_info *revs,\n \treturn 0;\n }\n \n-void read_revisions_from_stdin(struct rev_info *revs)\n+static void read_revisions_from_stdin(struct rev_info *revs)\n {\n \tchar line[1000];\n \n@@ -1227,7 +1227,7 @@ void parse_revision_opt(struct rev_info *revs, struct parse_opt_ctx_t *ctx,\n */\n int setup_revisions(int argc, const char **argv, struct rev_info *revs, const char *def)\n {\n-\tint i, flags, left, seen_dashdash;\n+\tint i, flags, left, seen_dashdash, read_from_stdin;\n \n \t/* First, search for \"--\" */\n \tseen_dashdash = 0;\n@@ -1245,6 +1245,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch\n \n \t/* Second, deal with arguments and options */\n \tflags = 0;\n+\tread_from_stdin = 0;\n \tfor (left = i = 1; i < argc; i++) {\n \t\tconst char *arg = argv[i];\n \t\tif (*arg == '-') {\n@@ -1283,6 +1284,16 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch\n \t\t\t\trevs->no_walk = 0;\n \t\t\t\tcontinue;\n \t\t\t}\n+\t\t\tif (!strcmp(arg, \"--stdin\")) {\n+\t\t\t\tif (revs->disable_stdin) {\n+\t\t\t\t\targv[left++] = arg;\n+\t\t\t\t\tcontinue;\n+\t\t\t\t}\n+\t\t\t\tif (read_from_stdin++)\n+\t\t\t\t\tdie(\"--stdin given twice?\");\n+\t\t\t\tread_revisions_from_stdin(revs);\n+\t\t\t\tcontinue;\n+\t\t\t}\n \n \t\t\topts = handle_revision_opt(revs, argc - i, argv + i, &left, argv);\n \t\t\tif (opts > 0) {\ndiff --git a/revision.h b/revision.h\nindex b6421a6..f64637b 100644\n--- a/revision.h\n+++ b/revision.h\n@@ -83,6 +83,8 @@ struct rev_info {\n \t\t\tuse_terminator:1,\n \t\t\tmissing_newline:1,\n \t\t\tdate_mode_explicit:1;\n+\tunsigned int\tdisable_stdin:1;\n+\n \tenum date_mode date_mode;\n \n \tunsigned int\tabbrev;\n@@ -128,8 +130,6 @@ struct rev_info {\n #define REV_TREE_DIFFERENT\t3\t/* Mixed changes */\n \n /* revision.c */\n-void read_revisions_from_stdin(struct rev_info *revs);\n-\n typedef void (*show_early_output_fn_t)(struct rev_info *, struct commit_list *);\n extern volatile show_early_output_fn_t show_early_output;\n \n"}]}