Re: [PATCH 1/2] t/t0021: convert the rot13-filter.pl script to C

[Matheus Tavares <matheus.bernardino@usp.br>]

On Sat, Jul 23, 2022 at 2:15 AM Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
>
> On Fri, Jul 22 2022, Matheus Tavares wrote:
>
> Looking a bit closer...
>
> > however, that we still use the script as a wrapper at
> > this commit, in order to minimize the amount of changes it introduces
> > and help reviewers. At the next commit we will properly remove the
> > script and adjust the affected tests to use test-tool.
>
> I'd prefer if we just squashed this, if you want to avoid some of the
> diff verbosity you could leave the PERL prereq on all the
> test_expect_success and remove it in a 2/2 (we just wouldn't run the
> test until then).
>
> But I think it's all boilerplate, so just doing it in one step would be
> better, reasoning about the in-between steps is harder IMO (e.g. "exec"
> escaping or whatever)

Sure, will do! My split attempt was to try to reduce the mental load
for the reviewers, but if it ended up making it harder instead of
helping, let's squash the two patches.

> > +     remote_caps = packet_read_and_check_capabilities(&supported_caps);
> > +     packet_check_and_write_capabilities(remote_caps, &requested_caps);
> > +     fprintf(logfile, "init handshake complete\n");
> > +
> > +     string_list_clear(&supported_caps, 0);
> > +     string_list_clear(remote_caps, 0);
>
> ..and here you're missing a free(), but I wonder why not just declare
> this string_list in this function, and pass it down instead?

Makes sense, will do.

> Not knowing much about the filtering mechanism, I wonder if this code
> here wouldn't be better as a built-in some day. I.e. isn't this all
> shimmy we need to talk to some arbitrary conversion filter, except for
> the rot13 part?
>
> So if we just invoked a "tr" with run_command() to do the actual rot13
> filtering we could do any sort of arbitrary replacement, and present a
> variant of this this command as a "if you can't be bothered with
> packet-line" in gitattributes(5)...

Hmm, maybe so. But I would expect that someone building a long running
process filter (as opposed to a "single-shot" filter, like the "tr"
use case)  would also want to have finer control over the
communication and "queueing" mechanics. And I'm not sure if that would
be feasible via an off-the-shelf solution packed with Git itself.

For example, while some filters may process the received paths
sequentially, Git-LFS will use the delay capability to queue and
download blobs in the background, examining the queue every time Git
asks for the list of currently available blobs.

Anyways, I could see these packet-line routines being exported as a
library for those writing such filters.