-
Notifications
You must be signed in to change notification settings - Fork 12
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 tr to output the list of files #8
use tr to output the list of files #8
Conversation
On DevOpsHiveHQ#7 it's reported that when using MacOs' default `find`, it makes `kubechc` to crash. This PR aims at leveraging `tr` command instead to print the `:` separated list of files instead of relying on a flag only available in specific versions of `find`.
kubech
Outdated
@@ -54,7 +54,7 @@ chmod 744 "${KUBECONFIG_SRC_DIR}" "${KUBECONFIG_DEST_DIR}" | |||
# Handeling Kube config files. | |||
_kubeconfig_files () { | |||
if [ -d "${KUBECONFIG_SRC_DIR}" ]; then | |||
KUBECONFIG_FILES="$(find "${KUBECONFIG_SRC_DIR}" -type f -printf '%p:')" | |||
KUBECONFIG_FILES="$(find "${KUBECONFIG_SRC_DIR}" -type f -print0 | tr '/n' ':')" |
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.
It should be -print | tr '\n' ':'
Could you please check if it works on MacOS?
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.
His branch as-is works for me on macOS 12 using -print0
. But i did notice "${KUBECONFIG_SRC_DIR}
looks at ~/.kube/config.src.d/
for me which is empty.
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.
@defenestration that's why it doesn't really work (it will put everything as 1 line and there is no new line in the output).
Could you please test it using -print | tr '\n' ':'
?
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.
Seems to work fine that way too, here is the diff against the branch in the PR for confirmation
diff --git a/kubech b/kubech
index 5576f70..68c60e9 100644
--- a/kubech
+++ b/kubech
@@ -54,7 +54,7 @@ chmod 744 "${KUBECONFIG_SRC_DIR}" "${KUBECONFIG_DEST_DIR}"
# Handeling Kube config files.
_kubeconfig_files () {
if [ -d "${KUBECONFIG_SRC_DIR}" ]; then
- KUBECONFIG_FILES="$(find "${KUBECONFIG_SRC_DIR}" -type f -print0 | tr '/n' ':')"
+ KUBECONFIG_FILES="$(find "${KUBECONFIG_SRC_DIR}" -type f -print | tr '\n' ':')"
fi
echo "${KUBECONFIG_FILES}${KUBECONFIG_ORIG}"
}
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.
@defenestration thanks a lot for your feedback 👌
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.
That's a good idea.
Thanks for the PR 🚀
I've added a small comment there.
On #7 it's reported that when using MacOs' default
find
, it makeskubechc
to crash.This PR aims at leveraging
tr
command instead to print the:
separated list of files instead of relying on a flag only available in specific versions offind
.Eventho it's not a very scientific test, it works on my MacOS Monterrey mac. There's no reason for it not to work in any widespread linux distribution.