Add /upload_sample route to allow upload new speakers using Web API #80
Conversation
…d new speakers using API * Added python-multipart to requirements
📝 WalkthroughWalkthroughThis change introduces a comprehensive setup and deployment infrastructure for an XTTSv2 FastAPI text-to-speech service. It adds installation and configuration automation scripts, multiple startup configurations (regular, low-VRAM, deepspeed), and significantly expands Python package dependencies to support the TTS system and its machine learning requirements. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Outside diff range and nitpick comments (2)
xtts_api_server/server.py (2)
Line range hint
195-195: Improve exception handling by using exception chaining.- raise HTTPException(status_code=400, detail=str(e)) + raise HTTPException(status_code=400, detail=str(e)) from eAlso applies to: 204-204, 224-224, 233-233, 298-298, 329-329, 353-353
Tools
Ruff
1-1:
TTS.api.TTSimported but unused (F401)Remove unused import:
TTS.api.TTS
Line range hint
150-150: Avoid redefining theget_speakersfunction.- @app.get("/speakers") + @app.get("/special_speakers") def get_speakers_special():Tools
Ruff
1-1:
TTS.api.TTSimported but unused (F401)Remove unused import:
TTS.api.TTS
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- requirements.txt (1 hunks)
- xtts_api_server/server.py (2 hunks)
Files skipped from review due to trivial changes (1)
- requirements.txt
Additional context used
Ruff
xtts_api_server/server.py
1-1:
TTS.api.TTSimported but unused (F401)Remove unused import:
TTS.api.TTS
10-10:
timeimported but unused (F401)Remove unused import:
time
12-12:
shutilimported but unused (F401)Remove unused import:
shutil
14-14:
argparse.ArgumentParserimported but unused (F401)Remove unused import:
argparse.ArgumentParser
15-15: Redefinition of unused
Pathfrom line 11 (F811)Remove definition:
Path
150-150: Redefinition of unused
get_speakersfrom line 145 (F811)
195-195: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling (B904)
204-204: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling (B904)
207-207: Do not perform function call
Filein argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable (B008)
224-224: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling (B904)
233-233: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling (B904)
298-298: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling (B904)
329-329: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling (B904)
353-353: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling (B904)
| @@ -1,5 +1,5 @@ | |||
| from TTS.api import TTS | |||
| from fastapi import BackgroundTasks, FastAPI, HTTPException, Request, Query | |||
| from fastapi import BackgroundTasks, FastAPI, HTTPException, Request, Query, File, UploadFile | |||
There was a problem hiding this comment.
Remove unused imports to clean up the code.
- from TTS.api import TTS
- import time
- import shutil
- from argparse import ArgumentParser
- from pathlib import PathCommittable suggestion was skipped due to low confidence.
| raise HTTPException(status_code=400, detail=str(e)) | ||
|
|
||
| @app.post("/upload_sample") | ||
| async def upload_sample(wavFile: UploadFile = File(...)): |
There was a problem hiding this comment.
Avoid using File(...) as a default argument.
- async def upload_sample(wavFile: UploadFile = File(...)):
+ async def upload_sample(wavFile: UploadFile = None):
+ if wavFile is None:
+ wavFile = File(...)Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| async def upload_sample(wavFile: UploadFile = File(...)): | |
| async def upload_sample(wavFile: UploadFile = None): | |
| if wavFile is None: | |
| wavFile = File(...) |
Tools
Ruff
207-207: Do not perform function call
Filein argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable (B008)
|
Great, thanks, I'll take a look tomorrow. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
requirements.txt (1)
1-169: Consider reducing dependency bloatThe requirements file includes many packages that seem unnecessary for the upload_sample route functionality:
- Multiple TTS engines (coqui-tts, pyttsx3)
- Various language processing tools (gruut, spacy, stanza)
- Multiple audio processing libraries
Consider maintaining separate requirement files:
requirements.txtfor core dependenciesrequirements-dev.txtfor development dependenciesrequirements-optional.txtfor optional featuresExample structure:
# requirements.txt (core dependencies) fastapi==0.109.0 python-multipart==0.0.20 python-dotenv==1.0.1 uvicorn==0.34.0 # requirements-optional.txt (TTS-specific dependencies) coqui-tts==0.24.1 librosa==0.10.2.post1 torch==2.5.1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
requirements.txt(1 hunks)
🔇 Additional comments (3)
requirements.txt (3)
122-122: LGTM! Required package for file upload functionalityThe addition of
python-multipartis necessary for handling file uploads in FastAPI.
38-38: Update FastAPI to the latest stable versionThe current FastAPI version (0.115.6) is outdated and may have security vulnerabilities.
Run this script to check for security advisories:
88-99: Verify CUDA compatibility with the systemThe inclusion of CUDA 12.x dependencies suggests GPU usage. Ensure these versions are compatible with:
- The installed NVIDIA drivers
- The GPU hardware
- The operating system
Run this script to check CUDA compatibility:
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (5)
start-deepspeed.sh (1)
7-7: Optionally capture and log the server PID.
Storing the background process PID can help with service management (stopping, monitoring). For example:- python -m xtts_api_server --deepspeed --listen &> log.txt & + python -m xtts_api_server --deepspeed --listen &> log.txt & + echo $! > server.pidstart-regular.sh (1)
1-10: Consider consolidating start scripts to reduce duplication.
All three start scripts follow the same pattern differing only by the flag (--listen,--lowvram,--deepspeed). You could merge them into a singlestart.shthat accepts a mode parameter (e.g.,./start.sh lowvram), reducing maintenance overhead.🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 3-3: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
conf.sh (2)
1-2: Enable strict error handling for robustness.
Addset -euo pipefailafter the shebang to catch failures early and avoid undefined variables.#!/bin/bash + set -euo pipefail clear
48-48: Quote variable expansions in file operations.
Unquoted paths can break if they contain spaces or special characters. Wrap expansions in quotes.- rm /home/dwemer/xtts-api-server/start.sh &>/dev/null + rm "/home/dwemer/xtts-api-server/start.sh" &>/dev/null ... - ln -sf $selected_file /home/dwemer/xtts-api-server/start.sh + ln -sf "$selected_file" "/home/dwemer/xtts-api-server/start.sh"Also applies to: 62-62
ddistro_install.sh (1)
19-19: Remove redundant virtual environment activation.
The script sources the same venv twice (lines 4 and here). Dropping the secondsourcewill simplify the flow.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
conf.sh(1 hunks)ddistro_install.sh(1 hunks)start-deepspeed.sh(1 hunks)start-lowvram.sh(1 hunks)start-regular.sh(1 hunks)
🧰 Additional context used
🪛 Shellcheck (0.10.0)
conf.sh
[error] 21-21: Can only exit with status 0-255. Other data should be written to stdout/stderr.
(SC2242)
ddistro_install.sh
[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.
(SC2148)
[warning] 2-2: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
start-deepspeed.sh
[warning] 3-3: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
start-lowvram.sh
[warning] 3-3: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
start-regular.sh
[warning] 3-3: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
🔇 Additional comments (1)
ddistro_install.sh (1)
15-15: Verify the complexsedsubstitution.
This in-place edit is brittle and may break if upstream formatting changes. Please test thissedinvocation against the target file or consider using a more robust approach (e.g., a Python script with clearer parsing).
| cd /home/dwemer/xtts-api-server/ | ||
|
|
||
| source /home/dwemer/python-tts/bin/activate |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling to directory change and dynamic path resolution.
Using a hard-coded cd without checking its success can lead to silent failures. Also, absolute paths reduce portability.
Apply this diff:
- cd /home/dwemer/xtts-api-server/
+ BASE_DIR="$(dirname "$0")"
+ cd "${BASE_DIR}" || { echo "Error: failed to change directory to ${BASE_DIR}"; exit 1; }🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 3-3: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
| cd /home/dwemer/xtts-api-server/ | ||
|
|
||
| source /home/dwemer/python-tts/bin/activate |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Ensure cd succeeds and use script-relative paths.
The cd command should exit on failure to prevent subsequent commands from running in an unexpected directory. Also, deriving the path dynamically makes the script more portable.
- cd /home/dwemer/xtts-api-server/
+ BASE_DIR="$(dirname "$0")"
+ cd "${BASE_DIR}" || { echo "Error: could not enter ${BASE_DIR}"; exit 1; }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| cd /home/dwemer/xtts-api-server/ | |
| source /home/dwemer/python-tts/bin/activate | |
| BASE_DIR="$(dirname "$0")" | |
| cd "${BASE_DIR}" || { echo "Error: could not enter ${BASE_DIR}"; exit 1; } | |
| source /home/dwemer/python-tts/bin/activate |
🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 3-3: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
| cd /home/dwemer/xtts-api-server/ | ||
|
|
||
| source /home/dwemer/python-tts/bin/activate |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error checking on directory change and derive path from script location.
This prevents silent failures if the directory is missing and makes the script agnostic of a fixed install path.
- cd /home/dwemer/xtts-api-server/
+ BASE_DIR="$(dirname "$0")"
+ cd "${BASE_DIR}" || { echo "Error: failed to change directory to ${BASE_DIR}"; exit 1; }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| cd /home/dwemer/xtts-api-server/ | |
| source /home/dwemer/python-tts/bin/activate | |
| BASE_DIR="$(dirname "$0")" | |
| cd "${BASE_DIR}" || { echo "Error: failed to change directory to ${BASE_DIR}"; exit 1; } | |
| source /home/dwemer/python-tts/bin/activate |
🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 3-3: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
| if [ ! -d /home/dwemer/python-tts ]; then | ||
| exit "XTTSv2 not installed" | ||
| fi |
There was a problem hiding this comment.
Fix invalid use of exit with a string.
exit only accepts numeric status codes. Use echo to print the message and then exit 1.
- if [ ! -d /home/dwemer/python-tts ]; then
- exit "XTTSv2 not installed"
+ if [ ! -d /home/dwemer/python-tts ]; then
+ echo "Error: XTTSv2 not installed" >&2
+ exit 1📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if [ ! -d /home/dwemer/python-tts ]; then | |
| exit "XTTSv2 not installed" | |
| fi | |
| if [ ! -d /home/dwemer/python-tts ]; then | |
| echo "Error: XTTSv2 not installed" >&2 | |
| exit 1 | |
| fi |
🧰 Tools
🪛 Shellcheck (0.10.0)
[error] 21-21: Can only exit with status 0-255. Other data should be written to stdout/stderr.
(SC2242)
| @@ -0,0 +1,34 @@ | |||
|
|
|||
| cd /home/dwemer/xtts-api-server | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Ensure cd command is checked.
If the directory change fails, the rest of the script will run in the wrong context. Wrap it with an exit on failure:
- cd /home/dwemer/xtts-api-server
+ cd /home/dwemer/xtts-api-server || { echo "Error: cannot cd to xtts-api-server"; exit 1; }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| cd /home/dwemer/xtts-api-server | |
| cd /home/dwemer/xtts-api-server || { echo "Error: cannot cd to xtts-api-server"; exit 1; } |
🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 2-2: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
|
|
||
| cd /home/dwemer/xtts-api-server |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add shebang and strict error handling.
There is no shebang at the top, and failures won't be caught. Prepend these lines:
+ #!/bin/bash
+ set -euo pipefail🧰 Tools
🪛 Shellcheck (0.10.0)
[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.
(SC2148)
[warning] 2-2: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
ddistro_install.sh (2)
1-1: Add shebang and strict mode at the top of the script.Without a shebang and strict error settings, the script’s behavior is undefined and errors may not be caught early. Add at the top:
#!/bin/bash set -euo pipefail🧰 Tools
🪛 Shellcheck (0.10.0)
[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.
(SC2148)
2-2: Guard directory change with error handling.If the
cdfails, subsequent commands will execute in the wrong context. Update to:- cd /home/dwemer/xtts-api-server + cd /home/dwemer/xtts-api-server || { echo "Error: cannot cd to xtts-api-server"; exit 1; }🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 2-2: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
🧹 Nitpick comments (2)
ddistro_install.sh (2)
8-10: Combine pip installation steps for clarity and performance.Instead of multiple invocations, you can group upgrades and requirements:
- python3 -m pip install --upgrade pip wheel ninja virtualenv - pip install setuptools==68.1.2 - pip install -r requirements.txt + pip install --upgrade pip wheel ninja virtualenv setuptools==68.1.2 + pip install -r requirements.txtThis reduces overhead and improves readability.
19-19: Remove redundant virtual environment activation.The venv is already activated at line 4. Activating it again is unnecessary and could mask activation failures.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
ddistro_install.sh(1 hunks)
🧰 Additional context used
🪛 Shellcheck (0.10.0)
ddistro_install.sh
[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.
(SC2148)
[warning] 2-2: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
|
|
||
| sed -i 's/checkpoint = load_fsspec(model_path, map_location=torch.device("cpu"))\["model"\]/checkpoint = load_fsspec(model_path, map_location=torch.device("cpu"), weights_only=False)["model"]/' /home/dwemer/python-tts/lib/python3.11/site-packages/TTS/tts/models/xtts.py | ||
|
|
||
| cp /home/dwemer/TheNarrator.wav speakers/TheNarrator.wav |
There was a problem hiding this comment.
Check file existence before copying speaker sample.
Copying /home/dwemer/TheNarrator.wav without verifying it exists will fail silently or with an unclear error. Add a guard:
if [[ ! -f "$SAMPLE_PATH" ]]; then
echo "Error: sample file not found at $SAMPLE_PATH"
exit 1
fi
cp "$SAMPLE_PATH" "${BASE_DIR}/speakers/"
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
conf.sh (1)
21-23: Fix invalid use of exit with a string.
exitonly accepts numeric status codes. Useechoto print the error message and thenexit 1. This matches the ShellCheck hint (SC2242) and the previous review feedback.if [ ! -d /home/dwemer/python-tts ]; then - exit "XTTSv2 not installed" + echo "Error: XTTSv2 not installed" >&2 + exit 1 fi🧰 Tools
🪛 Shellcheck (0.10.0)
[error] 22-22: Can only exit with status 0-255. Other data should be written to stdout/stderr.
(SC2242)
🧹 Nitpick comments (4)
conf.sh (4)
34-34: Correct the menu prompt typo.There's a duplicated article in the prompt. Update
"Select a an option"to"Select an option".-echo -e "Select a an option from the list:\n\n" +echo -e "Select an option from the list:\n\n"
47-51: Avoid numeric comparison on unvalidated input.Using
-eqon a non-numericselectioncan cause an error if the user inputs a non-integer. Either validate thatselectionis numeric before this check, or switch to string comparison (== "0").-if [ "$selection" -eq "0" ]; then +if [[ "$selection" == "0" ]]; thenOptionally, you can move this block after the numeric-validation block at lines 53–56.
63-63: Quote variables inlninvocation.Unquoted paths can break if
$selected_filecontains spaces or special characters. Wrap both source and target in quotes.-ln -sf $selected_file /home/dwemer/xtts-api-server/start.sh +ln -sf "$selected_file" "/home/dwemer/xtts-api-server/start.sh"
21-26: Use variables for installation paths.Hard-coded
/home/dwemer/...paths reduce portability. Consider defining variables at the top (e.g.,BASE_DIRandTTS_DIR) or using$HOME:+BASE_DIR="${HOME}/xtts-api-server" +TTS_DIR="${HOME}/python-tts" if [ ! -d /home/dwemer/python-tts ]; then - echo "Error: XTTSv2 not installed" >&2 - exit 1 + echo "Error: XTTSv2 not installed" >&2 + exit 1 fi -mapfile -t files < <(find /home/dwemer/xtts-api-server/ -name "start-*.sh") +mapfile -t files < <(find "$BASE_DIR" -name "start-*.sh")🧰 Tools
🪛 Shellcheck (0.10.0)
[error] 22-22: Can only exit with status 0-255. Other data should be written to stdout/stderr.
(SC2242)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
conf.sh(1 hunks)
🧰 Additional context used
🪛 Shellcheck (0.10.0)
conf.sh
[error] 22-22: Can only exit with status 0-255. Other data should be written to stdout/stderr.
(SC2242)
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
conf.sh (1)
19-21:⚠️ Potential issueFix invalid use of
exitwith a string
Theexitbuilt-in only accepts integer status codes. Writing an error message to stderr and then exiting with a non-zero code will correctly signal failure.if [ ! -d /home/dwemer/python-tts ]; then - exit "XTTSv2 not installed" + echo "Error: XTTSv2 not installed" >&2 + exit 1 fi🧰 Tools
🪛 Shellcheck (0.10.0)
[error] 20-20: Can only exit with status 0-255. Other data should be written to stdout/stderr.
(SC2242)
🧹 Nitpick comments (1)
conf.sh (1)
61-61: Quote the variable in thelncommand
Unquoted variables can break if the path contains whitespace or glob characters.-ln -sf $selected_file /home/dwemer/xtts-api-server/start.sh +ln -sf "$selected_file" /home/dwemer/xtts-api-server/start.sh
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
conf.sh(1 hunks)
🧰 Additional context used
🪛 Shellcheck (0.10.0)
conf.sh
[error] 20-20: Can only exit with status 0-255. Other data should be written to stdout/stderr.
(SC2242)
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
conf.sh (1)
19-21: Fix invalid use ofexitwith a string
exitonly accepts numeric status codes. Errors should be written to stderr and then exit with a non-zero status code.- if [ ! -d /home/dwemer/python-tts ]; then - exit "XTTSv2 not installed" - fi + if [ ! -d /home/dwemer/python-tts ]; then + echo "Error: XTTSv2 not installed" >&2 + exit 1 + fi🧰 Tools
🪛 Shellcheck (0.10.0)
[error] 20-20: Can only exit with status 0-255. Other data should be written to stdout/stderr.
(SC2242)
🧹 Nitpick comments (2)
conf.sh (2)
32-32: Typo in the selection prompt
There’s a duplicate article in the prompt"Select a an option...". It should read"Select an option...".- echo -e "Select a an option from the list:\n\n" + echo -e "Select an option from the list:\n\n"
23-23: Avoid hard-coded absolute paths
Consider defining a base directory variable (e.g.,XTTS_HOME="${XTTS_HOME:-$HOME/dwemer}") at the top and referencing it throughout. This improves portability and makes future relocations/configurations easier.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
conf.sh(1 hunks)
🧰 Additional context used
🪛 Shellcheck (0.10.0)
conf.sh
[error] 20-20: Can only exit with status 0-255. Other data should be written to stdout/stderr.
(SC2242)
|
|
||
| echo "You selected: $selected_file" | ||
|
|
||
| ln -sf $selected_file /home/dwemer/xtts-api-server/start.sh |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Quote path variables in ln -sf
Always quote expansions to handle paths containing spaces or special characters:
- ln -sf $selected_file /home/dwemer/xtts-api-server/start.sh
+ ln -sf "$selected_file" "/home/dwemer/xtts-api-server/start.sh"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ln -sf $selected_file /home/dwemer/xtts-api-server/start.sh | |
| ln -sf "$selected_file" "/home/dwemer/xtts-api-server/start.sh" |
| if [ "$selection" -eq "0" ]; then | ||
| echo "Disabling service. Run this again to enable" | ||
| rm /home/dwemer/xtts-api-server/start.sh &>/dev/null | ||
| exit 0 | ||
| fi | ||
|
|
||
| if ! [[ "$selection" =~ ^[0-9]+$ ]] || [ "$selection" -lt 1 ] || [ "$selection" -gt ${#files[@]} ]; then | ||
| echo "Invalid selection." | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Reorder input validation to prevent comparison errors
Using -eq on a non-numeric string will cause a runtime error. Check for numeric input first or use string comparison for the zero case. For example:
- if [ "$selection" -eq "0" ]; then
+ # Check for zero before numeric comparison
+ if [ "$selection" = "0" ]; then
echo "Disabling service. Run this again to enable"
rm /home/dwemer/xtts-api-server/start.sh &>/dev/null
exit 0
fi
- if ! [[ "$selection" =~ ^[0-9]+$ ]] || [ "$selection" -lt 1 ] || [ "$selection" -gt ${#files[@]} ]; then
+ if ! [[ "$selection" =~ ^[0-9]+$ ]] || [ "$selection" -lt 1 ] || [ "$selection" -gt ${#files[@]} ]; then
echo "Invalid selection."
exit 1
fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if [ "$selection" -eq "0" ]; then | |
| echo "Disabling service. Run this again to enable" | |
| rm /home/dwemer/xtts-api-server/start.sh &>/dev/null | |
| exit 0 | |
| fi | |
| if ! [[ "$selection" =~ ^[0-9]+$ ]] || [ "$selection" -lt 1 ] || [ "$selection" -gt ${#files[@]} ]; then | |
| echo "Invalid selection." | |
| exit 1 | |
| fi | |
| # Check for zero before numeric comparison | |
| if [ "$selection" = "0" ]; then | |
| echo "Disabling service. Run this again to enable" | |
| rm /home/dwemer/xtts-api-server/start.sh &>/dev/null | |
| exit 0 | |
| fi | |
| if ! [[ "$selection" =~ ^[0-9]+$ ]] || [ "$selection" -lt 1 ] || [ "$selection" -gt ${#files[@]} ]; then | |
| echo "Invalid selection." | |
| exit 1 | |
| fi |
* Added branch for Nvidia GT 10xx users (will use cuda 118)
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (5)
ddistro_install.sh (5)
1-2: Add strict error handling flags
To catch errors and undefined variables early, addset -euo pipefailright after the shebang.Suggested diff:
#!/bin/bash + set -euo pipefail
3-3: Validatecdsuccess
Ifcdfails, the script will continue in the wrong directory. Append an exit handler to guard against that.Suggested diff:
- cd /home/dwemer/xtts-api-server + cd /home/dwemer/xtts-api-server || { echo "Error: failed to cd to xtts-api-server"; exit 1; }
4-5: Parameterize hard-coded paths
Using fixed paths reduces portability. IntroduceBASE_DIRandVENV_DIRvariables so users can override them.Example:
- python3 -m venv /home/dwemer/python-tts - source /home/dwemer/python-tts/bin/activate + BASE_DIR="${BASE_DIR:-$HOME/xtts-api-server}" + VENV_DIR="${VENV_DIR:-$HOME/python-tts}" + + python3 -m venv "$VENV_DIR" + source "$VENV_DIR/bin/activate" + cd "$BASE_DIR" || exit 1
24-24: Avoid brittle in-placesededits
Patching installed packages withsedcan break on updates. Prefer delivering a proper patch or at least guard the file’s existence before editing.Example check:
FILE="/home/dwemer/python-tts/lib/python3.11/site-packages/TTS/tts/models/xtts.py" if [[ -f "$FILE" ]]; then sed -i 's/.../.../' "$FILE" else echo "Error: $FILE not found"; exit 1 fi
26-26: Ensure speaker sample exists before copying
Copying without checking the source file can fail with an unclear error. Add a guard to verify the path.Suggested diff:
- cp /home/dwemer/TheNarrator.wav speakers/TheNarrator.wav + SAMPLE_PATH="${SAMPLE_PATH:-/home/dwemer/TheNarrator.wav}" + if [[ ! -f "$SAMPLE_PATH" ]]; then + echo "Error: speaker sample not found at $SAMPLE_PATH"; exit 1 + fi + cp "$SAMPLE_PATH" "speakers/TheNarrator.wav"
🧹 Nitpick comments (4)
ddistro_install.sh (4)
10-15: Handle invalid input in GPU prompt
Currently any response other than “yes” falls back silently to the non-GPU path. Use a loop orcaseto re-prompt until valid.Example:
while true; do read -p "Do you own a GT10XX series GPU? (yes/no): " ans case "$ans" in [Yy]|[Yy][Ee][Ss]) torch_url="https://download.pytorch.org/whl/nightly/cu118"; break ;; [Nn]|[Nn][Oo]) torch_url="https://download.pytorch.org/whl/nightly/cu128"; break ;; *) echo "Please answer yes or no." ;; esac done
17-21: Consistent invocation of pip
Mixingpython3 -m pipand barepipcan install into different environments. Stick to one form—preferpython3 -m pip.Suggested diff:
- pip install setuptools==68.1.2 - pip install -r requirements.txt - pip uninstall -y torch torchaudio - pip install --pre -U torch torchaudio --index-url "$torch_url" + python3 -m pip install setuptools==68.1.2 + python3 -m pip install -r requirements.txt + python3 -m pip uninstall -y torch torchaudio + python3 -m pip install --pre -U torch torchaudio --index-url "$torch_url"
30-30: Verifyconf.shexecution
Ifconf.shis missing or not executable, the script will error out later. Add a pre-check.Example:
if [[ ! -x "./conf.sh" ]]; then echo "Error: conf.sh not found or not executable"; exit 1 fi ./conf.sh
40-40: Use consistent Python interpreter for server launch
To ensure the correct venv is used, explicitly invoke the venv’s Python.Suggested diff:
- python -m xtts_api_server --listen + "$VENV_DIR/bin/python" -m xtts_api_server --listen
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
ddistro_install.sh(1 hunks)
🧰 Additional context used
🪛 Shellcheck (0.10.0)
ddistro_install.sh
[error] 42-42: Couldn't parse this double quoted string. Fix to allow more checks.
(SC1073)
* Fixed unclosed "
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (5)
ddistro_install.sh (5)
1-2: Enable strict error handling for robustness.The script currently has a shebang but lacks strict mode, which can lead to silent failures or use of unset variables. Add
set -euo pipefailafter the shebang to ensure it exits on errors and undefined variables.
3-3: Parameterize and validate the working directory change.Avoid hard-coding
/home/dwemer/xtts-api-server. Introduce aBASE_DIRvariable (with a sensible default) and guard thecd:- cd /home/dwemer/xtts-api-server + BASE_DIR="${BASE_DIR:-/home/dwemer/xtts-api-server}" + cd "$BASE_DIR" || { echo "Error: cannot cd to $BASE_DIR"; exit 1; }🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 3-3: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
4-5: Refactor virtual environment setup to use configurable paths.Hard-coded VENV location reduces reusability. Use a
VENV_DIRvariable and reference it consistently:- python3 -m venv /home/dwemer/python-tts - source /home/dwemer/python-tts/bin/activate + VENV_DIR="${VENV_DIR:-$HOME/python-tts}" + python3 -m venv "$VENV_DIR" + source "$VENV_DIR/bin/activate"
24-24: Guard against missingxtts.pybefore applying in-place patch.Editing site-packages in place is brittle. At minimum, verify the file exists and consider using a patch file or upstream PR:
+ TTS_MODEL_FILE="$VENV_DIR/lib/python$(python3 -c 'import sys; print(f"{sys.version_info.major}.{sys.version_info.minor}")')/site-packages/TTS/tts/models/xtts.py" + if [[ -f "$TTS_MODEL_FILE" ]]; then + sed -i 's/checkpoint = load_fsspec(...)/checkpoint = load_fsspec(..., weights_only=False)/' "$TTS_MODEL_FILE" + else + echo "Error: $TTS_MODEL_FILE not found" >&2 + exit 1 + fi
26-26: Verify speaker sample file existence before copying.Directly copying
/home/dwemer/TheNarrator.wavcan fail without a clear message. ParameterizeSAMPLE_PATH, ensure the directory exists, and guard the copy:+ SAMPLE_PATH="${SAMPLE_PATH:-/home/dwemer/TheNarrator.wav}" + DEST_DIR="speakers" + if [[ ! -f "$SAMPLE_PATH" ]]; then + echo "Error: sample file '$SAMPLE_PATH' not found" >&2 + exit 1 + fi + mkdir -p "$DEST_DIR" + cp "$SAMPLE_PATH" "$DEST_DIR/"
🧹 Nitpick comments (3)
ddistro_install.sh (3)
10-15: Enhance GPU selection prompt validation (optional).The regex handles basic yes/no but doesn’t catch invalid inputs. Consider normalizing to lowercase and using a
casestatement with a default branch:read -p "Do you own a GT10XX series GPU? (yes/no): " gpu_answer gpu_answer="${gpu_answer,,}" case "$gpu_answer" in y|yes) torch_url="https://download.pytorch.org/whl/nightly/cu118" ;; n|no) torch_url="https://download.pytorch.org/whl/nightly/cu128" ;; *) echo "Invalid response: $gpu_answer"; exit 1 ;; esac
22-22: Remove or fix the commented-out command.The
#pip install xtts-api-serverline either needs context (why it fails) or should be removed to keep the script clean.
28-31: Ensureconf.shexecution is validated.If
conf.shfails or is missing, the script should stop and report an error:+ if [[ ! -f "./conf.sh" ]]; then + echo "Error: conf.sh not found"; exit 1 + fi + ./conf.sh || { echo "Error: conf.sh failed"; exit 1; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
ddistro_install.sh(1 hunks)
🧰 Additional context used
🪛 Shellcheck (0.10.0)
ddistro_install.sh
[warning] 3-3: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
ddistro_install.sh
Outdated
| python3 -m pip install --upgrade pip wheel ninja virtualenv | ||
| pip install setuptools==68.1.2 | ||
| pip install -r requirements.txt | ||
| pip uninstall -y torch torchaudio | ||
| pip install --pre -U torch torchaudio --index-url "$torch_url" |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consistently invoke pip via python3 -m pip within the venv.
Mixing bare pip and python3 -m pip can lead to ambiguity about which interpreter is used. Update all install/uninstall commands:
- pip install setuptools==68.1.2
- pip install -r requirements.txt
- pip uninstall -y torch torchaudio
- pip install --pre -U torch torchaudio --index-url "$torch_url"
+ python3 -m pip install setuptools==68.1.2
+ python3 -m pip install -r requirements.txt
+ python3 -m pip uninstall -y torch torchaudio
+ python3 -m pip install --pre -U torch torchaudio --index-url "$torch_url"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| python3 -m pip install --upgrade pip wheel ninja virtualenv | |
| pip install setuptools==68.1.2 | |
| pip install -r requirements.txt | |
| pip uninstall -y torch torchaudio | |
| pip install --pre -U torch torchaudio --index-url "$torch_url" | |
| python3 -m pip install --upgrade pip wheel ninja virtualenv | |
| python3 -m pip install setuptools==68.1.2 | |
| python3 -m pip install -r requirements.txt | |
| python3 -m pip uninstall -y torch torchaudio | |
| python3 -m pip install --pre -U torch torchaudio --index-url "$torch_url" |
ddistro_install.sh
Outdated
| python -m xtts_api_server --listen | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use the venv’s Python interpreter to launch the server.
Relying on system python may bypass the virtual environment. Switch to python3 (or $VENV_DIR/bin/python):
- python -m xtts_api_server --listen
+ python3 -m xtts_api_server --listen📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| python -m xtts_api_server --listen | |
| -python -m xtts_api_server --listen | |
| +python3 -m xtts_api_server --listen |
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
start-deepspeed.sh (1)
7-7: Ensurecdsucceeds and use script-relative paths.As flagged by ShellCheck (SC2164) and previously noted in past reviews, use the script's directory dynamically and exit if
cdfails.-cd /home/dwemer/xtts-api-server/ +BASE_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/.." +cd "${BASE_DIR}" || { echo "Error: could not change directory to ${BASE_DIR}" >&2; exit 1; }🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 7-7: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
🧹 Nitpick comments (2)
start-deepspeed.sh (2)
1-1: Enable strict mode for safer error handling.Add
set -euo pipefailafter the shebang to ensure the script exits on any error, catches unset variables, and fails on pipeline errors.#!/bin/bash + set -euo pipefail
9-9: Avoid hardcoding virtual environment paths.Make the virtual environment activation path configurable or relative to the script location to improve portability. For example:
-source /home/dwemer/python-tts/bin/activate +source "${BASE_DIR}/python-tts/bin/activate"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (1)
start-deepspeed.sh(1 hunks)
🧰 Additional context used
🪛 Shellcheck (0.10.0)
start-deepspeed.sh
[warning] 7-7: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (6)
ddistro_install.sh (6)
1-2: Enable strict mode for safer scripting.Add fail-fast and safer IFS. This prevents subtle errors from continuing unnoticed.
#!/bin/bash + +set -euo pipefail +# Safer word splitting for arrays/paths +IFS=$'\n\t'
3-5: Parameterize paths and guard cd; improve portability and reliability.Avoid hardcoded user paths and ensure directory change succeeds. This also addresses SC2164 from shellcheck.
-cd /home/dwemer/xtts-api-server -python3 -m venv /home/dwemer/python-tts -source /home/dwemer/python-tts/bin/activate +BASE_DIR="${BASE_DIR:-$HOME/xtts-api-server}" +VENV_DIR="${VENV_DIR:-$HOME/python-tts}" +cd "$BASE_DIR" || { echo "Error: cannot cd to $BASE_DIR"; exit 1; } +python3 -m venv "$VENV_DIR" +# shellcheck disable=SC1091 +source "$VENV_DIR/bin/activate"
17-23: Use the venv interpreter for all pip ops; avoid interpreter ambiguity.Standardize on python -m pip and keep the current ordering if you intend the nightly torch stack to override any torch specified in requirements.
python3 -m pip install --upgrade pip wheel ninja virtualenv -pip install setuptools==68.1.2 -pip install -r requirements.txt +python3 -m pip install setuptools==68.1.2 +python3 -m pip install -r requirements.txt # Ensure fresh installs of PyTorch stack including TorchCodec required by recent torchaudio -pip uninstall -y torch torchaudio torchcodec -pip install --pre -U torch torchaudio torchcodec --index-url "$torch_url" +python3 -m pip uninstall -y torch torchaudio torchcodec || true +python3 -m pip install --pre -U torch torchaudio torchcodec --index-url "$torch_url"Optional: If requirements.txt also pins torch/torchaudio, consider removing them there to avoid churn, or install requirements with
--no-depsafter the torch stack. Please confirm the desired precedence.
25-25: Avoid brittle sed patching of site-packages; compute path dynamically and back up.Hardcoding Python 3.11 and direct in-place edits are fragile and will break on upgrades or different environments. Prefer a targeted patch with backup based on the actual installed path.
-sed -i 's/checkpoint = load_fsspec(model_path, map_location=torch.device("cpu"))\["model"\]/checkpoint = load_fsspec(model_path, map_location=torch.device("cpu"), weights_only=False)["model"]/' /home/dwemer/python-tts/lib/python3.11/site-packages/TTS/tts/models/xtts.py +# Locate installed xtts.py and patch safely +XTTS_FILE="$(python3 - <<'PY' +import inspect, TTS.tts.models.xtts as m +print(inspect.getfile(m)) +PY +)" +if [[ -f "$XTTS_FILE" ]]; then + cp "$XTTS_FILE" "${XTTS_FILE}.bak" + python3 - "$XTTS_FILE" <<'PY' +import sys, pathlib +p = pathlib.Path(sys.argv[1]) +s = p.read_text() +old = 'load_fsspec(model_path, map_location=torch.device("cpu"))["model"]' +new = 'load_fsspec(model_path, map_location=torch.device("cpu"), weights_only=False)["model"]' +if old in s and new not in s: + p.write_text(s.replace(old, new)) +else: + print("Note: expected pattern not found or already patched", file=sys.stderr) +PY +else + echo "Error: xtts.py not found; cannot patch." >&2 + exit 1 +fiLong-term, prefer upstreaming this change or applying a proper patch file rather than mutating installed packages.
27-27: Validate sample file and ensure speakers dir exists before copying.Prevents confusing failures on missing source file or directory.
-cp /home/dwemer/TheNarrator.wav speakers/TheNarrator.wav +SAMPLE_PATH="${SAMPLE_PATH:-$HOME/TheNarrator.wav}" +mkdir -p speakers +if [[ ! -f "$SAMPLE_PATH" ]]; then + echo "Error: sample file not found at $SAMPLE_PATH" >&2 + exit 1 +fi +cp "$SAMPLE_PATH" "speakers/TheNarrator.wav"
41-41: Use the venv’s Python interpreter to run the server.Guarantees the correct environment and dependencies.
-python -m xtts_api_server --listen +"${VENV_DIR:-$HOME/python-tts}"/bin/python -m xtts_api_server --listen
🧹 Nitpick comments (3)
ddistro_install.sh (3)
10-15: Make CUDA index selection robust; optionally auto-detect GPU capability.The yes/no prompt works but is brittle. Consider auto-detecting compute capability via nvidia-smi and mapping to the correct CUDA wheel, with a manual fallback.
-read -p "Do you own a GT10XX series GPU? (yes/no): " gpu_answer -if [[ "$gpu_answer" =~ ^[Yy][Ee][Ss]$ || "$gpu_answer" =~ ^[Yy]$ ]]; then - torch_url="https://download.pytorch.org/whl/nightly/cu118" -else - torch_url="https://download.pytorch.org/whl/nightly/cu128" -fi +# Try to detect CUDA capability; fall back to prompt +torch_url="" +if command -v nvidia-smi >/dev/null 2>&1; then + cc="$(nvidia-smi --query-gpu=compute_cap --format=csv,noheader 2>/dev/null | head -n1 || true)" + case "$cc" in + 6.*) torch_url="https://download.pytorch.org/whl/nightly/cu118" ;; # Pascal + 7.*|8.*|9.*) torch_url="https://download.pytorch.org/whl/nightly/cu128" ;; # Turing/Ampere+/Blackwell + esac +fi +if [[ -z "${torch_url}" ]]; then + read -r -p "Use CUDA 11.8 wheels? [y/N]: " ans + if [[ "$ans" =~ ^[Yy]$ ]]; then torch_url="https://download.pytorch.org/whl/nightly/cu118"; else torch_url="https://download.pytorch.org/whl/nightly/cu128"; fi +fi +echo "Using PyTorch nightly index: $torch_url"Note: Please verify the mapping for your supported GPUs and desired CUDA runtime.
31-31: Ensure conf.sh exists and is executable; fail fast if not.Avoids silent no-ops or confusing errors later.
-./conf.sh +if [[ ! -x ./conf.sh ]]; then + echo "Error: ./conf.sh missing or not executable" >&2 + exit 1 +fi +./conf.sh
43-44: Either remove the trailing prompt or read input.Right now it only prints “Press Enter” without waiting. If you intend to pause, add a read.
-echo "Press Enter" +read -r -p "Press ENTER to exit... " _
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
ddistro_install.sh(1 hunks)
🧰 Additional context used
🪛 Shellcheck (0.10.0)
ddistro_install.sh
[warning] 3-3: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
🔇 Additional comments (1)
ddistro_install.sh (1)
1-44: Confirm PR scope alignment.PR objectives mention adding a new /upload_sample API route and python-multipart dependency. This script is an installation/startup helper and does not relate to that route. Verify whether this file belongs in this PR or should be split into a separate infra/setup PR to keep changes focused.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (8)
ddistro_install.sh (8)
1-1: Add strict/defensive shell options right after the shebang.Enable fail-fast behavior and safer word-splitting to avoid subtle breakages during long installs.
#!/bin/bash +set -euo pipefail +IFS=$'\n\t' +# Optional: surface a helpful message on unexpected errors +trap 'echo "Error on line ${LINENO}. Exiting." >&2' ERR
3-5: Parameterize hard-coded paths for portability.Make the script reusable across users/machines and easier to override in CI.
- cd /home/dwemer/xtts-api-server || { echo "Error: cannot cd to /home/dwemer/xtts-api-server"; exit 1; } - python3 -m venv /home/dwemer/python-tts - source /home/dwemer/python-tts/bin/activate + BASE_DIR="${BASE_DIR:-$HOME/xtts-api-server}" + VENV_DIR="${VENV_DIR:-$HOME/python-tts}" + cd "${BASE_DIR}" || { echo "Error: cannot cd to ${BASE_DIR}"; exit 1; } + python3 -m venv "${VENV_DIR}" + # shellcheck source=/dev/null + source "${VENV_DIR}/bin/activate" @@ - source /home/dwemer/python-tts/bin/activate +# shellcheck source=/dev/null + source "${VENV_DIR}/bin/activate"Also applies to: 40-40
36-36: Avoid brittle in-place sed patching of site-packages; compute site-packages dynamically.The hard-coded Python 3.11 path will break on other Python versions or layouts. Also consider carrying a patch file or upstreaming the change.
-sed -i 's/checkpoint = load_fsspec(model_path, map_location=torch.device("cpu"))\["model"\]/checkpoint = load_fsspec(model_path, map_location=torch.device("cpu"), weights_only=False)["model"]/' /home/dwemer/python-tts/lib/python3.11/site-packages/TTS/tts/models/xtts.py +# Derive site-packages path from the active venv +SITE_PKGS="$(python3 -c 'import site,sys; print((site.getsitepackages() or [site.getusersitepackages()])[0])')" +TARGET_FILE="${SITE_PKGS}/TTS/tts/models/xtts.py" +if [[ -f "${TARGET_FILE}" ]]; then + sed -i 's/checkpoint = load_fsspec(model_path, map_location=torch.device("cpu"))\["model"\]/checkpoint = load_fsspec(model_path, map_location=torch.device("cpu"), weights_only=False)["model"]/' "${TARGET_FILE}" +else + echo "Warning: ${TARGET_FILE} not found; skip patch." >&2 +fiIf you’d like, I can prepare a minimal patch file and apply it via patch -p0 for better traceability.
38-38: Guard the sample copy with an existence check (and use parameterized paths).Prevents confusing failures if the file is missing.
-cp /home/dwemer/TheNarrator.wav speakers/TheNarrator.wav +SAMPLE_PATH="${SAMPLE_PATH:-$HOME/TheNarrator.wav}" +if [[ ! -f "${SAMPLE_PATH}" ]]; then + echo "Error: sample file not found at ${SAMPLE_PATH}" >&2 + exit 1 +fi +mkdir -p "${BASE_DIR:-$PWD}/speakers" +cp "${SAMPLE_PATH}" "${BASE_DIR:-$PWD}/speakers/TheNarrator.wav"
3-3: Check the result of cd to prevent running in the wrong directory.If cd fails, later operations will patch/modify the wrong environment.
-cd /home/dwemer/xtts-api-server +cd /home/dwemer/xtts-api-server || { echo "Error: cannot cd to /home/dwemer/xtts-api-server"; exit 1; }
23-33: Use the venv’s interpreter for all pip actions; avoid mixing with bare pip.Prevents accidental use of system pip/interpreter and ambiguity.
-python3 -m pip install --upgrade pip wheel ninja virtualenv -pip install setuptools==68.1.2 -# Install app requirements without auto-pulling torch/torchaudio from deps -pip install --no-deps -r requirements.txt -# Pin to stable, CUDA-tagged PyTorch/Torchaudio that do not require TorchCodec -pip cache purge || true -pip uninstall -y torch torchaudio torchcodec torchvision || true -pip install --no-deps --no-cache-dir --index-url "$torch_url" "torch==${torch_ver}+${cu_tag}" "torchaudio==${torchaudio_ver}+${cu_tag}" -pip check || true -# Ensure fallback audio loader is available -pip install --no-cache-dir soundfile +python3 -m pip install --upgrade pip wheel ninja virtualenv +python3 -m pip install setuptools==68.1.2 +# Install app requirements without auto-pulling torch/torchaudio from deps +python3 -m pip install --no-deps -r requirements.txt +# Pin to stable, CUDA-tagged PyTorch/Torchaudio that do not require TorchCodec +python3 -m pip cache purge || true +python3 -m pip uninstall -y torch torchaudio torchcodec torchvision || true +python3 -m pip install --no-deps --no-cache-dir --index-url "$torch_url" \ + "torch==${torch_ver}+${cu_tag}" "torchaudio==${torchaudio_ver}+${cu_tag}" +# Fail if dependency conflicts are detected +python3 -m pip check +# Ensure fallback audio loader is available +python3 -m pip install --no-cache-dir soundfile
44-52: Fix contradictory prompts and start the server with the venv’s Python.Current messaging asks users to wait for Uvicorn output before you actually start the server and tells them to close the window (which kills the server). Start immediately after the confirmation and remove the “close this window” instruction.
-echo -echo "This will start CHIM XTTS to download the selected model" -echo "Wait for the message 'Uvicorn running on http://0.0.0.0:8020 (Press CTRL+C to quit)'" -echo "Then close this window. Press ENTER to continue" -read - -echo "please wait...." - -python -m xtts_api_server --listen +echo +echo "Press ENTER to start CHIM XTTS and download the selected model." +echo "After startup, look for: 'Uvicorn running on http://0.0.0.0:8020 (Press CTRL+C to quit)'." +echo "Keep this terminal open to keep the server running." +read -r +echo "Starting server, please wait..." +"${VENV_DIR:-$HOME/python-tts}"/bin/python -m xtts_api_server --listenIf you need the server to persist after logout, consider adding a systemd unit, or use tmux/screen.
52-52: Launch with the venv’s interpreter to avoid escaping the environment.Using system python may miss installed deps in the venv.
-python -m xtts_api_server --listen +"${VENV_DIR:-$HOME/python-tts}"/bin/python -m xtts_api_server --listen
🧹 Nitpick comments (4)
ddistro_install.sh (4)
10-21: Harden yes/no prompt parsing and avoid backslash escapes eating with read.Use read -r and a case statement; improves UX and robustness.
-read -p "Are you using a GT10XX series GPU? (yes/no): " gpu_answer -if [[ "$gpu_answer" =~ ^[Yy][Ee][Ss]$ || "$gpu_answer" =~ ^[Yy]$ ]]; then +read -r -p "Are you using a GT10XX series GPU? (yes/no): " gpu_answer +case "${gpu_answer}" in + [Yy]|[Yy][Ee][Ss]) cu_tag="cu118" torch_url="https://download.pytorch.org/whl/${cu_tag}" torch_ver="2.2.2" torchaudio_ver="2.2.2" -else + ;; + *) cu_tag="cu124" torch_url="https://download.pytorch.org/whl/${cu_tag}" torch_ver="2.4.0" torchaudio_ver="2.4.0" -fi + ;; +esac
31-31: Don’t mask dependency conflicts with|| true.If pip check fails, the environment is inconsistent and later steps may break at runtime.
-pip check || true +python3 -m pip check
40-41: Second venv activation is redundant.You already activated earlier; re-sourcing is unnecessary unless you intentionally deactivated.
-source /home/dwemer/python-tts/bin/activate +# (Redundant) already active; remove unless deliberately re-activating after a subshell.
54-55: Align the final prompt with intended behavior.If you expect a pause, add a read; otherwise, remove the dangling prompt.
-echo "Press Enter" +echo "Setup complete." +# If you want a pause here: +# read -r -p "Press ENTER to exit..."
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
ddistro_install.sh(1 hunks)
🧰 Additional context used
🪛 Shellcheck (0.10.0)
ddistro_install.sh
[warning] 3-3: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
ddistro_install.sh
Outdated
| #pip install xtts-api-server #Fails | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify xtts_api_server is importable before trying to run it.
requirements.txt install doesn’t show how xtts_api_server is provided (editable install, local package, etc.). Ensure it exists to avoid runtime failure.
Run this script to check for the module/package presence in the repo and potential entry-points:
Also applies to: 52-52
🏁 Script executed:
#!/bin/bash
# Search for a Python package/module named xtts_api_server
rg -nP -C2 '\bxtts_api_server\b' --type py
# Look for a setup.cfg/pyproject with "xtts_api_server" or console_scripts entrypoints
fd -H 'pyproject.toml' -x rg -nP -C2 'xtts_api_server|console_scripts'
fd -H 'setup.cfg' -x rg -nP -C2 'xtts_api_server|console_scripts'Length of output: 1773
Ensure the xtts_api_server package is installed and importable before invoking it
It looks like you have a local xtts_api_server/ module (so xtts_api_server is importable in the repo), but there’s no editable or regular install step in ddistro_install.sh, nor is there a console‐script entrypoint defined in your packaging. Without installing the package (or defining a CLI entrypoint), attempts to run xtts-api-server will fail.
• ddistro_install.sh (lines 34–35): replace the commented-out pip install line with a local install, for example:
pip install -e . # installs the local xtts_api_server package(or pip install . if you prefer a non-editable install) before you try to invoke the server.
• Alternatively, add a CLI entrypoint in your pyproject.toml so that pip install . also installs an executable:
[project.scripts]
xtts-api-server = "xtts_api_server.__main__:main"Then pip install . (or pip install -e .) will provide a xtts-api-server command.
🤖 Prompt for AI Agents
In ddistro_install.sh around lines 34-35, the script currently comments out
installing the local xtts_api_server package which means the xtts-api-server
command won’t be available; replace that commented line with a local install
(e.g., run pip install -e . or pip install .) before invoking the server, or
alternatively add a console-script entrypoint in pyproject.toml (project.scripts
mapping to xtts_api_server.__main__:main) so pip install . will create the
xtts-api-server executable.
Updated PyTorch installation to specific versions with CUDA support.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
ddistro_install.sh (3)
3-3: Handlecdfailure to avoid running in the wrong directory.If the
cdon Line [3] fails (typo, permissions, missing repo), the rest of the script keeps running and starts installing into whatever directory the caller happened to be in. Add an explicit guard so we fail fast.- cd /home/dwemer/xtts-api-server + cd /home/dwemer/xtts-api-server || { echo "Error: cannot cd to /home/dwemer/xtts-api-server"; exit 1; }
48-48: Check the sample file exists before copying.If
/home/dwemer/TheNarrator.wavis missing, thecpon Line [48] prints an error and the script keeps going, leaving the install half-baked. Guard it:-cp /home/dwemer/TheNarrator.wav speakers/TheNarrator.wav +sample_src=/home/dwemer/TheNarrator.wav +if [[ ! -f "$sample_src" ]]; then + echo "Error: sample file not found at $sample_src" + exit 1 +fi +cp "$sample_src" speakers/TheNarrator.wav
54-62: Fix the contradictory startup prompts.Lines [55-62] still tell users to “wait for” Uvicorn output and “close this window” before the server even starts, yet the server only launches after the
read. This blocks automation, confuses users, and the “close this window” instruction kills the server outright. Rework the flow so pressing ENTER starts the server immediately and drop the advice to close the terminal:-echo "This will start CHIM XTTS to download the selected model" -echo "Wait for the message 'Uvicorn running on http://0.0.0.0:8020 (Press CTRL+C to quit)'" -echo "Then close this window. Press ENTER to continue" -read - -echo "please wait...." - -python -m xtts_api_server --listen - -echo "Press Enter" +echo "Press ENTER to start CHIM XTTS and download the selected model." +echo "Watch for: 'Uvicorn running on http://0.0.0.0:8020 (Press CTRL+C to quit)'." +echo "Leave this terminal open (or run under tmux/screen/systemd) to keep the server alive." +read -r +echo "Starting server, please wait..." +python -m xtts_api_server --listen
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
ddistro_install.sh(1 hunks)
🧰 Additional context used
🪛 Shellcheck (0.11.0)
ddistro_install.sh
[warning] 3-3: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
ddistro_install.sh
Outdated
| #pip install xtts-api-server #Fails | ||
| fi | ||
|
|
||
| sed -i 's/checkpoint = load_fsspec(model_path, map_location=torch.device("cpu"))\["model"\]/checkpoint = load_fsspec(model_path, map_location=torch.device("cpu"), weights_only=False)["model"]/' /home/dwemer/python-tts/lib/python3.11/site-packages/TTS/tts/models/xtts.py |
There was a problem hiding this comment.
Stop hard-coding the Python 3.11 site-packages path.
Line [46] assumes the venv lives under .../python3.11/..., but python3 -m venv will follow whatever interpreter version is installed (3.10, 3.12, etc.). On anything but 3.11 the sed command fails, the patch is never applied, and the setup is silently broken. Resolve the path at runtime:
-sed -i 's/checkpoint = load_fsspec(model_path, map_location=torch.device("cpu"))\["model"\]/checkpoint = load_fsspec(model_path, map_location=torch.device("cpu"), weights_only=False)["model"]/' /home/dwemer/python-tts/lib/python3.11/site-packages/TTS/tts/models/xtts.py
+model_file="$(
+ python - <<'PY'
+import sysconfig, pathlib
+print(pathlib.Path(sysconfig.get_path("purelib")) / "TTS/tts/models/xtts.py")
+PY
+)"
+if [[ ! -f "$model_file" ]]; then
+ echo "Error: xtts.py not found at $model_file"
+ exit 1
+fi
+sed -i 's/checkpoint = load_fsspec(model_path, map_location=torch.device("cpu"))\["model"\]/checkpoint = load_fsspec(model_path, map_location=torch.device("cpu"), weights_only=False)["model"]/' "$model_file"🤖 Prompt for AI Agents
In ddistro_install.sh around line 46, the sed command hard-codes a python3.11
site-packages path which will fail for other interpreter versions; change it to
resolve the site-packages directory at runtime by invoking the target Python
interpreter to print its site-packages/purelib path and use that value in the
sed command so the replacement operates on the correct venv path (ensure the
python invocation matches the venv/interpreter you created and properly
quote/escape the resolved path when passing it to sed).
Updated CUDA version handling and installation process for PyTorch and related packages. Added user prompt for CUDA version selection and modified installation commands accordingly.
Added a comprehensive list of dependencies for the project.
Add LD_LIBRARY_PATH for NVIDIA libraries
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
start-regular.sh (1)
8-10: Add error handling to thecdcommand (unresolved from previous review).The directory change lacks error handling. If the directory doesn't exist, the script will silently continue and execute subsequent commands in the wrong context, leading to activation of the venv from an unexpected location.
Apply this diff to add error handling:
-cd /home/dwemer/xtts-api-server/ +cd /home/dwemer/xtts-api-server/ || { echo "Error: failed to change directory"; exit 1; }
🧹 Nitpick comments (1)
start-regular.sh (1)
4-5: Consider making CUDA version configurable.The CUDA version (12.8) is hardcoded. This may cause issues if the installed CUDA version differs. Consider allowing this to be overridden via an environment variable or auto-detected.
-if [ -d "/usr/local/cuda-12.8/bin" ]; then - export PATH="/usr/local/cuda-12.8/bin:$PATH" +CUDA_VERSION="${CUDA_VERSION:-12.8}" +CUDA_PATH="${CUDA_PATH:-/usr/local/cuda-${CUDA_VERSION}}" +if [ -d "${CUDA_PATH}/bin" ]; then + export PATH="${CUDA_PATH}/bin:$PATH"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
start-regular.sh(1 hunks)
🧰 Additional context used
🪛 Shellcheck (0.11.0)
start-regular.sh
[warning] 8-8: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
🔇 Additional comments (1)
start-regular.sh (1)
8-8: Partial review comment is incorrect — line 12 does not contain the hardcoded nvidia path cited.The hardcoded paths at lines 8 and 10 in start-regular.sh are correctly identified and should be made portable. However, the review incorrectly claims line 12 contains
/home/dwemer/python-tts/lib/python3.11/site-packages/nvidia/cu13/lib/— line 12 isdate > log.txt, which is not a hardcoded path.The nvidia CUDA library path does exist in other scripts (start-lowvram.sh:14, start-deepspeed.sh:10, ddistro_install.sh:83), but not in start-regular.sh. The "Also applies to: 10-10, 12-12" statement is partially incorrect.
Lines 8 and 10 warrant fixes; line 12 does not apply to this script.
Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
requirements5.txt (1)
1-177: Document the purpose and scope ofrequirements5.txtrelative to other requirements files.The file contains 177 strictly pinned dependencies spanning ML/NLP/TTS ecosystems (transformers, spacy, coqui-tts, stanza, gruut, torch + CUDA stack, etc.). This appears to be a complete environment setup, not a minimal set for an upload API route.
Please clarify:
- What is the rationale for creating a separate
requirements5.txt?- Is this intended to replace or supplement existing
requirements.txt?- Are all 177 dependencies necessary for the
/upload_samplefeature, or is this a broader environment manifest?- Why include extensive ML tooling (e.g., spacy, coqui-tts-trainer, deepspeed) for a file upload endpoint?
Consider adding a comment block at the top of the file documenting its purpose, deployment target, and relationship to other requirements files in the repository.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
requirements5.txt(1 hunks)
🔇 Additional comments (3)
requirements5.txt (3)
1-177: Mismatch between PR objectives and file content—verify scope.The PR objectives describe adding a
/upload_sampleroute to the Web API, but the file under review is a comprehensive requirements manifest containing 177 dependencies. The AI summary differs significantly from the PR objectives. Please clarify:
- Are the main code changes (the
/upload_sampleroute handler) included in a separate file not shown here?- Is
requirements5.txtintentionally separate fromrequirements.txt, and what is its purpose?- Why does a new upload route warrant such an extensive dependency set?
126-126: Now I'll verify the compatibility between these versions:Verify version compatibility: all three packages are compatible and current.
FastAPI requires python-multipart >=0.0.18. python-multipart 0.0.20 was released Dec 16, 2024 and satisfies this requirement. All three versions—python-multipart 0.0.20, FastAPI 0.115.6, and uvicorn 0.34.0—are recent stable releases from December 2024 with no known compatibility issues between them.
172-175: Based on the verification, the original review comment contains a factual error. Local version identifiers are PEP 440 compliant and follow the scheme:<public version identifier>[+<local version label>]. The+cu130notation is therefore not "non-PEP 440 versioning" as claimed.However, the underlying concern about installation practices is worth addressing. PyPI does not allow uploading packages with local version identifiers, and while PyTorch publishes wheels with this notation, the official guidance (from the earlier web search) indicates that using
--index-url https://download.pytorch.org/whl/cu130is the primary recommended approach rather than pinning+cu130suffixes in requirements files.
Reconsider pinning strategy for CUDA-specific PyTorch wheels.
The
+cu130suffix is PEP 440-compliant local version notation. However, PyTorch's official installation guidance recommends using the--index-urlflag (e.g.,https://download.pytorch.org/whl/cu130) rather than pinning CUDA variants in requirements.txt. This ensures better reproducibility and compatibility.Consider:
- Adding
--index-url https://download.pytorch.org/whl/cu130to your pip install command, or- Documenting the exact pip command needed to install these requirements with the correct PyTorch index
Conditionally add CUDA paths and libraries if the directory exists.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
start-deepspeed.sh (2)
7-7: Replace hardcoded paths with script-relative or environment-aware paths.Absolute paths like
/home/dwemer/...are not portable across development, test, and production environments. Use relative paths or environment variables to make this script reusable.Consider parameterizing critical paths:
+#!/bin/bash +# Configuration +PROJECT_DIR="${PROJECT_DIR:-$(dirname "$0")}" +VENV_PATH="${VENV_PATH:-${PROJECT_DIR}/../python-tts}" +CUDA_PATH="${CUDA_PATH:-/usr/local/cuda-12.8}" -if [ -d "/usr/local/cuda-12.8/bin" ]; then - export PATH="/usr/local/cuda-12.8/bin:$PATH" +if [ -d "${CUDA_PATH}/bin" ]; then + export PATH="${CUDA_PATH}/bin:$PATH" fi -cd /home/dwemer/xtts-api-server/ || exit 1 +cd "${PROJECT_DIR}" || { echo "Error: Failed to change directory to ${PROJECT_DIR}"; exit 1; } -source /home/dwemer/python-tts/bin/activate || exit 1 +source "${VENV_PATH}/bin/activate" || { echo "Error: Failed to activate venv at ${VENV_PATH}"; exit 1; } -if [ -d "/home/dwemer/python-tts/lib/python3.11/site-packages/nvidia/cu13/lib/" ]; +if [ -d "${VENV_PATH}/lib/python3.11/site-packages/nvidia/cu13/lib/" ]; then - export LD_LIBRARY_PATH=$LD_LIBRARY_PATH:/home/dwemer/python-tts/lib/python3.11/site-packages/nvidia/cu13/lib/ - export PATH=/home/dwemer/python-tts/lib/python3.11/site-packages/nvidia/cu13/bin:$PATH - export CUDA_HOME=/home/dwemer/python-tts/lib/python3.11/site-packages/nvidia/cu13 + export LD_LIBRARY_PATH="${LD_LIBRARY_PATH}:${VENV_PATH}/lib/python3.11/site-packages/nvidia/cu13/lib/" + export PATH="${VENV_PATH}/lib/python3.11/site-packages/nvidia/cu13/bin:${PATH}" + export CUDA_HOME="${VENV_PATH}/lib/python3.11/site-packages/nvidia/cu13" fiThis approach allows users to override paths via environment variables or symlinks without editing the script.
Also applies to: 9-9, 14-14, 15-15, 16-16
19-19: Consider capturing process PID when launching background task.Running the server with
&creates a background process, but the PID is not captured. This can make graceful shutdown or health monitoring difficult.Consider storing the PID for lifecycle management:
-python -m xtts_api_server --deepspeed --listen &> log.txt & +python -m xtts_api_server --deepspeed --listen &> log.txt & +echo $! > .xtts_server.pidThis allows other scripts to query or terminate the server by reading the PID from
.xtts_server.pid.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
requirements5.txt(1 hunks)start-deepspeed.sh(1 hunks)
🧰 Additional context used
🪛 Shellcheck (0.11.0)
start-deepspeed.sh
[warning] 7-7: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
| nvidia-cuda-nvcc | ||
| nvidia-cuda-profiler-api-cu13 | ||
| nvidia-cuda-profiler-api | ||
|
|
There was a problem hiding this comment.
Fix incomplete package specifications.
Lines 177–179 lack version constraints, and line 180 contains malformed data. These prevent reproducible builds and may cause installation failures.
Apply this diff to fix the incomplete entries:
-nvidia-cuda-nvcc
-nvidia-cuda-profiler-api-cu13
-nvidia-cuda-profiler-api
-+
+nvidia-cuda-nvcc==13.0.39
+nvidia-cuda-profiler-api-cu13==0.0.0a0
+nvidia-cuda-profiler-api==13.0.39If the intent is to omit these packages, remove them entirely. If they should truly have no version constraint, consider documenting why.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In requirements5.txt around lines 177 to 180, the three CUDA-related entries are
incomplete (no version constraints) and one line is malformed; update these
lines by either removing the packages entirely if they are not required, or
pinning them to explicit, tested versions (e.g., nvidia-cuda-nvcc==<version>,
nvidia-cuda-profiler-api-cu13==<version>, nvidia-cuda-profiler-api==<version>)
and remove the malformed line; if you intentionally want no pinning, leave them
out of this requirements file and add a comment explaining why unpinned CUDA
packages are acceptable for reproducibility.
| export PATH="/usr/local/cuda-12.8/bin:$PATH" | ||
| fi | ||
|
|
||
| cd /home/dwemer/xtts-api-server/ |
There was a problem hiding this comment.
Add error handling to cd and source commands.
Both commands can fail silently, allowing subsequent lines to execute in an unexpected state. This is a critical failure mode.
Apply this diff to add error handling:
-cd /home/dwemer/xtts-api-server/
+cd /home/dwemer/xtts-api-server/ || { echo "Error: Failed to change directory"; exit 1; }
-source /home/dwemer/python-tts/bin/activate
+source /home/dwemer/python-tts/bin/activate || { echo "Error: Failed to activate venv"; exit 1; }Also applies to: 9-9
🧰 Tools
🪛 Shellcheck (0.11.0)
[warning] 7-7: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
🤖 Prompt for AI Agents
start-deepspeed.sh lines 7 and 9: the cd and source commands can fail silently;
either enable strict shell failure handling at the top of the script (e.g., set
-euo pipefail) or add explicit checks immediately after each cd and source
invocation that detect failure, print a clear error message to stderr, and exit
with a non-zero status so the script stops on error.
Added symlink creation for CUDA libraries and updated environment variables for CUDA paths.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (6)
ddistro_install.sh (6)
16-76: Consistently usepython3 -m pipinstead of barepipwithin the venv.This issue was flagged in a previous review. Mixing bare
pipandpython3 -m pipcan lead to ambiguity about which interpreter is used.Replace all bare
pipinvocations withpython3 -m pipthroughout the script (lines 17, 19, 22–23, 36, 39, 61, 63, 66, 68, etc.). Example:-pip install setuptools==68.1.2 +python3 -m pip install setuptools==68.1.2 -pip install --no-deps -r requirements.txt +python3 -m pip install --no-deps -r requirements.txt
79-79: Check file existence before copying speaker sample.This issue was flagged in a previous review. Copying without verifying the source file exists will fail with an unclear error.
+if [[ ! -f "/home/dwemer/TheNarrator.wav" ]]; then + echo "Error: sample file not found at /home/dwemer/TheNarrator.wav" + exit 1 +fi cp /home/dwemer/TheNarrator.wav speakers/TheNarrator.wav
3-3: Wrapcdwith error handling.This issue was flagged in a previous review but remains unaddressed. If the directory change fails, subsequent commands run in the wrong context.
-cd /home/dwemer/xtts-api-server +cd /home/dwemer/xtts-api-server || { echo "Error: cannot cd to xtts-api-server"; exit 1; }
27-27: Resolve site-packages path dynamically instead of hard-coding Python 3.11.This issue was flagged in a previous review. Hard-coding
python3.11will fail silently on any other interpreter version (3.10, 3.12+). Notably, line 45 already demonstrates the correct approach usingpython3 -c 'import site; ...', but lines 27 and 73 still hard-code the path.Apply this dynamic resolution to both line 27 and line 73:
+model_file="$( + python3 - <<'PY' +import sysconfig +print(sysconfig.get_path("purelib") + "/TTS/tts/models/xtts.py") +PY +)" +if [[ ! -f "$model_file" ]]; then + echo "Error: xtts.py not found at $model_file" + exit 1 +fi -sed -i 's/checkpoint = load_fsspec(model_path, map_location=torch.device("cpu"))\["model"\]/checkpoint = load_fsspec(model_path, map_location=torch.device("cpu"), weights_only=False)["model"]/' /home/dwemer/python-tts/lib/python3.11/site-packages/TTS/tts/models/xtts.py +sed -i 's/checkpoint = load_fsspec(model_path, map_location=torch.device("cpu"))\["model"\]/checkpoint = load_fsspec(model_path, map_location=torch.device("cpu"), weights_only=False)["model"]/' "$model_file"Repeat this pattern for both occurrences (line 27 in the cu118 branch and line 73 in the cu128 branch).
Also applies to: 73-73
94-99: Resolve CUDA library paths dynamically instead of hard-coding Python 3.11.This issue relates to the hard-coded site-packages path flagged elsewhere (lines 27, 73). Hard-coding
python3.11will fail silently on other interpreter versions.-if [ -d "/home/dwemer/python-tts/lib/python3.11/site-packages/nvidia/cu13/lib/" ]; +cu13_lib_dir="$(python3 -c 'import sysconfig; print(sysconfig.get_path("purelib") + "/nvidia/cu13/lib")' 2>/dev/null)" +if [ -n "$cu13_lib_dir" ] && [ -d "$cu13_lib_dir" ]; then - export LD_LIBRARY_PATH=$LD_LIBRARY_PATH:/home/dwemer/python-tts/lib/python3.11/site-packages/nvidia/cu13/lib/ - export PATH=/home/dwemer/python-tts/lib/python3.11/site-packages/nvidia/cu13/bin:$PATH - export CUDA_HOME=/home/dwemer/python-tts/lib/python3.11/site-packages/nvidia/cu13 + export LD_LIBRARY_PATH=$LD_LIBRARY_PATH:$cu13_lib_dir + export PATH=$(python3 -c 'import sysconfig; print(sysconfig.get_path("purelib") + "/nvidia/cu13/bin")'):$PATH + export CUDA_HOME=$(python3 -c 'import sysconfig; print(sysconfig.get_path("purelib") + "/nvidia/cu13")') fi
103-103: Usepython3(or the venv's Python) instead of barepython.This issue was flagged in a previous review. Using bare
pythonmay bypass the venv or use an incorrect interpreter.-python -m xtts_api_server --listen $DEEPSPEED +python3 -m xtts_api_server --listen $DEEPSPEEDAlternatively, use the venv's Python directly for stronger isolation:
"${VENV_DIR:-/home/dwemer/python-tts}/bin/python" -m xtts_api_server --listen $DEEPSPEED
🧹 Nitpick comments (3)
ddistro_install.sh (3)
89-89: Useread -rto prevent backslash interpretation.The
readcommand without-rwill interpret backslashes as escape sequences, potentially causing unexpected behavior when users input certain characters.-read +read -r
101-101: Add defensive check forstart.shsymlink.The
readlinkcommand will fail silently ifstart.shdoesn't exist or isn't a symlink, andDEEPSPEEDwill be empty. Consider adding a check to confirm the file exists and provide clearer intent.-readlink start.sh | grep -q '/home/dwemer/xtts-api-server/start-deepspeed.sh' && export DEEPSPEED="--deepspeed" || export DEEPSPEED="" +if [ -L start.sh ]; then + readlink start.sh | grep -q 'start-deepspeed.sh' && export DEEPSPEED="--deepspeed" || export DEEPSPEED="" +else + export DEEPSPEED="" +fi
54-54: Remove or clarify commented-out code.Lines 54 and 72 contain commented
#pip install xtts-api-server #Failswith no explanation. These should either be removed or replaced with a clear TODO/FIXME explaining why installation fails and what the workaround is.- #pip install xtts-api-server #Fails + # TODO: xtts-api-server install fails; investigate dependency issue or install via editable mode (pip install -e .)Alternatively, remove these lines entirely if they're no longer relevant.
Also applies to: 72-72
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
ddistro_install.sh(1 hunks)
🧰 Additional context used
🪛 Shellcheck (0.11.0)
ddistro_install.sh
[warning] 3-3: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
Add cleanup step for previous deepspeed files.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (5)
ddistro_install.sh (5)
3-3: Guard thecdcommand with error handling.If the directory change fails, subsequent commands will execute in the wrong context, corrupting the environment setup. Previously flagged; still unresolved.
-cd /home/dwemer/xtts-api-server +cd /home/dwemer/xtts-api-server || { echo "Error: cannot cd to xtts-api-server"; exit 1; }
87-89: Fix contradictory user messaging and flow.The prompt tells the user to "wait for Uvicorn message" before starting the server, and then instructs them to "close this window"—which would kill the running process. This is confusing and incorrect. Clarify the sequence: the script will start the server immediately after the user presses Enter.
echo -echo "This will start CHIM XTTS to download the selected model" -echo "Wait for the message 'Uvicorn running on http://0.0.0.0:8020 (Press CTRL+C to quit)'" -echo "Then close this window. Press ENTER to continue" -read +echo "Press ENTER to start CHIM XTTS and download the selected model." +echo "After startup, look for: 'Uvicorn running on http://0.0.0.0:8020 (Press CTRL+C to quit)'." +read -r
1-2: Add strict error handling after shebang.The script lacks
set -euo pipefailfor proper error propagation and safe defaults (exit on error, undefined variables, pipe failures). This allows the script to silently continue when commands fail.#!/bin/bash +set -euo pipefail cd /home/dwemer/xtts-api-server
80-80: Verify speaker file exists before copying; parameterize paths.The copy operation has no existence check and hard-codes the sample path, making it fragile. If the file is missing or in a different location, the command silently fails or produces a cryptic error. Previously flagged; still unresolved.
+SAMPLE_FILE="/home/dwemer/TheNarrator.wav" +if [[ ! -f "$SAMPLE_FILE" ]]; then + echo "Error: sample file not found at $SAMPLE_FILE" >&2 + exit 1 +fi -cp /home/dwemer/TheNarrator.wav speakers/TheNarrator.wav +cp "$SAMPLE_FILE" speakers/TheNarrator.wav
28-28: Resolve the site-packages path at runtime instead of hard-coding python3.11.The
sedpath assumespython3.11butpython3 -m venvadapts to the installed Python version (3.10, 3.12, etc.). On any version other than 3.11, this command silently fails, leaving the patch unapplied. Previously flagged; still unresolved. Use the venv's interpreter to resolve the path dynamically.- sed -i 's/checkpoint = load_fsspec(model_path, map_location=torch.device("cpu"))\["model"\]/checkpoint = load_fsspec(model_path, map_location=torch.device("cpu"), weights_only=False)["model"]/' /home/dwemer/python-tts/lib/python3.11/site-packages/TTS/tts/models/xtts.py + model_file="$(/home/dwemer/python-tts/bin/python -c 'import sysconfig, pathlib; print(pathlib.Path(sysconfig.get_path("purelib")) / "TTS/tts/models/xtts.py")')" + if [[ ! -f "$model_file" ]]; then + echo "Error: xtts.py not found at $model_file" >&2 + exit 1 + fi + sed -i 's/checkpoint = load_fsspec(model_path, map_location=torch.device("cpu"))\["model"\]/checkpoint = load_fsspec(model_path, map_location=torch.device("cpu"), weights_only=False)["model"]/' "$model_file"
🧹 Nitpick comments (4)
ddistro_install.sh (4)
18-27: Consistently usepython3 -m pipfor clarity and robustness.Line 17 correctly uses
python3 -m pip, but subsequent lines revert to barepip. This inconsistency can lead to ambiguity about which interpreter is invoked. Standardize onpython3 -m pipthroughout.- pip install setuptools==68.1.2 - # Install app requirements without auto-pulling torch/torchaudio from deps - pip install --no-deps -r requirements.txt + python3 -m pip install setuptools==68.1.2 + # Install app requirements without auto-pulling torch/torchaudio from deps + python3 -m pip install --no-deps -r requirements.txt # Pin to stable, CUDA-tagged PyTorch/Torchaudio that do not require TorchCodec pip cache purge || true - pip uninstall -y torch torchaudio torchcodec torchvision || true - pip install --no-deps --no-cache-dir --index-url "$torch_url" "torch==${torch_ver}+${cu_tag}" "torchaudio==${torchaudio_ver}+${cu_tag}" - pip check || true + python3 -m pip uninstall -y torch torchaudio torchcodec torchvision || true + python3 -m pip install --no-deps --no-cache-dir --index-url "$torch_url" "torch==${torch_ver}+${cu_tag}" "torchaudio==${torchaudio_ver}+${cu_tag}" + python3 -m pip check || true # Ensure fallback audio loader is available - pip install --no-cache-dir soundfile + python3 -m pip install --no-cache-dir soundfile
37-44: Standardize onpython3 -m pipfor consistency across all CUDA paths.Both the CUDA 13.0 and CUDA 12.8 branches use bare
pipinconsistently. Line 46 already demonstrates the correct pattern for dynamic resolution; apply the same discipline to all pip invocations.Also applies to: 62-72
102-102: Resolve the start script symlink target using the venv's Python and parameterized paths.This grep command hard-codes
/home/dwemer/xtts-api-server/start-deepspeed.sh, making it brittle to path changes. Additionally,readlinkoutput may be a relative symlink; use the venv's Python to compute it reliably.-readlink start.sh | grep -q '/home/dwemer/xtts-api-server/start-deepspeed.sh' && export DEEPSPEED="--deepspeed" || export DEEPSPEED="" +SYMLINK_TARGET="$(readlink -f start.sh)" || SYMLINK_TARGET="" +[[ "$SYMLINK_TARGET" == *"start-deepspeed.sh"* ]] && export DEEPSPEED="--deepspeed" || export DEEPSPEED=""
3-5: Parameterize base directory and venv paths to improve portability and reusability.Hard-coding
/home/dwemerpaths makes this script specific to one user and environment. Allow overrides via environment variables with sensible defaults so users can adapt it to their setup without editing the script.Add near the top (after shebang and
set -euo pipefail):# Allow users to override paths via environment variables BASE_DIR="${BASE_DIR:-$HOME/xtts-api-server}" VENV_DIR="${VENV_DIR:-$HOME/python-tts}" SAMPLE_FILE="${SAMPLE_FILE:-$HOME/TheNarrator.wav}" cd "$BASE_DIR" || { echo "Error: cannot cd to $BASE_DIR"; exit 1; } python3 -m venv "$VENV_DIR" source "$VENV_DIR/bin/activate"This allows users to run:
BASE_DIR=/opt/xtts VENV_DIR=/opt/venv ./ddistro_install.sh
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
ddistro_install.sh(1 hunks)
🧰 Additional context used
🪛 Shellcheck (0.11.0)
ddistro_install.sh
[warning] 3-3: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
| if [ -d "/home/dwemer/python-tts/lib/python3.11/site-packages/nvidia/cu13/lib/" ]; | ||
| then | ||
| export LD_LIBRARY_PATH=$LD_LIBRARY_PATH:/home/dwemer/python-tts/lib/python3.11/site-packages/nvidia/cu13/lib/ | ||
| export PATH=/home/dwemer/python-tts/lib/python3.11/site-packages/nvidia/cu13/bin:$PATH | ||
| export CUDA_HOME=/home/dwemer/python-tts/lib/python3.11/site-packages/nvidia/cu13 | ||
| fi |
There was a problem hiding this comment.
Use runtime path resolution for CUDA libraries instead of hard-coded python3.11.
The conditional path and environment variables hard-code python3.11. This will fail silently on other Python versions.
-if [ -d "/home/dwemer/python-tts/lib/python3.11/site-packages/nvidia/cu13/lib/" ];
+CUDA_LIB_PATH="$(/home/dwemer/python-tts/bin/python -c 'import sysconfig, pathlib; p = pathlib.Path(sysconfig.get_path("purelib")) / "nvidia/cu13/lib"; print(p if p.exists() else "")')"
+if [ -n "$CUDA_LIB_PATH" ] && [ -d "$CUDA_LIB_PATH" ];
then
- export LD_LIBRARY_PATH=$LD_LIBRARY_PATH:/home/dwemer/python-tts/lib/python3.11/site-packages/nvidia/cu13/lib/
- export PATH=/home/dwemer/python-tts/lib/python3.11/site-packages/nvidia/cu13/bin:$PATH
- export CUDA_HOME=/home/dwemer/python-tts/lib/python3.11/site-packages/nvidia/cu13
+ export LD_LIBRARY_PATH="${LD_LIBRARY_PATH}:${CUDA_LIB_PATH}"
+ export PATH="$(dirname "$CUDA_LIB_PATH")/bin:$PATH"
+ export CUDA_HOME="$(dirname "$CUDA_LIB_PATH")/.."
fiCommittable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In ddistro_install.sh around lines 95-100, the script hard-codes a python3.11
site-packages path for CUDA libs; replace the hard-coded path with a
runtime-resolved site-packages base (call python to print the site-packages or
sysconfig path and store it in a variable) and then build the nvidia/cu13 lib
and bin paths from that variable before checking/setting LD_LIBRARY_PATH, PATH
and CUDA_HOME so the script works for other Python versions/environments.
Add prompt for clean install option before setup.
Add process termination for port 8020 before installation
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (7)
ddistro_install.sh (7)
24-24: Consistently usepython3 -m pipinstead of barepip.Mixing bare
pipandpython3 -m pipwithin an activated venv can lead to ambiguity. Usepython3 -m pipthroughout for clarity and consistency.Apply this pattern throughout the script:
- pip install setuptools==68.1.2 - pip install --no-deps -r requirements.txt - pip cache purge || true - pip uninstall -y torch torchaudio torchcodec torchvision || true + python3 -m pip install setuptools==68.1.2 + python3 -m pip install --no-deps -r requirements.txt + python3 -m pip cache purge || true + python3 -m pip uninstall -y torch torchaudio torchcodec torchvision || trueAlso applies to: 26-26, 28-28, 29-29, 43-43, 46-46, 70-70, 72-72
3-3: Checkcdcommand for failure.Line 3 lacks error handling; if the directory change fails, the script will continue in the wrong context.
- cd /home/dwemer/xtts-api-server + cd /home/dwemer/xtts-api-server || { echo "Error: cannot cd to xtts-api-server"; exit 1; }
34-34: Resolve site-packages path at runtime instead of hard-codingpython3.11.Lines 34 and 80 hard-code
/home/dwemer/python-tts/lib/python3.11/site-packages/which will fail silently on Python 3.10, 3.12, or any other version. The script already demonstrates the correct pattern at line 52; apply it here.The
sedcommand will silently succeed even if the path is wrong, leaving the model file unfixed.Use the runtime resolution pattern (as shown in line 52) for both occurrences:
- sed -i 's/checkpoint = load_fsspec(model_path, map_location=torch.device("cpu"))\["model"\]/checkpoint = load_fsspec(model_path, map_location=torch.device("cpu"), weights_only=False)["model"]/' /home/dwemer/python-tts/lib/python3.11/site-packages/TTS/tts/models/xtts.py + model_file="$(python3 -c 'import site, pathlib; print(pathlib.Path(site.getsitepackages()[0]) / "TTS/tts/models/xtts.py")')" + if [[ ! -f "$model_file" ]]; then + echo "Error: xtts.py not found at $model_file" >&2 + exit 1 + fi + sed -i 's/checkpoint = load_fsspec(model_path, map_location=torch.device("cpu"))\["model"\]/checkpoint = load_fsspec(model_path, map_location=torch.device("cpu"), weights_only=False)["model"]/' "$model_file"Apply the same fix at line 80.
Also applies to: 80-80
86-86: Check source file existence before copying.Line 86 copies
/home/dwemer/TheNarrator.wavwithout verifying it exists, leading to silent or unclear failures.+ sample_file="/home/dwemer/TheNarrator.wav" + if [[ ! -f "$sample_file" ]]; then + echo "Error: sample file not found at $sample_file" >&2 + exit 1 + fi - cp /home/dwemer/TheNarrator.wav speakers/TheNarrator.wav + cp "$sample_file" speakers/TheNarrator.wav
101-106: Resolve CUDA library paths at runtime instead of hard-codingpython3.11.Lines 101–106 hard-code the site-packages path for CUDA libraries. On Python versions other than 3.11, the path check at line 101 will fail silently, and the environment variables (
LD_LIBRARY_PATH,PATH,CUDA_HOME) will never be set, causing CUDA lookups to fail at runtime.Resolve the path dynamically (matching the pattern from line 52):
- if [ -d "/home/dwemer/python-tts/lib/python3.11/site-packages/nvidia/cu13/lib/" ]; - then - export LD_LIBRARY_PATH=$LD_LIBRARY_PATH:/home/dwemer/python-tts/lib/python3.11/site-packages/nvidia/cu13/lib/ - export PATH=/home/dwemer/python-tts/lib/python3.11/site-packages/nvidia/cu13/bin:$PATH - export CUDA_HOME=/home/dwemer/python-tts/lib/python3.11/site-packages/nvidia/cu13 - fi + cuda_lib_path="$(python3 -c 'import site, pathlib; p = pathlib.Path(site.getsitepackages()[0]) / "nvidia/cu13/lib"; print(p if p.exists() else "")')" + if [[ -n "$cuda_lib_path" ]] && [[ -d "$cuda_lib_path" ]]; then + export LD_LIBRARY_PATH="${LD_LIBRARY_PATH}:${cuda_lib_path}" + export PATH="$(dirname "$cuda_lib_path")/bin:$PATH" + export CUDA_HOME="$(dirname "$(dirname "$cuda_lib_path")")" + fi
110-110: Install the localxtts_api_serverpackage and use the venv's Python explicitly.Line 110 attempts to run
-m xtts_api_serverusing barepython, but:
- The
xtts_api_servermodule is never installed (nopip install .orpip install -e .anywhere in the script).- Bare
pythonmay not resolve to the venv's Python; usepython3or the explicit path.This will fail at runtime with a module-not-found error.
Solution A (recommended): Install the local package before running it.
After line 90 (after
conf.sh), add:# Install the local xtts_api_server package python3 -m pip install -e . || python3 -m pip install .Then update line 110 to use the venv's
python3explicitly:- python -m xtts_api_server --listen $DEEPSPEED + /home/dwemer/python-tts/bin/python3 -m xtts_api_server --listen $DEEPSPEEDSolution B (alternative): Add a console-script entrypoint in
pyproject.toml.If a
pyproject.tomlexists in the repo root, add:[project.scripts] xtts-api-server = "xtts_api_server.__main__:main"Then ensure
pip install .orpip install -e .is run (as in Solution A), and call the executable directly:/home/dwemer/python-tts/bin/xtts-api-server --listen $DEEPSPEED
1-5: Add strict error handling withset -euo pipefail.Without
set -euo pipefail, the script will silently continue even after failures (e.g., if asedcommand doesn't match, ifcpfails, or if variable expansion fails). This leaves the setup in an inconsistent state.Add this after the shebang:
#!/bin/bash + set -euo pipefail +This will:
- Exit immediately on any command failure (
-e).- Fail if an undefined variable is referenced (
-u).- Propagate failures through pipelines (
-o pipefail).
🧹 Nitpick comments (1)
ddistro_install.sh (1)
9-9: Useread -rto prevent backslash interpretation.Lines with
readcommands (9, 17, 37, 96) should use the-rflag to avoid treating backslashes as escape sequences, which can cause unexpected behavior.- read -p "Do you want to perform a clean install? (yes/no): " clean_install + read -r -p "Do you want to perform a clean install? (yes/no): " clean_install - read -p "Are you using a GT10XX series GPU? (yes/no): " gpu_answer + read -r -p "Are you using a GT10XX series GPU? (yes/no): " gpu_answer - read -p "New: Use CUDA13. Needs windows updated drivers. RTX 50XX should use this. Usey cuda13? (yes/no): " gpu5_answer + read -r -p "New: Use CUDA13. Needs windows updated drivers. RTX 50XX should use this. Usey cuda13? (yes/no): " gpu5_answer - read + read -rAlso applies to: 17-17, 37-37, 96-96
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
ddistro_install.sh(1 hunks)
🧰 Additional context used
🪛 Shellcheck (0.11.0)
ddistro_install.sh
[warning] 3-3: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
Add virtual environment setup for clean install.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (5)
ddistro_install.sh (5)
1-12: Add strict error handling and parameterize hard-coded paths.The script lacks
set -euo pipefailfor robust error handling, and line 3'scdcommand is unguarded—if it fails, subsequent commands execute in the wrong directory. Additionally, hard-coded/home/dwemerpaths make the script non-portable.(These issues were flagged in previous reviews.)
Apply this diff:
#!/bin/bash +set -euo pipefail -cd /home/dwemer/xtts-api-server -python3 -m venv /home/dwemer/python-tts -source /home/dwemer/python-tts/bin/activate +BASE_DIR="${BASE_DIR:-$HOME/xtts-api-server}" +VENV_DIR="${VENV_DIR:-$HOME/python-tts}" + +cd "$BASE_DIR" || { echo "Error: cannot cd to $BASE_DIR"; exit 1; } +python3 -m venv "$VENV_DIR" +source "$VENV_DIR/bin/activate"
44-44: Resolve Python version path dynamically instead of hard-codingpython3.11.Lines 44 and 90 hard-code
/home/dwemer/python-tts/lib/python3.11/..., which breaks silently on Python 3.10, 3.12, or other versions. Line 62 shows the correct approach; replicate that pattern here.(This critical issue was flagged in previous reviews and remains unresolved.)
Apply this diff to both line 44 and line 90:
- sed -i 's/checkpoint = load_fsspec(model_path, map_location=torch.device("cpu"))\["model"\]/checkpoint = load_fsspec(model_path, map_location=torch.device("cpu"), weights_only=False)["model"]/' /home/dwemer/python-tts/lib/python3.11/site-packages/TTS/tts/models/xtts.py + model_file="$(python3 -c 'import sysconfig, pathlib; print(pathlib.Path(sysconfig.get_path("purelib")) / "TTS/tts/models/xtts.py")')" + if [[ ! -f "$model_file" ]]; then + echo "Error: xtts.py not found at $model_file" >&2 + exit 1 + fi + sed -i 's/checkpoint = load_fsspec(model_path, map_location=torch.device("cpu"))\["model"\]/checkpoint = load_fsspec(model_path, map_location=torch.device("cpu"), weights_only=False)["model"]/' "$model_file"Also applies to: 90-90
96-96: Check file existence before copying and use parameterized paths.Copying without verifying the source exists will fail with an unclear error. Additionally, the hard-coded
/home/dwemer/path is not portable.(This issue was flagged in previous reviews.)
Apply this diff:
-cp /home/dwemer/TheNarrator.wav speakers/TheNarrator.wav +SAMPLE_PATH="${SAMPLE_PATH:-$HOME/TheNarrator.wav}" +if [[ ! -f "$SAMPLE_PATH" ]]; then + echo "Error: sample file not found at $SAMPLE_PATH" >&2 + exit 1 +fi +cp "$SAMPLE_PATH" "speakers/TheNarrator.wav"
111-116: Resolve CUDA paths dynamically instead of hard-codingpython3.11.This block hard-codes
/home/dwemer/python-tts/lib/python3.11/.../nvidia/cu13/, which fails silently on other Python versions or install locations.(This major issue was flagged in previous reviews and remains unresolved.)
Apply this diff:
-# Add CUDA to PATH if the directory exists -if [ -d "/home/dwemer/python-tts/lib/python3.11/site-packages/nvidia/cu13/lib/" ]; +# Add CUDA to PATH if the directory exists +CUDA_LIB_PATH="$(python3 -c 'import sysconfig, pathlib; p = pathlib.Path(sysconfig.get_path("purelib")) / "nvidia/cu13/lib"; print(p if p.exists() else "")' 2>/dev/null || echo "")" +if [ -n "$CUDA_LIB_PATH" ] && [ -d "$CUDA_LIB_PATH" ]; then - export LD_LIBRARY_PATH=$LD_LIBRARY_PATH:/home/dwemer/python-tts/lib/python3.11/site-packages/nvidia/cu13/lib/ - export PATH=/home/dwemer/python-tts/lib/python3.11/site-packages/nvidia/cu13/bin:$PATH - export CUDA_HOME=/home/dwemer/python-tts/lib/python3.11/site-packages/nvidia/cu13 + export LD_LIBRARY_PATH="${LD_LIBRARY_PATH}:${CUDA_LIB_PATH}" + export PATH="$(dirname "$CUDA_LIB_PATH")/bin:$PATH" + export CUDA_HOME="$(dirname "$CUDA_LIB_PATH")/.." fi
71-71: Install the local package and use the venv's Python explicitly.Lines 71 and 89 show failed attempts to install
xtts-api-servervia pip; line 120 then tries to run the module without installing it first, which will fail with "No module named xtts_api_server." Additionally, line 120 uses barepythoninstead ofpython3, which is not guaranteed to be the venv's Python.(Package installation and invocation issues were flagged in previous reviews and remain unresolved.)
Add a local install step after line 100 (after
./conf.sh), and update line 120:./conf.sh +# Install the local xtts_api_server package +python3 -m pip install -e . || python3 -m pip install . + echo echo "This will start CHIM XTTS to download the selected model" echo "Wait for the message 'Uvicorn running on http://0.0.0.0:8020 (Press CTRL+C to quit)'" echo "Then close this window. Press ENTER to continue" read echo "please wait...." # Add CUDA to PATH if the directory exists ... -python -m xtts_api_server --listen $DEEPSPEED +python3 -m xtts_api_server --listen $DEEPSPEEDAlternatively, if
pyproject.tomldefines a console-script entrypoint (e.g.,xtts-api-server = "xtts_api_server.__main__:main"), you can invoke it directly afterpip install .:xtts-api-server --listen $DEEPSPEEDIf you'd like, I can help verify the package structure and generate the appropriate
pyproject.tomlconfiguration if a console-script entrypoint isn't already defined.Also applies to: 89-89, 100-106, 120-120
🧹 Nitpick comments (1)
ddistro_install.sh (1)
25-25: Parameterize and safely clean the cache directory.Line 25 hard-codes a user path and Python version, and assumes the directory exists. Use a safer approach:
-rm /home/dwemer/.cache/torch_extensions/py311_cu130/transformer_inference/* +CACHE_DIR="${HOME}/.cache/torch_extensions/py311_cu130/transformer_inference" +if [ -d "$CACHE_DIR" ]; then + rm -f "$CACHE_DIR"/* +fiNote: The
py311version hardcoding is a separate issue related to Python version detection; consider extracting and parameterizing this in a future refactor.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
ddistro_install.sh(1 hunks)
🧰 Additional context used
🪛 Shellcheck (0.11.0)
ddistro_install.sh
[warning] 3-3: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
| read -p "Do you want to perform a clean install? (yes/no): " clean_install | ||
| if [[ "$clean_install" =~ ^[Yy][Ee][Ss]$ || "$clean_install" =~ ^[Yy]$ ]]; then | ||
| rm -fr /home/dwemer/python-tts/* | ||
| python3 -m venv /home/dwemer/python-tts | ||
| source /home/dwemer/python-tts/bin/activate | ||
| fi |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Update hard-coded paths to use environment variables.
Once parameterization is applied (from the previous comment), update this block:
read -p "Do you want to perform a clean install? (yes/no): " clean_install
if [[ "$clean_install" =~ ^[Yy][Ee][Ss]$ || "$clean_install" =~ ^[Yy]$ ]]; then
- rm -fr /home/dwemer/python-tts/*
- python3 -m venv /home/dwemer/python-tts
- source /home/dwemer/python-tts/bin/activate
+ rm -fr "$VENV_DIR"/*
+ python3 -m venv "$VENV_DIR"
+ source "$VENV_DIR/bin/activate"
fi🤖 Prompt for AI Agents
In ddistro_install.sh around lines 17 to 22, the script uses the hard-coded path
/home/dwemer/python-tts; replace this with a parameterized variable (e.g.
INSTALL_DIR) defaulting to $HOME/python-tts, ensure you quote the variable
everywhere, create parent dirs with mkdir -p before venv creation, use rm -rf
"$INSTALL_DIR"/* for clean installs, run python3 -m venv "$INSTALL_DIR" and
source "$INSTALL_DIR/bin/activate"; also validate/export INSTALL_DIR at the top
if not provided.
| torch_ver="2.2.2" | ||
| torchaudio_ver="2.2.2" | ||
| python3 -m pip install --upgrade pip wheel ninja virtualenv | ||
| pip install setuptools==68.1.2 |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Consistently use python3 -m pip instead of bare pip in the venv.
Mixing bare pip (lines 34, 36, 39, 53, 78, 80, 85) and python3 -m pip (line 33, 52, 77) creates ambiguity about which Python interpreter is active. Once activated in a venv, bare pip usually works, but it's not guaranteed and is not portable across environments.
For consistency and portability, replace all bare pip invocations with python3 -m pip throughout this section.
Also applies to: 36-36, 39-39, 53-53, 56-56, 78-78, 80-80, 85-85
🤖 Prompt for AI Agents
ddistro_install.sh lines 34, 36, 39, 53, 56, 78, 80, 85: several commands use
bare pip while nearby lines use python3 -m pip, causing ambiguity about which
interpreter is invoked; replace each bare `pip` invocation on these lines with
`python3 -m pip` (keeping the same arguments and versions) so all package
installs consistently run via the intended Python interpreter (e.g., change `pip
install setuptools==68.1.2` to `python3 -m pip install setuptools==68.1.2` and
do the same for every other bare pip occurrence listed).
Updated deepseed option description to specify requirement for cuda13 on 50xx series GPUs.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @start-gpu.sh:
- Line 8: The script does a bare cd "$REPO_DIR" without checking success; change
it to verify the directory exists and the cd succeeded (e.g., test -d
"$REPO_DIR" and/or check the exit status of cd "$REPO_DIR") and immediately exit
with a non‑zero code and a clear error message if it fails, so subsequent steps
(virtualenv creation etc.) never run in the wrong location.
- Around line 11-16: The script creates the virtual environment ($VENV_DIR) but
never installs dependencies, so restapi.py will fail (missing python-multipart);
after creating or activating the venv in start-gpu.sh, run pip install -r
requirements.txt (or pip install python-multipart and other required packages)
using the venv's python/pip executable (reference VENV_DIR) and ensure
requirements.txt includes python-multipart; add the install step so both the
first-time creation branch and the existing-venv branch verify/install
dependencies before proceeding.
🧹 Nitpick comments (2)
start-gpu.sh (2)
4-4: Remove unused variable.
REPO_URLis defined but never used in the script.🧹 Proposed fix
-REPO_URL="https://github.com/Dwemer-Dynamics/chatterbox"
21-21: Add validation before launching the service.The script attempts to run
restapi.pywithout verifying that the file exists or that all prerequisites are met. This can lead to unclear error messages.✅ Proposed fix
# Launch the service +if [ ! -f "restapi.py" ]; then + echo "Error: restapi.py not found in $REPO_DIR" + exit 1 +fi + +echo "Starting REST API service..." python3 restapi.py
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
start-gpu.sh
🧰 Additional context used
🪛 Shellcheck (0.11.0)
start-gpu.sh
[warning] 4-4: REPO_URL appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 8-8: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
start-gpu.sh
Outdated
| REPO_DIR="$BASE_DIR/chatterbox" | ||
| VENV_DIR="$REPO_DIR/venv" | ||
|
|
||
| cd "$REPO_DIR" |
There was a problem hiding this comment.
Add error handling for directory change.
If the directory doesn't exist or isn't accessible, the script will continue executing in the wrong location, potentially creating the virtual environment in an unexpected place.
🔧 Proposed fix
-cd "$REPO_DIR"
+cd "$REPO_DIR" || { echo "Error: Cannot access directory $REPO_DIR"; exit 1; }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| cd "$REPO_DIR" | |
| cd "$REPO_DIR" || { echo "Error: Cannot access directory $REPO_DIR"; exit 1; } |
🧰 Tools
🪛 Shellcheck (0.11.0)
[warning] 8-8: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
🤖 Prompt for AI Agents
In @start-gpu.sh at line 8, The script does a bare cd "$REPO_DIR" without
checking success; change it to verify the directory exists and the cd succeeded
(e.g., test -d "$REPO_DIR" and/or check the exit status of cd "$REPO_DIR") and
immediately exit with a non‑zero code and a clear error message if it fails, so
subsequent steps (virtualenv creation etc.) never run in the wrong location.
start-gpu.sh
Outdated
| if [ ! -d "$VENV_DIR" ]; then | ||
| echo "Creating Python virtual environment..." | ||
| python3 -m venv venv | ||
| else | ||
| echo "Virtual environment already exists." | ||
| fi |
There was a problem hiding this comment.
Missing dependency installation step.
The script creates a virtual environment but never installs the required dependencies. According to the PR description, this should install packages including the newly added python-multipart. Without this step, restapi.py will fail due to missing dependencies.
📦 Proposed fix
# Create virtual environment if it doesn't exist
if [ ! -d "$VENV_DIR" ]; then
echo "Creating Python virtual environment..."
python3 -m venv venv
+ echo "Installing dependencies..."
+ source venv/bin/activate
+ pip install --upgrade pip
+ pip install -r requirements.txt
else
echo "Virtual environment already exists."
+ source venv/bin/activate
fi
-
-# Activate virtual environment
-source venv/bin/activateCommittable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In @start-gpu.sh around lines 11 - 16, The script creates the virtual
environment ($VENV_DIR) but never installs dependencies, so restapi.py will fail
(missing python-multipart); after creating or activating the venv in
start-gpu.sh, run pip install -r requirements.txt (or pip install
python-multipart and other required packages) using the venv's python/pip
executable (reference VENV_DIR) and ensure requirements.txt includes
python-multipart; add the install step so both the first-time creation branch
and the existing-venv branch verify/install dependencies before proceeding.
There was a problem hiding this comment.
Actionable comments posted: 11
🤖 Fix all issues with AI agents
In @ddistro_install.sh:
- Line 96: The CUDA13 install branch is missing the same sed patch applied in
the CUDA128 branch that replaces the checkpoint load call in
TTS/tts/models/xtts.py (the sed command that adds weights_only=False), causing
inconsistent behavior; modify ddistro_install.sh so the same sed replacement
(the sed invocation that alters load_fsspec(..., weights_only=False)["model"])
is run for the CUDA13 path as well (or move the patching to a common
post-install step executed for both branches), ensuring the xtts.py replacement
is applied consistently across both CUDA13 and CUDA128 installations.
- Line 50: The script is directly patching installed package files with sed
(modifying TTS/tts/models/xtts.py to change the load_fsspec call to include
weights_only=False), which is fragile; instead create a validated patch apply
step that locates the virtualenv path dynamically (use a VENV_PATH variable
rather than hardcoded python3.11 path), verify the target file exists, run patch
--dry-run against xtts.py and only apply if the dry-run succeeds, and exit with
an error if the patch cannot be applied; alternatively document the change or
upstream/fork the package to add a configurable weights_only option for
load_fsspec rather than forcing weights_only=False at runtime.
- Around line 1-128: Summary: the install script lacks error handling, uses
hardcoded paths, has no validation or logging, and mixes indentation styles; fix
by adding strict mode and a trap (use set -euo pipefail and a trap on ERR),
introduce configurable top-level variables (BASE_DIR, VENV_PATH, NARRATOR_PATH)
and replace literal "/home/dwemer/..." references with those variables (used
where venv is created/activated, site-packages paths, cp and cd), add a small
check_command function and call it for required commands (python3, lsof, pip) to
fail fast, add a LOG_FILE and redirect stdout/stderr to it via exec >(tee -a
"$LOG_FILE") 2>&1 for install logging, and normalize indentation to spaces
across the file; update usages of DEEPSPEED/xtts_api_server/start scripts to
reference BASE_DIR/VENV_PATH variables accordingly.
- Line 53: The user prompt string in the read -p call assigns to variable
gpu5_answer but contains a typo "Usey cuda13?"; update the prompt text to read
"Use cuda13? (yes/no): " so the question is spelled correctly and still includes
the context about CUDA13 and driver note while leaving the variable name
gpu5_answer unchanged.
- Around line 21-28: The destructive rm -fr /home/dwemer/python-tts/* in the
clean-install branch is unsafe; before removing the venv contents (triggered by
the clean_install variable), validate and canonicalize the target path, ensure
it exactly matches the expected venv directory (e.g.,
"/home/dwemer/python-tts"), reject suspicious or root paths, prompt the user for
a secondary explicit confirmation (yes typed), and create a backup (move or tar)
of the existing venv directory before deleting; only perform the removal after
these checks and confirmations, then recreate the venv and source it as
currently done.
- Around line 30-31: The rm command that deletes the transformer_inference cache
can fail silently; update ddistro_install.sh to first check that the
transformer_inference directory exists and is readable, then delete files with a
safe rm invocation and explicitly handle errors: if the directory is missing
skip with a logged notice, otherwise run a protected removal (use -- or -f/-r as
appropriate) and if rm fails capture and log the error and exit non‑zero so the
failure is visible. Reference the existing rm invocation that targets the
transformer_inference cache directory and replace it with the described
existence check + guarded removal + error logging/exit.
- Around line 9-11: Add strict failure handling and replace hardcoded paths with
configurable variables: set "set -euo pipefail" at the top, use variables like
INSTALL_DIR and VENV_DIR instead of "/home/dwemer/...", and check the return
status of the cd command (the current "cd /home/dwemer/xtts-api-server"
invocation) and exit if it fails. Before creating the venv (the "python3 -m venv
/home/dwemer/python-tts" step) validate that INSTALL_DIR exists and VENV_DIR
parent is writable (create directories if needed with proper permissions), and
ensure "source .../bin/activate" is only run after the venv was successfully
created; fail with an explanatory message on any error.
- Around line 102-106: The script blindly runs the cp and ./conf.sh steps; add
pre-flight checks to ensure /home/dwemer/TheNarrator.wav exists and the target
directory (speakers/) is present/writable before running cp (create speakers/
with mkdir -p if missing), and verify ./conf.sh exists and is executable before
invoking it (or exit with a clear error). In practice, update the block around
the cp command and the invocation of conf.sh to perform file/directory existence
checks (using test -f/-d or similar), emit descriptive error messages, and exit
non‑zero on failure so the installer fails fast with a clear reason.
- Line 126: Before running "python -m xtts_api_server --listen $DEEPSPEED"
verify the installation and module availability by checking the prior install
command exit code and performing an import check (e.g., run a quick "python -c
'import xtts_api_server'" or "python -m pip show xtts_api_server"); if the check
fails, log a clear error and exit non‑zero instead of attempting to start
xtts_api_server; ensure the script references the same "xtts_api_server" and
"$DEEPSPEED" variables so the start command only runs after successful
validation.
🧹 Nitpick comments (1)
ddistro_install.sh (1)
61-61: Remove commented-out code or document why it's kept.Multiple commented-out lines with notes like
#Failssuggest an incomplete or uncertain implementation. This makes the script harder to maintain and understand.Either:
- Remove the commented code if it's no longer needed
- Document why these approaches failed and what the correct solution is
- Open an issue to track fixing the underlying problems (e.g., why
pip install xtts-api-serverfails)Would you like me to open an issue to track investigating why certain installation methods fail?
Also applies to: 77-77, 90-90, 95-95
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
ddistro_install.sh
🧰 Additional context used
🪛 Shellcheck (0.11.0)
ddistro_install.sh
[warning] 9-9: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
🔇 Additional comments (1)
ddistro_install.sh (1)
1-8: Good user communication.The header clearly communicates the port conflict constraint, which helps prevent runtime issues.
| #!/bin/bash | ||
|
|
||
| echo "=== CHIM XTTS Installation ===" | ||
| echo "" | ||
| echo "NOTE: CHIM XTTS and Chatterbox use the same port (8020)." | ||
| echo " Only one can be enabled at a time." | ||
| echo "" | ||
|
|
||
| cd /home/dwemer/xtts-api-server | ||
| python3 -m venv /home/dwemer/python-tts | ||
| source /home/dwemer/python-tts/bin/activate | ||
|
|
||
|
|
||
| PIDS=$(lsof -ti:8020) | ||
| if [ -n "$PIDS" ]; then | ||
| echo "Killing processes listening on port 8020: $PIDS" | ||
| kill -9 $PIDS | ||
| fi | ||
|
|
||
|
|
||
| echo "This will take a while so please wait." | ||
|
|
||
| read -p "Do you want to perform a clean install? (yes/no): " clean_install | ||
| if [[ "$clean_install" =~ ^[Yy][Ee][Ss]$ || "$clean_install" =~ ^[Yy]$ ]]; then | ||
| rm -fr /home/dwemer/python-tts/* | ||
| python3 -m venv /home/dwemer/python-tts | ||
| source /home/dwemer/python-tts/bin/activate | ||
| fi | ||
|
|
||
| # Clean previous deepspeed stuff | ||
| rm /home/dwemer/.cache/torch_extensions/py311_cu130/transformer_inference/* | ||
| # Ask user about GPU | ||
| read -p "Are you using a GT10XX series GPU? (yes/no): " gpu_answer | ||
| if [[ "$gpu_answer" =~ ^[Yy][Ee][Ss]$ || "$gpu_answer" =~ ^[Yy]$ ]]; then | ||
| cu_tag="cu118" | ||
| torch_url="https://download.pytorch.org/whl/${cu_tag}" | ||
| torch_ver="2.2.2" | ||
| torchaudio_ver="2.2.2" | ||
| python3 -m pip install --upgrade pip wheel ninja virtualenv | ||
| pip install setuptools==68.1.2 | ||
| # Install app requirements without auto-pulling torch/torchaudio from deps | ||
| pip install --no-deps -r requirements.txt | ||
| # Pin to stable, CUDA-tagged PyTorch/Torchaudio that do not require TorchCodec | ||
| pip cache purge || true | ||
| pip uninstall -y torch torchaudio torchcodec torchvision || true | ||
| pip install --no-deps --no-cache-dir --index-url "$torch_url" "torch==${torch_ver}+${cu_tag}" "torchaudio==${torchaudio_ver}+${cu_tag}" | ||
| pip check || true | ||
| # Ensure fallback audio loader is available | ||
| pip install --no-cache-dir soundfile | ||
| sed -i 's/checkpoint = load_fsspec(model_path, map_location=torch.device("cpu"))\["model"\]/checkpoint = load_fsspec(model_path, map_location=torch.device("cpu"), weights_only=False)["model"]/' /home/dwemer/python-tts/lib/python3.11/site-packages/TTS/tts/models/xtts.py | ||
|
|
||
| else | ||
| read -p "New: Use CUDA13. Needs windows updated drivers. RTX 50XX should use this. Usey cuda13? (yes/no): " gpu5_answer | ||
| if [[ "$gpu5_answer" =~ ^[Yy][Ee][Ss]$ || "$gpu5_answer" =~ ^[Yy]$ ]]; then | ||
| cu_tag="cu130" | ||
| torch_url="https://download.pytorch.org/whl/${cu_tag}" | ||
| echo "Using torch: $torch_url" | ||
| python3 -m pip install --upgrade pip wheel ninja virtualenv | ||
| pip install setuptools==68.1.2 | ||
| # Install app requirements without auto-pulling torch/torchaudio from deps | ||
| #pip install --no-deps -r requirements5.txt --index-url=$torch_url | ||
| pip install -r requirements5.txt --extra-index-url=https://download.pytorch.org/whl/cu130 | ||
| # Pin to stable, CUDA-tagged PyTorch/Torchaudio that do not require TorchCodec | ||
| pip check || true | ||
| # Ensure fallback audio loader is available | ||
| pip install --no-cache-dir soundfile | ||
| # Fix symlinks | ||
| LIBDIR=$(python3 -c 'import site; print(site.getsitepackages()[0])')/nvidia/cu13/lib | ||
| for f in "$LIBDIR"/lib*.so.*; do | ||
| base=$(basename "$f") | ||
| link="${f%%.so.*}.so" | ||
| if [ ! -e "$link" ]; then | ||
| echo "Creating symlink: $(basename "$link") -> $base" | ||
| ln -s "$base" "$link" | ||
| fi | ||
| done | ||
| #pip install xtts-api-server #Fails | ||
|
|
||
| else | ||
| cu_tag="cu128" | ||
| torch_url="https://download.pytorch.org/whl/${cu_tag}" | ||
| echo "Using torch: $torch_url" | ||
| python3 -m pip install --upgrade pip wheel ninja virtualenv | ||
| pip install setuptools==68.1.2 | ||
| # Install app requirements without auto-pulling torch/torchaudio from deps | ||
| pip install --no-deps -r requirements.txt | ||
| # Pin to stable, CUDA-tagged PyTorch/Torchaudio that do not require TorchCodec | ||
| pip cache purge || true | ||
| pip uninstall -y torch torchaudio torchcodec torchvision || true | ||
| #pip install --index-url "$torch_url" torch torchaudio torchcodec torchvision | ||
| pip install torch==2.7.0+cu128 torchaudio==2.7.0+cu128 torchvision==0.22.0+cu128 --index-url=https://download.pytorch.org/whl/ | ||
| pip check || true | ||
| # Ensure fallback audio loader is available | ||
| pip install --no-cache-dir soundfile | ||
| #pip install xtts-api-server #Fails | ||
| sed -i 's/checkpoint = load_fsspec(model_path, map_location=torch.device("cpu"))\["model"\]/checkpoint = load_fsspec(model_path, map_location=torch.device("cpu"), weights_only=False)["model"]/' /home/dwemer/python-tts/lib/python3.11/site-packages/TTS/tts/models/xtts.py | ||
|
|
||
| fi | ||
| fi | ||
|
|
||
|
|
||
| cp /home/dwemer/TheNarrator.wav speakers/TheNarrator.wav | ||
|
|
||
| source /home/dwemer/python-tts/bin/activate | ||
|
|
||
| ./conf.sh | ||
|
|
||
| echo | ||
| echo "This will start CHIM XTTS to download the selected model" | ||
| echo "Wait for the message 'Uvicorn running on http://0.0.0.0:8020 (Press CTRL+C to quit)'" | ||
| echo "Then close this window. Press ENTER to continue" | ||
| read | ||
|
|
||
| echo "please wait...." | ||
|
|
||
| # Add CUDA to PATH if the directory exists | ||
| if [ -d "/home/dwemer/python-tts/lib/python3.11/site-packages/nvidia/cu13/lib/" ]; | ||
| then | ||
| export LD_LIBRARY_PATH=$LD_LIBRARY_PATH:/home/dwemer/python-tts/lib/python3.11/site-packages/nvidia/cu13/lib/ | ||
| export PATH=/home/dwemer/python-tts/lib/python3.11/site-packages/nvidia/cu13/bin:$PATH | ||
| export CUDA_HOME=/home/dwemer/python-tts/lib/python3.11/site-packages/nvidia/cu13 | ||
| fi | ||
|
|
||
| readlink start.sh | grep -q '/home/dwemer/xtts-api-server/start-deepspeed.sh' && export DEEPSPEED="--deepspeed" || export DEEPSPEED="" | ||
|
|
||
| python -m xtts_api_server --listen $DEEPSPEED | ||
|
|
||
| echo "Press Enter" |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add overall error handling and improve script robustness.
The script lacks a comprehensive error handling strategy, which makes it difficult to debug when things go wrong and creates a poor user experience. Additionally, the extensive use of hardcoded paths makes the script non-portable.
Recommended improvements:
- Add error handling at the top:
#!/bin/bash
set -euo pipefail # Exit on error, undefined variables, and pipe failures
# Trap errors and provide helpful messages
trap 'echo "ERROR: Installation failed at line $LINENO. Check output above for details." >&2' ERR- Use configurable paths instead of hardcoded ones:
# Configuration section at top of file
BASE_DIR="${BASE_DIR:-$HOME/xtts-api-server}"
VENV_PATH="${VENV_PATH:-$HOME/python-tts}"
NARRATOR_PATH="${NARRATOR_PATH:-$HOME/TheNarrator.wav}"- Add a validation function:
check_command() {
if ! command -v "$1" &> /dev/null; then
echo "ERROR: Required command '$1' not found. Please install it first."
exit 1
fi
}
check_command python3
check_command lsof
check_command pip- Add logging:
LOG_FILE="./install_$(date +%Y%m%d_%H%M%S).log"
exec > >(tee -a "$LOG_FILE") 2>&1
echo "Installation log: $LOG_FILE"- Fix indentation: The script uses mixed tabs and spaces. Choose one and be consistent (spaces are recommended for bash scripts).
🧰 Tools
🪛 Shellcheck (0.11.0)
[warning] 9-9: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
🤖 Prompt for AI Agents
In @ddistro_install.sh around lines 1 - 128, Summary: the install script lacks
error handling, uses hardcoded paths, has no validation or logging, and mixes
indentation styles; fix by adding strict mode and a trap (use set -euo pipefail
and a trap on ERR), introduce configurable top-level variables (BASE_DIR,
VENV_PATH, NARRATOR_PATH) and replace literal "/home/dwemer/..." references with
those variables (used where venv is created/activated, site-packages paths, cp
and cd), add a small check_command function and call it for required commands
(python3, lsof, pip) to fail fast, add a LOG_FILE and redirect stdout/stderr to
it via exec >(tee -a "$LOG_FILE") 2>&1 for install logging, and normalize
indentation to spaces across the file; update usages of
DEEPSPEED/xtts_api_server/start scripts to reference BASE_DIR/VENV_PATH
variables accordingly.
| cd /home/dwemer/xtts-api-server | ||
| python3 -m venv /home/dwemer/python-tts | ||
| source /home/dwemer/python-tts/bin/activate |
There was a problem hiding this comment.
Add error handling and avoid hardcoded paths.
Multiple critical issues:
- No error handling if
cdfails (also flagged by shellcheck SC2164) - Hardcoded absolute paths (
/home/dwemer/) make the script non-portable - No validation that paths exist or are writable
If cd fails, all subsequent operations execute in the wrong directory, potentially causing data corruption or security issues.
🛡️ Recommended fixes
-cd /home/dwemer/xtts-api-server
-python3 -m venv /home/dwemer/python-tts
-source /home/dwemer/python-tts/bin/activate
+# Determine script directory and use relative paths
+SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" || { echo "Failed to determine script directory"; exit 1; }
+cd "$SCRIPT_DIR" || { echo "Failed to navigate to $SCRIPT_DIR"; exit 1; }
+
+# Allow user to configure paths or use defaults
+VENV_PATH="${VENV_PATH:-$HOME/python-tts}"
+python3 -m venv "$VENV_PATH" || { echo "Failed to create virtual environment at $VENV_PATH"; exit 1; }
+source "$VENV_PATH/bin/activate" || { echo "Failed to activate virtual environment"; exit 1; }Additionally, consider adding set -euo pipefail at the top of the script to fail fast on errors.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| cd /home/dwemer/xtts-api-server | |
| python3 -m venv /home/dwemer/python-tts | |
| source /home/dwemer/python-tts/bin/activate | |
| # Determine script directory and use relative paths | |
| SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" || { echo "Failed to determine script directory"; exit 1; } | |
| cd "$SCRIPT_DIR" || { echo "Failed to navigate to $SCRIPT_DIR"; exit 1; } | |
| # Allow user to configure paths or use defaults | |
| VENV_PATH="${VENV_PATH:-$HOME/python-tts}" | |
| python3 -m venv "$VENV_PATH" || { echo "Failed to create virtual environment at $VENV_PATH"; exit 1; } | |
| source "$VENV_PATH/bin/activate" || { echo "Failed to activate virtual environment"; exit 1; } |
🧰 Tools
🪛 Shellcheck (0.11.0)
[warning] 9-9: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
🤖 Prompt for AI Agents
In @ddistro_install.sh around lines 9 - 11, Add strict failure handling and
replace hardcoded paths with configurable variables: set "set -euo pipefail" at
the top, use variables like INSTALL_DIR and VENV_DIR instead of
"/home/dwemer/...", and check the return status of the cd command (the current
"cd /home/dwemer/xtts-api-server" invocation) and exit if it fails. Before
creating the venv (the "python3 -m venv /home/dwemer/python-tts" step) validate
that INSTALL_DIR exists and VENV_DIR parent is writable (create directories if
needed with proper permissions), and ensure "source .../bin/activate" is only
run after the venv was successfully created; fail with an explanatory message on
any error.
| PIDS=$(lsof -ti:8020) | ||
| if [ -n "$PIDS" ]; then | ||
| echo "Killing processes listening on port 8020: $PIDS" | ||
| kill -9 $PIDS | ||
| fi |
There was a problem hiding this comment.
Use graceful shutdown before SIGKILL.
Using kill -9 (SIGKILL) unconditionally prevents processes from cleaning up resources, which can cause data loss or corruption. Additionally, there's no validation that the PIDs belong to expected processes.
♻️ Recommended approach for graceful shutdown
PIDS=$(lsof -ti:8020)
if [ -n "$PIDS" ]; then
echo "Killing processes listening on port 8020: $PIDS"
- kill -9 $PIDS
+ # Try graceful shutdown first
+ kill $PIDS 2>/dev/null || true
+ sleep 2
+ # Force kill if still running
+ REMAINING=$(lsof -ti:8020)
+ if [ -n "$REMAINING" ]; then
+ echo "Force killing remaining processes: $REMAINING"
+ kill -9 $REMAINING
+ fi
fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| PIDS=$(lsof -ti:8020) | |
| if [ -n "$PIDS" ]; then | |
| echo "Killing processes listening on port 8020: $PIDS" | |
| kill -9 $PIDS | |
| fi | |
| PIDS=$(lsof -ti:8020) | |
| if [ -n "$PIDS" ]; then | |
| echo "Killing processes listening on port 8020: $PIDS" | |
| # Try graceful shutdown first | |
| kill $PIDS 2>/dev/null || true | |
| sleep 2 | |
| # Force kill if still running | |
| REMAINING=$(lsof -ti:8020) | |
| if [ -n "$REMAINING" ]; then | |
| echo "Force killing remaining processes: $REMAINING" | |
| kill -9 $REMAINING | |
| fi | |
| fi |
| echo "This will take a while so please wait." | ||
|
|
||
| read -p "Do you want to perform a clean install? (yes/no): " clean_install | ||
| if [[ "$clean_install" =~ ^[Yy][Ee][Ss]$ || "$clean_install" =~ ^[Yy]$ ]]; then | ||
| rm -fr /home/dwemer/python-tts/* | ||
| python3 -m venv /home/dwemer/python-tts | ||
| source /home/dwemer/python-tts/bin/activate | ||
| fi |
There was a problem hiding this comment.
Add safeguards for destructive operations.
The rm -fr command deletes the entire virtual environment without any backup or additional confirmation. If the hardcoded path is misconfigured or if there's a typo, this could delete unintended directories.
🛡️ Recommended safeguards
read -p "Do you want to perform a clean install? (yes/no): " clean_install
if [[ "$clean_install" =~ ^[Yy][Ee][Ss]$ || "$clean_install" =~ ^[Yy]$ ]]; then
- rm -fr /home/dwemer/python-tts/*
+ # Verify path before deletion
+ if [ -d "$VENV_PATH" ] && [[ "$VENV_PATH" == *"python-tts"* ]]; then
+ echo "WARNING: About to delete $VENV_PATH"
+ read -p "Are you absolutely sure? Type 'DELETE' to confirm: " confirm
+ if [ "$confirm" = "DELETE" ]; then
+ rm -fr "$VENV_PATH"
+ else
+ echo "Clean install cancelled"
+ exit 0
+ fi
+ fi
python3 -m venv /home/dwemer/python-tts
source /home/dwemer/python-tts/bin/activate
fiCommittable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In @ddistro_install.sh around lines 21 - 28, The destructive rm -fr
/home/dwemer/python-tts/* in the clean-install branch is unsafe; before removing
the venv contents (triggered by the clean_install variable), validate and
canonicalize the target path, ensure it exactly matches the expected venv
directory (e.g., "/home/dwemer/python-tts"), reject suspicious or root paths,
prompt the user for a secondary explicit confirmation (yes typed), and create a
backup (move or tar) of the existing venv directory before deleting; only
perform the removal after these checks and confirmations, then recreate the venv
and source it as currently done.
| # Clean previous deepspeed stuff | ||
| rm /home/dwemer/.cache/torch_extensions/py311_cu130/transformer_inference/* |
There was a problem hiding this comment.
Add error handling for cache cleanup.
The cache cleanup operation on Line 31 will fail silently if the path doesn't exist, if there are permission issues, or if the wildcard doesn't match any files.
♻️ Recommended fix
# Clean previous deepspeed stuff
-rm /home/dwemer/.cache/torch_extensions/py311_cu130/transformer_inference/*
+CACHE_DIR="$HOME/.cache/torch_extensions/py311_cu130/transformer_inference"
+if [ -d "$CACHE_DIR" ]; then
+ echo "Cleaning deepspeed cache at $CACHE_DIR"
+ rm -f "$CACHE_DIR"/* || echo "Warning: Failed to clean some cache files"
+fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Clean previous deepspeed stuff | |
| rm /home/dwemer/.cache/torch_extensions/py311_cu130/transformer_inference/* | |
| # Clean previous deepspeed stuff | |
| CACHE_DIR="$HOME/.cache/torch_extensions/py311_cu130/transformer_inference" | |
| if [ -d "$CACHE_DIR" ]; then | |
| echo "Cleaning deepspeed cache at $CACHE_DIR" | |
| rm -f "$CACHE_DIR"/* || echo "Warning: Failed to clean some cache files" | |
| fi |
🤖 Prompt for AI Agents
In @ddistro_install.sh around lines 30 - 31, The rm command that deletes the
transformer_inference cache can fail silently; update ddistro_install.sh to
first check that the transformer_inference directory exists and is readable,
then delete files with a safe rm invocation and explicitly handle errors: if the
directory is missing skip with a logged notice, otherwise run a protected
removal (use -- or -f/-r as appropriate) and if rm fails capture and log the
error and exit non‑zero so the failure is visible. Reference the existing rm
invocation that targets the transformer_inference cache directory and replace it
with the described existence check + guarded removal + error logging/exit.
| sed -i 's/checkpoint = load_fsspec(model_path, map_location=torch.device("cpu"))\["model"\]/checkpoint = load_fsspec(model_path, map_location=torch.device("cpu"), weights_only=False)["model"]/' /home/dwemer/python-tts/lib/python3.11/site-packages/TTS/tts/models/xtts.py | ||
|
|
||
| else | ||
| read -p "New: Use CUDA13. Needs windows updated drivers. RTX 50XX should use this. Usey cuda13? (yes/no): " gpu5_answer |
There was a problem hiding this comment.
Fix typo in user prompt.
"Usey cuda13?" should be "Use cuda13?"
📝 Fix
- read -p "New: Use CUDA13. Needs windows updated drivers. RTX 50XX should use this. Usey cuda13? (yes/no): " gpu5_answer
+ read -p "New: Use CUDA13. Needs updated drivers. RTX 50XX should use this. Use cuda13? (yes/no): " gpu5_answer🤖 Prompt for AI Agents
In @ddistro_install.sh at line 53, The user prompt string in the read -p call
assigns to variable gpu5_answer but contains a typo "Usey cuda13?"; update the
prompt text to read "Use cuda13? (yes/no): " so the question is spelled
correctly and still includes the context about CUDA13 and driver note while
leaving the variable name gpu5_answer unchanged.
| # Ensure fallback audio loader is available | ||
| pip install --no-cache-dir soundfile | ||
| #pip install xtts-api-server #Fails | ||
| sed -i 's/checkpoint = load_fsspec(model_path, map_location=torch.device("cpu"))\["model"\]/checkpoint = load_fsspec(model_path, map_location=torch.device("cpu"), weights_only=False)["model"]/' /home/dwemer/python-tts/lib/python3.11/site-packages/TTS/tts/models/xtts.py |
There was a problem hiding this comment.
Inconsistent patching across installation paths.
This line has the same critical sed patching issue as Line 50. However, this patch is only applied in the CUDA128 installation path and not in the CUDA13 path (lines 52-78), creating inconsistent behavior.
Either:
- The CUDA13 path is missing this patch (bug), or
- The CUDA13 path doesn't need this patch (should be documented why)
Refer to the review comment on Line 50 for recommended fixes to the patching approach.
🤖 Prompt for AI Agents
In @ddistro_install.sh at line 96, The CUDA13 install branch is missing the same
sed patch applied in the CUDA128 branch that replaces the checkpoint load call
in TTS/tts/models/xtts.py (the sed command that adds weights_only=False),
causing inconsistent behavior; modify ddistro_install.sh so the same sed
replacement (the sed invocation that alters load_fsspec(...,
weights_only=False)["model"]) is run for the CUDA13 path as well (or move the
patching to a common post-install step executed for both branches), ensuring the
xtts.py replacement is applied consistently across both CUDA13 and CUDA128
installations.
| cp /home/dwemer/TheNarrator.wav speakers/TheNarrator.wav | ||
|
|
||
| source /home/dwemer/python-tts/bin/activate | ||
|
|
||
| ./conf.sh |
There was a problem hiding this comment.
Validate file existence before operations.
Both the cp and ./conf.sh commands will fail with unclear errors if the files don't exist or aren't accessible. This creates a poor user experience during installation.
♻️ Recommended validation
+# Copy speaker sample if available
+NARRATOR_SOURCE="/home/dwemer/TheNarrator.wav" # TODO: Make configurable
+if [ -f "$NARRATOR_SOURCE" ]; then
-cp /home/dwemer/TheNarrator.wav speakers/TheNarrator.wav
+ cp "$NARRATOR_SOURCE" speakers/TheNarrator.wav
+else
+ echo "Warning: TheNarrator.wav not found at $NARRATOR_SOURCE"
+fi
source /home/dwemer/python-tts/bin/activate
-./conf.sh
+if [ -x ./conf.sh ]; then
+ ./conf.sh
+else
+ echo "ERROR: conf.sh not found or not executable"
+ exit 1
+fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| cp /home/dwemer/TheNarrator.wav speakers/TheNarrator.wav | |
| source /home/dwemer/python-tts/bin/activate | |
| ./conf.sh | |
| # Copy speaker sample if available | |
| NARRATOR_SOURCE="/home/dwemer/TheNarrator.wav" # TODO: Make configurable | |
| if [ -f "$NARRATOR_SOURCE" ]; then | |
| cp "$NARRATOR_SOURCE" speakers/TheNarrator.wav | |
| else | |
| echo "Warning: TheNarrator.wav not found at $NARRATOR_SOURCE" | |
| fi | |
| source /home/dwemer/python-tts/bin/activate | |
| if [ -x ./conf.sh ]; then | |
| ./conf.sh | |
| else | |
| echo "ERROR: conf.sh not found or not executable" | |
| exit 1 | |
| fi |
🤖 Prompt for AI Agents
In @ddistro_install.sh around lines 102 - 106, The script blindly runs the cp
and ./conf.sh steps; add pre-flight checks to ensure
/home/dwemer/TheNarrator.wav exists and the target directory (speakers/) is
present/writable before running cp (create speakers/ with mkdir -p if missing),
and verify ./conf.sh exists and is executable before invoking it (or exit with a
clear error). In practice, update the block around the cp command and the
invocation of conf.sh to perform file/directory existence checks (using test
-f/-d or similar), emit descriptive error messages, and exit non‑zero on failure
so the installer fails fast with a clear reason.
| export CUDA_HOME=/home/dwemer/python-tts/lib/python3.11/site-packages/nvidia/cu13 | ||
| fi | ||
|
|
||
| readlink start.sh | grep -q '/home/dwemer/xtts-api-server/start-deepspeed.sh' && export DEEPSPEED="--deepspeed" || export DEEPSPEED="" |
There was a problem hiding this comment.
Simplify and add error handling for deepspeed detection.
The command using readlink and grep is fragile and assumes start.sh is a symlink. The logic can fail silently if:
start.shdoesn't existstart.shis not a symlink- The path doesn't match exactly
♻️ Recommended approach
-readlink start.sh | grep -q '/home/dwemer/xtts-api-server/start-deepspeed.sh' && export DEEPSPEED="--deepspeed" || export DEEPSPEED=""
+# Check if deepspeed should be enabled
+if [ -L start.sh ]; then
+ TARGET=$(readlink start.sh)
+ if [[ "$TARGET" == *"start-deepspeed.sh"* ]]; then
+ export DEEPSPEED="--deepspeed"
+ echo "Deepspeed mode enabled"
+ else
+ export DEEPSPEED=""
+ fi
+else
+ export DEEPSPEED=""
+ echo "Note: start.sh is not a symlink, deepspeed mode not auto-detected"
+fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| readlink start.sh | grep -q '/home/dwemer/xtts-api-server/start-deepspeed.sh' && export DEEPSPEED="--deepspeed" || export DEEPSPEED="" | |
| # Check if deepspeed should be enabled | |
| if [ -L start.sh ]; then | |
| TARGET=$(readlink start.sh) | |
| if [[ "$TARGET" == *"start-deepspeed.sh"* ]]; then | |
| export DEEPSPEED="--deepspeed" | |
| echo "Deepspeed mode enabled" | |
| else | |
| export DEEPSPEED="" | |
| fi | |
| else | |
| export DEEPSPEED="" | |
| echo "Note: start.sh is not a symlink, deepspeed mode not auto-detected" | |
| fi |
|
|
||
| readlink start.sh | grep -q '/home/dwemer/xtts-api-server/start-deepspeed.sh' && export DEEPSPEED="--deepspeed" || export DEEPSPEED="" | ||
|
|
||
| python -m xtts_api_server --listen $DEEPSPEED |
There was a problem hiding this comment.
Verify installation before starting the server.
The server is started without verifying that the installation succeeded or that the xtts_api_server module is available. This can result in unclear error messages if any previous step failed.
✅ Recommended validation
+# Verify installation succeeded
+if ! python -c "import xtts_api_server" 2>/dev/null; then
+ echo "ERROR: xtts_api_server module not found. Installation may have failed."
+ echo "Check the output above for errors."
+ exit 1
+fi
+
+echo "Starting XTTS API server..."
python -m xtts_api_server --listen $DEEPSPEED📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| python -m xtts_api_server --listen $DEEPSPEED | |
| # Verify installation succeeded | |
| if ! python -c "import xtts_api_server" 2>/dev/null; then | |
| echo "ERROR: xtts_api_server module not found. Installation may have failed." | |
| echo "Check the output above for errors." | |
| exit 1 | |
| fi | |
| echo "Starting XTTS API server..." | |
| python -m xtts_api_server --listen $DEEPSPEED |
🤖 Prompt for AI Agents
In @ddistro_install.sh at line 126, Before running "python -m xtts_api_server
--listen $DEEPSPEED" verify the installation and module availability by checking
the prior install command exit code and performing an import check (e.g., run a
quick "python -c 'import xtts_api_server'" or "python -m pip show
xtts_api_server"); if the check fails, log a clear error and exit non‑zero
instead of attempting to start xtts_api_server; ensure the script references the
same "xtts_api_server" and "$DEEPSPEED" variables so the start command only runs
after successful validation.
3 participants
Summary by CodeRabbit
Release Notes
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.