Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bash-forwarder-template should define the same header as bash-template #921

Closed
Latankar opened this issue Nov 22, 2016 · 6 comments · Fixed by #923
Closed

bash-forwarder-template should define the same header as bash-template #921

Latankar opened this issue Nov 22, 2016 · 6 comments · Fixed by #923
Labels
bug universal Zip, tar.gz, tgz and bash issues

Comments

@Latankar
Copy link

Latankar commented Nov 22, 2016

both templates define different headers

bash-template:

#!/usr/bin/env bash

bash-forwarder-template:

#!/bin/sh

Executing a bash-forwarder-template based script (example-app) the following message is displayed:

./example-app: 21: [: example-app: unexpected operator
./example-app: 29: ./incoming-messages-processor-app: [[: not found
@muuki88 muuki88 added universal Zip, tar.gz, tgz and bash issues bug labels Nov 22, 2016
@muuki88
Copy link
Contributor

muuki88 commented Nov 22, 2016

Thanks for the report. Would you mind sending a pull request to fix this? I'm not sure why none of the existing tests in src/sbt-test/bash found this.

@jan0sch
Copy link
Contributor

jan0sch commented Nov 22, 2016

I guess that on most linux systems you won't notice because they usually link /bin/sh to bash.

@muuki88
Copy link
Contributor

muuki88 commented Nov 22, 2016

the /bin/sh is on purpose as this more general, I was wondering about #!/usr/bin/env bash

@jan0sch
Copy link
Contributor

jan0sch commented Nov 22, 2016

If /bin/sh is used then the script must be sh compatible which is often not the case because of the mentioned fact that most linux distributions replace sh with bash. This means that such errors will only trigger on systems that don't replace sh with bash (bsds for example).
Using /usr/bin/env bash requires that bash is installed but provides a cross plattform way to execute them.
I don't know if a sh script will run on a bash though.

@jan0sch
Copy link
Contributor

jan0sch commented Nov 27, 2016

According to debian (https://wiki.debian.org/Shell) they used to use bash as bin/sh replacement but switched to dash as bin/sh replacement from Squeeze on forward.
I haven't researched this in depth for other distributions but if we consider the fact that most distributions replace bin/sh with something to their liking then it may be the safer bet to use #!/usr/bin/env bash in general. This explicitely requires a bash on the system and will circumvent any possible bugs that might occur if bin/sh has been replaced with some esoteric shell we might not even have heard about. ;-)

Shall I provide a PR that adjusts the forwarder-template to the env solution?

@muuki88
Copy link
Contributor

muuki88 commented Nov 27, 2016

Thanks for the research 👍 I found the pull request were a similar fix was introduced (#167).

then it may be the safer bet to use #!/usr/bin/env bash

Sounds good

Shall I provide a PR that adjusts the forwarder-template to the env solution?

That would be awesome :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug universal Zip, tar.gz, tgz and bash issues
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants