Skip to content

Fix install.sh for macOS Bash 3.2 compatibility#12

Open
vladimireduardo wants to merge 1 commit intoPhilipStark:masterfrom
vladimireduardo:fix/install-macos-bash-compat
Open

Fix install.sh for macOS Bash 3.2 compatibility#12
vladimireduardo wants to merge 1 commit intoPhilipStark:masterfrom
vladimireduardo:fix/install-macos-bash-compat

Conversation

@vladimireduardo
Copy link
Copy Markdown

@vladimireduardo vladimireduardo commented Mar 25, 2026

Summary

  • Remove pipefail from set -euo pipefail — not supported in Bash 3.2 (macOS default). No pipes in the script, so it was unnecessary.
  • Convert CRLF line endings to LF — Windows-style \r\n causes $'\r': command not found errors on macOS/Linux.

Environment tested

  • macOS 15 (Darwin 24.6.0)
  • Apple M1
  • Bash 3.2.57

Test plan

  • Ran bash install.sh on macOS M1 — installs all 21 skills + 1 agent + 5 knowledge files successfully

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Chores
    • Optimized installation script error handling to improve reliability during setup.

- Remove `pipefail` from `set -euo pipefail` — not supported in Bash 3.2
  which ships with macOS (including Apple Silicon/M1). No pipes are used
  in the script, so `pipefail` was unnecessary.
- Convert CRLF line endings to LF — Windows-style line endings cause
  `$'\r': command not found` errors on macOS/Linux.

Tested on macOS 15 (Darwin 24.6.0) with Bash 3.2.57 on Apple M1.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 25, 2026

📝 Walkthrough

Walkthrough

The install.sh script's shell safety settings were modified from set -euo pipefail to set -eu, removing pipefail handling while retaining error exit and unset variable error detection. All functional operations remain unchanged.

Changes

Cohort / File(s) Summary
Shell Safety Configuration
install.sh
Modified shell safety flags from set -euo pipefail to set -eu, removing pipefail handling while preserving error exit and unset variable error behaviors. Functional directory checks, file copying, and output operations remain intact.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 A rabbit scripts with careful care,
Removing pipes from the air,
set -eu now takes the lead,
Errors caught at lightful speed,
Simpler paths, the same good deed! 🌟

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: fixing shell compatibility for macOS Bash 3.2 by removing the unsupported pipefail option.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
install.sh (1)

2-2: Restore pipefail using a fallback pattern for Bash 3.2 compatibility.

The current set -eu removes pipefail, which weakens error detection in pipeline operations. Since pipefail was introduced in Bash 3.0 and is available in Bash 3.2, use a conditional pattern to enable it safely across shells.

Suggested pattern
 set -eu
+set -o pipefail 2>/dev/null || true
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@install.sh` at line 2, Replace the bare "set -eu" with a portable pattern
that restores pipefail when supported: use "set -euo pipefail 2>/dev/null || set
-eu" (or add that right after the existing "set -eu") so shells with pipefail
enable it and older Bash versions fall back to the original flags; update the
line containing "set -eu" accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@install.sh`:
- Line 5: The script header and summary lines currently contain a hardcoded "20
skills" string that can drift; change those static strings (the comment at the
top and the summary around lines 29-30) to use the same dynamic skill count
variable/command used later (the SKILL_COUNT or the find/grep count computed
near the code that prints the dynamic count at ~line 82) so the displayed number
is computed at runtime; update the two static messages to interpolate or format
with that computed count instead of the literal "20 skills".

---

Nitpick comments:
In `@install.sh`:
- Line 2: Replace the bare "set -eu" with a portable pattern that restores
pipefail when supported: use "set -euo pipefail 2>/dev/null || set -eu" (or add
that right after the existing "set -eu") so shells with pipefail enable it and
older Bash versions fall back to the original flags; update the line containing
"set -eu" accordingly.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: a8707941-c262-4746-970f-b77b04bcc714

📥 Commits

Reviewing files that changed from the base of the PR and between a236afc and 3790f38.

📒 Files selected for processing (1)
  • install.sh

set -eu

# Book Genesis V4 — Installer for macOS/Linux
# Installs 20 skills + 1 agent + knowledge base to ~/.claude/
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Hardcoded “20 skills” text can drift from actual installs.

Line 5 and Line 29 are static, but Line 82 reports a dynamic count. This can mislead users when repo content changes (e.g., 21 skills).

Proposed patch
-# Installs 20 skills + 1 agent + knowledge base to ~/.claude/
+# Installs skills + 1 agent + knowledge base to ~/.claude/
@@
-echo -e "${YELLOW}Installing 20 skills + 1 agent + knowledge base${NC}"
+echo -e "${YELLOW}Installing skills + 1 agent + knowledge base${NC}"

Also applies to: 29-30

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@install.sh` at line 5, The script header and summary lines currently contain
a hardcoded "20 skills" string that can drift; change those static strings (the
comment at the top and the summary around lines 29-30) to use the same dynamic
skill count variable/command used later (the SKILL_COUNT or the find/grep count
computed near the code that prints the dynamic count at ~line 82) so the
displayed number is computed at runtime; update the two static messages to
interpolate or format with that computed count instead of the literal "20
skills".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants