Fix setup.bat PY_MINOR detection and run-all.bat START command quoting#11
Fix setup.bat PY_MINOR detection and run-all.bat START command quoting#11chainchopper with Copilot wants to merge 2 commits into
Conversation
Co-authored-by: chainchopper <36168576+chainchopper@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Fixes Windows batch-script quoting/parsing issues that were breaking optional llama-cpp-python install in setup.bat and service launching in run-all.bat.
Changes:
- Update
setup.batto detect Python minor version viasys.version_info[1]usingfor /f usebackqto avoid batch parser truncation. - Fix
START cmd /kquoting inrun-all.batso paths with spaces don’t get mis-parsed as commands. - Apply the same
STARTquoting fix to therun-all.batgenerator section insidesetup.bat.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| setup.bat | Fixes PY_MINOR detection and corrects quoting when generating run-all.bat entries. |
| run-all.bat | Fixes start ... cmd /k command-line quoting so backend/frontend windows launch reliably. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -184,7 +184,7 @@ if %errorlevel% neq 0 ( | |||
| echo Installing llama-cpp-python (optional - GGUF inference)... | |||
| "%PIP%" uninstall llama-cpp-python -y >nul 2>&1 | |||
| set "LLAMA_CPP_OK=1" | |||
There was a problem hiding this comment.
PY_MINOR is derived via a for /f command substitution, but it's never cleared or validated. If the Python invocation fails (e.g., transient venv/python issue), PY_MINOR could retain a pre-existing environment value and select the wrong wheel branch. Consider set "PY_MINOR=" before the loop and adding a guard that falls back to the generic pip install llama-cpp-python --only-binary :all: path when PY_MINOR is not set.
| set "LLAMA_CPP_OK=1" | |
| set "LLAMA_CPP_OK=1" | |
| set "PY_MINOR=" |
…ation - run-all.bat: Replace broken nested-quote START commands with clean delegation to run-backend.bat / run-frontend.bat helper scripts - setup.bat: Fix generated run-all.bat lines to use ^!ROOT^!\run-backend.bat and ^!ROOT^!\run-frontend.bat with proper caret-escaped vars - backend/Dockerfile: Add cmake, migrate llama.cpp build from make to CMake, update COPY paths to build/bin/ to match CMake output layout Supersedes conflicting PRs #8, #9, #10, #11, #12 Co-authored-by: chainchopper <36168576+chainchopper@users.noreply.github.com>
Two Windows batch script bugs caused
setup.batto crash during the optionalllama-cpp-pythoninstall andrun-all.batto fail launching services with'Frontend:' is not recognized.setup.bat— PY_MINOR detectionThe
for /f "tokens=2"form withplatform.python_version_tuple()[1]had two defects: the)in the expression terminated thefor /fclause early (batch parser truncation), andtokens=2always yielded empty since the output is a single token. Replaced withusebackqbacktick form usingsys.version_info[1]:run-all.bat— START command quotingcmd /k ^line continuation with unescaped inner path quotes caused CMD to treat path components likeFrontend:as commands. Collapsed to a single line with""path""doubled-quote escaping:Same fix applied to
setup.bat's generator section so freshly-createdrun-all.batfiles are also correct.Original prompt
This pull request was created from Copilot chat.
🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.