Fix bazel installation for github codespace #49472
Conversation
dentiny
force-pushed
the
hjiang/fix-github-codespace-install
branch
from
34b6a39 to
82c5d3f
Compare
dentiny
force-pushed
the
hjiang/fix-github-codespace-install
branch
2 times, most recently
from
d34ea2f to
b08f55f
Compare
ci/env/install-bazel.sh
Outdated
| @@ -95,6 +95,10 @@ fi | |||
|
|
|||
| bazel --version | |||
There was a problem hiding this comment.
If this line works then this means it's part of path already? Seems our installation target is bin/ folder?
There was a problem hiding this comment.
No, checkout the issue linked; I'm interested in this issue since I met the same issue when I was using github codespace.
There was a problem hiding this comment.
Executable accessible could be installed in the current directory.
There was a problem hiding this comment.
Don't we always install to target="$HOME/bin/bazel" or target="/bin/bazel"
There was a problem hiding this comment.
For me particularly,
@SaSikun ➜ /workspaces/ray (master) $ echo $HOME
/home/codespace
+ bazel --version
bazel 6.5.0
+ which bazel
/home/codespace/bin/bazel
And bazel does install under $HOME/bin/bazel, but it's not in PATH
Signed-off-by: dentiny <[email protected]>
dentiny
force-pushed
the
hjiang/fix-github-codespace-install
branch
from
b08f55f to
ce2c12d
Compare
ci/env/install-bazel.sh
Outdated
|
|
||
| # Append bazel executable directory to system `PATH`. | ||
| BAZEL_EXEC_BIN=$(dirname "$(which bazel)") | ||
| echo "export PATH=${BAZEL_EXEC_BIN}:\$PATH" >> ~/.bashrc |
There was a problem hiding this comment.
I'm not sure if it's good idea to change user's .bashrc. Should we instead update the doc instead telling people to put the installation path to the PATH?
There was a problem hiding this comment.
TBH I still prefer to add it into system executable path, and it's commonly used in a lot of tools that I use:
[ -f ~/.fzf.bash ] && source ~/.fzf.bash
eval "$(thefuck --alias)"
There was a problem hiding this comment.
I feel it might have unintended side effect: at least we should do PATH=:$PATH:${BAZEL_EXEC_BIN} instead of PATH=${BAZEL_EXEC_BIN}:\$PATH
There was a problem hiding this comment.
Emm I don't see much difference? Just search order. BTW in the latest commit there's only Readme doc change but no code change
Signed-off-by: dentiny <[email protected]>
Fix issue: #49470
The problem is: not all the platform defaults binary installation to default system executable path, so we have to append them manually to the
PATH.