Re: [PATCH v7 10/10] convert: add filter.<driver>.process option
- From
Torsten Bögershausen <tboegi@web.de>
- Date
- Sep 10, 2016, 06:29 UTC
- Message-ID
- <20160910062919.GB11001@tb-raspi>
- In-Reply-To
- <20160908182132.50788-11-larsxschneider@gmail.com>
On Thu, Sep 08, 2016 at 08:21:32PM +0200, larsxschneider@gmail.com wrote: []
Show 13 quoted lines
> +packet: git> git-filter-client > +packet: git> version=2 > +packet: git> version=42 > +packet: git> 0000 > +packet: git< git-filter-server > +packet: git< version=2 > +packet: git> clean=true > +packet: git> smudge=true > +packet: git> not-yet-invented=true > +packet: git> 0000 > +packet: git< clean=true > +packet: git< smudge=true > +packet: git< 0000
It's probalby only me who has difficulties to distinguish '>' from '<'.
packet: git> git-filter-client packet: git> version=2 packet: git> version=42 packet: git> 0000 packet: filter> git-filter-server packet: filter> version=2
(Otherwise the dialoge description is nice)
Show 13 quoted lines
> +------------------------ > +Supported filter capabilities in version 2 are "clean" and > +"smudge". > + > +Afterwards Git sends a list of "key=value" pairs terminated with > +a flush packet. The list will contain at least the filter command > +(based on the supported capabilities) and the pathname of the file > +to filter relative to the repository root. Right after these packets > +Git sends the content split in zero or more pkt-line packets and a > +flush packet to terminate content. > +------------------------ > +packet: git> command=smudge\n > +packet: git> pathname=path/testfile.dat\n
How do we send pathnames the have '\n' ? Not really recommended, but allowed. And here I am a little bit lost, is each of the lines packed into a pkt-line ? command=smudge is packet as pkt-line and pathname= is packed into another one ? (The we don't need the '\n' at all) Or go both lines into one pkt-line (thats what I think), then we don't need the '\n' afther the pathname. And in this case the pathname is always the last element, and a '\n' may occur in the pathname, since we know the length of the packet we know how long the pathname must be.
Show 5 quoted lines
> +packet: git> 0000 > +packet: git> CONTENT > +packet: git> 0000 > +------------------------ > +
Show 25 quoted lines
> +The filter is expected to respond with a list of "key=value" pairs > +terminated with a flush packet. If the filter does not experience > +problems then the list must contain a "success" status. Right after > +these packets the filter is expected to send the content in zero > +or more pkt-line packets and a flush packet at the end. Finally, a > +second list of "key=value" pairs terminated with a flush packet > +is expected. The filter can change the status in the second list. > +------------------------ > +packet: git< status=success\n > +packet: git< 0000 > +packet: git< SMUDGED_CONTENT > +packet: git< 0000 > +packet: git< 0000 # empty list! > +------------------------ > + > +If the result content is empty then the filter is expected to respond > +with a success status and an empty list. > +------------------------ > +packet: git< status=success\n > +packet: git< 0000 > +packet: git< 0000 # empty content! > +packet: git< 0000 # empty list! > +------------------------ > + > +In case the filter cannot or does not want to process the content,
Does not want ? I can see something like "I read through the file, there is nothing to do. So Git, I don't send anything back, you know where the file is.
Show 24 quoted lines
> +it is expected to respond with an "error" status. Depending on the > +`filter.<driver>.required` flag Git will interpret that as error > +but it will not stop or restart the filter process. > +------------------------ > +packet: git< status=error\n > +packet: git< 0000 > +------------------------ > + > +If the filter experiences an error during processing, then it can > +send the status "error" after the content was (partially or > +completely) sent. Depending on the `filter.<driver>.required` flag > +Git will interpret that as error but it will not stop or restart the > +filter process. > +------------------------ > +packet: git< status=success\n > +packet: git< 0000 > +packet: git< HALF_WRITTEN_ERRONEOUS_CONTENT > +packet: git< 0000 > +packet: git< status=error\n > +packet: git< 0000 > +------------------------ > + > +If the filter dies during the communication or does not adhere to > +the protocol then Git will stop the filter process and restart it
My personal comment: When a filter is mis-behaving, Git should say so loud and clear, and die(). The filter process can be left running, so that it can be debugged.
Here I stopped the review for a moment, I still think that Git shouldn't kill anything, because we loose the ability to debug these processes.