Re: racy test failure in tb/upload-pack-filters

[Taylor Blau <me@ttaylorr.com>]

On Wed, Aug 05, 2020 at 05:26:58AM -0400, Jeff King wrote:
> On Wed, Aug 05, 2020 at 11:06:41AM +0200, SZEDER Gábor wrote:
>
> > > We can fix it by using test_i18ngrep, which just makes this grep a noop
> > > in the poison mode.
> >
> > I wonder whether changing that die to
> >
> >   die("%s: %s", _("remote error"), buffer + 4)
> >
> > would be better.
>
> That would definitely work, but it seems sad to have to make our code
> uglier. Plus I think it would hurt translations that want to format
> differently (e.g., would an RTL language want to swap the order?).

This is reason enough to not do this; I know that we are pretty averse
to translation lego, and even though this one seems innocuous, I think
it may be a deal breaker for such languages.

> It also wouldn't help other poison uses that could be expecting a "%s"
> to be filled.  Another option would be to make our poison code more
> realistic by copying placeholders like "%s" into the poison string. That
> would fix this problem, and allow some tests to relax a bit (e.g., if
> I'm looking for an error message that contains a filename, I _could_
> grep for just that filename, which would never actually be translated).
>
> But I think that gets pretty tricky, as we'd have to understand the
> whole set of placeholders (e.g., that "%s" is complete after two bytes,
> but "%lu" needs three bytes).

Might be a good direction to go in, but I agree it's outside of the
scope of this patch.

> Anyway, it seemed like limiting the damage to the tests themselves was
> the least bad thing.
>
> By the way, grepping for "remote error:" shows that when we get an error
> over sideband 3 we produce the same message but _don't_ translate it.
> That seems inconsistent.
>
> -Peff

Thanks,
Taylor