feat: add automatic profile setup for install script#1006
Open
leoafarias wants to merge 8 commits intomainfrom
Open
feat: add automatic profile setup for install script#1006leoafarias wants to merge 8 commits intomainfrom
leoafarias wants to merge 8 commits intomainfrom
Conversation
- Add shell detection (bash/zsh/fish) for profile file discovery - Add automatic CI PATH configuration (GITHUB_PATH, BASH_ENV, Azure DevOps) - Add --auto-setup flag for interactive shell profile modification - Auto-detect if FVM is already configured in profile - In CI environments, automatically export PATH and configure CI-specific env files - Add helper to re-run with --auto-setup if manual setup desired This allows the install script to work seamlessly in CI without requiring manual PATH configuration steps after installation.
Improvements based on research of industry best practices: Shell Detection (macOS/Linux/BSD compatible): - Use portable `ps` command instead of /proc/$PPID/comm - Handle login shells with leading dash (e.g., -bash) - Defensive error handling for edge cases Profile File Priority (correct per shell docs): - Bash: .bash_profile → .bash_login → .bashrc → .profile - Zsh: .zprofile → .zshrc (recommended for PATH setup) - Fish: Respects $XDG_CONFIG_HOME CI Auto-Setup (works without manual PATH config): - GitHub Actions: Writes to $GITHUB_PATH - CircleCI: Writes to $BASH_ENV - Azure DevOps: Uses ##vso[task.prependpath] command - GitLab/Travis/Jenkins: Uses export PATH - Always exports PATH for current step Security Fixes: - Use grep -F for fixed string matching (prevents regex injection) - Skip comment lines when checking PATH config - Fix BASH_ENV writability check logic
Document comparing FVM's install.sh against NVM, Homebrew, and Bun install scripts. Key findings: - FVM shell detection (ps + $SHELL) is most robust - FVM duplicate detection (grep -qF, skip comments) is best-in-class - FVM CI auto-setup is unique - no other installer does this - Current "instructions only" approach matches Homebrew philosophy Recommends minor enhancement: PROFILE env var override (NVM-compatible)
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
- Add PROFILE env var override (NVM-compatible): set to specific file or /dev/null to skip detection - Use platform-aware bash profile priority: .bash_profile first on macOS (login shells), .bashrc first on Linux (interactive shells) - Use platform-aware zsh profile priority: .zprofile on macOS, .zshrc on Linux (matches Homebrew behavior) - Expand unknown shell fallback to try more profile files - Document PROFILE environment variable in usage help Based on verified analysis of NVM, Homebrew, and Bun install scripts.
- Use [[:space:]] instead of \s in grep for POSIX/BSD compatibility (macOS grep treats \s as literal backslash+s in BRE mode) - Use detected profile file in "source" instruction instead of hardcoded ~/.bashrc - Make migration non-destructive: backup ~/.fvm_flutter to ~/.fvm_flutter.bak.TIMESTAMP instead of deleting - Only remove /usr/local/bin/fvm symlink if it points to a known FVM location (prevents removing unrelated symlinks)
Add reusable helper functions to eliminate code duplication: - find_first_file(): profile file detection with priority list - try_remove_file(): file removal with sudo fallback - try_remove_directory(): directory removal with optional backup - print_musl_warning(): Alpine Linux musl libc warning Refactored functions: - get_profile_file(): uses find_first_file (reduced ~30 lines) - migrate_from_v1(): uses try_remove_* helpers (reduced ~25 lines) - do_uninstall(): uses try_remove_* helpers (reduced ~25 lines) - musl detection: uses print_musl_warning (reduced ~8 lines) Net reduction: 72 lines (737 → 665 effective)
- Add INT/TERM/HUP to trap for robust cleanup on interrupts - Inline print_musl_warning (was only called twice) - Simplify is_ci to check only supported platforms (GitHub Actions, Azure DevOps, CircleCI) plus generic CI=true - Remove redundant system bin check from validate_install_base (already covered by HOME/* path requirement) Net reduction: 20 lines (702 -> 691)
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.
This allows the install script to work seamlessly in CI without requiring
manual PATH configuration steps after installation.