search

plexus / chemacs

Emacs profile switcher
652 stars 45 forks source link

fix non-standard shebang #23

Closed Yumasi closed 4 years ago

Yumasi commented 4 years ago

The only paths you can take for granted are /bin/sh and /usr/bin/env. Using /bin/bash may not work on every system (for example on NixOS). This commit fixes this by using /bin/sh instead.

Signed-off-by: Guillaume Pagnoux guillaume.pagnoux@lse.epita.fr

plexus commented 4 years ago

Did you review the script for bashisms?

Yumasi commented 4 years ago

Did you review the script for bashisms?

I have modified it to make it POSIX here. While, it's functionaly identical, the output differs a bit (hence why it is not part of this PR). Let me know if you want me to add it. I can also make a more "conservative" version if you like.

plexus commented 4 years ago

I just need to know that everything that is there will work on bourne shell the same it would on bash, otherwise I can't safely merge it. Your version of install.sh removes some functionality so I can't accept that.

Yumasi commented 4 years ago

I've updated the PR and removed the bashisms without modifying much of the script architecture. I had to remove chemacs_home, since it used BASH_SOURCE and was basically equivalent to $(dirname $0) in the way it was used in the script.

plexus commented 4 years ago

Thanks!

plexus commented 4 years ago

Not sure the directory detection will work as reliably as before, there are quite a few edge cases, see https://stackoverflow.com/questions/59895/how-to-get-the-source-directory-of-a-bash-script-from-within-the-script-itself/246128#246128 for discussion. However we don't expect people to do exotic things with this script like sourcing it directly, symlinking it or adding it to the $PATH, so I'm going to assume this is fine. Thanks again for helping to make chemacs better.