Skip to content

Add setup.sh to facilitate bootstrap on portable#12801

Open
idlesign wants to merge 1 commit intoComfy-Org:masterfrom
idlesign:feat/setupsh
Open

Add setup.sh to facilitate bootstrap on portable#12801
idlesign wants to merge 1 commit intoComfy-Org:masterfrom
idlesign:feat/setupsh

Conversation

@idlesign
Copy link

@idlesign idlesign commented Mar 6, 2026

Hi,

I'd like to propose a helper script that saves time bootstrapping portable version on Linux.

All setup can be as easy as

./setup.sh --manager --torch=nvidia --torch-nightly

It's also easy to maintain and update default torch versions.
Yet the script requires uv to be installed.

I've just added a small hint in the README, but it seems that other instructions in that could be also simplified further.

Looking forward for any feedback.

@coderabbitai
Copy link

coderabbitai bot commented Mar 6, 2026

📝 Walkthrough

Walkthrough

This pull request introduces automated environment setup tooling. A new setup.sh script handles virtual environment creation, dependency installation, and flexible PyTorch installation with support for multiple vendors (AMD, Intel, NVIDIA) and nightly builds. Accompanying README.md documentation provides usage examples and notes the uv dependency requirement. The changes are purely additive with no modifications to existing code.

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title directly describes the main change: adding a setup.sh script to facilitate bootstrap on portable (Linux) systems, which aligns with the PR's primary objective.
Description check ✅ Passed The description is clearly related to the changeset, explaining the purpose of the setup.sh script, its usage example, and the added README documentation with appropriate context.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

Tip

Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs).
Share your feedback on Discord.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (2)
README.md (1)

223-232: Good documentation addition.

Clear and helpful section that introduces the setup helper script with a practical example.

Minor note: Line 228 uses ; as a comment prefix, which is unconventional in shell (typically # is used). While ; at line start is technically valid bash (empty command), using # would be more idiomatic.

📝 Minor style suggestion
 ```shell
-; install ComfyUI & Manager dependencies + nightly PyTorch build for Nvidia
+# install ComfyUI & Manager dependencies + nightly PyTorch build for Nvidia
 ./setup.sh --manager --torch=nvidia --torch-nightly
</details>

<details>
<summary>🤖 Prompt for AI Agents</summary>

Verify each finding against the current code and only fix it if needed.

In @README.md around lines 223 - 232, Update the shell example comment in the
README so it uses the conventional shell comment prefix: replace the leading ';'
on the line before the setup.sh invocation with '#' in the code block that
demonstrates running ./setup.sh --manager --torch=nvidia --torch-nightly (i.e.,
change the commented description line in the shell snippet to "# install ComfyUI
& Manager dependencies + nightly PyTorch build for Nvidia").


</details>

</blockquote></details>
<details>
<summary>setup.sh (1)</summary><blockquote>

`37-45`: **Consider warning on unknown arguments.**

Unknown arguments are silently ignored (line 43). This could mask user typos such as `--manger` instead of `--manager`, leading to unexpected behavior without feedback.

Also, the `shift` statements inside the case branches have no effect since the `for` loop iterates over a snapshot of `"$@"` and doesn't rely on positional parameter shifting.

<details>
<summary>🔧 Suggested improvement</summary>

```diff
 for i in "$@"; do
   case $i in
-    --manager) MANAGER=true; shift ;;
-    --torch=*) TORCH="${i#*=}"; shift ;;
-    --torch-nightly) NIGHTLY=true; shift ;;
-    *) ;; # skip unknown
+    --manager) MANAGER=true ;;
+    --torch=*) TORCH="${i#*=}" ;;
+    --torch-nightly) NIGHTLY=true ;;
+    *) echo "Warning: unknown option '$i' ignored" ;;
   esac
 done
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@setup.sh` around lines 37 - 45, The argument parsing silently ignores unknown
options and contains ineffective shift calls; update the parser in setup.sh (the
for loop + case handling of --manager, --torch=*, --torch-nightly and the
default *) to both emit a clear warning to stderr when an unknown arg is
encountered and fix shifting behavior: either remove the pointless shift calls
inside the for-loop or replace the for ... in "$@" loop with a while [ $# -gt 0
]; do case "$1" in ... ) ...; shift ;; esac; done so shifting works correctly
while still setting MANAGER, TORCH and NIGHTLY as before; ensure unknown-arg
handling prints a warning (e.g., to >&2) rather than silently skipping.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@setup.sh`:
- Around line 47-58: The install sequence in setup.sh causes torch packages in
requirements.txt to be downloaded twice when the --torch path is used; modify
the script so that when the --torch flag is set you either (a) remove/filter
torch, torchvision, torchaudio entries from the pip install of
requirements.txt/manager_requirements.txt (the uv pip install -r
requirements.txt and uv pip install -r manager_requirements.txt calls) before
that step, or (b) install the vendor-specific torch packages first and then
install requirements.txt with a filtered/--no-deps or a temporary requirements
file to exclude torch entries; update the logic around the uv pip install calls
and the --torch handling to ensure torch is only downloaded/installed once and
keep MANAGER branch behavior consistent.

---

Nitpick comments:
In `@README.md`:
- Around line 223-232: Update the shell example comment in the README so it uses
the conventional shell comment prefix: replace the leading ';' on the line
before the setup.sh invocation with '#' in the code block that demonstrates
running ./setup.sh --manager --torch=nvidia --torch-nightly (i.e., change the
commented description line in the shell snippet to "# install ComfyUI & Manager
dependencies + nightly PyTorch build for Nvidia").

In `@setup.sh`:
- Around line 37-45: The argument parsing silently ignores unknown options and
contains ineffective shift calls; update the parser in setup.sh (the for loop +
case handling of --manager, --torch=*, --torch-nightly and the default *) to
both emit a clear warning to stderr when an unknown arg is encountered and fix
shifting behavior: either remove the pointless shift calls inside the for-loop
or replace the for ... in "$@" loop with a while [ $# -gt 0 ]; do case "$1" in
... ) ...; shift ;; esac; done so shifting works correctly while still setting
MANAGER, TORCH and NIGHTLY as before; ensure unknown-arg handling prints a
warning (e.g., to >&2) rather than silently skipping.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 88de7708-e3bf-4afd-b980-8e4376d401aa

📥 Commits

Reviewing files that changed from the base of the PR and between 3b93d5d and 0893b7c.

📒 Files selected for processing (2)
  • README.md
  • setup.sh

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant