Gitk --all error when there are more than 797 refs in a repository

Gitk --all error when there are more than 797 refs in a repository

[Murphy, John]

There is a error when running  gitk --all when there are more than 797 refs in a repository.
We get an error message:

Error reading commits: fatal ambiguous argument '3': unknown revision or path not in the working tree.
Use '--' to separate paths from revisions.

I believe issue is with this line of the code in proc parseviewrevs:

       if {[catch {set ids [eval exec git rev-parse "$revs"]} err]}

When there are more than 797 refs the output of git rev-parse is too large to fit into the string, ids.

797 refs = 32,677 bytes.
798 refs = 32,718 bytes my guess is a little too close for comfort to 32,768 bytes.

As I was deleting refs locally the error message would change from '3' to any char [A-Z,0-9].

I am a novice tcl programmer but is seems like ids could be an array.
There are also many other areas in the code where git rev-parse is called and using array may also be necessary.

We were using:
git 1.6.3.2.314.ge3519

and then I upgraded to test if there was a change:
git 1.6.5.rc1.18.g401ce7

We are also using:
tcl 8.4.1
cygwin 1.5.25-7
Windows XP Pro SP3

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

[Pat Thoyts]

"Murphy, John" <john.murphy@bankofamerica.com> writes:

>There is a error when running  gitk --all when there are more than 797 refs in a repository.
>We get an error message:
>
>Error reading commits: fatal ambiguous argument '3': unknown revision or path not in the working tree.
>Use '--' to separate paths from revisions.
>
>I believe issue is with this line of the code in proc parseviewrevs:
>
>       if {[catch {set ids [eval exec git rev-parse "$revs"]} err]}
>
>When there are more than 797 refs the output of git rev-parse is too large to fit into the string, ids.
>
>797 refs = 32,677 bytes.
>798 refs = 32,718 bytes my guess is a little too close for comfort to 32,768 bytes.
>
>As I was deleting refs locally the error message would change from '3' to any char [A-Z,0-9].
>
>I am a novice tcl programmer but is seems like ids could be an array.
>There are also many other areas in the code where git rev-parse is called and using array may also be necessary.
>

Tcl strings can eat all your memory. However, there is a limit to the
size of the command line argument passed to CreateProcess.  MSDN says
of the lpCommandLine parameter:
  "The maximum length of this string is 32K characters."
A solution for this case will be to use a pipe to read the responses
instead of having it all returned to the caller.
The following patch might be sufficient:

--- patch begins -----

[PATCH] Avoid command-line limits when executing git rev-parse on windows.

This patch solves the problem handling large numbers of references
reported by John Murphy that is due to limits in executing processes
in Windows by reading the rev-parse result over a pipe.

Signed-off-by: Pat Thoyts <patthoyts@users.sourceforge.net>
---
 gitk |   14 ++++++++++++--
 1 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/gitk b/gitk
index 1306178..1bd7d65 100755
--- a/gitk
+++ b/gitk
@@ -236,13 +236,23 @@ proc parseviewargs {n arglist} {
     return $allknown
 }
 
+proc git-rev-parse {args} {
+    set ids {}
+    set pipe [open |[linsert $args 0 git rev-parse] r]
+    while {[gets $pipe line] != -1} {
+        lappend ids $line
+    }
+    close $pipe
+    return $ids
+}
+    
 proc parseviewrevs {view revs} {
     global vposids vnegids
 
     if {$revs eq {}} {
 	set revs HEAD
     }
-    if {[catch {set ids [eval exec git rev-parse $revs]} err]} {
+    if {[catch {set ids [git-rev-parse $revs]} err]} {
 	# we get stdout followed by stderr in $err
 	# for an unknown rev, git rev-parse echoes it and then errors out
 	set errlines [split $err "\n"]
@@ -273,7 +283,7 @@ proc parseviewrevs {view revs} {
     set pos {}
     set neg {}
     set sdm 0
-    foreach id [split $ids "\n"] {
+    foreach id $ids {
 	if {$id eq "--gitk-symmetric-diff-marker"} {
 	    set sdm 4
 	} elseif {[string match "^*" $id]} {
-- 
1.6.4.msysgit.0



-- 
Pat Thoyts                            http://www.patthoyts.tk/
PGP fingerprint 2C 6E 98 07 2C 59 C8 97  10 CE 11 E6 04 E0 B9 DD

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

[Johannes Sixt]

Pat Thoyts schrieb:
> "Murphy, John" <john.murphy@bankofamerica.com> writes:
>> There is a error when running  gitk --all when there are more than 797 refs in a repository.
>> We get an error message:
>>
>> Error reading commits: fatal ambiguous argument '3': unknown revision or path not in the working tree.
>> Use '--' to separate paths from revisions.
>>
>> I believe issue is with this line of the code in proc parseviewrevs:
>>
>>       if {[catch {set ids [eval exec git rev-parse "$revs"]} err]}
>>
>> When there are more than 797 refs the output of git rev-parse is too large to fit into the string, ids.
>>
>> 797 refs = 32,677 bytes.
>> 798 refs = 32,718 bytes my guess is a little too close for comfort to 32,768 bytes.
>>
>> As I was deleting refs locally the error message would change from '3' to any char [A-Z,0-9].

I cannot reproduce the error. I have a repository with 100 commits in a
linear history and 5000 refs (50 refs per commit). They are named
refs/heads/branch-XXXX. I don't see any problems with 'gitk --all'.

> +proc git-rev-parse {args} {
> +    set ids {}
> +    set pipe [open |[linsert $args 0 git rev-parse] r]
> +    while {[gets $pipe line] != -1} {
> +        lappend ids $line
> +    }
> +    close $pipe
> +    return $ids
> +}
> +    
>  proc parseviewrevs {view revs} {
>      global vposids vnegids
>  
>      if {$revs eq {}} {
>  	set revs HEAD
>      }
> -    if {[catch {set ids [eval exec git rev-parse $revs]} err]} {
> +    if {[catch {set ids [git-rev-parse $revs]} err]} {

Sorry, but you are changing the wrong end of git rev-parse. The limit is
on the command line, but if you run 'gitk --all', then $revs is simply
"--all" - no limit is exceeded. You changed the output of rev-parse, but
there is no limit on how much Tcl can eat of rev-parse's output.

The error must be in some other git invocation.

-- Hannes

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

[Paul Mackerras]

Pat Thoyts writes:

> Tcl strings can eat all your memory. However, there is a limit to the
> size of the command line argument passed to CreateProcess.  MSDN says
> of the lpCommandLine parameter:
>   "The maximum length of this string is 32K characters."
> A solution for this case will be to use a pipe to read the responses
> instead of having it all returned to the caller.
> The following patch might be sufficient:

I knew about the 32k command-line limit under windows, but I don't see
how that applies in this case unless it is $revs that is too long (and
if that is the case then I don't see how your patch helps).  Is there
also a 32k limit on the size of data returned by a command executed
with [exec]?

Paul.

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

[Murphy, John]

Johannes Sixt writes:

> I cannot reproduce the error. I have a repository with 100 commits in
a
> linear history and 5000 refs (50 refs per commit). They are named
> refs/heads/branch-XXXX. I don't see any problems with 'gitk --all'.

That still leave you with only 100 unique refs.
You need over 797 unique refs.

> The error must be in some other git invocation.

I put many debug pop-ups around the code and I believe that this the
call that is dying on.
This is the only part of the code that has the error text "fatal:
ambiguous argument".


Paul Mackerras writes:

> I knew about the 32k command-line limit under windows, but I don't see
> how that applies in this case unless it is $revs that is too long (and
> if that is the case then I don't see how your patch helps).  Is there
> also a 32k limit on the size of data returned by a command executed
> with [exec]?

In this case $revs is "--all"

I believe what I am experiencing is a 32K limit with [exec]

Additional info:
My repo has: 17,737 commits

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

[Johannes Sixt]

Murphy, John schrieb:
> Paul Mackerras writes:
> 
>> I knew about the 32k command-line limit under windows, but I don't see
>> how that applies in this case unless it is $revs that is too long (and
>> if that is the case then I don't see how your patch helps).  Is there
>> also a 32k limit on the size of data returned by a command executed
>> with [exec]?
> 
> In this case $revs is "--all"
> 
> I believe what I am experiencing is a 32K limit with [exec]

But in order to have a $revs that exceeds 32K, you would already have to
invoke gitk with a huge command line that exceeds the limit (but this is
not possible), no?

How do you run gitk?

-- Hannes

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

[Murphy, John]

Johannes Sixt writes:

> But in order to have a $revs that exceeds 32K, you would already have
to
> invoke gitk with a huge command line that exceeds the limit (but this
is
> not possible), no?

>How do you run gitk?

gitk --all

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

[Johannes Sixt]

Murphy, John schrieb:
> Johannes Sixt writes:
> 
>> But in order to have a $revs that exceeds 32K, you would already have
> to
>> invoke gitk with a huge command line that exceeds the limit (but this
> is
>> not possible), no?
> 
>> How do you run gitk?
> 
> gitk --all

I see it. Here is a bash script that creates a repository that reproduces
the error. It is important that refs which sort alphabetically earlier
also point to earlier commits.

-- snip --
#!/bin/bash
git init
echo initial > file && git add file && git commit -m initial
for ((i = 0; i < 1000; i++))
do
	echo $i > file &&
	git commit -m $i file > /dev/null &&
	printf -v l "branch-%04d" $i &&
	git update-ref refs/heads/$l HEAD
done
git gc
-- snip --

On Windows, 'gitk --all' starts with branch-0797, on Linux it starts with
branch-0999 aka master.

I'm just throwing this out to interested parties; I'll not look into it at
this time.

Thanks,
-- Hannes

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

[Pat Thoyts]

Johannes Sixt <j.sixt@viscovery.net> writes:

>Murphy, John schrieb:
>
>On Windows, 'gitk --all' starts with branch-0797, on Linux it starts with
>branch-0999 aka master.

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.
John, if this works for you can you also check that editing and
creating new gitk views on your real repository continues to work ok.

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
     }
     if {[catch {set ids [eval exec git rev-parse $revs]} err]} {
        # we get stdout followed by stderr in $err

-- 
Pat Thoyts                            http://www.patthoyts.tk/
PGP fingerprint 2C 6E 98 07 2C 59 C8 97  10 CE 11 E6 04 E0 B9 DD

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

[Murphy, John]

Pat Thoyts writes:

> John, if this works for you can you also check that editing and
> creating new gitk views on your real repository continues to work ok.

Works like a charm.
I look forward to it coming in a new version of git.
Thank you very much.

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

[Junio C Hamano]

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"?

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

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

[Junio C Hamano]

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

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

[Pat Thoyts]

(nobody) writes:

>Junio C Hamano <gitster@pobox.com> writes:
>
>> Pat Thoyts <patthoyts@users.sourceforge.net> writes:
>>
>> 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.

Not exactly. The problem is that the call to parseviewrevs will expand
--all into a tcl list containing all the revision ids. We can do some
testing if we dig into this with the tcl console:
 % llength [set revs [parseviewrevs {} --all]]
 1001
 % string length $revs
 41040
In start_rev-list this list gets added to the command line for git-log
in the $args variable. This is always going to exceed windows'
commandline limit (32k).

Some testing shows that a number of rev-parse arguments do not get
expanded into a list of ids. All these can be ignored. But --all,
--tags and --branches do. Maybe --remotes as well.
These arguments are accetable to git-log so it looks to me like they
can be left as-is.

The vposids and vnegids arrays are getting used for something
though. So the patch is not complete. They appear to be caching the
set of revisions present in the current view for use in updatecommits
to do something.

So - needs more work.

-- 
Pat Thoyts                            http://www.patthoyts.tk/
PGP fingerprint 2C 6E 98 07 2C 59 C8 97  10 CE 11 E6 04 E0 B9 DD

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

[Alex Riesen]

On Tue, Sep 22, 2009 at 02:39, Junio C Hamano <gitster@pobox.com> wrote:
> Pat Thoyts <patthoyts@users.sourceforge.net> writes:
>>      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).
>

And it is a race condition. By the time git log has got --all list of references
it may look completely different to what gitk has.

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

[Paul Mackerras]

Alex Riesen writes:

> On Tue, Sep 22, 2009 at 02:39, Junio C Hamano <gitster@pobox.com> wrote:
> > Pat Thoyts <patthoyts@users.sourceforge.net> writes:
> >>      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).
> >
> 
> And it is a race condition. By the time git log has got --all list of references
> it may look completely different to what gitk has.

Yes, exactly.  Until git log understands --stdin, I think the only
real solution is to disable the view update optimization on windows.

Paul.

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

[Paul Mackerras]

Pat Thoyts writes:

> 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.

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

[Junio C Hamano]

Paul Mackerras <paulus@samba.org> writes:

> 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.

Hmph.

The negative ones you can learn by giving --boundary, but I do not think
the set of starting points are something you can get out of log output.

Even if you could, you would have the same issue giving them from the
command line anyway.  The right solution would likely to be to give the
same --stdin option as rev-list to "git log", I think.

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

[Paul Mackerras]

Junio C Hamano writes:

> Paul Mackerras <paulus@samba.org> writes:
> 
> > 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.
> 
> Hmph.
> 
> The negative ones you can learn by giving --boundary, but I do not think
> the set of starting points are something you can get out of log output.
> 
> Even if you could, you would have the same issue giving them from the
> command line anyway.  The right solution would likely to be to give the
> same --stdin option as rev-list to "git log", I think.

A --stdin option to git log would be great, but it doesn't seem to be
implemented yet.  How hard would it be to add?

Paul.

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

[Junio C Hamano]

Paul Mackerras <paulus@samba.org> writes:

> Junio C Hamano writes:
>
>> Even if you could, you would have the same issue giving them from the
>> command line anyway.  The right solution would likely to be to give the
>> same --stdin option as rev-list to "git log", I think.
>
> A --stdin option to git log would be great, but it doesn't seem to be
> implemented yet.  How hard would it be to add?

Fairly trivial.  Only lightly tested with things like:

    $ (echo ^master; echo next) | ./git log --stdin
    $ (echo ^master; echo jc/log-tz) | ./git format-patch --stdin --stdout

I am not signing this off because...

 - I do not want to think about what would happen if you give "-p" option,
   nor if our "pager" infrastructure is set up to allow us to do a
   sensible thing; and

 - This disables --stdin for blame as it wants to read contents from its
   standard input when given "-" as the file argument.  I suspect there
   probably are similar commands that use setup_revisions() and do not use
   a "--stdin" option but still read from the standard input, and they
   need to be fixed similarly, but I want somebody else to do the audit.

At least, not yet.

That is, making it work for "log" is trivial---making sure it won't break
unsuspecting bystanders is much more time-consuming work.

-- >8 --
Subject: teach --stdin to "log" family

Move the logic to read revs from standard input that rev-list knows about
from it to revision machinery, so that all the users of setup_revisions()
can feed the list of revs from the standard input when "--stdin" is used
on the command line.

Allow some users of the revision machinery that want different semantics
from the "--stdin" option to disable it by setting an option in the
rev_info structure.

This also cleans up the kludge made to bundle.c via cut and paste.

---
 builtin-blame.c     |    1 +
 builtin-diff-tree.c |    1 +
 builtin-rev-list.c  |    7 -------
 bundle.c            |    7 -------
 revision.c          |   15 +++++++++++++--
 revision.h          |    4 ++--
 6 files changed, 17 insertions(+), 18 deletions(-)

diff --git a/builtin-blame.c b/builtin-blame.c
index 7512773..b0aa530 100644
--- a/builtin-blame.c
+++ b/builtin-blame.c
@@ -2352,6 +2352,7 @@ parse_done:
 			die_errno("cannot stat path '%s'", path);
 	}
 
+	revs.disable_stdin = 1;
 	setup_revisions(argc, argv, &revs, NULL);
 	memset(&sb, 0, sizeof(sb));
 
diff --git a/builtin-diff-tree.c b/builtin-diff-tree.c
index 79cedb7..2380c21 100644
--- a/builtin-diff-tree.c
+++ b/builtin-diff-tree.c
@@ -104,6 +104,7 @@ int cmd_diff_tree(int argc, const char **argv, const char *prefix)
 	git_config(git_diff_basic_config, NULL); /* no "diff" UI options */
 	opt->abbrev = 0;
 	opt->diff = 1;
+	opt->disable_stdin = 1;
 	argc = setup_revisions(argc, argv, opt, NULL);
 
 	while (--argc > 0) {
diff --git a/builtin-rev-list.c b/builtin-rev-list.c
index 42cc8d8..f6a56f3 100644
--- a/builtin-rev-list.c
+++ b/builtin-rev-list.c
@@ -306,7 +306,6 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
 	struct rev_info revs;
 	struct rev_list_info info;
 	int i;
-	int read_from_stdin = 0;
 	int bisect_list = 0;
 	int bisect_show_vars = 0;
 	int bisect_find_all = 0;
@@ -349,12 +348,6 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
 			bisect_show_vars = 1;
 			continue;
 		}
-		if (!strcmp(arg, "--stdin")) {
-			if (read_from_stdin++)
-				die("--stdin given twice?");
-			read_revisions_from_stdin(&revs);
-			continue;
-		}
 		usage(rev_list_usage);
 
 	}
diff --git a/bundle.c b/bundle.c
index df95e15..a7c9987 100644
--- a/bundle.c
+++ b/bundle.c
@@ -204,7 +204,6 @@ int create_bundle(struct bundle_header *header, const char *path,
 	int i, ref_count = 0;
 	char buffer[1024];
 	struct rev_info revs;
-	int read_from_stdin = 0;
 	struct child_process rls;
 	FILE *rls_fout;
 
@@ -257,12 +256,6 @@ int create_bundle(struct bundle_header *header, const char *path,
 	argc = setup_revisions(argc, argv, &revs, NULL);
 
 	for (i = 1; i < argc; i++) {
-		if (!strcmp(argv[i], "--stdin")) {
-			if (read_from_stdin++)
-				die("--stdin given twice?");
-			read_revisions_from_stdin(&revs);
-			continue;
-		}
 		return error("unrecognized argument: %s'", argv[i]);
 	}
 
diff --git a/revision.c b/revision.c
index 9fc4e8d..45c5de8 100644
--- a/revision.c
+++ b/revision.c
@@ -953,7 +953,7 @@ int handle_revision_arg(const char *arg, struct rev_info *revs,
 	return 0;
 }
 
-void read_revisions_from_stdin(struct rev_info *revs)
+static void read_revisions_from_stdin(struct rev_info *revs)
 {
 	char line[1000];
 
@@ -1227,7 +1227,7 @@ void parse_revision_opt(struct rev_info *revs, struct parse_opt_ctx_t *ctx,
  */
 int setup_revisions(int argc, const char **argv, struct rev_info *revs, const char *def)
 {
-	int i, flags, left, seen_dashdash;
+	int i, flags, left, seen_dashdash, read_from_stdin;
 
 	/* First, search for "--" */
 	seen_dashdash = 0;
@@ -1245,6 +1245,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch
 
 	/* Second, deal with arguments and options */
 	flags = 0;
+	read_from_stdin = 0;
 	for (left = i = 1; i < argc; i++) {
 		const char *arg = argv[i];
 		if (*arg == '-') {
@@ -1283,6 +1284,16 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch
 				revs->no_walk = 0;
 				continue;
 			}
+			if (!strcmp(arg, "--stdin")) {
+				if (revs->disable_stdin) {
+					argv[left++] = arg;
+					continue;
+				}
+				if (read_from_stdin++)
+					die("--stdin given twice?");
+				read_revisions_from_stdin(revs);
+				continue;
+			}
 
 			opts = handle_revision_opt(revs, argc - i, argv + i, &left, argv);
 			if (opts > 0) {
diff --git a/revision.h b/revision.h
index b6421a6..f64637b 100644
--- a/revision.h
+++ b/revision.h
@@ -83,6 +83,8 @@ struct rev_info {
 			use_terminator:1,
 			missing_newline:1,
 			date_mode_explicit:1;
+	unsigned int	disable_stdin:1;
+
 	enum date_mode date_mode;
 
 	unsigned int	abbrev;
@@ -128,8 +130,6 @@ struct rev_info {
 #define REV_TREE_DIFFERENT	3	/* Mixed changes */
 
 /* revision.c */
-void read_revisions_from_stdin(struct rev_info *revs);
-
 typedef void (*show_early_output_fn_t)(struct rev_info *, struct commit_list *);
 extern volatile show_early_output_fn_t show_early_output;