Re: master: t5800-remote-helpers.sh hangs on test "pulling from remote remote"

From: Pete Wyckoff <pw@padd.com>
Date: Apr 22, 2012, 02:17 UTC
Message-ID: <20120422021753.GB11892@padd.com>
In-Reply-To: <20120421233521.GB20473@ecki>

drizzd@aon.at wrote on Sun, 22 Apr 2012 01:35 +0200:

Show 20 quoted lines

> Hi Pete,
> 
> Thanks for solving the mystery.
> 
> On Sat, Apr 21, 2012 at 04:15:24PM -0400, Pete Wyckoff wrote:
> > 
> > Not sure yet what is the right way to fix this.  And I don't know
> > if it's just git-remote-testgit that has the problem because it
> > forks an importer, or if other remote helpers would be similarly
> > affected.
> > 
> > The easiest fix is to invoke "python -u" in
> > git-remote-testgit.py, or do
> > 
> >     sys.stdin = os.fdopen(sys.stdin.fileno(), 'r', 0)
> > 
> > But I'm not sure if this works on all systems.
> 
> Which part are you worried about? Maybe the patch below is more
> portable?

Your way works. But it doesn't handle universal newlines. I don't know if the windows platforms turn that on for stdio. In fact, I'm not sure we should be worried about sys.stdin buffering on windows at all.

Show 5 quoted lines

> > We could also consider adding a handshake after "export", to give the
> > helper time to get ready.
> 
> Not sure I understand. The helper is throwing away some input. It seems
> to me there is nothing one can do to prevent such brokeness.

The handshake would give the helper time to get its streams straightened up. It would do something like:

    read "export\n"
    fork importer, listening to stdin
    write "export-ready\n" to stdout

And git push would wait for the "export-ready" line before it unleases the stream from fast-export. But as Sverre said, that's a protocol change, which is more difficult. And maybe not warranted if we think this can also be attributed to helper brokenness.

I think we should reopen stdin like this to fix the problem now. I'll investigate if it exists on windows too, and see if it is possible to fix there too.

> It would be nice if the deadlock could be detected and if git would time
> out in such a case, instead of hanging forever. But rather long delays
> can also be the result of the remote helper legimitately waiting for
> data from a slow network.

Tough, generally.

		-- Pete

Show 53 quoted lines

> Clemens
> ---
> 
> diff --git a/git-remote-testgit.py b/git-remote-testgit.py
> index 616e772..bdd1116 100644
> --- a/git-remote-testgit.py
> +++ b/git-remote-testgit.py
> @@ -135,6 +135,22 @@ def update_local_repo(repo):
>      return repo
>  
>  
> +def read_line_fd(fd):
> +    """Read from fd until end of line or EOF, and no further.
> +    """
> +
> +    line = ''
> +
> +    while True:
> +        c = os.read(fd, 1)
> +        if c == '':
> +            break
> +        line += c
> +        if c == '\n':
> +            break
> +
> +    return line
> +
>  def do_import(repo, args):
>      """Exports a fast-import stream from testgit for git to import.
>      """
> @@ -149,7 +165,7 @@ def do_import(repo, args):
>      refs = [ref]
>  
>      while True:
> -        line = sys.stdin.readline()
> +        line = read_line_fd(sys.stdin.fileno())
>          if line == '\n':
>              break
>          if not line.startswith('import '):
> @@ -211,9 +227,7 @@ def read_one_line(repo):
>          time.sleep(int(sleepy))
>          debug("Awake, to readline")
>  
> -    line = sys.stdin.readline()
> -
> -    cmdline = line
> +    cmdline = read_line_fd(sys.stdin.fileno())
>  
>      if not cmdline:
>          warn("Unexpected EOF")
> -- 
> 1.7.10
>