-
Notifications
You must be signed in to change notification settings - Fork 1.2k
🐛 Quote path with -p env in envtest-setup #1513
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
🐛 Quote path with -p env in envtest-setup #1513
Conversation
The path could have spaces, single quotes, etc in in, so this quotes the path and escapes single quotes so we can export properly.
FYI @vincepri |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: DirectXMan12, vincepri The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/retest |
hmm... looks like a flaky timing test? Occaisional timeouts waiting for etcd of kube-apiserver to start, esp on the "it should be fast to set up and tear down" tests. /retest |
// - shell strings that are next to each other are concatenated (so "a""b""c" == "abc") | ||
// - you can intermix quote styles using the above | ||
// - so `'"'"'` --> CLOSE_QUOTE + "'" + OPEN_QUOTE | ||
shellQuoted := strings.ReplaceAll(path, "'", `'"'"'`) |
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.
So the result is supposed to look something like the following?
$ export FOO='/foo/'"'"'Application Support'"'"''
$ echo $FOO
/foo/'Application Support'
The path could have spaces, single quotes, etc in in, so this quotes the
path and escapes single quotes so we can export properly.
This is very relevant on darwin, where our default store path contains spaces (it includes "Application Support")