diff mbox series

[FFmpeg-devel,v3,2/5] ffbuild/libversion.sh: add shebang

Message ID 20240409135034.95513-2-jdek@itanimul.li
State Accepted
Commit fcfd17dbb4a6cf270cdd82e91c21a5efdc878d12
Headers show
Series [FFmpeg-devel,v3,1/5] configure: simplify bigendian check | expand

Checks

Context Check Description
yinshiyou/make_loongarch64 success Make finished
yinshiyou/make_fate_loongarch64 success Make fate finished
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

J. Dekker April 9, 2024, 1:50 p.m. UTC
The implicit interpreter is dependent on the environment, and isn't
guaranteed to be /bin/sh. Some packagers call this script directly, and
in certain environments such as containers using qemu-user through
binfmt_misc emulation on Linux it doesn't fallback to /bin/sh.

To fix these cases we add the interpreter explicitly.

Signed-off-by: J. Dekker <jdek@itanimul.li>
---
 ffbuild/libversion.sh | 1 +
 1 file changed, 1 insertion(+)

Comments

Marth64 April 9, 2024, 9:52 p.m. UTC | #1
> +#!/bin/sh
Might I suggest `#!/usr/bin/env sh` instead for this case?
I tend to prefer it from a portability and usability perspective,
but I can imagine for sh it might not matter.

I am not close to the patch that you are working on.
But thought to throw this out there in case there is a
platform or user relying on unique behaviour here.

Thank you for your time,
Henrik Gramner April 9, 2024, 10:27 p.m. UTC | #2
On Tue, Apr 9, 2024 at 11:52 PM Marth64 <marth64@proxyid.net> wrote:
> > +#!/bin/sh
> Might I suggest `#!/usr/bin/env sh` instead for this case?
> I tend to prefer it from a portability and usability perspective,
> but I can imagine for sh it might not matter.

/bin/sh exists on virtually every *NIX system whereas /usr/bin/env is
not ubiquitous, so that seems like a terrible idea that would achieve
the opposite result.
Marth64 April 9, 2024, 10:52 p.m. UTC | #3
> so that seems like a terrible idea that would achieve
> the opposite result.

I respectfully disagree.

Neither approach is universal or POSIX specified.
So while I agree it can be left as-is since only
a basic Bourne shell is needed, I would not just
dismiss it/write it off as a terrible idea.

There are reasons why #!/usr/bin/env is colloquially preferred
to launch an interpreter in most shell script cases.
For example, there are systems where /bin/sh is NOT
a POSIX compliant shell. Alternatively, what if I have a different
or compliant `sh` in my PATH, but not necessarily in /bin?
This is not a scenario I made up, one can quickly research to see.

I will not bikeshed the topic further, we can go either way,
but I don't think it qualifies as a "terrible idea".
Marth64 April 9, 2024, 11 p.m. UTC | #4
Regardless -- it can be left as is . I digress from the topic.

Thank you,
diff mbox series

Patch

diff --git a/ffbuild/libversion.sh b/ffbuild/libversion.sh
index a94ab58057..ecaa90cde6 100755
--- a/ffbuild/libversion.sh
+++ b/ffbuild/libversion.sh
@@ -1,3 +1,4 @@ 
+#!/bin/sh
 toupper(){
     echo "$@" | tr abcdefghijklmnopqrstuvwxyz ABCDEFGHIJKLMNOPQRSTUVWXYZ
 }