git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [RFC 4/4 v3] Add cat-blob report fifo from fast-import to remote-helper.

From
Florian Achleitner <florian.achleitner.2.6.31@gmail.com>
Date
Jul 22, 2012, 21:03 UTC
Message-ID
<5489458.8D23shS0RV@flomedio>
In-Reply-To
<20120721154437.GC19860@burratino>
On Saturday 21 July 2012 10:44:37 Jonathan Nieder wrote:
Show 14 quoted lines
> Florian Achleitner wrote:
> > On Saturday 21 July 2012 09:48:34 Jonathan Nieder wrote:
> >> To sum up: I think we should just stick to pipes --- why all this fifo
> >> complication?
> > 
> > People didn't like pipe variant (prexec_cb not being compatible to
> > windows'
> > process creation model), so I learned about fifos and implemented a
> > (basic) fifo variant. *shrug*
> 
> Ok, can you elaborate on that?  What does it mean that preexec_cb is
> not compatible to windows' process creation model?  Don't the people
> of the future working on this code deserve to know about that, too, so
> they don't break it?

Let's discuss how to describe the solution after we decide which of the two variants we choose.

Summarizing the earlier discussion (in this thread as replies to version 1 of the patch) more verbosely: I used the prexec_cb feature of start_command which allows to install a callback function that is called just before exec. It's purpose is to close the other pipe end in each of the processes that inherited both ends after fork. The pipe is created in transport-helper.c, because it forks fast-import as well as the remote-helper, which inherit the pipe's file descriptors.

On Windows, processses are not forked but spawned from new and therefore can't inherit pipe file descriptors. So we had the idea to use a fifo, which can be opened after the creation of the process. Also there are named pipes on Windows which are similar to fifos (though I've never used them).

Mostly out of curiosity I played around with fifos and implemented this additional cat-blob-pipe feature, like a proof-of-concept.

The first version used the pipes because this feature already exists.
> 
> Come on --- I'm not asking these questions just to make your life
> difficult.  Please make it easy to understand your code changes and to
> keep them maintained.

The reason why I resent the patch in version 3 these days is that I the problem of blocking open calls leading to potential deadlocks. Of course I should tell future maintainers better what its all about, if we really want that feature.

Hope that helps, Florian

Previous: Jonathan NiederNext: Jonathan Nieder
Message 50 of 57 in “[RFC 0/4]”
  1. Florian AchleitnerJun 4, 2012
  2. 1/4 Implement a basic remote helper vor svn in C.Florian Achleitner, Jun 4, 2012
  3. 2/4 Integrate remote-svn into svn-fe/Makefile.Florian Achleitner, Jun 4, 2012
  4. 3/4 Add svndump_init_fd to allow reading dumps from arbitrary FDs.Florian Achleitner, Jun 4, 2012
  5. 4/4 Add cat-blob report pipe from fast-import to remote-helper.Florian Achleitner, Jun 4, 2012
  6. David Michael BarrJun 5, 2012
  7. Jeff KingJun 5, 2012
  8. David Michael BarrJun 5, 2012
  9. Jeff KingJun 5, 2012
  10. Florian AchleitnerJun 5, 2012
  11. Jeff KingJun 6, 2012
  12. Florian AchleitnerJun 6, 2012
  13. Johannes SixtJun 5, 2012
  14. Jeff KingJun 5, 2012
  15. Florian AchleitnerJun 5, 2012
  16. Johannes SixtJun 5, 2012
  17. David Michael BarrJun 5, 2012
  18. 0/4 Florian Achleitner, Jun 29, 2012
  19. 1/4 Implement a basic remote helper for svn in C.Florian Achleitner, Jun 29, 2012
  20. Jonathan NiederJul 2, 2012
  21. Jonathan NiederJul 6, 2012
  22. Florian AchleitnerJul 6, 2012
  23. Florian AchleitnerJul 26, 2012
  24. Jonathan NiederJul 26, 2012
  25. Florian AchleitnerJul 26, 2012
  26. Jonathan NiederJul 28, 2012
  27. Florian AchleitnerJul 30, 2012
  28. Jonathan NiederJul 30, 2012
  29. Florian AchleitnerJul 30, 2012
  30. Jonathan NiederJul 30, 2012
  31. Florian AchleitnerJul 31, 2012
  32. Jonathan NiederJul 31, 2012
  33. Florian AchleitnerAug 1, 2012
  34. Jonathan NiederAug 1, 2012
  35. Florian AchleitnerAug 12, 2012
  36. Jonathan NiederAug 12, 2012
  37. Florian AchleitnerAug 12, 2012
  38. Jonathan NiederAug 12, 2012
  39. Jonathan NiederAug 12, 2012
  40. Junio C HamanoJul 26, 2012
  41. Florian AchleitnerJul 30, 2012
  42. Steven MichalskeJul 26, 2012
  43. 2/4 Integrate remote-svn into svn-fe/Makefile.Florian Achleitner, Jun 29, 2012
  44. 3/4 Add svndump_init_fd to allow reading dumps from arbitrary FDs.Florian Achleitner, Jun 29, 2012
  45. 4/4 Add cat-blob report fifo from fast-import to remote-helper.Florian Achleitner, Jun 29, 2012
  46. Florian AchleitnerJul 21, 2012
  47. Jonathan NiederJul 21, 2012
  48. Florian AchleitnerJul 21, 2012
  49. Jonathan NiederJul 21, 2012
  50. Florian AchleitnerJul 22, 2012
  51. Jonathan NiederJul 22, 2012
  52. Jonathan NiederJul 21, 2012
  53. Jonathan NiederJul 26, 2012
  54. Florian AchleitnerJul 26, 2012
  55. Jonathan NiederJul 26, 2012
  56. Florian AchleitnerJul 27, 2012
  57. Jonathan NiederJul 28, 2012

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.