Re: [PATCH v5 02/10] add--helper: create builtin helper for interactive add

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Mar 8, 2019, 20:48 UTC
Message-ID: <nycvar.QRO.7.76.6.1903082138350.41@tvgsbejvaqbjf.bet>
In-Reply-To: <xmqqzhqpt483.fsf@gitster-ct.c.googlers.com>

Hi Junio,

On Thu, 21 Feb 2019, Junio C Hamano wrote:

Show 20 quoted lines

> "Daniel Ferreira via GitGitGadget" <gitgitgadget@gmail.com> writes:
> 
> > diff --git a/git.c b/git.c
> > index 2dd588674f..cb42591f37 100644
> > --- a/git.c
> > +++ b/git.c
> > @@ -444,6 +444,7 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv)
> >  
> >  static struct cmd_struct commands[] = {
> >  	{ "add", cmd_add, RUN_SETUP | NEED_WORK_TREE },
> > +	{ "add--helper", cmd_add__helper, RUN_SETUP | NEED_WORK_TREE },
> >  	{ "am", cmd_am, RUN_SETUP | NEED_WORK_TREE },
> >  	{ "annotate", cmd_annotate, RUN_SETUP | NO_PARSEOPT },
> >  	{ "apply", cmd_apply, RUN_SETUP_GENTLY },
> 
> When adding a more complex replacement command we often cannot write
> RUN_SETUP nor NEED_WORK_TREE here and instead do the job ourselves
> in cmd_ourcommand() the hard way.  But because this one is only for
> helping "add -i", we can let the git.c::run_builtin() aka "git
> potty" do the hard part, which is very nice.

Sadly, I ran into a quite curious (and seemingly equally insurmountable) problem with this approach: when I use `system()` to execute a Git built-in from the Perl script, and when that built-in then consumes even a single byte of stdin, that file descriptor gets closed in the Perl process, and no amount of file descriptor duplication and redirecting that I could come up helped. After calling the built-in helper to run, say, the `update` command, the main interactive add process always exited on me due to EOF on stdin, and I could not make it work.

To be sure, this happens only with MSYS2's Perl. If I try the same on Linux (WSL, really), everything works Just Fine.

So my current approach is actually the other way round: I started implementing the main loop in C, and implemented all commands except for `patch` so far, and I think I will just hand off to the Perl script for that command for now.

Unfortunately that means that my current state looks already quite a bit different than what you reviewed. My hope is, however, that it will make for a nice "story" once I have a version to show that I think could be applied as-is.

Ciao, Dscho