Re: [PATCH] parse-options: make parse_options_check() test-only

[Junio C Hamano <gitster@pobox.com>]

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

> On Tue, Mar 01 2022, Junio C Hamano wrote:
>
>> The array of options given to the parse-options API is sanity
>> checked for reuse of a single-letter option for multiple entries and
>> other programmer mistakes by calling parse_options_check() from
>> parse_options_start().  This allows our developers to catch silly
>> mistakes early, but all callers of parse-options API pays this cost.
>> Once the set of options in an array is validated and passes this
>> check, until a programmer modifies the array, there is no way for it
>> to fail the check, which is wasteful.
>
> That's not true due to the "git rev-parse --parseopt" interface. I'd be

Meaning that a parse-options array can be fed by "rev-parse --parseopt"
and having the sanity check enabled does help the use case?  Even there,
I would say that once the script writer finishes developing the script
that uses "rev-parse --parseopt", setting the parseopt input in stone,
there is no need to check the same thing over and over again.  Am I
mistaken?  Does "rev-parse --parseopt" that is fed the same input
sometimes trigger the sanity check and sometimes not?

> I see the benifit of Johannes's suggestion of checking this once (but
> with t0012-help.sh etc. we're nowhere near being able to do that).
>
> Now this runs for the whole test suite, so our tests will have the the
> same behavior.

The code for sanity check is there ONLY to help those who develop
while they develop, and it is logical to enable it during our tests.
There is no reason to trigger the sanity check in the end-user
environment, no?

> So aren't we shaving microseconds off the runtime here?

No, the problem I have with the runtime check is more at the
conceptual level.  Those who remove assert() by setting _NDEBUG
would not be doing so to save nanoseconds, either.