Skip to content

Don't assume that gcloud is in the same place as gsutil. #695

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Apr 15, 2019
Merged

Don't assume that gcloud is in the same place as gsutil. #695

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions lib/common.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -943,11 +943,16 @@ common::set_cloud_binaries () {
logecho -n "Checking/setting cloud tools: "

for GSUTIL in "$(which gsutil)" /opt/google/google-cloud-sdk/bin/gsutil; do
[[ -x $GSUTIL ]] && break
if [[ -x "$GSUTIL" ]]; then
break
fi
done

# gcloud should be in the same place
GCLOUD=${GSUTIL/gsutil/gcloud}
for GCLOUD in "${GSUTIL}/gsutil/gcloud" "$(which gcloud)"; do
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is different in meaning because in the original code, ${GSUTIL/gsutil/gcloud}is doing a pattern substitution (brace expansion). In your change you are treating the slashes as path delimiters in a filesystem. So the first value in the list won't be executable (it most likely doesn't exist). And in the Docker container the path to gcloud is probably not in the PATH, so the second command probably fails too ("which" will fail).

That's probably why it broke in the container, is my guess.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

${GSUTIL/gsutil/gcloud}is doing a pattern substitution (brace expansion)

Wow, yeah I totally overlooked that. My fault for changing it without understanding its effects, but I had no idea bash could do pattern substitution in this manner, and just assumed that was a path.

Thanks for the explanation.

if [[ -x "$GCLOUD" ]]; then
break
fi
done

if [[ -x "$GSUTIL" && -x "$GCLOUD" ]]; then
logecho -r $OK
Expand Down