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

Proper fix for dirname in env.sh #23

Closed
hohwille opened this issue Dec 17, 2018 · 10 comments
Closed

Proper fix for dirname in env.sh #23

hohwille opened this issue Dec 17, 2018 · 10 comments
Labels
enhancement New feature or request help wanted Extra attention is needed linux specific for linux OS (debian, ubunutu, suse, etc.) scripts related to shell scripts (bash and CMD)
Milestone

Comments

@hohwille
Copy link
Member

User @maihacke changed env.sh to use $(dirname "$0"):
b9cca78#diff-59bbdf4f0007715da1871587ba4ffd3d

I assume this was some kind of bugfix for MacOS. Also this seems more reliable to me as $BASH_SOURCE is proprietary and only works in bash. What is the purpose why this was ever changed to this?

User @vapadwal wants to change env.sh back to $(dirname "$BASH_SOURCE"):
985a51e#diff-59bbdf4f0007715da1871587ba4ffd3d

As I do not like to have git wars, I would kindly ask all participants to clarify the reasons for these changes and find a peaceful solution.

@hohwille hohwille added this to the release:3.0.0 milestone Dec 17, 2018
@hohwille hohwille added enhancement New feature or request help wanted Extra attention is needed linux specific for linux OS (debian, ubunutu, suse, etc.) scripts related to shell scripts (bash and CMD) labels Dec 17, 2018
@hohwille
Copy link
Member Author

hohwille commented Dec 19, 2018

I was able to reproduce the error somehow:

Jorgs-MacBook-Pro:devon hohwille$ . env.sh
dirname: illegal option -- b
usage: dirname path
IDE environment has been initialized.
Jorgs-MacBook-Pro:devon hohwille$ echo $0
-bash

So in some cases bash has -bash as $0 what is causing the issue (then dirname is -).
Hence I think both "fixes" are wrong.
IMHO we need an if that tests if we are in bash and then uses $BASH_SOURCE and otherwise uses $0 to make it work in any shell.

@hohwille
Copy link
Member Author

So IMHO $BASH_SOURCE refers to the file that has been sourced (via . script or source script) whereas $0 refers to the invocation of a script (will be path/script or script depending on how you called the script). Hence $0 never makes sense for env.sh. The question is more how can we make env.sh work with other shells such as zsh?

@maihacke
Copy link
Member

maihacke commented Dec 19, 2018

Maybe this could be a solution (please try an report, if it works):

#!/bin/bash
#
# Source this script from a BASH shell to setup the shell for the corresponding project.
# 
pushd "$(dirname "${BASH_SOURCE:-$_}")" >/dev/null
export OASP_PROJECT_HOME=`pwd`

. $PWD/scripts/environment-project

popd > /dev/null

@hohwille
Copy link
Member Author

hohwille commented Dec 19, 2018

Create test.sh with this content:

echo "0=$0"
echo "BASH_SOURCE=$BASH_SOURCE"
echo "_=$_"

This will produce:

$ . ./test.sh
0=-bash
BASH_SOURCE=./test.sh
_=BASH_SOURCE=./test.sh

I hope this makes it clear. $0 was always wrong, had been fixed and was accidentally be reintroduced via PR (I assume some copy & paste from old source).
$BASH_SOURCE is correct but will only work in bash.

@hohwille
Copy link
Member Author

$ zsh
% . ./test.sh
0=./test.sh
BASH_SOURCE=
_=BASH_SOURCE=

So in zsh the $0 variant is correct.

@hohwille
Copy link
Member Author

$_ seems to be the last command (even within the current script)

@maihacke
Copy link
Member

yepp, nonsense :-/

@maihacke
Copy link
Member

maihacke commented Dec 19, 2018

maybe we should go with ${BASH_SOURCE[0]-$0} then
Should work in Bash and ZSH

➜  ~ cat test.sh 
echo "0=$0"
echo "BASH_SOURCE=$BASH_SOURCE"
echo "_=$_"
echo "${BASH_SOURCE[0]-$0}"

➜  ~ . ./test.sh 
0=./test.sh
BASH_SOURCE=
_=BASH_SOURCE=
./test.sh

➜  ~ bash
bash-3.2$ . ./test.sh 
0=bash
BASH_SOURCE=./test.sh
_=BASH_SOURCE=./test.sh
./test.sh

@hohwille
Copy link
Member Author

hohwille commented Jan 8, 2019

We only need to review and merge PR #25 that fixes this issue together with #21.

hohwille added a commit that referenced this issue Feb 8, 2019
* #21: improve OS support

#23: proper fix for dirname of sourced script

* #21: iTerm2 support with tabs now working

* #21: improved windows documentation

* #21: improve Mac support

* #21: improved Mac Readme

* #21: Improved IDEenv for linux/Mac

* #21: fixed assembly packaging for OS variants, improved Mac keybindings
@hohwille
Copy link
Member Author

fixed with PR #21

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed linux specific for linux OS (debian, ubunutu, suse, etc.) scripts related to shell scripts (bash and CMD)
Projects
None yet
Development

No branches or pull requests

2 participants