fix: install.sh venv detection, Windows path support, and install dir#7
Merged
Merged
Conversation
- Default INSTALL_DIR to current working directory (pwd) instead of $HOME - Skip venv creation if already inside an active venv ($VIRTUAL_ENV is set) - Search for existing venv activate script before creating a new one - Detect Windows (Git Bash/MSYS2) activate path (Scripts/ vs bin/) - Fix hardcoded bin/activate in generated start_lmstudio_mcp.sh - Add error trap for clearer failure messages Fixes: install breaks out to $HOME when run from Pinokio or any pre-configured venv environment on Windows.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Reported by a user running the install script inside Pinokio on Windows with a pre-existing venv.
Three bugs were present:
Install directory defaulted to
$HOME—INSTALL_DIR="$HOME/lmstudio-mcp"caused the script to break out of the user's working directory and install in their Windows home folder instead of where Pinokio (or any other launcher) invoked the script from.No detection of an already-active venv — The script unconditionally created a new venv even if
$VIRTUAL_ENVwas already set (i.e. the user was already inside one). This broke Pinokio environments and any other tool that pre-activates a venv before running the script.Hardcoded
bin/activatepath breaks on Windows — On Windows (Git Bash / MSYS2), the activate script lives atScripts/activate, notbin/activate. The generatedstart_lmstudio_mcp.shhad the same hardcoded path.Fix
INSTALL_DIRnow defaults to$(pwd)— install happens where the script is run from.already_in_venv()checks$VIRTUAL_ENVand$CONDA_DEFAULT_ENVbefore creating anything.find_existing_venv()searches common venv directory names (venv,.venv,env, etc.) before creating a new one.get_activate_path()triesbin/activate,Scripts/activate, andscripts/activatein order — works on Linux, macOS, and Windows.start_lmstudio_mcp.shanddev_setup.shnow use the detected path rather than the hardcodedvenv/bin/activate.traponERRso failures print the line number instead of silently dying.Testing
Verified the following scenarios are handled correctly:
venv/bin/activate)venv/Scripts/activate)$HOME