diff mbox series

Re* [PATCH 7/8] ci: run style check on GitHub and GitLab

Message ID xmqqwmlvcx9g.fsf_-_@gitster.g (mailing list archive)
State Accepted
Commit 58696bfcaacc50323e596112124b41242fde23de
Headers show
Series Re* [PATCH 7/8] ci: run style check on GitHub and GitLab | expand

Commit Message

Junio C Hamano July 8, 2024, 10:52 p.m. UTC
Junio C Hamano <gitster@pobox.com> writes:

> I think the consensus from the last discussion we had was to allow
> scripts that rely on bash-isms to say "#!/usr/bin/env bash" because
> we know /bin/sh can legitimately be not bash and we assume bash may
> not be installed as /bin/bash.

Let's do this before we forget.

------- >8 ------------- >8 ------------- >8 -------
Subject: [PATCH] ci: unify bash calling convention

Under ci/ hierarchy, we run scripts under either "sh" (any Bourne
compatible POSIX shell would work) or specifically "bash" (as they
require features from bash, e.g., $(parameter/pattern/string}
expansion).  As we have the CI envionment under our control, we can
expect that /bin/sh will always be fine to run the scripts that only
require Bourne, but we may not know where "bash" gets installed
depending on distros.

So let's make sure we start these scripts with either one of these:

	#!/bin/sh
	#!/usr/bin/env bash

Yes, the latter has to assume that everybody installs "env" at that
path and not as /bin/env or /usr/local/bin/env, but this currently
is the best we could do.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 ci/check-directional-formatting.bash | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Justin Tobler July 8, 2024, 11:17 p.m. UTC | #1
On 24/07/08 03:52PM, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
> 
> > I think the consensus from the last discussion we had was to allow
> > scripts that rely on bash-isms to say "#!/usr/bin/env bash" because
> > we know /bin/sh can legitimately be not bash and we assume bash may
> > not be installed as /bin/bash.
> 
> Let's do this before we forget.
> 
> ------- >8 ------------- >8 ------------- >8 -------
> Subject: [PATCH] ci: unify bash calling convention
> 
> Under ci/ hierarchy, we run scripts under either "sh" (any Bourne
> compatible POSIX shell would work) or specifically "bash" (as they
> require features from bash, e.g., $(parameter/pattern/string}
> expansion).  As we have the CI envionment under our control, we can

s/envionment/environment

> expect that /bin/sh will always be fine to run the scripts that only
> require Bourne, but we may not know where "bash" gets installed
> depending on distros.
> 
> So let's make sure we start these scripts with either one of these:
> 
> 	#!/bin/sh
> 	#!/usr/bin/env bash
> 
> Yes, the latter has to assume that everybody installs "env" at that
> path and not as /bin/env or /usr/local/bin/env, but this currently
> is the best we could do.

Makes sense to me to be consistent and I also think `#!/usr/bin/env bash`
is probably the best route. Other than the small typo this looks good to
me.

-Justin
brian m. carlson July 9, 2024, 12:56 a.m. UTC | #2
On 2024-07-08 at 22:52:11, Junio C Hamano wrote:
> Subject: [PATCH] ci: unify bash calling convention
> 
> Under ci/ hierarchy, we run scripts under either "sh" (any Bourne
> compatible POSIX shell would work) or specifically "bash" (as they
> require features from bash, e.g., $(parameter/pattern/string}
> expansion).  As we have the CI envionment under our control, we can
> expect that /bin/sh will always be fine to run the scripts that only
> require Bourne, but we may not know where "bash" gets installed
> depending on distros.
> 
> So let's make sure we start these scripts with either one of these:
> 
> 	#!/bin/sh
> 	#!/usr/bin/env bash
> 
> Yes, the latter has to assume that everybody installs "env" at that
> path and not as /bin/env or /usr/local/bin/env, but this currently
> is the best we could do.

This seems sensible.  We know that /bin/sh is not POSIX-compatible on
some proprietary Unices, but we're not targeting those in CI, and Git
for Windows, macOS, and all of the major open-source Linux and BSD
distros have /bin/sh as a POSIX-compatible shell.

/usr/bin/env bash is also safer than /bin/bash, because bash is not in
/usr/bin on most of the BSDs.  Every other project I've seen writes
/usr/bin/env, so I think that's a fairly safe assumption most places,
and I agree that it's the best we can do.

So I think is the right move.
diff mbox series

Patch

diff --git a/ci/check-directional-formatting.bash b/ci/check-directional-formatting.bash
index e6211b141a..3cbbb7030e 100755
--- a/ci/check-directional-formatting.bash
+++ b/ci/check-directional-formatting.bash
@@ -1,4 +1,4 @@ 
-#!/bin/bash
+#!/usr/bin/env bash
 
 # This script verifies that the non-binary files tracked in the Git index do
 # not contain any Unicode directional formatting: such formatting could be used