fix(tasks): stop expanding possibly-empty arrays bare under set -u - #278
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 10 included reviews per hour; 6 remain after this review. 📝 WalkthroughWalkthroughThe pull request adds regression coverage for unsafe empty-array expansion in strict-mode Bash task scripts. It also updates the serve and test tasks to conditionally expand optional arrays. ChangesBash array safety
Suggested reviewers: Priority: ⬇️ Low Merge Risk: ⚪ Minimal · up to The task scripts now omit empty optional argument arrays safely while preserving populated arguments. No unresolved merge-readiness risk was identified. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
macOS bash 3.2.57 treats "${ARR[@]}" on an empty array as an unbound
variable under set -u, so serve.bash failed before the server started for
any non-cuda13 bundle. Use the portable ${ARR[@]+"${ARR[@]}"} form in
serve.bash and test.bash, and add a test that scans every task script for
the class.
|
@coderabbitai review |
|
1cf8aac to
9779263
Compare
1 participant
Summary
macOS ships bash 3.2.57, where
"${ARR[@]}"on an empty array is a hard error underset -urather than an empty expansion:serve.bashdeclaresUV_BUNDLE_ARGS=(), fills it only for cuda13 bundles, and expands it bare, somise run servewith any other bundle fails on a stock macOS shell before the server starts.SERVER_ARGS=("$@")on the two lines below it is the same shape with a different trigger: empty whenever the task is run with no arguments.test.bashcarries the same shape twice (ARGS,model_node_ids).The portable form
${ARR[@]+"${ARR[@]}"}expands to nothing when the array is empty and is identical otherwise."${ARR[@]:-}"is not a substitute, because it injects one empty argument.The guard
tools/ci/tests/test_bash_empty_array_expansion.pyscans every task script underset -uand keys on the arrays that can actually be empty: declared literally empty, or seeded from the script's own arguments. Two positive controls stop it passing vacuously, and a second test pins the detector against the crashing shape and its fixed form, quoted and unquoted, including an unsafe expansion sitting beside a guarded one.Verification
bash -non both scripts under 3.2.57; the empty-array crash and the guarded form reproduced directly on 3.2.57 (output above).pytest -q tools/ci/tests: 314 passed, including the two new tests.ruff format --checkandruff check --select E,F,I,UP,Bclean on the new test.Summary by CodeRabbit
Bug Fixes
Tests