-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Use bash with source #4761
Use bash with source #4761
Conversation
b20da2f
to
95e8787
Compare
@@ -13,7 +13,7 @@ ENVTEST_ASSETS_DIR=$(shell pwd)/testbin | |||
test: manifests generate fmt vet | |||
mkdir -p ${ENVTEST_ASSETS_DIR} | |||
test -f ${ENVTEST_ASSETS_DIR}/setup-envtest.sh || curl -sSLo ${ENVTEST_ASSETS_DIR}/setup-envtest.sh https://raw.githubusercontent.com/kubernetes-sigs/controller-runtime/master/hack/setup-envtest.sh | |||
source ${ENVTEST_ASSETS_DIR}/setup-envtest.sh; fetch_envtest_tools $(ENVTEST_ASSETS_DIR); setup_envtest_env $(ENVTEST_ASSETS_DIR); go test ./... -coverprofile cover.out | |||
base -c "source ${ENVTEST_ASSETS_DIR}/setup-envtest.sh; fetch_envtest_tools $(ENVTEST_ASSETS_DIR); setup_envtest_env $(ENVTEST_ASSETS_DIR); go test ./... -coverprofile cover.out" |
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.
base -c "source ${ENVTEST_ASSETS_DIR}/setup-envtest.sh; fetch_envtest_tools $(ENVTEST_ASSETS_DIR); setup_envtest_env $(ENVTEST_ASSETS_DIR); go test ./... -coverprofile cover.out" | |
bash -c "source ${ENVTEST_ASSETS_DIR}/setup-envtest.sh; fetch_envtest_tools $(ENVTEST_ASSETS_DIR); setup_envtest_env $(ENVTEST_ASSETS_DIR); go test ./... -coverprofile cover.out" |
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.
Fixed.
@varshaprasad96 could you verify this change also works on a Mac? |
This patch calls the commands with `source` with the bash directly. This way these commands also work on OS like Debian where `/bin/sh` is actually not the bash, but another maybe not compatitible shell like `dash`. Signed-off-by: Sven Haardiek <sven@haardiek.de>
95e8787
to
1c5b687
Compare
This will be fixed by kubernetes-sigs/kubebuilder#2149 upstream then pulled into operator-sdk. If the fix is not accepted upstream, I'd like to port those changes here. |
/hold |
Description of the change:
This patch calls the commands with
source
with the bash directly.Motivation for the change:
This way these commands also work on OS like Debian where
/bin/sh
is actuallynot the bash, but another maybe not compatitible shell like
dash
.Checklist
If the pull request includes user-facing changes, extra documentation is required:
changelog/fragments
(seechangelog/fragments/00-template.yaml
)website/content/en/docs