[PATCH v2 5/5] Documentation: add caveats about I/O buffering for check-{attr,ignore}

2013-04-11 Thread Adam Spiers
check-attr and check-ignore have the potential to deadlock callers
which do not read back the output in real-time.  For example, if a
caller writes N paths out and then reads N lines back in, it risks
becoming blocked on write() to check-*, and check-* is blocked on
write back to the caller.  Somebody has to buffer; the pipe buffers
provide some leeway, but they are limited.

Thanks to Peff for pointing this out:

http://article.gmane.org/gmane.comp.version-control.git/220534

Signed-off-by: Adam Spiers g...@adamspiers.org
---
 Documentation/git-check-attr.txt   |  5 +
 Documentation/git-check-ignore.txt |  5 +
 Documentation/git.txt  | 16 +---
 3 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/Documentation/git-check-attr.txt b/Documentation/git-check-attr.txt
index 5abdbaa..a7be80d 100644
--- a/Documentation/git-check-attr.txt
+++ b/Documentation/git-check-attr.txt
@@ -56,6 +56,11 @@ being queried and info can be either:
 'set';;when the attribute is defined as true.
 value;;  when a value has been assigned to the attribute.
 
+Buffering happens as documented under the `GIT_FLUSH` option in
+linkgit:git[1].  The caller is responsible for avoiding deadlocks
+caused by overfilling an input buffer or reading from an empty output
+buffer.
+
 EXAMPLES
 
 
diff --git a/Documentation/git-check-ignore.txt 
b/Documentation/git-check-ignore.txt
index 7e3cabc..8e1f7ab 100644
--- a/Documentation/git-check-ignore.txt
+++ b/Documentation/git-check-ignore.txt
@@ -81,6 +81,11 @@ not.  (Without this option, it would be impossible to tell 
whether the
 absence of output for a given file meant that it didn't match any
 pattern, or that the output hadn't been generated yet.)
 
+Buffering happens as documented under the `GIT_FLUSH` option in
+linkgit:git[1].  The caller is responsible for avoiding deadlocks
+caused by overfilling an input buffer or reading from an empty output
+buffer.
+
 EXIT STATUS
 ---
 
diff --git a/Documentation/git.txt b/Documentation/git.txt
index 6a875f2..eecdb15 100644
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -808,13 +808,15 @@ for further details.
 
 'GIT_FLUSH'::
If this environment variable is set to 1, then commands such
-   as 'git blame' (in incremental mode), 'git rev-list', 'git log',
-   and 'git whatchanged' will force a flush of the output stream
-   after each commit-oriented record have been flushed.   If this
-   variable is set to 0, the output of these commands will be done
-   using completely buffered I/O.   If this environment variable is
-   not set, Git will choose buffered or record-oriented flushing
-   based on whether stdout appears to be redirected to a file or not.
+   as 'git blame' (in incremental mode), 'git rev-list', 'git
+   log', 'git check-attr', 'git check-ignore', and 'git
+   whatchanged' will force a flush of the output stream after
+   each commit-oriented record have been flushed.  If this
+   variable is set to 0, the output of these commands will be
+   done using completely buffered I/O.  If this environment
+   variable is not set, Git will choose buffered or
+   record-oriented flushing based on whether stdout appears to be
+   redirected to a file or not.
 
 'GIT_TRACE'::
If this variable is set to 1, 2 or true (comparison
-- 
1.8.2.1.342.gfa7285d

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 5/5] Documentation: add caveats about I/O buffering for check-{attr,ignore}

2013-04-11 Thread Junio C Hamano
Adam Spiers g...@adamspiers.org writes:

 diff --git a/Documentation/git-check-ignore.txt 
 b/Documentation/git-check-ignore.txt
 index 7e3cabc..8e1f7ab 100644
 --- a/Documentation/git-check-ignore.txt
 +++ b/Documentation/git-check-ignore.txt
 @@ -81,6 +81,11 @@ not.  (Without this option, it would be impossible to tell 
 whether the
  absence of output for a given file meant that it didn't match any
  pattern, or that the output hadn't been generated yet.)
  
 +Buffering happens as documented under the `GIT_FLUSH` option in
 +linkgit:git[1].  The caller is responsible for avoiding deadlocks
 +caused by overfilling an input buffer or reading from an empty output
 +buffer.
 +
  EXIT STATUS
  ---
  
 diff --git a/Documentation/git.txt b/Documentation/git.txt
 index 6a875f2..eecdb15 100644
 --- a/Documentation/git.txt
 +++ b/Documentation/git.txt
 @@ -808,13 +808,15 @@ for further details.
  
  'GIT_FLUSH'::
   If this environment variable is set to 1, then commands such
 - as 'git blame' (in incremental mode), 'git rev-list', 'git log',
 - and 'git whatchanged' will force a flush of the output stream
 - after each commit-oriented record have been flushed.   If this
 - variable is set to 0, the output of these commands will be done
 - using completely buffered I/O.   If this environment variable is
 - not set, Git will choose buffered or record-oriented flushing
 - based on whether stdout appears to be redirected to a file or not.
 + as 'git blame' (in incremental mode), 'git rev-list', 'git
 + log', 'git check-attr', 'git check-ignore', and 'git
 + whatchanged' will force a flush of the output stream after
 + each commit-oriented record have been flushed.  If this
 + variable is set to 0, the output of these commands will be
 + done using completely buffered I/O.  If this environment
 + variable is not set, Git will choose buffered or
 + record-oriented flushing based on whether stdout appears to be
 + redirected to a file or not.

Reflowing of the text is very much unappreciated X-.  

It took me five minutes to spot that you only added check-attr and
check-ignore and forgot to adjust that commit-oriented record to
an updated reality, where you now have commands that produce
non-commit-oriented record to the output.

It would have been far simpler to review if it were like this, don't
you think?

   If this environment variable is set to 1, then commands such
   as 'git blame' (in incremental mode), 'git rev-list', 'git log',
 - and 'git whatchanged' will force a flush of the output stream
 - after each commit-oriented record have been flushed.   If this
 + 'git check-attr', 'git check-ignore', and 'git whatchanged' will
 + force a flush of the output stream
 + after each record have been flushed.   If this
   variable is set to 0, the output of these commands will be done
   using completely buffered I/O.   If this environment variable is
   not set, Git will choose buffered or record-oriented flushing
   based on whether stdout appears to be redirected to a file or not.

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html