Re: [RFC/PATCH 0/3] fix "Funny: git -p submodule summary"

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Jan 27, 2009, 16:31 UTC
Message-ID: <alpine.DEB.1.00.0901271728130.3586@pacific.mpi-cbg.de>
In-Reply-To: <20090127062512.GA10487@coredump.intra.peff.net>

Hi,

On Tue, 27 Jan 2009, Jeff King wrote:

Show 17 quoted lines

> On Thu, Jan 08, 2009 at 04:07:08PM +0100, Johannes Schindelin wrote:
> 
> > Just try this with a submodule that has more changes than fit on a 
> > screen:
> > 
> > 	$ git -p submodule summary
> > 
> > In my tests, it consistently fscks up my console.  I wonder if this is 
> > related to ea27a18(spawn pager via run_command interface).
> 
> OK, here is a patch series that fixes the problem:
> 
>   1/3: git: s/run_command/run_builtin/
>   2/3: run_command: handle missing command errors more gracefully
>   3/3: git: use run_command to execute dashed externals
> 
> 1 is a cleanup, 2 is infrastructure support, and 3 is the actual fix.

I like the patch series, well designed and concise (especially with the fixes Hannes proposed).

Show 6 quoted lines

> There are two potential downsides to the fix:
> 
>  1. There is an extra fork and a parent process sitting in memory for
>     dashed externals. This is pretty necessary to any fix, since
>     something has to wait to do pager cleanup, and we can't rely on the
>     child to do so.

Actually, I think this is a good thing; that way, we can catch segmentation fault properly and display an error message in the pager. That was not possible previously.

>  2. A failed attempt to execute a dashed external results in an extra
>     fork. For builtins, this has no impact, since they take precedence.
>     For aliases, though, it means we will do an extra fork before
>     realizing that there is no dashed external and trying the alias.

All the more reason to build more programs in :-)

Ciao, Dscho