diff --git a/docs/linux-install-fixes.md b/docs/linux-install-fixes.md new file mode 100644 index 00000000..cdbea862 --- /dev/null +++ b/docs/linux-install-fixes.md @@ -0,0 +1,178 @@ +# Linux Installation Fixes + +Investigation and fixes for two bugs affecting the installer on Linux and WSL. +Reported in issue #137. Fixed in PRs #148 and #149. + +--- + +## How to Build and Run Locally + +### Prerequisites + +Install Go (required to compile from source): + +```bash +curl -fsSL https://go.dev/dl/go1.23.4.linux-amd64.tar.gz | sudo tar -C /usr/local -xzf - +echo 'export PATH="/usr/local/go/bin:$PATH"' >> ~/.bashrc && source ~/.bashrc +``` + +### Compile + +```bash +cd installer +go build -o ~/gentleman.dots ./cmd/gentleman-installer +``` + +### Run + +```bash +~/gentleman.dots +``` + +### Test with Docker (no Go required) + +```bash +docker build -f Dockerfile.test -t gentleman-test --no-cache . +docker run -it --rm gentleman-test +``` + +--- + +## Bug 1 — Homebrew fails with "Bash is required to interpret this script." + +### Symptom + +On Debian/Ubuntu/WSL, the Homebrew installation step fails immediately: + +``` +🍺 Installing Homebrew package manager... + (You may be prompted for your password) + +Bash is required to interpret this script. +``` + +### Root Cause + +`getHomebrewScript()` in `installer/internal/tui/interactive.go` generated a script +with `#!/bin/sh` that called the Homebrew installer via: + +```sh +/bin/sh -c "$(curl -fsSL https://raw.githubusercontent.com/Homebrew/install/HEAD/install.sh)" +``` + +On Debian/Ubuntu, `/bin/sh` is symlinked to **dash**, not bash. Homebrew's `install.sh` +checks `$BASH_VERSION` at the very top and aborts if it is empty — which it always is +under dash. + +### Why `stepInstallHomebrew` in `installer.go` was not the issue + +The Homebrew step is registered with `Interactive: true` in `model.go`: + +```go +m.Steps = append(m.Steps, InstallStep{ + ID: "homebrew", + Interactive: true, + ... +}) +``` + +This means `runNextStep()` always routes it through `runInteractiveStep()` → +`getHomebrewScript()` → `tea.ExecProcess`. The `stepInstallHomebrew` function in +`installer.go` is never called for this path. + +### Fix — `installer/internal/tui/interactive.go` + +```diff +- script := fmt.Sprintf(`#!/bin/sh ++ script := fmt.Sprintf(`#!/bin/bash + set -e + ... +- /bin/sh -c "$(curl -fsSL https://raw.githubusercontent.com/Homebrew/install/HEAD/install.sh)" ++ NONINTERACTIVE=1 /bin/bash -c "$(curl -fsSL https://raw.githubusercontent.com/Homebrew/install/HEAD/install.sh)" +``` + +Two changes: +1. Shebang changed to `#!/bin/bash`. +2. Installer call uses explicit `/bin/bash` and `NONINTERACTIVE=1` to skip + post-install interactive prompts that previously caused a false `exit status 1`. + +### PR + +**#148** — `fix(homebrew): use bash and NONINTERACTIVE=1 in interactive install script` + +--- + +## Bug 2 — Shell change fails with "chsh: PAM: Authentication failure" + +### Symptom + +After Homebrew and shell installation, the step that sets the default shell fails: + +``` +🔐 Changing default shell... + (You may need to enter your password) + +Password: +chsh: PAM: Authentication failure +``` + +### Root Cause + +`getSetShellScript()` uses `chsh -s "$SHELL_PATH"` which requires PAM authentication. +In Docker containers and minimal Linux environments, PAM is not fully configured, so +this always fails — even when the user has full `sudo` access with `NOPASSWD`. + +### Fix — `installer/internal/tui/interactive.go` + +```diff +- chsh -s "$SHELL_PATH" +- +- echo "" +- echo "✅ Default shell changed to $SHELL_PATH" ++ if chsh -s "$SHELL_PATH" 2>/dev/null; then ++ echo "" ++ echo "✅ Default shell changed to $SHELL_PATH" ++ elif sudo usermod -s "$SHELL_PATH" "$(whoami)" 2>/dev/null; then ++ echo "" ++ echo "✅ Default shell changed to $SHELL_PATH (via usermod)" ++ else ++ echo "" ++ echo "⚠️ Could not change default shell automatically." ++ echo " Run manually: chsh -s $SHELL_PATH" ++ fi +``` + +Strategy: +1. Try `chsh` first — works on real machines where PAM is properly configured. +2. Fall back to `sudo usermod` — works in Docker and environments with sudo access. +3. If both fail — show a clear manual instruction instead of crashing. + +### PR + +**#149** — `fix(setshell): fallback to sudo usermod when chsh fails PAM auth` + +--- + +## Test Result + +Full installation with both fixes applied on Ubuntu 22.04 (Docker): + +``` +✨ Installation Complete! ✨ + + • OS: linux + • Terminal: alacritty + • Shell: fish + • Window Manager: tmux + • Font: Iosevka Term Nerd Font + • Editor: Neovim with Gentleman config +``` + +--- + +## Files Modified + +| File | Change | +|------|--------| +| `installer/internal/tui/interactive.go` | Homebrew script: `#!/bin/sh` → `#!/bin/bash`, `/bin/sh` → `NONINTERACTIVE=1 /bin/bash` | +| `installer/internal/tui/interactive.go` | Shell change: `chsh` with fallback to `sudo usermod` | diff --git a/installer/internal/tui/installation_steps_test.go b/installer/internal/tui/installation_steps_test.go index aeb9809b..cd792de5 100644 --- a/installer/internal/tui/installation_steps_test.go +++ b/installer/internal/tui/installation_steps_test.go @@ -1,6 +1,7 @@ package tui import ( + "encoding/json" "os" "path/filepath" "testing" @@ -774,6 +775,214 @@ func TestBackupOptions(t *testing.T) { }) } +func TestPreserveFileDuringOperation(t *testing.T) { + t.Run("restores existing file after overwrite", func(t *testing.T) { + tmpDir := t.TempDir() + path := filepath.Join(tmpDir, "opencode.json") + original := `{"plugins":{"existing":true}}` + + if err := os.WriteFile(path, []byte(original), 0644); err != nil { + t.Fatalf("write original file: %v", err) + } + + err := preserveFileDuringOperation(path, func() error { + return os.WriteFile(path, []byte(`{"plugins":{"installer":true}}`), 0644) + }) + if err != nil { + t.Fatalf("preserveFileDuringOperation returned error: %v", err) + } + + got, err := os.ReadFile(path) + if err != nil { + t.Fatalf("read restored file: %v", err) + } + if string(got) != original { + t.Fatalf("expected original contents %q, got %q", original, string(got)) + } + }) + + t.Run("restores existing file after deletion", func(t *testing.T) { + tmpDir := t.TempDir() + path := filepath.Join(tmpDir, "opencode.json") + original := `{"mcps":{"keep":true}}` + + if err := os.WriteFile(path, []byte(original), 0644); err != nil { + t.Fatalf("write original file: %v", err) + } + + err := preserveFileDuringOperation(path, func() error { + return os.Remove(path) + }) + if err != nil { + t.Fatalf("preserveFileDuringOperation returned error: %v", err) + } + + got, err := os.ReadFile(path) + if err != nil { + t.Fatalf("read restored file: %v", err) + } + if string(got) != original { + t.Fatalf("expected original contents %q, got %q", original, string(got)) + } + }) + + t.Run("does not create file when none existed before", func(t *testing.T) { + tmpDir := t.TempDir() + path := filepath.Join(tmpDir, "opencode.json") + + err := preserveFileDuringOperation(path, func() error { + return nil + }) + if err != nil { + t.Fatalf("preserveFileDuringOperation returned error: %v", err) + } + + if _, err := os.Stat(path); !os.IsNotExist(err) { + t.Fatalf("expected file to remain absent, stat err = %v", err) + } + }) +} + +func TestMergeJSONFileMissingKeys(t *testing.T) { + t.Run("creates destination file when it does not exist", func(t *testing.T) { + tmpDir := t.TempDir() + src := filepath.Join(tmpDir, "default-opencode.json") + dst := filepath.Join(tmpDir, "opencode.json") + defaultContents := `{"theme":"gentleman"}` + + if err := os.WriteFile(src, []byte(defaultContents), 0644); err != nil { + t.Fatalf("write source file: %v", err) + } + + action, err := mergeJSONFileMissingKeys(src, dst) + if err != nil { + t.Fatalf("mergeJSONFileMissingKeys returned error: %v", err) + } + if action != "created" { + t.Fatalf("expected action created, got %q", action) + } + + got, err := os.ReadFile(dst) + if err != nil { + t.Fatalf("read destination file: %v", err) + } + if string(got) != defaultContents { + t.Fatalf("expected destination contents %q, got %q", defaultContents, string(got)) + } + }) + + t.Run("merges only missing keys into existing destination file", func(t *testing.T) { + tmpDir := t.TempDir() + src := filepath.Join(tmpDir, "default-opencode.json") + dst := filepath.Join(tmpDir, "opencode.json") + defaultContents := `{ + "$schema": "https://opencode.ai/config.json", + "agent": { + "gentleman": { + "mode": "primary", + "tools": { + "edit": true, + "write": true + } + }, + "dangerous-gentleman": { + "mode": "all" + } + }, + "autoupdate": true +}` + existingContents := `{ + "agent": { + "gentleman": { + "mode": "custom", + "tools": { + "edit": false + } + } + } +}` + + if err := os.WriteFile(src, []byte(defaultContents), 0644); err != nil { + t.Fatalf("write source file: %v", err) + } + if err := os.WriteFile(dst, []byte(existingContents), 0644); err != nil { + t.Fatalf("write existing destination file: %v", err) + } + + action, err := mergeJSONFileMissingKeys(src, dst) + if err != nil { + t.Fatalf("mergeJSONFileMissingKeys returned error: %v", err) + } + if action != "merged" { + t.Fatalf("expected action merged, got %q", action) + } + + got, err := os.ReadFile(dst) + if err != nil { + t.Fatalf("read destination file: %v", err) + } + + var merged map[string]any + if err := json.Unmarshal(got, &merged); err != nil { + t.Fatalf("unmarshal merged destination file: %v", err) + } + + if merged["$schema"] != "https://opencode.ai/config.json" { + t.Fatalf("expected $schema to be added, got %#v", merged["$schema"]) + } + if merged["autoupdate"] != true { + t.Fatalf("expected autoupdate to be added, got %#v", merged["autoupdate"]) + } + + agent := merged["agent"].(map[string]any) + gentleman := agent["gentleman"].(map[string]any) + if gentleman["mode"] != "custom" { + t.Fatalf("expected existing agent.gentleman.mode to be preserved, got %#v", gentleman["mode"]) + } + + tools := gentleman["tools"].(map[string]any) + if tools["edit"] != false { + t.Fatalf("expected existing agent.gentleman.tools.edit to be preserved, got %#v", tools["edit"]) + } + if tools["write"] != true { + t.Fatalf("expected missing agent.gentleman.tools.write to be added, got %#v", tools["write"]) + } + + if _, ok := agent["dangerous-gentleman"]; !ok { + t.Fatal("expected missing agent.dangerous-gentleman object to be added") + } + }) + + t.Run("returns preserved when no keys are missing", func(t *testing.T) { + tmpDir := t.TempDir() + src := filepath.Join(tmpDir, "default-opencode.json") + dst := filepath.Join(tmpDir, "opencode.json") + contents := `{ + "$schema": "https://opencode.ai/config.json", + "agent": { + "gentleman": { + "mode": "primary" + } + } +}` + + if err := os.WriteFile(src, []byte(contents), 0644); err != nil { + t.Fatalf("write source file: %v", err) + } + if err := os.WriteFile(dst, []byte(contents), 0644); err != nil { + t.Fatalf("write destination file: %v", err) + } + + action, err := mergeJSONFileMissingKeys(src, dst) + if err != nil { + t.Fatalf("mergeJSONFileMissingKeys returned error: %v", err) + } + if action != "preserved" { + t.Fatalf("expected action preserved, got %q", action) + } + }) +} + // Helper functions func simulateKeyPress(m Model, key string) (Model, interface{}) { var msg tea.Msg diff --git a/installer/internal/tui/installer.go b/installer/internal/tui/installer.go index d53084b4..77e38a18 100644 --- a/installer/internal/tui/installer.go +++ b/installer/internal/tui/installer.go @@ -1,6 +1,7 @@ package tui import ( + "encoding/json" "fmt" "os" "path/filepath" @@ -42,6 +43,121 @@ func wrapStepError(stepID, stepName, description string, cause error) error { } } +// preserveFileDuringOperation keeps an existing user-managed file intact while +// an external installer mutates or recreates the surrounding config directory. +func preserveFileDuringOperation(path string, operation func() error) error { + originalContents, err := os.ReadFile(path) + existed := err == nil + if err != nil && !os.IsNotExist(err) { + return fmt.Errorf("read preserved file: %w", err) + } + + mode := os.FileMode(0o644) + if existed { + info, statErr := os.Stat(path) + if statErr != nil { + return fmt.Errorf("stat preserved file: %w", statErr) + } + mode = info.Mode() + } + + operationErr := operation() + if !existed { + return operationErr + } + + if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + if operationErr != nil { + return fmt.Errorf("operation failed: %w; additionally failed to recreate preserved file directory: %v", operationErr, err) + } + return fmt.Errorf("recreate preserved file directory: %w", err) + } + + if err := os.WriteFile(path, originalContents, mode); err != nil { + if operationErr != nil { + return fmt.Errorf("operation failed: %w; additionally failed to restore preserved file: %v", operationErr, err) + } + return fmt.Errorf("restore preserved file: %w", err) + } + + return operationErr +} + +// mergeJSONFileMissingKeys creates a config file from the repo default when it +// does not exist, or merges only missing keys into an existing JSON object. +func mergeJSONFileMissingKeys(src, dst string) (string, error) { + srcContents, err := os.ReadFile(src) + if err != nil { + return "", fmt.Errorf("read source json: %w", err) + } + + var srcJSON map[string]any + if err := json.Unmarshal(srcContents, &srcJSON); err != nil { + return "", fmt.Errorf("parse source json: %w", err) + } + + dstContents, err := os.ReadFile(dst) + if os.IsNotExist(err) { + if err := system.CopyFile(src, dst); err != nil { + return "", err + } + return "created", nil + } + if err != nil { + return "", fmt.Errorf("read destination json: %w", err) + } + + var dstJSON map[string]any + if err := json.Unmarshal(dstContents, &dstJSON); err != nil { + return "", fmt.Errorf("parse destination json: %w", err) + } + + if !mergeMissingJSONKeys(dstJSON, srcJSON) { + return "preserved", nil + } + + mergedContents, err := json.MarshalIndent(dstJSON, "", " ") + if err != nil { + return "", fmt.Errorf("marshal merged json: %w", err) + } + mergedContents = append(mergedContents, '\n') + + info, err := os.Stat(dst) + if err != nil { + return "", fmt.Errorf("stat destination json: %w", err) + } + if err := os.WriteFile(dst, mergedContents, info.Mode()); err != nil { + return "", fmt.Errorf("write merged json: %w", err) + } + + return "merged", nil +} + +// mergeMissingJSONKeys recursively copies only missing object keys from src to +// dst. Existing values in dst always win. +func mergeMissingJSONKeys(dst, src map[string]any) bool { + changed := false + + for key, srcValue := range src { + dstValue, exists := dst[key] + if !exists { + dst[key] = srcValue + changed = true + continue + } + + srcMap, srcIsMap := srcValue.(map[string]any) + dstMap, dstIsMap := dstValue.(map[string]any) + if srcIsMap && dstIsMap { + if mergeMissingJSONKeys(dstMap, srcMap) { + changed = true + } + } + } + + return changed +} + // executeStep runs the actual installation for a step func executeStep(stepID string, m *Model) error { switch stepID { @@ -1144,9 +1260,15 @@ func stepInstallNvim(m *Model) error { // Skip on Termux - OpenCode doesn't support Android if !m.SystemInfo.IsTermux { SendLog(stepID, "Installing OpenCode (optional)...") - system.RunWithLogs(`curl -fsSL https://opencode.ai/install | bash`, nil, func(line string) { - SendLog(stepID, line) - }) + opencodeConfigPath := filepath.Join(homeDir, ".config", "opencode", "opencode.json") + if err := preserveFileDuringOperation(opencodeConfigPath, func() error { + result := system.RunWithLogs(`curl -fsSL https://opencode.ai/install | bash`, nil, func(line string) { + SendLog(stepID, line) + }) + return result.Error + }); err != nil { + SendLog(stepID, "⚠️ Could not preserve existing OpenCode config during install: "+err.Error()) + } } else { SendLog(stepID, "Skipping OpenCode (not supported on Termux)") } @@ -1161,7 +1283,21 @@ func stepInstallNvim(m *Model) error { SendLog(stepID, "Warning: could not remove legacy opencode/skill dir: "+err.Error()) } system.EnsureDir(filepath.Join(openCodeDir, "skills")) - system.CopyFile(filepath.Join(repoDir, "GentlemanOpenCode/opencode.json"), filepath.Join(openCodeDir, "opencode.json")) + opencodeConfigPath := filepath.Join(openCodeDir, "opencode.json") + configAction, err := mergeJSONFileMissingKeys(filepath.Join(repoDir, "GentlemanOpenCode/opencode.json"), opencodeConfigPath) + if err != nil { + return wrapStepError("nvim", "Install Neovim", + "Failed to configure OpenCode config", + err) + } + switch configAction { + case "created": + SendLog(stepID, "⚙️ Installed default OpenCode config") + case "merged": + SendLog(stepID, "⚙️ Merged missing keys into existing OpenCode config") + default: + SendLog(stepID, "⚙️ Preserved existing OpenCode config") + } system.CopyFile(filepath.Join(repoDir, "GentlemanOpenCode/themes/gentleman.json"), filepath.Join(openCodeDir, "themes/gentleman.json")) system.CopyDir(filepath.Join(repoDir, "GentlemanOpenCode", "skills"), filepath.Join(openCodeDir, "skills")) system.CopyFile(filepath.Join(repoDir, "GentlemanOpenCode/AGENTS.md"), filepath.Join(openCodeDir, "AGENTS.md")) diff --git a/installer/internal/tui/interactive.go b/installer/internal/tui/interactive.go index 3be8c2b3..8f5ae256 100644 --- a/installer/internal/tui/interactive.go +++ b/installer/internal/tui/interactive.go @@ -70,13 +70,13 @@ func getHomebrewScript(m *Model) (string, error) { } brewPrefix := system.GetBrewPrefix() - script := fmt.Sprintf(`#!/bin/sh + script := fmt.Sprintf(`#!/bin/bash set -e echo "" echo "🍺 Installing Homebrew package manager..." echo " (You may be prompted for your password)" echo "" -/bin/sh -c "$(curl -fsSL https://raw.githubusercontent.com/Homebrew/install/HEAD/install.sh)" +NONINTERACTIVE=1 /bin/bash -c "$(curl -fsSL https://raw.githubusercontent.com/Homebrew/install/HEAD/install.sh)" echo "" echo "📝 Configuring shell to use Homebrew..." @@ -327,10 +327,18 @@ echo "" echo "🔐 Changing default shell..." echo " (You may need to enter your password)" echo "" -chsh -s "$SHELL_PATH" +if chsh -s "$SHELL_PATH" 2>/dev/null; then + echo "" + echo "✅ Default shell changed to $SHELL_PATH" +elif sudo usermod -s "$SHELL_PATH" "$(whoami)" 2>/dev/null; then + echo "" + echo "✅ Default shell changed to $SHELL_PATH (via usermod)" +else + echo "" + echo "⚠️ Could not change default shell automatically." + echo " Run manually: chsh -s $SHELL_PATH" +fi -echo "" -echo "✅ Default shell changed to $SHELL_PATH" echo " Please log out and log back in for changes to take effect." echo "" echo "Press Enter to continue..."