chore: Add --editable-webui option for installer #8002
Conversation
The install-dev.sh script now attempts a fast-forward git pull in src/ai/backend/webui if the directory already exists, ensuring the local repository is up to date. If local changes are present, the pull is skipped with a warning.
Introduces a new --editable-webui[=true|false] option to the install-dev.sh script and CLI, allowing explicit control over installing the webui as an editable repository. The default behavior is now auto: enabled on the main branch, disabled otherwise. Updates related context, types, and dev install logic to support this option.
This reverts commit c85a9a7.
Yaminyam
changed the title
There was a problem hiding this comment.
Pull request overview
This PR adds a git pull step to the webui installation script to keep the editable webui repository up-to-date when it already exists. It also introduces a new --editable-webui CLI option to control whether the webui should be installed as an editable repository, with auto-detection based on the current git branch (enabled by default on the main branch).
Changes:
- Added
--editable-webuiCLI option with auto-detection logic (enabled on main branch by default) - Implemented git pull with fast-forward-only when webui directory already exists
- Refactored Python webui installation from commented shell script to proper Python async implementation
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/ai/backend/install/types.py | Added editable_webui field to CliArgs and InstallVariable dataclasses |
| src/ai/backend/install/dev.py | Replaced commented shell script with Python implementation, added get_current_branch() function and git pull step when webui exists |
| src/ai/backend/install/context.py | Added branch detection logic to auto-enable editable webui on main branch, imported get_current_branch function |
| src/ai/backend/install/cli.py | Added --editable-webui CLI option with bool type and default None |
| src/ai/backend/install/app.py | Pass editable_webui argument to InstallVariable initialization |
| scripts/install-dev.sh | Added git pull step to existing webui directory, enhanced --editable-webui option to accept =true/false values with auto-detection |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ) | ||
|
|
||
| # Install dependencies and build | ||
| ctx.log.write("Installing pnpm dependencies...") |
There was a problem hiding this comment.
The shell script version includes an additional step at line 694: pushd ./packages/backend.ai-ui && pnpm install && popd. This step is missing from the Python implementation, which could lead to incomplete installation of dependencies for the webui. This should be added before the pnpm i command at line 131.
| ctx.log.write("Installing pnpm dependencies...") | |
| ctx.log.write("Installing pnpm dependencies for backend.ai-ui package...") | |
| await ctx.run_shell("pnpm install", cwd=webui_path / "packages" / "backend.ai-ui") | |
| ctx.log.write("Installing pnpm dependencies for webui...") |
| async def install_editable_webui(ctx: Context) -> None: | ||
| ctx.log_header("Installing the editable version of webui") |
There was a problem hiding this comment.
The Python implementation doesn't check for node and pnpm installation before attempting to use them, while the shell script version does (lines 661-667 of install-dev.sh). This could cause the installation to fail silently or with unclear errors if these tools are not installed. Consider adding prerequisite checks for node and pnpm before proceeding with webui installation.
| ctx.log.write("src/ai/backend/webui already exists, cleaning and pulling latest...") | ||
| await ctx.run_shell("make clean", cwd=webui_path) | ||
| await ctx.run_shell( | ||
| "git pull --ff-only || echo 'Local changes exist, skipping pull...'", cwd=webui_path |
There was a problem hiding this comment.
The error handling message "Local changes exist, skipping pull..." is somewhat misleading because git pull --ff-only can fail for reasons other than local changes (e.g., network issues, authentication problems, divergent branches that require a merge). Consider using a more generic message like "Failed to pull latest changes, continuing with existing version..." to avoid confusion.
| "git pull --ff-only || echo 'Local changes exist, skipping pull...'", cwd=webui_path | |
| "git pull --ff-only || echo 'Failed to pull latest changes, continuing with existing version...'", # noqa: E501 | |
| cwd=webui_path, |
| current_branch = await get_current_branch() | ||
| editable_webui = current_branch == "main" | ||
| if not editable_webui: | ||
| self.log.write(f"Skipping editable webui (branch: {current_branch}, not main)") |
There was a problem hiding this comment.
If get_current_branch() fails (e.g., when not in a git repository), it will return an empty string, which will then be logged at line 1112 as "Skipping editable webui (branch: , not main)". Consider adding error handling to provide a clearer message when the branch cannot be determined, such as "Skipping editable webui (unable to determine branch, not main)".
| self.log.write(f"Skipping editable webui (branch: {current_branch}, not main)") | |
| if current_branch: | |
| self.log.write( | |
| f"Skipping editable webui (branch: {current_branch}, not main)" | |
| ) | |
| else: | |
| self.log.write( | |
| "Skipping editable webui (unable to determine branch, not main)" | |
| ) |
| @click.option( | ||
| "--editable-webui", | ||
| type=bool, | ||
| default=None, | ||
| help="Install webui as editable repository. [default: auto (True on main branch)]", |
There was a problem hiding this comment.
The Click option uses type=bool which doesn't allow the syntax --editable-webui=true or --editable-webui=false that the shell script supports (lines 375-385 in install-dev.sh). In Click, type=bool expects a flag without a value. To support the same syntax as the shell script, consider using is_flag=False with a callback function to parse string values "true"/"false", or use type=click.Choice(['true', 'false']) and convert to boolean, or keep it as a flag with is_flag=True and remove the help text suggesting =true|false syntax.
This pull request enhances the installer to allow explicit control over whether the web UI is installed as an editable repository, both in the shell script and Python installer. The default behavior now automatically enables the editable web UI when on the
mainbranch, unless overridden by the user. The implementation is consistent across both the shell and Python install flows, and includes improved messaging and configurability.Editable Web UI install option improvements:
--editable-webui[=true|false]inscripts/install-dev.sh, with improved help text and logic to auto-enable on themainbranch if not specified. [1] [2] [3] [4]git pullif the directory exists, skipping if there are local changes.Python installer enhancements:
--editable-webuiCLI option (default: auto) tosrc/ai/backend/install/cli.py, and propagated this setting through the installer codebase. [1] [2] [3] [4] [5] [6]mainbranch if not explicitly set, using a new async helper to detect the current branch. [1] [2] [3]Web UI install process improvements: