Re: [PATCH v2 0/5] Git filter protocol

From: Jakub Narębski <jnareb@gmail.com>
Date: Jul 27, 2016, 19:08 UTC
Message-ID: <579906C5.1050809@gmail.com>
In-Reply-To: <20160727000605.49982-1-larsxschneider@gmail.com>

W dniu 2016-07-27 o 02:06, larsxschneider@gmail.com pisze:

Show 11 quoted lines

> From: Lars Schneider <larsxschneider@gmail.com>
> 
> Hi,
> 
> thanks a lot for the extensive reviews. I tried to address all mentioned
> concerns and summarized them below. The most prominent changes since v1 are
> the following:
> * Git offers a number of filter capabilities that a filter can request
>   (right now only "smudge" and "clean" - in the future maybe "cleanFromFile",
>   "smudgeToFile", and/or "stream")
> * pipe communication uses a packet format (pkt-line) based protocol

I wonder if it would make sense to support both whole-file pipe communication, and packet format (pkt-line) pipe communication.

The problem with whole-file pipe communication (original proposal for
new filter protocol is that it needs file size upfront.  For some types
of filters it is not a problem:
 - if a filtered file has the same size as original, like for rot13
   example in the test for the feature
 - if you can calculate the resulting file size from original size,
   like for most if not all encryption formats (that includes GPG,
   uudecode, base64, quoted-printable, hex, etc.); same for decryption,
   and from converting between fixed-width encodings
 - if resulting file size is saved somewhere that is easy to get, like
   for LFS solutions (I think).

For other filters it is serious problem. For example indent, keyword expansion, rezipping with zero compression (well, maybe not this one, but at least the reverse of it), converting between encodings where at least one is variable width (like UTF-8),...

IMHO writing whole-file persistent filters is easier than using pkt-line. On the other hand using pkt-line allow for more detailed progress report.

> * a long running filter application is defined with "filter.<driver>.process"

I hope that won't confuse Git users into trying to use single-shot filters with a new protocol...

Show 38 quoted lines

> ## Torsten:
> * add "\n" line terminator after version in init sequence
> * prepare big file for EXPENSIVE tests once
> * set "#!/usr/bin/perl" as shebang for rot13.pl to mimic other Perl test scripts
> * add test_have_prereq PERL to t0021
> 
> ## Ramsay:
> * use write_in_full(process->in, nbuf.buf, nbuf.len) to avoid unneccesary strlen call
> * use read_in_full to read data that exceeds MAX_IO_SIZE properly
> * fix test case to check for large file filtering
> 
> ## Jakub:
> * use standard input/standard output instead of stdin/stdout [in description/documentation]
> * replace global variable "cmd_process_map" with a function parameter where possible
> * avoid "strbuf_reset" after STRBUF_INIT
> * align test_config_global
> * rename rot13.pl to rot13-filter.pl
> * make Perl style consistent
> * describe hard coded filenames in test filter header
> * improve docs
> * add filter capabilities field (enables cleanToFile, smudgeFromFile, and/or stream later)
> * explain that content size in bytes is encoded in ASCII
> * consistent line ending for die call in Perl (without "\n")
> * make rot13 test filter die in case of failure (instead of returning "fail")
> 
> ## Eric:
> * flush explicitly in Perl test filter
> * do not initialize variables to NULL if they are set unconditionally
> * fix no-op stop_protocol_filter
> * use off_t instead of size_t
> * improve test filter int parsing ($filelen =~ /\A\d+\z/ or die "bad filelen: $filelen")
> 
> ## Peff:
> * use pkt-line protocol
> * do not use Perl autodie
> 
> ## Remi:
> * remove spaces after '<'