Fix crash on any shell other than bash

[daniandtheweb]

The "need_to_be_sourced.sh" script crashes on any shell other than bash.

This small change should allow the script to work fine on any linux shell.

[jeroen-mostert]

If it's supposed to work for shells other than bash, shouldn't the shebang just be #!/bin/sh rather than hardcode bash?

[daniandtheweb]

You're right, I forgot to change it.

[jeroen-mostert]

I missed the fact that the thing was changed to use BASH_SOURCE in the meantime. That makes the use of any shell other than Bash pointless, of course, unless they happen to support this Bash extension, which is unlikely. In typical Unix fashion, robustly detecting if a script is being sourced is needlessly complicated. With this in mind, something that only gives the warning on bash and just does nothing on other shells seems good enough.

[daniandtheweb]

I've tested the command on bash, zsh and fish and it detects BASH_SOURCE fine with all of them so I think 99% of the cases should be covered with that.

[jeroen-mostert]

How are you testing it? On systems where sh is symlinked to /bin/bash, this will end up invoking bash and run as intended. But if you run zsh env_rocm.sh it will just output nothing, same as when /bin/sh is symlinked to /bin/zsh, since the condition is never true. Implicitly using bash is fine (IMO) but then of course my point about using /bin/sh no longer stands. :P

[daniandtheweb]

Ok, I understand what you mean by that, I was testing it differently and the results were incomplete and wrong. In this case let's just say this pr can still see improvements. The main goal is achieved, avoid the crash on any shell other than bash, but showing the error on any other shell seems quite more complicated. I'll try to search for a better way to do it.

Thanks for the comment, otherwise I would've thought everything was okay now.

[jeroen-mostert]

Although making it work for any POSIX shell when using /bin/sh is a noble goal I think it may be overshooting the mark. This repo can't build if bash is not installed on the system as multiple scripts explicitly rely on it. Therefore, using #!/bin/bash in this script and relying on a bashism to check if it's being executed rather than sourced is fine, I think. Users of other shells will just get bash telling them not to run the script, unless they use an exotic way of launching the script. When they use source the bash conditional should silently fail on most shells (possibly not all, haven't tested) and the script will end up running, as intended.

The only thing to be aware of is that sourced scripts may be running in other shells that the user is using for their front-end work, and so other than that bash check the rest should be POSIX, but as long as the script is doing nothing exciting but some exports that's fine.

[lamikr]

I like from the 1-line change to use if [[ "${BASH_SOURCE[0]}" == "${0}" ]]; then and I tested that it works on all linux distros I have. So I will cherry-pick the ea7da11ab commit.

But @jeroen-mostert is right that that the build scripts will anyway rely on bash-features, so it's ok to keep other scripts now also to relying on bash. (And probably even more in future because I think the "readarray -t a < /path/to/filename" could be good in future to read the files that specify binfo files)

[lamikr]

@daniandtheweb Thanks, I merged now the relevant part from bash change. It's now on master and 612 release branches.