Re: tl/push-branches-is-an-alias-for-all

From: Junio C Hamano <gitster@pobox.com>
Date: May 12, 2023, 17:23 UTC
Message-ID: <xmqq3541qxad.fsf@gitster.g>
In-Reply-To: <CABPp-BFLOpsm6wg+CidEHaQhJ4FTnszfj8SUeBWxkFZgSeJ8Nw@mail.gmail.com>

Elijah Newren <newren@gmail.com> writes:

Show 13 quoted lines

> On Thu, May 11, 2023 at 6:22 PM Junio C Hamano <gitster@pobox.com> wrote:
>>
>> * tl/push-branches-is-an-alias-for-all (2023-05-06) 1 commit
>>   (merged to 'next' on 2023-05-09 at 1d8c3e5dcf)
>>  + push: introduce '--branches' option
>>
>>  "git push --all" gained an alias "git push --branches".
>>
>>  Will merge to 'master'.
>>  source: <20230506113408.61529-2-tenglong.tl@alibaba-inc.com>
>
> This topic breaks the tests for me; it needs a trivial one-character
> fix: https://lore.kernel.org/git/pull.1532.git.1683904156670.gitgitgadget@gmail.com/

I wonder what made this breakage hidden. I usually run the tests after accepting a series standalone, and I run the full test suite (excluding the ones that depend on things I do not have, like p4) with bash and dash before pushing anything out, and I haven't seen GitHub Actions break due to this.

Ah, it is because t/Makefile essentially uses $(TEST_SHELL_PATH_SQ) to run $@, instead of directly doing "./$@", in both cases with and without probe. This is very much deliberate in that we do not want to rely on and get affected by whichever shell choice the system administrator made when installing "/bin/sh".

I'll apply your patch primarily for consistency (a shell script file that begins with "#!bin/sh" never is a good thing, and everybody else begins with "#!/bin/sh") for now, but I think the justification of your patch is questionable in that we do not want our developers to rely on "./t5583-push-branches.sh" to work [*], as these scripts are designed to run under $(TEST_SHELL_PATH).

Thanks.

[Footnote]

 * We could, for example, make t/t[0-9][0-9][0-9]-*.sh scripts
   consistently use bogus shebang line to catch possible breakage to
   t/Makefile that bypasses $(TEST_SHELL_PATH) in the future.  If
   that happens, developers who expect ./t5583-push-branches.sh to
   work will be broken.