Conversation
Closes #394 The install script showed both fish and POSIX source commands regardless of the user's shell. On systems with fish config present (e.g. DGX Spark), this confused bash users by suggesting they source the fish env script. Now detects the current shell via $SHELL and shows only the relevant command. Also fixes the wget fallback in resolve_redirect to use portable sed instead of grep -oP (Perl regex) which is unavailable on some systems.
Functions write_env_script_fish, add_source_line, and verify_checksum used variable names that collided with their callers in setup_path and main. Since POSIX sh has no local scope, write_env_script_fish overwrote _env_script, causing the fish env path to be sourced in .bashrc/.profile instead of the POSIX env script. Use function-prefixed variable names to avoid all cross-function clobbering.
Adds e2e/test-install.sh with 12 test cases covering: - POSIX and fish env script syntax correctness - rc files (.bashrc, .profile, .zshrc) source the correct env script - fish conf.d sources the fish env script - existing rc file content is preserved (no overwrites) - no duplicate source lines on re-runs - shell-aware guidance output (POSIX vs fish) - variable clobbering regression (the #394 root cause) - .profile creation when no rc files exist Adds .github/workflows/test-install.yml that runs on changes to install.sh or the test script.
…ying rc files Remove all rc file modification logic (env scripts, source lines, fish conf.d setup). The installer now prints shell-appropriate instructions when ~/.local/bin is not on PATH, leaving dotfile changes to the user. Rewrite e2e tests as true end-to-end tests that download the latest release, verify the binary is installed correctly, and check PATH guidance output per shell.
drew
changed the title
pimlock
approved these changes
Mar 17, 2026
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
2 participants
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.
Summary
Rewrites
install.shto stop modifying user dotfiles entirely. Instead of writing env scripts and injecting source lines into.bashrc,.zshrc,.profile, etc., the installer now prints shell-appropriate instructions when~/.local/binis not on the user's PATH.This eliminates the class of bugs from #394 (variable clobbering, wrong env script sourced into wrong shell) by removing the rc file modification machinery altogether.
Related Issue
Closes #394
Changes
install.sh: Remove all rc file modification logic (setup_path,write_env_script_sh,write_env_script_fish,add_source_line, env script generation, fish conf.d setup,--no-modify-pathflag). When the install dir is not on PATH, printexport PATH="..."for POSIX shells orfish_add_pathfor fish.e2e/install/: verify the binary is installed to the correct directory, is executable, runs, and that PATH guidance output is correct per shell (sh, bash, zsh, fish).Testing
mise run pre-commitpassesTests added:
Checklist