Fix: Quote path variables in shell scripts to handle space characters in directory paths#642
Open
vknaik wants to merge 2 commits intollm-d:mainfrom
Open
Fix: Quote path variables in shell scripts to handle space characters in directory paths#642vknaik wants to merge 2 commits intollm-d:mainfrom
vknaik wants to merge 2 commits intollm-d:mainfrom
Conversation
- Fixed unquoted path variables causing word splitting in paths with spaces - Updated standup.sh: quoted pushd, realpath, source, find, and sed expressions - Updated env.sh: quoted source, find, mkdir, rm, cp, touch, ls commands - Updated functions.sh: quoted extensive path operations (mkdir, rm, cp, touch, cat, ls, find, rsync) - Updated run.sh: quoted pushd, realpath, source, find, mkdir, rm, ls commands - Updated teardown.sh: quoted pushd, realpath, source commands - Updated e2e.sh: quoted pushd, realpath, source, find, ls commands Resolves issue where standup.sh and other scripts would fail when run from directories containing spaces in their path.
…mmit.sh - Fixed unquoted path variables in all setup/steps/*.sh scripts (00-10) - Fixed util/setup_precommit.sh path variable quoting - Ensures proper handling of directory paths containing spaces - Completes the space-handling fixes across the repository Related to previous commit fixing setup/*.sh scripts
Collaborator
|
This PR contains Bash scripts for the various steps which have been deprecated in favor of Python. Please remove the files under |
Author
|
@namasl You are correct. I will remove the .sh files under |
namasl
reviewed
Feb 6, 2026
| for workload_type in ${LLMDBENCH_HARNESS_PROFILE_HARNESS_LIST}; do | ||
| llmdbench_execute_cmd "${LLMDBENCH_CONTROL_KCMD} --namespace ${LLMDBENCH_HARNESS_NAMESPACE} delete configmap $workload_type-profiles --ignore-not-found" ${LLMDBENCH_CONTROL_DRY_RUN} ${LLMDBENCH_CONTROL_VERBOSE} | ||
| llmdbench_execute_cmd "${LLMDBENCH_CONTROL_KCMD} --namespace ${LLMDBENCH_HARNESS_NAMESPACE} create configmap $workload_type-profiles --from-file=${LLMDBENCH_CONTROL_WORK_DIR}/workload/profiles/${workload_type}" ${LLMDBENCH_CONTROL_DRY_RUN} ${LLMDBENCH_CONTROL_VERBOSE} | ||
| llmdbench_execute_cmd "${LLMDBENCH_CONTROL_KCMD} --namespace ${LLMDBENCH_HARNESS_NAMESPACE} create configmap $workload_type-profiles --from-file=\"${LLMDBENCH_CONTROL_WORK_DIR}/workload/profiles/${workload_type}\"" ${LLMDBENCH_CONTROL_DRY_RUN} ${LLMDBENCH_CONTROL_VERBOSE} |
Collaborator
There was a problem hiding this comment.
Not functional, but adding back spacing for clarity
Suggested change
| llmdbench_execute_cmd "${LLMDBENCH_CONTROL_KCMD} --namespace ${LLMDBENCH_HARNESS_NAMESPACE} create configmap $workload_type-profiles --from-file=\"${LLMDBENCH_CONTROL_WORK_DIR}/workload/profiles/${workload_type}\"" ${LLMDBENCH_CONTROL_DRY_RUN} ${LLMDBENCH_CONTROL_VERBOSE} | |
| llmdbench_execute_cmd "${LLMDBENCH_CONTROL_KCMD} --namespace ${LLMDBENCH_HARNESS_NAMESPACE} create configmap $workload_type-profiles --from-file=\"${LLMDBENCH_CONTROL_WORK_DIR}/workload/profiles/${workload_type}\"" ${LLMDBENCH_CONTROL_DRY_RUN} ${LLMDBENCH_CONTROL_VERBOSE} |
namasl
reviewed
Feb 6, 2026
|
|
||
| export LLMDBENCH_RUN_EXPERIMENT_ID_PREFIX="" | ||
|
|
||
| for treatment in $(ls ${LLMDBENCH_CONTROL_WORK_DIR}/workload/profiles/${workload_type}/*.yaml); do |
namasl
reviewed
Feb 6, 2026
| export LLMDBENCH_HARNESS_EXPERIMENT_PROFILE=$(echo $treatment | rev | cut -d '/' -f 1 | rev) | ||
|
|
||
| tf=$(cat ${treatment} | grep "#treatment" | tail -1 | $LLMDBENCH_CONTROL_SCMD 's/^#//' || true) | ||
| if [[ -f ${LLMDBENCH_CONTROL_WORK_DIR}/workload/profiles/${workload_type}/treatment_list/$tf ]]; then |
namasl
reviewed
Feb 6, 2026
|
|
||
| local_results_dir=${LLMDBENCH_CONTROL_WORK_DIR}/results/${LLMDBENCH_RUN_EXPERIMENT_RESULTS_DIR_SUFFIX} | ||
| local_analysis_dir=${LLMDBENCH_CONTROL_WORK_DIR}/analysis/${LLMDBENCH_RUN_EXPERIMENT_RESULTS_DIR_SUFFIX} | ||
| llmdbench_execute_cmd "mkdir -p ${local_results_dir}_${i} && mkdir -p ${local_analysis_dir}_${i}" \ |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #641
Problem:
Scripts fail when run from directories containing spaces in their path due to unquoted variables causing word splitting.
Solution:
Changes:
Testing:
Verified scripts can now run from directories with spaces in the path.
Commits: