back to the pipe-topic.

On Wednesday 01 August 2012 12:42:48 Jonathan Nieder wrote:
> Hi again,
> Florian Achleitner wrote:
> > When the first line arrives at the remote-helper, it starts importing one
> > line at a time, leaving the remaining lines in the pipe.
> > For importing it requires the data from fast-import, which would be mixed
> > with import lines or queued at the end of them.
> Oh, good catch.
> The way it's supposed to work is that in a bidi-import, the remote
> helper reads in the entire list of refs to be imported and only once
> the newline indicating that that list is over arrives starts writing
> its fast-import stream.  We could make this more obvious by not
> spawning fast-import until immediately before writing that newline.
> This needs to be clearly documented in the git-remote-helpers(1) page
> if the bidi-import command is introduced.
> If a remote helper writes commands for fast-import before that newline
> comes, that is a bug in the remote helper, plain and simple.  It might
> be fun to diagnose this problem:

This would require all existing remote helpers that use 'import' to be ported 
to the new concept, right? Probably there is no other..

>       static void pipe_drained_or_die(int fd, const char *msg)
>       {
>               char buf[1];
>               int flags = fcntl(fd, F_GETFL);
>               if (flags < 0)
>                       die_errno("cannot get pipe flags");
>               if (fcntl(fd, F_SETFL, flags | O_NONBLOCK))
>                       die_errno("cannot set up non-blocking pipe read");
>               if (read(fd, buf, 1) > 0)
>                       die("%s", msg);
>               if (fcntl(fd, F_SETFL, flags))
>                       die_errno("cannot restore pipe flags");
>       }
>       ...
>       for (i = 0; i < nr_heads; i++) {
>               write "import %s\n", to_fetch[i]->name;
>       }
>               sleep(1);
>       pipe_drained_or_die("unexpected output from remote helper before
> fast-import launch");
>       if (get_importer(transport, &fastimport))
>               die("couldn't run fast-import");
>       write_constant(data->helper->in, "\n");

I still don't believe that sharing the input pipe of the remote helper is 
worth the hazzle.
It still requires an additional pipe to be setup, the one from fast-import to 
the remote-helper, sharing one FD at the remote helper.
It still requires more than just stdin, stdout, stderr.

I would suggest to use a fifo. It can be openend independently, after forking 
and on windows they have named pipes with similar semantics, so I think this 
could be easily ported. 
I would suggest the following changes:
- add a capability to the remote helper 'bidi-import', or 'bidi-pipe'. This 
signals that the remote helper requires data from fast-import.

- add a command 'bidi-import', or 'bidi-pipe' that is tells the remote helper 
which filename the fifo is at, so that it can open it and read it when it 
handles 'import' commands.

- transport-helper.c creates the fifo on demand, i.e. on seeing the capability, 
in the gitdir or in /tmp.

- fast-import gets the name of the fifo as a command-line argument. The 
alternative would be to add a command, but that's not allowed, because it 
changes the stream semantics.
Another alternative would be to use the existing --cat-pipe-fd argument. But 
that requires to open the fifo before execing fast-import and makes us 
dependent on the posix model of forking and inheriting file descriptors, while 
opening a fifo in fast-import would not.
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to