-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Call bash only when necessary on macOS #54769
Conversation
185b7b8
to
b6a77fc
Compare
Following @waynew advice from PR #54216
I have modified this PR too in order to fulfil the new release strategy. |
Skip test test_shell_properly_handled_on_macOS if platform is not macOS
I would like to know if this PR will be finally merged |
Please @xeacott, could you revise this PR, it is open for nearly two months. Thank you in advance. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me!
Please @xeacott, could you revise this PR, it solves an issue introduced in #54136. By accepting this pull request, #54079 could be closed since it was duplicated in #54136 in order to rebased commits into 2019.2.1 instead of 2019.2 I mention @weswhet too, because I don't know if @xeacott is actively working on this project since I haven't heard from him for a long time. Thank you all in advance!! |
What does this PR do?
This PR fixes a bug introduced on PR #54136 (and possibly on PR #54079 too) that calls
/bin/bash
when the user's default shell is not bash.Also, users that have set a different version of bash as their default shell, they might be calling the wrong shell.
For example, for people with
zsh
as default shell the current behaviour is not right, neither for people with/usr/local/bin/bash
as their mainshell
since the current code is invoking/bin/bash
instead the right one.New Behavior
The new behaviour introduced in this PR invokes the right bash shell only when the users default shell is bash.
This is done in this way to load the right environment as it was intended in PR #54136.
Tests written?
✅
Commits signed with GPG?
✅