Re: [PATCH] Wait for git diff to finish in git difftool

From: David Aguilar <davvid@gmail.com>
Date: Apr 22, 2009, 08:26 UTC
Message-ID: <20090422082652.GA32698@gmail.com>
In-Reply-To: <81b0412b0904220027t7afd1ed7xc57c479ef8cdb6b9@mail.gmail.com>

On  0, Alex Riesen <raa.lkml@gmail.com> wrote:

> In ActivetState Perl, exec does not wait for the started program. This
> breaks difftool tests and may cause unexpected behaviour: git difftool
> has returned, but the rest of code (diff and possibly the interactive
> program are still running in the background.

Thanks for keeping an eye on portability.

There's a tiny typo in the commit message (Active't'State) that maybe you can tweak before applying? ('you' being Junio)

For whatever it's worth,

Acked-by: David Aguilar <davvid@gmail.com>

> I usually don't care for exit code in a pure UI tool, so the kill signal
> is just ORed together with the real exit code just to provide indication
> of error.

This seems reasonable. The exit code isn't very important in the common 'show-me-the-diff' read-only scenario, and I wouldn't expect anyone to rely on difftool being exactly exit-code equivalent to git-diff.

Show 13 quoted lines

> diff --git a/git-difftool.perl b/git-difftool.perl
> index 948ff7f..bd828c2 100755
> --- a/git-difftool.perl
> +++ b/git-difftool.perl
> @@ -82,4 +82,5 @@ sub generate_command
>  }
> 
>  setup_environment();
> -exec(generate_command());
> +my $rc = system(generate_command());
> +exit($rc | ($rc >> 8));
> -- 
> 1.6.3.rc0.45.g63634

-- 
		David