Re: [PATCH v2 1/2] t: add "test-tool genbytes" to avoid slow tr(1) on macOS

[Patrick Steinhardt <ps@pks.im>]

On Sun, Nov 08, 2026 at 07:15:03PM +0000, Kristofer Karlsson via GitGitGadget wrote:
[snip]
> This makes t4205 much faster on macOS in CI.  The Linux jobs, shown
> for comparison, only save the few seconds that GNU tr(1) took, which
> is within the usual noise:
> 
>   job                before    after   change
>   osx-gcc            821.1s   243.3s   -70.4%
>   osx-reftable       911.0s   194.0s   -78.7%
>   linux-asan-ubsan   226.7s   222.1s    -2.0%
>   linux-leaks        320.2s   298.8s    -6.7%

Nice improvement.

> diff --git a/t/helper/test-genbytes.c b/t/helper/test-genbytes.c
> new file mode 100644
> index 0000000000..b760326b24
> --- /dev/null
> +++ b/t/helper/test-genbytes.c
> @@ -0,0 +1,61 @@

Okay. The diff is unfortunately quite useless as we do the file rename
plus all the changes in a single step. It might've been helpful to split
this up into:

  1. A rename of the file.

  2. A refactoring of `cmd_genzeros()` to use the write buffer.

  3. A change to introduce `cmd_genbytes()`.

But maybe I'm also overdoing this.

> +#define DISABLE_SIGN_COMPARE_WARNINGS
> +
> +#include "test-tool.h"
> +#include "git-compat-util.h"
> +#include "hex-ll.h"
> +
> +/*
> + * Write "count" copies of "byte" to stdout, or an endless stream of them if
> + * "count" is negative.
> + */
> +static int generate_bytes(char byte, intmax_t count)
> +{
> +	char buf[256 * 1024];
> +	ssize_t n;
> +
> +	memset(buf, byte, sizeof(buf));
> +
> +	/* Writing out individual bytes is slow... */
> +	while (count < 0)
> +		if (xwrite(1, buf, ARRAY_SIZE(buf)) < 0)
> +			die_errno("write error");
> +
> +	while (count > 0) {
> +		n = xwrite(1, buf,
> +			   count < ARRAY_SIZE(buf)
> +			   ? count : ARRAY_SIZE(buf));
> +
> +		if (n < 0)
> +			die_errno("write error");
> +
> +		count -= n;
> +	}

Okay. These two loops are a bit ugly if you ask me, but they are carried
over from preexisting code. So this is good enough.

> +	return 0;
> +}
> +
> +static intmax_t parse_count(int argc, const char **argv, int pos)
> +{
> +	return pos < argc ? strtoimax(argv[pos], NULL, 0) : -1;
> +}

We can use `git_parse_signed()` instead.

> diff --git a/t/t0613-reftable-write-options.sh b/t/t0613-reftable-write-options.sh
> index 0b27d75fe3..b2032a799d 100755
> --- a/t/t0613-reftable-write-options.sh
> +++ b/t/t0613-reftable-write-options.sh
> @@ -146,7 +146,7 @@ test_expect_success 'small block size fails with large reflog message' '
>  	(
>  		cd repo &&
>  		test_commit A &&
> -		test-tool genzeros 500 | tr "\000" "a" >logmsg &&
> +		test-tool genbytes 61 500 >logmsg &&
>  		cat >expect <<-EOF &&
>  		fatal: update_ref failed for ref ${SQ}refs/heads/logme${SQ}: reftable: transaction failure: entry too large
>  		EOF

I wonder about the regression in readability though. Maybe that's just
me, but I don't happen to know all ASCII values by heart, so if I were
to read this I would now wonder what's so special about 61. And the next
thing to wonder about is whether this is supposed to encode "=" (decimal
61) or "a" (hexadecimal 61), and without reading the code of genbytes I
would not be able to tell.

So can we maybe make the underpinning logic a bit more intelligent, e.g.
by accepting "\NNN" or "\xNN" syntax instead of two hex characters? That
might also make it more extensible going forward if the next developer
wants to make genbytes take and repeat strings of arbitrary length
as-is.

Patrick