Subject: Re: [PATCH 0/6] test: make the test suite work with zsh

From: Junio C Hamano <[email-masked]>
Date: Fri, 31 Mar 2023 18:30:41 -0700

Felipe Contreras <[email-masked]> writes:

>> The criteria is more about what our developers are expected to be
>> familiar with, and what is reasonable to force our developers to
>> become sufficiently familiar with.
>
> That is not true.

We probably should agree to disagree.  Let me respond by picking the
first example from your message and then stop.

> This patch 77e572653b (t0050: fix printf format strings for
> portability, 2010-12-21) fixed a problem that was specific with dash.
> Did our developers have to learn the details of such issue? No.

The code before that change was feeding "\xc3\xa4" to printf,
expecting that it would be an acceptable way to spell hexadecimal
byte values, which was wrong.  The commit improved portability by
rewriting them to spell the same byte values in octal.

Yes, our developers have to learn to avoid hexadecimal byte values
and the commit serves as a reminder for them to learn from.  When a
developer writes printf format with '\xCC' hexadecimal, reviewers
would need to catch it as a mistake, and that commit makes a good
reference why we insist on such a rule.

By learning that practice, our developers will be trained to write
scripts that not just work with bash but also with dash, which is a
small good thing.  But still, it *is* forcing our developers to
learn one more rule.

There is a trade off: is it worth supporting dash by forcing our
developers to stick to the rule to write octal and not hex?  dash is
used as the default for some distros and considered one of the
standard ones, and is worth supporting even if we need to stay away
from some stuff people may have picked up from other shells like
bash.

If it were a different shell, the equation may have been different.
If industry standards like POSIX.1 required supporting hex literals,
the equation may also have been different.

	Note: though, we do not blindly say "it is in POSIX, your
        shell behaves differently and we won't support it".

In any case, once we declare that "we aim for our scripts and tests
to work with dash and bash and these other shells", our developers
are forced to stick to intersection of these supported ones.

It takes a judgement call.  And "don't write literals you feed
printf in hex, instead do so in octal, because printf built into
some shells do not like it" is something reasonable to force our
developers to stick to (as it allows "dash" to be thrown into the
set of supported shells).  I however do not see "don't use path or
status or options or 0 as shell variable names" falls into the same
category (even though it may allow "zsh" in native mode to be
included into the supported shells).  Do benefits outweigh
downsides?

And somebody has to draw that line.  Judgement calls are just that.
There are no absolutely right or wrong answers and they will not
please everybody.  Some folks may not agree with where the line gets
drawn.  Tough.

But the job of maintainer is not about being loved by everybody.
Just drawing the line somewhere in order to allow folks to move on,
without having everybody dragged into and getting stuck in endless
arguments, in which there is no absolute right or wrong.  That is
what needs to be done, and that is what I just did.