Re: Looking for help to understand external filter driver code

Lars Schneider [off-list ref] writes:

Git writes --> 4 byte filename length
Git writes --> filename string

Why limit to 32GB?  Perhaps NUL termination is more appropriate
here?

Git writes --> 4 byte content length
Git writes --> content string
Git reads <-- 4 byte filtered content length
Git reads <-- filtered content

Do you really need to force the sender to know the length in
advance?  Together with the sequential nature of the above exchange,
i.e. the filter is forbidden from producing even a single byte of
its output before reading everything Git feeds it, you are making it
impossible to use filters that perform streaming conversion.

Of course, with the "sequential" thing, you do not have to worry
about deadlocking hence no need for poll/select, but I am not sure
that is a good thing.

On 19 Jul 2016, at 23:33, Junio C Hamano [off-list ref] wrote:

Lars Schneider [off-list ref] writes:

quoted

Git writes --> 4 byte filename length
Git writes --> filename string

Why limit to 32GB?  Perhaps NUL termination is more appropriate
here?

OK, I will use NUL termination for the filename. 
You're also right about the limit - I will use 8 byte to encode the 
content length.

quoted

Git writes --> 4 byte content length
Git writes --> content string
Git reads <-- 4 byte filtered content length
Git reads <-- filtered content

Do you really need to force the sender to know the length in
advance?  Together with the sequential nature of the above exchange,
i.e. the filter is forbidden from producing even a single byte of
its output before reading everything Git feeds it, you are making it
impossible to use filters that perform streaming conversion.

That is correct. However, for my particular use case streaming
conversion wouldn't be useful anyways:
https://github.com/github/git-lfs/pull/1382

Of course, with the "sequential" thing, you do not have to worry
about deadlocking hence no need for poll/select, but I am not sure
that is a good thing.

Thanks for the confirmation. I consider to exchange a "filter protocol 
version" right after the filter process has started. That way someone 
could add a more evolved "filter driver protocol" later on that supports 
streaming and the external filter could pick whatever protocol is most
appropriate (and supported). Could that be an acceptable compromise
to get a serious review of the "sequential" thing?

Thanks,
Lars

On 07/20/2016 12:01 AM, Lars Schneider wrote:

On 19 Jul 2016, at 23:33, Junio C Hamano [off-list ref] wrote:

quoted

Lars Schneider [off-list ref] writes:

quoted

Git writes --> 4 byte filename length
Git writes --> filename string

Why limit to 32GB?  Perhaps NUL termination is more appropriate
here?

OK, I will use NUL termination for the filename.
You're also right about the limit - I will use 8 byte to encode the
content length.

Is there any reason to encode the file length in binary format?
With all the discussions about big endianess, little endianess, 4GiB or 
32 GiB.
How about simply writing the length as ASCII ?

Unless we don't want to have a "spare" field for future extensions,
it could be good to add an option field, which may be empty.
On top of that, do we want a field separator different from the line
separator ?

How about this:
<options><TAB><length><TAB><filename><NUL>

<options> may be "var=value;var2=value2" or simply ""

W dniu 2016-07-19 o 23:33, Junio C Hamano pisze:

Lars Schneider [off-list ref] writes:

quoted

quoted

Git writes --> 4 byte filename length
Git writes --> filename string

Why limit to 32GB?  Perhaps NUL termination is more appropriate
here?

Errr, I think limiting _filename_ to 32GB is a reasonable
assumption, for forever...
 

quoted

quoted

Git writes --> 4 byte content length
Git writes --> content string

...while limiting file _content_ to 32GB might not be
future-proof enough.

;-)
-- 
Jakub Narębski

On 20 Jul 2016, at 10:59, Jakub Narębski [off-list ref] wrote:

W dniu 2016-07-19 o 23:33, Junio C Hamano pisze:

quoted

Lars Schneider [off-list ref] writes:

quoted

quoted

Git writes --> 4 byte filename length
Git writes --> filename string

Why limit to 32GB?  Perhaps NUL termination is more appropriate
here?

Errr, I think limiting _filename_ to 32GB is a reasonable
assumption, for forever...

Well, Java packages can get reaaaally long :D

quoted

quoted

quoted

Git writes --> 4 byte content length
Git writes --> content string

...while limiting file _content_ to 32GB might not be
future-proof enough.

;-)

I think this is what Junio meant to say. At least I 
interpreted it that way.


- Lars

On Tue, Jul 19, 2016 at 02:33:09PM -0700, Junio C Hamano wrote:

quoted

Git writes --> 4 byte content length
Git writes --> content string
Git reads <-- 4 byte filtered content length
Git reads <-- filtered content

Do you really need to force the sender to know the length in
advance?  Together with the sequential nature of the above exchange,
i.e. the filter is forbidden from producing even a single byte of
its output before reading everything Git feeds it, you are making it
impossible to use filters that perform streaming conversion.

Another option: use pkt-lines with a flush packet to indicate
end-of-input. That allows arbitrary sized data, with streaming, and
reuses existing concepts from git. There is proportional overhead, but
it's only 4 bytes per 64k, which is a tiny percent.

It does make some implementations easier if they know the size ahead of
time, though, so if we are _sure_ that nobody will want streaming later,
it may not be a good tradeoff. If we do print a size ahead of time, the
"normal" thing in git would be to do so in base-10 ascii followed by a
newline (e.g., as found in "cat-file --batch", or fast-import's "data"
command).

-Peff