Re: [PATCH 6/9] difftool: replace system call with Git::command_noisy

From: Alex Riesen <raa.lkml@gmail.com>
Date: Mar 17, 2012, 10:50 UTC
Message-ID: <CALxABCYiOpQavW3qz+Xx-qjadaF3sAQ3DHAAwzRRBDXm6MAnOw@mail.gmail.com>
In-Reply-To: <CAJDDKr4+0iWoZhxo6kMVa0YUtDzmrH=XTZnDqQdbnM6TJ41UDg@mail.gmail.com>

Resend for vger archives. Damn that Android GMail client.

On Sat, Mar 17, 2012 at 03:48, David Aguilar <davvid@gmail.com> wrote:

Show 22 quoted lines

> On Fri, Mar 16, 2012 at 6:59 PM, Tim Henigan <tim.henigan@gmail.com> wrote:
>> The Git.pm module includes functions intended to standardize working
>> with Git repositories in Perl scripts. This commit teaches difftool
>> to use Git::command_noisy rather than a system call to run the diff
>> command.
>
> Git::command_noisy() calls _cmd_exec() which calls _execv_git_cmd()
> which does a fork() + exec('git', @_) + waitpid();
>
> We were avoiding exec() for portability reasons, as Alex explained in
> 677fbff88f368ed6ac52438ddbb530166ec1d5d1:
>
> # ActiveState Perl for Win32 does not implement POSIX semantics of
> # exec* system call. It just spawns the given executable and finishes
> # the starting program, exiting with code 0.
> # system will at least catch the errors returned by git diff,
> # allowing the caller of git difftool better handling of failures.
>
> Is this no longer a concern?  Does Git.pm need a similar portability
> caveat, or  does it avoid the problem altogether since it uses fork()
> + exec() + waitpid()?  (if this is true then it implies that this
> change is fine).

It _might_ work. Cygwin kind of has fork(2), it even works (kind of: it is a *very* expensive thing to do). There are also other ifs and whens, but it is worth a test. It's a nice clean up to have.