diff mbox

[dpdk-dev] Fix bash path in shebangs

Message ID 20170727201212.9252-1-asomers@gmail.com (mailing list archive)
State Changes Requested, archived
Headers

Checks

Context Check Description
ci/checkpatch warning coding style issues
ci/Intel-compilation success Compilation OK

Commit Message

asomers@gmail.com July 27, 2017, 8:12 p.m. UTC 
  From: Alan Somers <asomers@gmail.com>

"/bin/bash" is a Linuxism.  "/usr/bin/env bash" is portable.

Signed-off-by Alan Somers <asomers@gmail.com>
---
 examples/performance-thread/l3fwd-thread/test.sh | 2 +-
 usertools/dpdk-setup.sh                          | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

Comments

Thomas Monjalon Sept. 13, 2017, 9:37 a.m. UTC | #1 
27/07/2017 22:12, asomers@gmail.com:
> From: Alan Somers <asomers@gmail.com>
> 
> "/bin/bash" is a Linuxism.  "/usr/bin/env bash" is portable.

Why is it an issue?

Can you run dpdk-setup.sh on a non-Linux system?
asomers@gmail.com Sept. 13, 2017, 2:35 p.m. UTC | #2 
On Wed, Sep 13, 2017 at 3:37 AM, Thomas Monjalon <thomas@monjalon.net> wrote:
> 27/07/2017 22:12, asomers@gmail.com:
>> From: Alan Somers <asomers@gmail.com>
>>
>> "/bin/bash" is a Linuxism.  "/usr/bin/env bash" is portable.
>
> Why is it an issue?
>
> Can you run dpdk-setup.sh on a non-Linux system?
>

Nope, because even dpdk-setup.sh assumes that bash is located at
/bin/bash.  But "/usr/bin/env bash" works everywhere.
Thomas Monjalon Sept. 13, 2017, 3:39 p.m. UTC | #3 
13/09/2017 16:35, alan somers:
> On Wed, Sep 13, 2017 at 3:37 AM, Thomas Monjalon <thomas@monjalon.net> wrote:
> > 27/07/2017 22:12, asomers@gmail.com:
> >> From: Alan Somers <asomers@gmail.com>
> >>
> >> "/bin/bash" is a Linuxism.  "/usr/bin/env bash" is portable.
> >
> > Why is it an issue?
> >
> > Can you run dpdk-setup.sh on a non-Linux system?
> 
> Nope, because even dpdk-setup.sh assumes that bash is located at
> /bin/bash.  But "/usr/bin/env bash" works everywhere.

No, I mean: can you run dpdk-setup.sh on a non-Linux system after your change?

This script configures a Linux system, so I want to understand
what situation you are trying to fix.
asomers@gmail.com Sept. 13, 2017, 3:55 p.m. UTC | #4 
On Wed, Sep 13, 2017 at 9:39 AM, Thomas Monjalon <thomas@monjalon.net> wrote:
> 13/09/2017 16:35, alan somers:
>> On Wed, Sep 13, 2017 at 3:37 AM, Thomas Monjalon <thomas@monjalon.net> wrote:
>> > 27/07/2017 22:12, asomers@gmail.com:
>> >> From: Alan Somers <asomers@gmail.com>
>> >>
>> >> "/bin/bash" is a Linuxism.  "/usr/bin/env bash" is portable.
>> >
>> > Why is it an issue?
>> >
>> > Can you run dpdk-setup.sh on a non-Linux system?
>>
>> Nope, because even dpdk-setup.sh assumes that bash is located at
>> /bin/bash.  But "/usr/bin/env bash" works everywhere.
>
> No, I mean: can you run dpdk-setup.sh on a non-Linux system after your change?
>
> This script configures a Linux system, so I want to understand
> what situation you are trying to fix.

I'm using Ceph, which imports DPDK whole (and several other 3rd party
projects too).  I'm not sure which parts of these 3rd party projects
Ceph is actually using, but it's easier to fix the bash path
everywhere than to determine which places need it to be fixed.  And
AFAIK it doesn't cause any problems on any modern Unix derivative.
-Alan
Thomas Monjalon Sept. 13, 2017, 4:51 p.m. UTC | #5 
13/09/2017 17:55, alan somers:
> On Wed, Sep 13, 2017 at 9:39 AM, Thomas Monjalon <thomas@monjalon.net> wrote:
> > 13/09/2017 16:35, alan somers:
> >> On Wed, Sep 13, 2017 at 3:37 AM, Thomas Monjalon <thomas@monjalon.net> wrote:
> >> > 27/07/2017 22:12, asomers@gmail.com:
> >> >> From: Alan Somers <asomers@gmail.com>
> >> >>
> >> >> "/bin/bash" is a Linuxism.  "/usr/bin/env bash" is portable.
> >> >
> >> > Why is it an issue?
> >> >
> >> > Can you run dpdk-setup.sh on a non-Linux system?
> >>
> >> Nope, because even dpdk-setup.sh assumes that bash is located at
> >> /bin/bash.  But "/usr/bin/env bash" works everywhere.
> >
> > No, I mean: can you run dpdk-setup.sh on a non-Linux system after your change?
> >
> > This script configures a Linux system, so I want to understand
> > what situation you are trying to fix.
> 
> I'm using Ceph, which imports DPDK whole (and several other 3rd party
> projects too).  I'm not sure which parts of these 3rd party projects
> Ceph is actually using, but it's easier to fix the bash path
> everywhere than to determine which places need it to be fixed.  And
> AFAIK it doesn't cause any problems on any modern Unix derivative.

If I understand well, you don't know which case it is fixing,
but you prefer the shebang being this way.

I am a bit reluctant to fix something if we don't know what is the bug.
Maybe it does not hurt, but there can be some drawbacks:
	- the chosen bash binary depends on the environment PATH
	- it makes impossible to add some options in the shebang
diff mbox

Patch

diff --git a/examples/performance-thread/l3fwd-thread/test.sh b/examples/performance-thread/l3fwd-thread/test.sh
index b7718b622..eb1fe2dc2 100755
--- a/examples/performance-thread/l3fwd-thread/test.sh
+++ b/examples/performance-thread/l3fwd-thread/test.sh
@@ -1,4 +1,4 @@ 
-#!/bin/bash
+#!/usr/bin/env bash
 
 case "$1" in
 
diff --git a/usertools/dpdk-setup.sh b/usertools/dpdk-setup.sh
index c4fec5a63..ebf36f830 100755
--- a/usertools/dpdk-setup.sh
+++ b/usertools/dpdk-setup.sh
@@ -1,4 +1,4 @@ 
-#! /bin/bash
+#! /usr/bin/env bash
 
 #   BSD LICENSE
 #