fix(installer): detect Linux distribution and persist shell environme…#10
Conversation
…nt profiles
Issue 1: Linux Distribution Identification Failure
- Root Cause: Installer scripts only checked `uname -s` ("Linux") and `uname -m`,
lacking inspection of `/etc/os-release`, `/etc/debian_version`, or `lsb_release`.
When a C compiler was preinstalled, no OS distribution was reported.
- Fix: Introduced `detect_distro()` to safely parse `/etc/os-release` (PRETTY_NAME, ID),
`/etc/debian_version`, `lsb_release`, `/etc/redhat-release`, and `/etc/alpine-release`.
The detected distribution (e.g., Debian GNU/Linux 13 trixie) is logged in header
summaries and package manager messages.
Issue 2: Environment Settings Disappearing Across Sessions and Reboots
- Root Cause: Previously, `append_shell_rc()` skipped modifying profile files on
interactive runs without `--yes`. Additionally, system-wide / root installs lacked
`/etc/profile.d/mako.sh` integration, causing environment variables to vanish in
new shells or after system reboot.
- Fix: Updated `append_shell_rc()` to automatically write `/etc/profile.d/mako.sh` for
system-wide / root installs (standard on Debian and standard Linux distros to persist
across reboots & all login/subshell sessions) and append to user profile files
(`~/.bashrc`, `~/.profile`, `~/.zshrc`, `~/.bash_profile`).
Files modified:
- scripts/install-linux.sh
- scripts/install-release.sh
- dist/install-linux.sh
- dist/install-release.sh
📝 WalkthroughWalkthroughBoth one-shot installers now detect host distributions, report package-manager selection, persist Mako environment setup across shell profiles and reboots, and provide expanded installation status and activation guidance. ChangesInstaller environment setup
Estimated code review effort: 3 (Moderate) | ~25 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
scripts/install-linux.sh (1)
168-188: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
idandversionare parsed but never used.shellcheck flags both as unused (SC2034).
nameis the only field consumed by the finalif;id/versionare dead assignments in every branch (os-release, lsb_release, redhat-release, alpine-release). If the intent was to later drive package-manager selection fromid(which would also avoid the separatecommand -v apt-get/dnf/...probing done inensure_clang), consider returning/using it; otherwise drop the dead assignments.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/install-linux.sh` around lines 168 - 188, Remove the unused id and version variables and all assignments to them from the OS detection logic, keeping name and its existing fallback behavior unchanged. Update the local declaration and the /etc/os-release, lsb_release, redhat-release, and alpine-release branches within the OS detection function.Source: Linters/SAST tools
scripts/install-release.sh (1)
168-188: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
idandversionare parsed but never used.Same unused-variable pattern as
scripts/install-linux.sh(SC2034) —nameis the only field consumed;id/versionare dead in every branch.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/install-release.sh` around lines 168 - 188, In the OS detection logic, remove the unused id and version variables and their assignments, retaining only name parsing and the existing fallback behavior. Update the local declaration and all affected branches within the surrounding OS detection function.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/install-linux.sh`:
- Around line 330-343: Restrict the system-wide profile hook in the system
profile branch to shared/system installation prefixes, matching the existing
PREFIX-versus-HOME distinction used by the .profile branch. Update the condition
surrounding sys_profile_dir and sys_profile_file so a prefix under the current
user’s HOME, such as the default /root/.local, cannot create
/etc/profile.d/mako.sh; retain the existing hook installation behavior for
genuinely shared prefixes.
In `@scripts/install-release.sh`:
- Around line 330-343: The system-wide profile update in the
`/etc/profile.d/mako.sh` block must only run when the configured `$PREFIX` is
shared and readable by other users. Add the same PREFIX accessibility/ownership
guard used by `scripts/install-linux.sh` before writing `sys_profile_file`, so
sudo-driven installs with a private default `$HOME/.local` update only the user
profile instead.
---
Nitpick comments:
In `@scripts/install-linux.sh`:
- Around line 168-188: Remove the unused id and version variables and all
assignments to them from the OS detection logic, keeping name and its existing
fallback behavior unchanged. Update the local declaration and the
/etc/os-release, lsb_release, redhat-release, and alpine-release branches within
the OS detection function.
In `@scripts/install-release.sh`:
- Around line 168-188: In the OS detection logic, remove the unused id and
version variables and their assignments, retaining only name parsing and the
existing fallback behavior. Update the local declaration and all affected
branches within the surrounding OS detection function.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 429d3781-1247-488d-a616-bf031b0eee14
⛔ Files ignored due to path filters (2)
dist/install-linux.shis excluded by!**/dist/**dist/install-release.shis excluded by!**/dist/**
📒 Files selected for processing (2)
scripts/install-linux.shscripts/install-release.sh
| if [[ "${EUID:-$(id -u)}" -eq 0 ]] || [[ -w /etc/profile.d ]]; then | ||
| local sys_profile_dir="/etc/profile.d" | ||
| local sys_profile_file="$sys_profile_dir/mako.sh" | ||
| if mkdir -p "$sys_profile_dir" 2>/dev/null && { | ||
| cat > "$sys_profile_file" <<EOF | ||
| # Mako environment configuration | ||
| $src_line | ||
| EOF | ||
| chmod 644 "$sys_profile_file" 2>/dev/null | ||
| }; then | ||
| echo " updated system profile: $sys_profile_file (persists across reboots & all users)" | ||
| updated_any=1 | ||
| fi | ||
| fi |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
System-wide profile hook can point at an inaccessible path for other users.
Writing /etc/profile.d/mako.sh is gated solely on EUID==0/writability, with no check that the install location is actually shared/readable. Default PREFIX is $HOME/.local; a typical curl | sudo bash run resets HOME to /root, so this writes a system-wide hook sourcing /root/.local/share/mako/env.sh. Other users' shells will silently fail the [ -f ... ] test (no traversal permission into /root), so Mako never actually becomes available for them despite the profile hook existing — contradicting the "automatically available ... after reboot" messaging later in the script. Note the .profile branch below already checks "$PREFIX" == "$HOME"* for exactly this kind of distinction; the system-profile branch doesn't apply the same care.
🛠️ Proposed fix: only install the system-wide hook for shared/system prefixes
- if [[ "${EUID:-$(id -u)}" -eq 0 ]] || [[ -w /etc/profile.d ]]; then
+ if { [[ "${EUID:-$(id -u)}" -eq 0 ]] || [[ -w /etc/profile.d ]]; } && [[ "$PREFIX" != "$HOME"* ]]; then📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if [[ "${EUID:-$(id -u)}" -eq 0 ]] || [[ -w /etc/profile.d ]]; then | |
| local sys_profile_dir="/etc/profile.d" | |
| local sys_profile_file="$sys_profile_dir/mako.sh" | |
| if mkdir -p "$sys_profile_dir" 2>/dev/null && { | |
| cat > "$sys_profile_file" <<EOF | |
| # Mako environment configuration | |
| $src_line | |
| EOF | |
| chmod 644 "$sys_profile_file" 2>/dev/null | |
| }; then | |
| echo " updated system profile: $sys_profile_file (persists across reboots & all users)" | |
| updated_any=1 | |
| fi | |
| fi | |
| if { [[ "${EUID:-$(id -u)}" -eq 0 ]] || [[ -w /etc/profile.d ]]; } && [[ "$PREFIX" != "$HOME"* ]]; then | |
| local sys_profile_dir="/etc/profile.d" | |
| local sys_profile_file="$sys_profile_dir/mako.sh" | |
| if mkdir -p "$sys_profile_dir" 2>/dev/null && { | |
| cat > "$sys_profile_file" <<EOF | |
| # Mako environment configuration | |
| $src_line | |
| EOF | |
| chmod 644 "$sys_profile_file" 2>/dev/null | |
| }; then | |
| echo " updated system profile: $sys_profile_file (persists across reboots & all users)" | |
| updated_any=1 | |
| fi | |
| fi |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/install-linux.sh` around lines 330 - 343, Restrict the system-wide
profile hook in the system profile branch to shared/system installation
prefixes, matching the existing PREFIX-versus-HOME distinction used by the
.profile branch. Update the condition surrounding sys_profile_dir and
sys_profile_file so a prefix under the current user’s HOME, such as the default
/root/.local, cannot create /etc/profile.d/mako.sh; retain the existing hook
installation behavior for genuinely shared prefixes.
| if [[ "${EUID:-$(id -u)}" -eq 0 ]] || [[ -w /etc/profile.d ]]; then | ||
| local sys_profile_dir="/etc/profile.d" | ||
| local sys_profile_file="$sys_profile_dir/mako.sh" | ||
| if mkdir -p "$sys_profile_dir" 2>/dev/null && { | ||
| cat > "$sys_profile_file" <<EOF | ||
| # Mako environment configuration | ||
| $src_line | ||
| EOF | ||
| chmod 644 "$sys_profile_file" 2>/dev/null | ||
| }; then | ||
| echo " updated system profile: $sys_profile_file (persists across reboots & all users)" | ||
| updated_any=1 | ||
| fi | ||
| fi |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
System-wide profile hook can point at an inaccessible path for other users.
Same issue as scripts/install-linux.sh: the /etc/profile.d/mako.sh write is gated only on privilege, not on whether $PREFIX (default $HOME/.local) is actually shared/readable by other users, which breaks silently for sudo-driven installs where HOME=/root.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/install-release.sh` around lines 330 - 343, The system-wide profile
update in the `/etc/profile.d/mako.sh` block must only run when the configured
`$PREFIX` is shared and readable by other users. Add the same PREFIX
accessibility/ownership guard used by `scripts/install-linux.sh` before writing
`sys_profile_file`, so sudo-driven installs with a private default
`$HOME/.local` update only the user profile instead.
2 participants
…nt profiles
Issue 1: Linux Distribution Identification Failure
uname -s("Linux") anduname -m, lacking inspection of/etc/os-release,/etc/debian_version, orlsb_release. When a C compiler was preinstalled, no OS distribution was reported.detect_distro()to safely parse/etc/os-release(PRETTY_NAME, ID),/etc/debian_version,lsb_release,/etc/redhat-release, and/etc/alpine-release. The detected distribution (e.g., Debian GNU/Linux 13 trixie) is logged in header summaries and package manager messages.Issue 2: Environment Settings Disappearing Across Sessions and Reboots
append_shell_rc()skipped modifying profile files on interactive runs without--yes. Additionally, system-wide / root installs lacked/etc/profile.d/mako.shintegration, causing environment variables to vanish in new shells or after system reboot.append_shell_rc()to automatically write/etc/profile.d/mako.shfor system-wide / root installs (standard on Debian and standard Linux distros to persist across reboots & all login/subshell sessions) and append to user profile files (~/.bashrc,~/.profile,~/.zshrc,~/.bash_profile).Files modified:
Summary by CodeRabbit
New Features
Bug Fixes