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

Re: Designing the filter process protocol (was: Re: [PATCH v3 10/10] convert: add filter.<driver>.process option)

From
Lars Schneider <larsxschneider@gmail.com>
Date
Aug 5, 2016, 10:32 UTC
Message-ID
<FE7B04D2-C017-42AF-BD30-6160D251028C@gmail.com>
In-Reply-To
<607c07fe-5b6f-fd67-13e1-705020c267ee@gmail.com>
Show 57 quoted lines
> On 03 Aug 2016, at 20:30, Jakub Narębski <jnareb@gmail.com> wrote:
> 
> The ultimate goal is to be able to run filter drivers faster for both `clean`
> and `smudge` operations.  This is done by starting filter driver once per
> git command invocation, instead of once per file being processed.  Git needs
> to pass actual contents of files to filter driver, and get its output.
> 
> We want the protocol between Git and filter driver process to be extensible,
> so that new features can be added without modifying protocol.
> 
> 
> 1. CONFIGURATION
> 
> As I wrote, there are different ways of configuring new-type filter driver:
> 
> ...
> 
> # Using a single variable for new filter type, and decide on which phase
>   (which operation) is supported by filter driver during the handshake
>   *(current approach)*
> 
>   	[filter "protocol"]
>   		process = rot13-filtes.pl
> 
>   PROS: per-file and per-command filters possible with precedence rule;
>         extensible to other types of drivers: textconv, diff, etc.
>         only one invocation for commands which use both clean and smudge
>   CONS: need single driver to be responsible for both clean and smudge;
>         need to run driver to know that it does not support given
>           operation (workaround exists)
> 
> 
> 2. HANDSHAKE (INITIALIZATION)
> 
> Next, there is deciding on and designing the handshake between Git (between
> Git command) and the filter driver process.  With the `filter.<driver>.process`
> solution the driver needs to tell which operations among (for now) "clean"
> and "smudge" it does support.  Plus it provides a way to extend protocol,
> adding new features, like support for streaming, cleaning from file or
> smudging to file, providing size upfront, perhaps even progress report.
> 
> Current handshake consist of filter driver printing a signature, version
> number and capabilities, in that order.  Git checks that it is well formed
> and matches expectations, and notes which of "clean" and "smudge" operations
> are supported by the filter.
> 
> There is no interaction from the Git side in the handshake, for example to
> set options and expectations common to all files being filtered.  Take
> one possible extension of protocol: supporting streaming.  The filter
> driver needs to know whether it needs to read all the input, or whether
> it can start printing output while input is incoming (e.g. to reduce
> memory consumption)... though we may simply decide it to be next version
> of the protocol.
> 
> On the other hand if the handshake began with Git sending some initializer
> info to the filter driver, we probably could detect one-shot filter
> misconfigured as process-filter.
OK, I'll look into this.
Show 6 quoted lines
> Note that we need some way of deciding where handshake ends, either by
> specifying number of entries (currently: three lines / pkt-line packets),
> or providing some terminator ("smart" transport protocol uses flush packet
> for this).
> 
> ...
Would you be OK with Peff's suggestion?
  version=2
  clean=true
  smudge=true
  0000
http://public-inbox.org/git/20160803224619.bwtbvmslhuicx2qi%40sigill.intra.peff.net/
Show 43 quoted lines
> 3. SENDING CONTENTS (FILE TO BE FILTERED AND FILTER OUTPUT)
> 
> Next thing to design is decision how to send contents to be filtered
> to the filter driver process, and how to get filtered output from the
> filter driver process.
> 
> ...
> 
> # Send/receive data file by file, using some kind of chunking,
>   with a end-of-file marker.  The solution used by Git is
>   pkt-line, with flush packet used to signal end of file.
> 
>   This is protocol used by the current implementation.
> 
>   PROS:
>   - no need to know size upfront, so easier streaming support
>   - you can signal error that happened during output, after
>     some data were sent, as well as error known upfront
>   - tracing support for free (GIT_TRACE_PACKET)
>   CONS:
>   - filter driver program slightly more difficult to implement
>   - some negligible amount of overhead
> 
> If we want in the end to implement streaming, then the last solution
> is the way to go.
> 
> 
> 4. PER-FILE HANDSHAKE - SENDING FILE TO FILTER
> 
> Let's assume that for simplicity we want to implement (for now) only
> the synchronous (non-streaming) case, where we send whole contents
> of a file to filter driver process, and *then* read filter driver
> output.
> ...
> 
> If we are using pkt-line, then the convention is that text lines
> are terminated using LF ("\n") character.  This needs to be stated
> explicitly in the documentation for filter.<driver>.process writers.
> 
>    git> packet:  [operation] clean size=67\n
> 
> We could denote that it is operation name, but it is obvious from
> position in the stream, thus not really needed.

I would prefer not to mix command and size in one packet as it makes parsing a little more difficult.

Show 9 quoted lines
> ...
> 
> The Git would sent contents of the file to be filtered, using
> as many pack lines as needed (note: large file support needs
> to be tested, at least as expensive test).  Flush packet is
> used to signal the end of the file.
> 
>    git> packets:  <file contents>
>    git> flush packet

If expensive tests are enabled the test suite will process data larger then max pkt size.

Show 8 quoted lines
> 5. FILTER DRIVER PROCESS RESPONSE
> 
> First filter should, in my opinion, reply that it received the
> request (or the command, in the case of streaming supported).
> Also, in this response it can provide further information to
> Git process.
> 
>    git< packet: [received]  ok size=67\n

I think this would be different for real streaming and the current non-streaming... therefore it would complicate the protocol?! I wonder if it is truly necessary.

Show 5 quoted lines
> This response could be used to refuse to filter specific file
> upfront (for example if the file is not present in the artifactory
> for git-LFS solutions).
> 
>   git< packet: [rejected]  reject\n
Reject is already supported in v4.
Show 7 quoted lines
> We can even provide the reasoning to Git (maybe in the future
> extension)... or filter driver can print the explanation to the
> standard error (but then, no --quiet / --verbose support).
> 
>   git< packet: [rejected]  reject with-message\n
>   git< packet: [message]   File not found on server\n
>   git< flush packet

I think Git shouldn't care about these details. If the filter needs to tell something then it should use stderr.

Show 18 quoted lines
> Another response, which I think should be standarized, or at
> least described in the documentation, is filter driver refusing
> to filter further (e.g. git-LFS and network is down), to be not
> restarted by Git.
> 
>   git< packet: [quit]      quit msg=Server error\n
> 
> or
> 
>   git< packet: [quit]      quit Server error\n
> 
> or
> 
>   git< packet: [quit]      quit with-message\n
>   git< packet: [message]   Server error\n
>   git< flush packet
> 
> Maybe this is over-engineering, but I don't think so.
Interesting idea! I will look into this for v5!
Show 29 quoted lines
> Next comes the output from the filter driver (filtered contents),
> using possibly multiple pkt-lines, ending with a flush packet:
> 
>    git< packets:  <filtered contents>
>    git< flush packet
> 
> Note that empty file would consist of zero pack lines of contents,
> and one flush packet.
> 
> Finally, to allow handling of [resumable] errors that occurred
> during sending file contents, especially for the future streaming
> filters case, we want to confirm that we send whole file
> successfully.
> 
>    git< packet: [status]   success\n
> 
> If there was an error during process, making data receives so far
> invalid, filter driver should tell about it
> 
>    git< packet: [status]   fail\n
> 
> or
> 
>    git< packet: [status]   reject\n
> 
> This may happen for example for UCS-2 <-> UTF-8 filter when invalid
> byte sequence is encountered.  This may happen for git-LFS if the
> server fails during fetch, and spare / slave server doesn't have
> a file.
Correct!
> We may want to quit filtering at this point, and not to send another
> file.
> 
>   git< packet: [status]    quit\n

I don't get this one. Git would restart the filter as soon as it finds another file that needs to be filtered, right?

Thanks a lot for this write up!
- Lars
Previous: Jakub NarębskiNext: Lars Schneider
Message 41 of 100 in “Git filter protocol”
  1. 00/10 Git filter protocollarsxschneider@gmail.com, Jul 29, 2016
  2. 01/10 pkt-line: extract set_packet_header()larsxschneider@gmail.com, Jul 29, 2016
  3. Jakub NarębskiJul 30, 2016
  4. Lars SchneiderAug 1, 2016
  5. Jakub NarębskiAug 3, 2016
  6. Lars SchneiderAug 5, 2016
  7. 02/10 pkt-line: add direct_packet_write() and direct_packet_write_data()larsxschneider@gmail.com, Jul 29, 2016
  8. Jakub NarębskiJul 30, 2016
  9. Lars SchneiderAug 1, 2016
  10. Jakub NarębskiAug 3, 2016
  11. Lars SchneiderAug 5, 2016
  12. 03/10 pkt-line: add packet_flush_gentle()larsxschneider@gmail.com, Jul 29, 2016
  13. Jakub NarębskiJul 30, 2016
  14. Lars SchneiderAug 1, 2016
  15. Torstem BögershausenJul 31, 2016
  16. Lars SchneiderJul 31, 2016
  17. Torsten BögershausenAug 2, 2016
  18. Lars SchneiderAug 5, 2016
  19. 04/10 pkt-line: call packet_trace() only if a packet is actually sendlarsxschneider@gmail.com, Jul 29, 2016
  20. Jakub NarębskiJul 30, 2016
  21. Lars SchneiderAug 1, 2016
  22. Jakub NarębskiAug 3, 2016
  23. 05/10 pack-protocol: fix maximum pkt-line sizelarsxschneider@gmail.com, Jul 29, 2016
  24. Jakub NarębskiJul 30, 2016
  25. Lars SchneiderAug 1, 2016
  26. 07/10 convert: quote filter names in error messageslarsxschneider@gmail.com, Jul 29, 2016
  27. 06/10 run-command: add clean_on_exit_handlerlarsxschneider@gmail.com, Jul 29, 2016
  28. Johannes SixtJul 30, 2016
  29. Lars SchneiderAug 1, 2016
  30. Johannes SixtAug 2, 2016
  31. Lars SchneiderAug 2, 2016
  32. 08/10 convert: modernize testslarsxschneider@gmail.com, Jul 29, 2016
  33. 09/10 convert: generate large test files only oncelarsxschneider@gmail.com, Jul 29, 2016
  34. 10/10 convert: add filter.<driver>.process optionlarsxschneider@gmail.com, Jul 29, 2016
  35. Jakub NarębskiJul 30, 2016
  36. Jakub NarębskiJul 31, 2016
  37. Lars SchneiderJul 31, 2016
  38. Jakub NarębskiJul 31, 2016
  39. Lars SchneiderAug 1, 2016
  40. Designing the filter process protocol (was: Re: [PATCH v3 10/10] convert: add filter.<driver>.process option)Jakub Narębski, Aug 3, 2016
  41. Lars SchneiderAug 5, 2016
  42. Lars SchneiderAug 6, 2016
  43. Jakub NarębskiAug 3, 2016
  44. Jakub NarębskiJul 31, 2016
  45. Lars SchneiderAug 1, 2016
  46. Jakub NarębskiAug 4, 2016
  47. Lars SchneiderAug 3, 2016
  48. Jakub NarębskiAug 4, 2016
  49. Lars SchneiderAug 5, 2016
  50. 00/12 Git filter protocollarsxschneider@gmail.com, Aug 3, 2016
  51. 01/12 pkt-line: extract set_packet_header()larsxschneider@gmail.com, Aug 3, 2016
  52. Junio C HamanoAug 3, 2016
  53. Jeff KingAug 3, 2016
  54. Jeff KingAug 3, 2016
  55. Junio C HamanoAug 4, 2016
  56. Lars SchneiderAug 5, 2016
  57. Junio C HamanoAug 5, 2016
  58. Lars SchneiderAug 5, 2016
  59. Junio C HamanoAug 5, 2016
  60. Lars SchneiderAug 3, 2016
  61. 07/12 run-command: add clean_on_exit_handlerlarsxschneider@gmail.com, Aug 3, 2016
  62. Jeff KingAug 3, 2016
  63. Lars SchneiderAug 3, 2016
  64. Jeff KingAug 3, 2016
  65. Lars SchneiderAug 3, 2016
  66. Jeff KingAug 3, 2016
  67. Lars SchneiderAug 5, 2016
  68. Torsten BögershausenAug 5, 2016
  69. Lars SchneiderAug 5, 2016
  70. 03/12 pkt-line: add packet_flush_gentle()larsxschneider@gmail.com, Aug 3, 2016
  71. Jeff KingAug 3, 2016
  72. Junio C HamanoAug 4, 2016
  73. 02/12 pkt-line: add direct_packet_write() and direct_packet_write_data()larsxschneider@gmail.com, Aug 3, 2016
  74. 08/12 convert: quote filter names in error messageslarsxschneider@gmail.com, Aug 3, 2016
  75. 05/12 pkt-line: add functions to read/write flush terminated packet streamslarsxschneider@gmail.com, Aug 3, 2016
  76. 09/12 convert: modernize testslarsxschneider@gmail.com, Aug 3, 2016
  77. 12/12 convert: add filter.<driver>.process shutdown command optionlarsxschneider@gmail.com, Aug 3, 2016
  78. 06/12 pack-protocol: fix maximum pkt-line sizelarsxschneider@gmail.com, Aug 3, 2016
  79. 04/12 pkt-line: call packet_trace() only if a packet is actually sendlarsxschneider@gmail.com, Aug 3, 2016
  80. 11/12 convert: add filter.<driver>.process optionlarsxschneider@gmail.com, Aug 3, 2016
  81. Junio C HamanoAug 3, 2016
  82. Lars SchneiderAug 3, 2016
  83. Jeff KingAug 3, 2016
  84. Lars SchneiderAug 5, 2016
  85. Junio C HamanoAug 3, 2016
  86. Lars SchneiderAug 3, 2016
  87. Junio C HamanoAug 3, 2016
  88. Lars SchneiderAug 3, 2016
  89. Torsten BögershausenAug 5, 2016
  90. Lars SchneiderAug 5, 2016
  91. Junio C HamanoAug 5, 2016
  92. Jeff KingAug 5, 2016
  93. Lars SchneiderAug 6, 2016
  94. Jeff KingAug 6, 2016
  95. Lars SchneiderAug 6, 2016
  96. Jeff KingAug 8, 2016
  97. Lars SchneiderAug 8, 2016
  98. Jeff KingAug 8, 2016
  99. Torsten BögershausenAug 6, 2016
  100. 10/12 convert: generate large test files only oncelarsxschneider@gmail.com, Aug 3, 2016

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.