All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Lars Schneider <larsxschneider@gmail.com>
Cc: git@vger.kernel.org, peff@peff.net, tboegi@web.de, e@80x24.org,
	ttaylorr@github.com, peartben@gmail.com
Subject: Re: [PATCH v5 5/5] convert: add "status=delayed" to filter process protocol
Date: Fri, 02 Jun 2017 11:21:31 +0900	[thread overview]
Message-ID: <xmqq60gf1650.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <20170601082203.50397-6-larsxschneider@gmail.com> (Lars Schneider's message of "Thu, 1 Jun 2017 10:22:03 +0200")

Lars Schneider <larsxschneider@gmail.com> writes:

> +Delay
> +^^^^^
> +
> +If the filter supports the "delay" capability, then Git can send the
> +flag "can-delay" after the filter command and pathname. This flag
> +denotes that the filter can delay filtering the current blob (e.g. to
> +compensate network latencies) by responding with no content but with
> +the status "delayed" and a flush packet.
> +------------------------
> +packet:          git> command=smudge
> +packet:          git> pathname=path/testfile.dat
> +packet:          git> can-delay=1
> +packet:          git> 0000
> +packet:          git> CONTENT
> +packet:          git> 0000
> +packet:          git< status=delayed
> +packet:          git< 0000
> +------------------------
> +
> +If the filter supports the "delay" capability then it must support the
> +"list_available_blobs" command. If Git sends this command, then the
> +filter is expected to return a list of pathnames of blobs that are
> +available. The list must be terminated with a flush packet followed
> +by a "success" status that is also terminated with a flush packet. If
> +no blobs for the delayed paths are available, yet, then the filter is
> +expected to block the response until at least one blob becomes
> +available. The filter can tell Git that it has no more delayed blobs
> +by sending an empty list.
> +------------------------
> +packet:          git> command=list_available_blobs
> +packet:          git> 0000
> +packet:          git< pathname=path/testfile.dat
> +packet:          git< pathname=path/otherfile.dat
> +packet:          git< 0000
> +packet:          git< status=success
> +packet:          git< 0000
> +------------------------
> +
> +After Git received the pathnames, it will request the corresponding
> +blobs again. These requests contain a pathname and an empty content
> +section. The filter is expected to respond with the smudged content
> +in the usual way as explained above.
> +------------------------
> +packet:          git> command=smudge
> +packet:          git> pathname=path/testfile.dat
> +packet:          git> 0000
> +packet:          git> 0000  # empty content!
> +packet:          git< status=success
> +packet:          git< 0000
> +packet:          git< SMUDGED_CONTENT
> +packet:          git< 0000
> +packet:          git< 0000  # empty list, keep "status=success" unchanged!
> +------------------------

Random things that come to mind (which I suspect has been dealt with
in code but not described in the above in detail):

 * Using pathname as a "delay key" would mean that we assume that a
   single helper process is spawned to serve only a single Git
   invocation (as opposed to be sitting as a daemon accepting
   requests from instances of Git invoked much later than the daemon
   started), which is OK, but it is somewhat unclear when a filter
   is allowed to discard the requests that used to be outstanding
   from the write-up.  Imagine Git asked for A and B, both of which
   was delayed, and then asked for a list of available ones and
   learned that A is now ready.  

   - Is it an error to ask for B at this point, and if so how is the
     error handled?  Or will it turn into a blocking operation?

   - As A is available, Git can ask for it.  After retrieving
     smudged content for A, can Git ask for it again?  Or is it an
     error to do so, without starting over with another can-delay=1
     request for the same path?

 * The pathname does not have to be encoded/quoted in any way as it
   is just a payload on the tail end of a packet line, so I am
   guessing it is just a sequence raw bytes.  It is somewhat unclear
   in the above write-up.  Also, is this considered to be a part of
   "textual" exchange over the packet-line protocol, where it is
   customary to remove the trailing LF from the packet?  "We do not
   support a file whose name ends with LF" may or may not be an
   acceptable stance (I am personally OK, but some people who use
   Git may not), but it needs to be documented if there is such an
   issue.

 * The continued request throws an empty content in the above
   illustration.  Is a filter allowed/encouraged to assume that an
   empty content in the request is a continuation?  I am guessing
   that the answer is NO (otherwise you cannot filter an empty
   file), and it somehow need to be documented, perhaps?


  reply	other threads:[~2017-06-02  2:21 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-06-01  8:21 [PATCH v5 0/5] convert: add "status=delayed" to filter process protocol Lars Schneider
2017-06-01  8:21 ` [PATCH v5 1/5] t0021: keep filter log files on comparison Lars Schneider
2017-06-01  8:22 ` [PATCH v5 2/5] t0021: make debug log file name configurable Lars Schneider
2017-06-01  8:22 ` [PATCH v5 3/5] t0021: write "OUT" only on success Lars Schneider
2017-06-01  8:22 ` [PATCH v5 4/5] convert: move multiple file filter error handling to separate function Lars Schneider
2017-06-18  7:20   ` Torsten Bögershausen
2017-06-18 11:47     ` Lars Schneider
2017-06-19 17:18       ` Torsten Bögershausen
2017-06-19 17:47         ` Lars Schneider
2017-06-01  8:22 ` [PATCH v5 5/5] convert: add "status=delayed" to filter process protocol Lars Schneider
2017-06-02  2:21   ` Junio C Hamano [this message]
2017-06-05 11:36     ` Lars Schneider
2017-06-24 14:19   ` Jeff King
2017-06-24 17:22     ` Lars Schneider
2017-06-24 18:51       ` Junio C Hamano
2017-06-24 20:36         ` Jeff King
2017-06-24 20:32       ` Jeff King
2017-06-01  9:44 ` [PATCH v5 0/5] " Junio C Hamano
2017-06-02  2:06 ` Junio C Hamano
2017-06-24 14:23 ` Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqq60gf1650.fsf@gitster.mtv.corp.google.com \
    --to=gitster@pobox.com \
    --cc=e@80x24.org \
    --cc=git@vger.kernel.org \
    --cc=larsxschneider@gmail.com \
    --cc=peartben@gmail.com \
    --cc=peff@peff.net \
    --cc=tboegi@web.de \
    --cc=ttaylorr@github.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.