On Saturday 28 July 2012 02:00:31 Jonathan Nieder wrote: > Thanks for explaining. Now we've discussed a few different approproaches, > none of which is perfect. > > a. use --cat-blob-fd, no FIFO > > Doing this unconditionally would break platforms that don't support > --cat-blob-fd=(descriptor >2), like Windows, so we'd have to: > > * Make it conditional --- only do it (1) we are not on Windows and > (2) the remote helper requests backflow by advertising the > import-bidi capability. > > * Let the remote helper know what's going on by using > "import-bidi" instead of "import" in the command stream to > initiate the import.
Generally I like your prefered solution. I think there's one problem: The pipe needs to be created before the fork, so that the fd can be inherited. There is no way of creating it if the remote-helper advertises a capability, because it is already forked then. This would work with fifos, though. We could: - add a capability: bidi-import. - make transport-helper create a fifo if the helper advertises it. - add a command for remote-helpers, like 'bidi-import <pipename>' that makes the remote helper open the fifo at <pipename> and use it. - fast-import is forked after the helper, so we do already know if there will be a back-pipe. If yes, open it in transport-helper and pass the fd as command line argument cat-blob-fd. --> fast-import wouldn't need to be changed, but we'd use a fifo, and we get rid of the env-vars. (I guess it could work on windows too). What do you think? > > b. use envvars to pass around FIFO path > > This complicates the fast-import interface and makes debugging hard. > It would be nice to avoid this if we can, but in case we can't, it's > nice to have the option available. > > c. transport-helper.c uses FIFO behind the scenes. > > Like (a), except it would require a fast-import tweak (boo) and > would work on Windows (yea) > > d. use --cat-blob-fd with FIFO > > Early scripted remote-svn prototypes did this to fulfill "fetch" > requests. > > It has no advantage over "use --cat-blob-fd, no FIFO" except being > easier to implement as a shell script. I'm listing this just for > comparison; since (a) looks better in every way, I don't see any > reason to pursue this one. > > Since avoiding deadlocks with bidirectional communication is always a > little subtle, it would be nice for this to be implemented once in > transport-helper.c rather than each remote helper author having to > reimplement it again. As a result, my knee-jerk ranking is a > c > > b > d. > > Sane? > Jonathan -- 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
