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

Check that tools/bazel is not a directory before attempting to exec #10477

Closed
wants to merge 1 commit into from

Conversation

@aaliddell
Copy link
Contributor

aaliddell commented Dec 22, 2019

Fixes #10476

@googlebot googlebot added the cla: yes label Dec 22, 2019
@philwo

This comment has been minimized.

Copy link
Member

philwo commented Jan 2, 2020

Thank you! I'll merge this.

@@ -191,7 +191,7 @@ if [[ ! -x $BAZEL_REAL ]]; then
fi

readonly wrapper="${workspace_dir}/tools/bazel"
if [[ -x "$wrapper" ]]; then
if [[ -x "$wrapper" && ! -d "$wrapper" ]]; then

This comment has been minimized.

Copy link
@philwo

philwo Jan 2, 2020

Member

Code review brought up the question of why not just use -f here instead of ! -d:

if [[ -x "$wrapper" && -f "$wrapper" ]]; then

It looks fine to me, because symlinks are handled correctly that way, too ("If file is a symbolic link, test will fully dereference it and then evaluate the expression against the file referenced"), but wanted to check with you.

This comment has been minimized.

Copy link
@aaliddell

aaliddell Jan 2, 2020

Author Contributor

Honestly I just went with what it was before, so I didn’t really think about alternatives. If there’s a preferred change, I’m fine with that 👍

This comment has been minimized.

Copy link
@aaliddell

aaliddell Jan 2, 2020

Author Contributor

I’m also not able to make the changes to the pr currently, so feel free to edit it yourself if you want

@bazel-io bazel-io closed this in 145896c Jan 2, 2020
@aaliddell aaliddell deleted the aaliddell:fix-directory-exec branch Jan 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.