Re: [PATCH/RFD] builtin-revert.c: release index lock when cherry-picking an empty commit

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

Jeff King <peff@peff.net> writes:

>   1. Declare appended newline a forbidden style, fix all existing cases
>      in the test suite, and be on the lookout for new ones.
>
>      The biggest problem with this option is that we have no automated
>      way of policing. Such tests will just silently pass on the broken
>      platform.
>
>   2. Have test_run_ canonicalize the snippet by removing trailing
>      newlines.
>
>   3. Declare FreeBSD's /bin/sh unfit for git consumption, and require
>      bash for the test suite.
>
> I think (2) is the most reasonable option of those choices.
>
> We could also try to convince FreeBSD that it's a bug, but that doesn't
> change the fact that the tests are broken on every existing version.

If this part from your analysis is true for a shell:

> eval 'false
>
> '
> echo status is $?
>
> generates:
> ...
>   status is 0

I would be very tempted to declare that shell is unfit for any serious
use, not just for test suite.  Removing the empty line at the end of a
scriptlet that such a broken shell misinterprets as an empty command
that is equivalent to ":" (or "true") might hide breakages in the test
suite, but

 (1) eval "$string" is used outside of test suite, most notably "am" and
     "bisect".  I think "am"'s use is safe, but I wouldn't be surprised if
     the scriptlet "bisect" internally creates has empty lines if only for
     debuggability; and more importantly

 (2) who knows what _other_ things may be broken in such a shell?