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

Elijah Newren [off-list ref] writes:

On Thu, May 11, 2023 at 6:22 PM Junio C Hamano [off-list ref] wrote:

quoted

* 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: [ref]

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/ (local)

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.