Re: [PATCH] git-remote-testgit: fix race when spawning fast-import

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 22, 2012, 04:50 UTC
Message-ID: <xmqqty0cxtcd.fsf@junio.mtv.corp.google.com>
In-Reply-To: <20120421234555.GA11808@padd.com>

Pete Wyckoff <pw@padd.com> writes:

Show 33 quoted lines

> Fix this by setting stdin to unbuffered.
>
> Signed-off-by: Pete Wyckoff <pw@padd.com>
> ---
>
> This is the one that might be suitable for upstream.  It fixes
> the problem with this particular remote helper.
>
>  git-remote-testgit.py     |    7 +++++++
>  t/t5800-remote-helpers.sh |   11 +++++++++++
>  2 files changed, 18 insertions(+)
>
> diff --git a/git-remote-testgit.py b/git-remote-testgit.py
> index 3dc4851..5f3ebd2 100644
> --- a/git-remote-testgit.py
> +++ b/git-remote-testgit.py
> @@ -22,6 +22,7 @@ except ImportError:
>      _digest = sha.new
>  import sys
>  import os
> +import time
>  sys.path.insert(0, os.getenv("GITPYTHONLIB","."))
>  
>  from git_remote_helpers.util import die, debug, warn
> @@ -204,6 +205,11 @@ def read_one_line(repo):
>      """Reads and processes one command.
>      """
>  
> +    sleepy = os.environ.get("GIT_REMOTE_TESTGIT_SLEEPY")
> +    if sleepy:
> +        debug("Sleeping %d sec before readline" % int(sleepy))
> +        time.sleep(int(sleepy))
> +

If I understand your explanation correctly, the primary purpose of the remote-testgit is to test the parts of the system that talk to remote helpers that are used in production in the t/t5800 script, and this "sleep" is to make it easier to trigger the particular bug you fixed in *this* script. The bug is _not_ in the parts of the system being tested, but is in this test scaffolding.

If that is the case, then it should not be enabled unconditionally. When somebody wants to see if remote-testgit was broken again (perhaps after observing occassional hangs), the environment should be set when running the test, but not in t5800.