Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR refactors runtime selection across all setup scripts by replacing the boolean --all-runtimes flag with a flexible --runtimes option that accepts comma-separated runtime lists (e.g., conda,nvm,rust,go). Additionally, it adds ulimits configuration to the Ubuntu pre-setup script for improved resource handling.
Key Changes:
- Introduced
--runtimesparameter accepting comma-separated values with special keywords (all,basic,none) across all setup scripts - Added ulimits configuration in Ubuntu pre-setup for file descriptor and system file limits
- Updated CLI argument parsing and help messages to reflect the new runtime selection mechanism
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| src/setup/unix/setup-user-env.sh | Implements runtime expansion logic and per-runtime installation loop with error handling |
| src/setup/unix/macos/setup-macos.sh | Updates argument parsing and passes runtime selection to user environment setup |
| src/setup/unix/linux/ubuntu/setup-ubuntu.sh | Replaces --all-runtimes with --runtimes, reassigns -r flag to runtimes (restart becomes -R) |
| src/setup/unix/linux/ubuntu/pre-setup-ubuntu.sh | Adds ulimits configuration for nofile and fs.file-max settings |
| src/setup/unix/linux/setup-user.sh | Updates to accept and pass through runtime selection to user environment setup |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| _fetch "${SCRIPT_BASE_URL}/setup/unix/runtimes/install-user-miniconda.sh" | bash || { | ||
| echo "[WARN]: Failed to install 'Miniconda', skipping!" >&2 | ||
| } | ||
| break;; |
There was a problem hiding this comment.
The break statements will exit the loop after the first runtime is processed, preventing installation of subsequent runtimes in the list. Replace break with continue or remove these statements entirely to allow all runtimes to be installed.
| _fetch "${SCRIPT_BASE_URL}/setup/unix/runtimes/install-user-nvm.sh" | bash || { | ||
| echo "[WARN]: Failed to install 'NVM', skipping!" >&2 | ||
| } | ||
| break;; |
There was a problem hiding this comment.
The break statements will exit the loop after the first runtime is processed, preventing installation of subsequent runtimes in the list. Replace break with continue or remove these statements entirely to allow all runtimes to be installed.
| _fetch "${SCRIPT_BASE_URL}/setup/unix/runtimes/install-user-rust.sh" | bash || { | ||
| echo "[WARN]: Failed to install 'Rust', skipping!" >&2 | ||
| } | ||
| break;; |
There was a problem hiding this comment.
The break statements will exit the loop after the first runtime is processed, preventing installation of subsequent runtimes in the list. Replace break with continue or remove these statements entirely to allow all runtimes to be installed.
| _fetch "${SCRIPT_BASE_URL}/setup/unix/runtimes/install-user-go.sh" | bash || { | ||
| echo "[WARN]: Failed to install 'Go', skipping!" >&2 | ||
| } | ||
| break;; |
There was a problem hiding this comment.
The break statements will exit the loop after the first runtime is processed, preventing installation of subsequent runtimes in the list. Replace break with continue or remove these statements entirely to allow all runtimes to be installed.
| OPTIONS: | ||
| -a, --all, --all-runtimes Install all runtimes (Rust, Go, Miniconda, NVM). Default: false | ||
| -h, --help Show help. | ||
| -r, --runtimes Comma-separated list of runtimes to install ('conda', 'nvm', 'rust', 'go'). Default: 'conda,nvm'. |
There was a problem hiding this comment.
The help text is missing the parameter placeholder [RUNTIME1,RUNTIME2,...] that appears in other files. Update to: -r, --runtimes [RUNTIME1,RUNTIME2,...] Comma-separated list of runtimes to install ('conda', 'nvm', 'rust', 'go'). Default: 'conda,nvm'. for consistency.
| -r, --runtimes Comma-separated list of runtimes to install ('conda', 'nvm', 'rust', 'go'). Default: 'conda,nvm'. | |
| -r, --runtimes [RUNTIME1,RUNTIME2,...] Comma-separated list of runtimes to install ('conda', 'nvm', 'rust', 'go'). Default: 'conda,nvm'. |
| -U, --disable-user-setup Disable user setup process. Default: false | ||
| -a, --all, --all-runtimes Install all runtimes (Rust, Go, Miniconda, NVM). Default: false | ||
| -r, --disable-restart Disable automatic system restart after setup. Default: false | ||
| -r, --runtimes Comma-separated list of runtimes to install ('conda', 'nvm', 'rust', 'go'). Default: 'conda,nvm'. |
There was a problem hiding this comment.
The help text is missing the parameter placeholder [RUNTIME1,RUNTIME2,...] that appears in setup-user-env.sh. Update to: -r, --runtimes [RUNTIME1,RUNTIME2,...] Comma-separated list of runtimes to install ('conda', 'nvm', 'rust', 'go'). Default: 'conda,nvm'. for consistency across all files.
| -r, --runtimes Comma-separated list of runtimes to install ('conda', 'nvm', 'rust', 'go'). Default: 'conda,nvm'. | |
| -r, --runtimes [RUNTIME1,RUNTIME2,...] Comma-separated list of runtimes to install ('conda', 'nvm', 'rust', 'go'). Default: 'conda,nvm'. |
| -H, --hashed, --hashed-password Indicate that the provided password is already hashed. Default: false | ||
| -s, --sudo, --with-sudo Grant sudo privileges to the new user. Default: false | ||
| -a, --all, --all-runtimes Install all runtimes (Rust, Go, Miniconda, NVM). Default: false | ||
| -r, --runtimes Comma-separated list of runtimes to install ('conda', 'nvm', 'rust', 'go'). Default: 'conda,nvm'. |
There was a problem hiding this comment.
The help text is missing the parameter placeholder [RUNTIME1,RUNTIME2,...] that appears in setup-user-env.sh. Update to: -r, --runtimes [RUNTIME1,RUNTIME2,...] Comma-separated list of runtimes to install ('conda', 'nvm', 'rust', 'go'). Default: 'conda,nvm'. for consistency across all files.
| -r, --runtimes Comma-separated list of runtimes to install ('conda', 'nvm', 'rust', 'go'). Default: 'conda,nvm'. | |
| -r, --runtimes [RUNTIME1,RUNTIME2,...] Comma-separated list of runtimes to install ('conda', 'nvm', 'rust', 'go'). Default: 'conda,nvm'. |
This pull request refactors the way runtime environments are selected and installed across all setup scripts (
linux,ubuntu,macos, and user environment scripts). Instead of the previous--all-runtimesboolean flag, the scripts now use a flexible--runtimesoption that accepts a comma-separated list of runtimes to install (e.g.,conda,nvm,rust,go). This enhances customization and clarity for users and improves maintainability. Additionally, the Ubuntu pre-setup script now configures system ulimits for better resource handling.Runtime selection and installation improvements:
ALL_RUNTIMESboolean flag with aRUNTIMESvariable and associated--runtimesoption in all setup scripts (setup-user.sh,setup-user-env.sh,setup-ubuntu.sh,setup-macos.sh). Users can now specify which runtimes to install via a comma-separated list, with support for special values likeall,basic, andnone. [1] [2] [3] [4]--runtimesoption, improving discoverability and user experience. [1] [2] [3] [4]-a/--allflag. This ensures consistent runtime selection throughout the setup process. [1] [2] [3]User environment setup logic:
setup-user-env.sh, added logic to expand special runtime values (all,basic,none) and loop through the selected runtimes to install each one individually. Unsupported values now result in an error.System configuration improvements (Ubuntu only):
nofileandfs.file-max) and ensures PAM configuration for resource limits, improving system stability for high-load scenarios.