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

Add || true to .bashrc lines in case set -e is on #3

Closed
wants to merge 1 commit into from
Closed

Add || true to .bashrc lines in case set -e is on #3

wants to merge 1 commit into from

Conversation

gibfahn
Copy link

@gibfahn gibfahn commented Nov 8, 2017

In the (super-rare) case that someone does something like:

set -e
. ~/.bashrc

and the nvm lines are the last two lines in the shell script, and the nvm directory doesn't exist, the bashrc will return 1 as the exit code, killing the parent shell.

To be honest I'm not sure it's worth fixing this edge case, most people who aren't writing scripts don't have set -e in their shell, and most people writing shell scripts aren't sourcing their

An alternative workaround for this is to add ; true (or ; : for maximum opacity) instead of || true), but I feel like || true is most obvious.

I tested this by doing:

▶▶▶ SOURCE_STR="\nexport NVM_DIR=\"${PROFILE_INSTALL_DIR}\"\n[ -s \"\$NVM_DIR/nvm.sh\" ] && \\. \"\$NVM_DIR/nvm.sh\" || true  # This loads nvm\n"
COMPLETION_STR="[ -s \"\$NVM_DIR/bash_completion\" ] && \\. \"\$NVM_DIR/bash_completion\" || true # This loads nvm bash_completion\n"
▶▶▶ printf "${COMPLETION_STR}"
[ -s "$NVM_DIR/bash_completion" ] && \. "$NVM_DIR/bash_completion" || true # This loads nvm bash_completion
▶▶▶ printf "${SOURCE_STR}"

export NVM_DIR=""
[ -s "$NVM_DIR/nvm.sh" ] && \. "$NVM_DIR/nvm.sh" || true  # This loads nvm

and it seems to work, I should probably test it more though.

Refs: nodejs/build#988

In the (super-rare) case that someone does something like:

```bash
set -e
. ~/.bashrc
```

and the nvm lines are the last two lines in the shell script, the bashrc
will return 1 as the exit code, killing the parent shell.

An alternative workaround for this is to add `; true` (or `; :` for
maximum opacity) instead of `|| true`), but I feel like `|| true` is
most obvious.
@ljharb
Copy link
Owner

ljharb commented Nov 8, 2017

Wrong location :-) and duplicate of nvm-sh#1659

@ljharb ljharb closed this Nov 8, 2017
@gibfahn gibfahn deleted the set-e branch November 8, 2017 19:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants