[0/2] cat-file: add a --stdin-cmd mode

Message

Philippe Blain via GitGitGadget Jan. 19, 2022, 5:58 p.m. UTC

This WIP patch is mostly stealing code from builtin/update-ref.c and
implementing the same sort of prefixed command-mode that it supports. I.e.
in addition to --batch now supporting:

<object> LF


It'll support with --stdin-cmd, with and without -z, respectively:

object <object> NL
object <object> NUL


The plus being that we can now implement additional commands:

fflush NL
fflush NUL


That command simply calls fflush(stdout), which could be done as an emergent
effect before by feeding the input a "NL".

I think this will be useful for other things, e.g. a not-trivial part of
"cat-file --batch" time is spent on parsing its argument and seeing if it's
a revision, ref etc.

So we could e.g. add a command that only accepts a full-length 40 character
SHA-1, or switch the --format output mid-request etc.

 1. https://lore.kernel.org/git/[e-mail]/

requires ee4d43041d ab/cat-file

John Cai (2):
  strvec.c: add a strvec_split_delim()
  cat-file: add a --stdin-cmd mode

 builtin/cat-file.c  | 128 +++++++++++++++++++++++++++++++++++++++++++-
 strvec.c            |  23 ++++++++
 strvec.h            |   8 +++
 t/t1006-cat-file.sh |  72 +++++++++++++++++++++++++
 4 files changed, 230 insertions(+), 1 deletion(-)


base-commit: 00780c9af44409a68481c82f63a97bd18bb2593e
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1191%2Fjohn-cai%2Fjc-cat-file-stdin-cmd-mode-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1191/john-cai/jc-cat-file-stdin-cmd-mode-v1
Pull-Request: https://github.com/git/git/pull/1191

Comments

Taylor Blau Jan. 19, 2022, 7:38 p.m. UTC | #1

On Wed, Jan 19, 2022 at 05:58:40PM +0000, John Cai via GitGitGadget wrote:
> I think this will be useful for other things, e.g. a not-trivial part of
> "cat-file --batch" time is spent on parsing its argument and seeing if it's
> a revision, ref etc.
>
> So we could e.g. add a command that only accepts a full-length 40 character
> SHA-1, or switch the --format output mid-request etc.

I would like to see a more concrete proposal or need for this sort of
thing before we go too far down adding a new mode to a command so
fundamental as cat-file is.

Between your two proposals for other commands that you could add, I am
not convinced that either of them needs to be in cat-file itself. IOW,
you could easily inject another process in between which verifies that
the provided objects are 40 character SHA-1s.

The latter, changing the output format in-process, seems dubious to me.
Is the start-up time of cat-file so slow (and you need to change formats
so often) that the two together are unbearable? I'd be surprised if they
were (and if so, we should focus our efforts on improving Git's start-up
time).

In the meantime, this is quite an invasive way to provide callers a way
to control the output stream. If there really is a need to just force
cat-file to flush more often, perhaps consider designating a special
signal that we could treat as a request to flush the output stream?

Thanks,
Taylor

John Cai Jan. 21, 2022, 5:46 p.m. UTC | #2

On 19 Jan 2022, at 14:38, Taylor Blau wrote:

> On Wed, Jan 19, 2022 at 05:58:40PM +0000, John Cai via GitGitGadget wrote:
>> I think this will be useful for other things, e.g. a not-trivial part of
>> "cat-file --batch" time is spent on parsing its argument and seeing if it's
>> a revision, ref etc.
>>
>> So we could e.g. add a command that only accepts a full-length 40 character
>> SHA-1, or switch the --format output mid-request etc.
>
> I would like to see a more concrete proposal or need for this sort of
> thing before we go too far down adding a new mode to a command so
> fundamental as cat-file is.

Thanks for the feedback! I realized I should have made this an RFC first.
I’ll submit an RFC separately.

>
> Between your two proposals for other commands that you could add, I am
> not convinced that either of them needs to be in cat-file itself. IOW,
> you could easily inject another process in between which verifies that
> the provided objects are 40 character SHA-1s.
>
> The latter, changing the output format in-process, seems dubious to me.
> Is the start-up time of cat-file so slow (and you need to change formats
> so often) that the two together are unbearable? I'd be surprised if they
> were (and if so, we should focus our efforts on improving Git's start-up
> time).
>
> In the meantime, this is quite an invasive way to provide callers a way
> to control the output stream. If there really is a need to just force
> cat-file to flush more often, perhaps consider designating a special
> signal that we could treat as a request to flush the output stream?
>
> Thanks,
> Taylor