Skip to content

Commit 5356010

Browse files
consolidate duplicated logic into helper funcs and add tests
1 parent 8f0fb13 commit 5356010

6 files changed

Lines changed: 256 additions & 40 deletions

File tree

cmd/root.go

Lines changed: 5 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,12 @@ package cmd
33
import (
44
"fmt"
55
"os"
6-
"path/filepath"
76
"strings"
87

98
"github.com/go-nv/goenv/internal/cmdutil"
109
"github.com/go-nv/goenv/internal/config"
1110
"github.com/go-nv/goenv/internal/manager"
11+
"github.com/go-nv/goenv/internal/migration"
1212
"github.com/go-nv/goenv/internal/utils"
1313
"github.com/go-nv/goenv/internal/vscode"
1414
"github.com/go-nv/goenv/internal/workflow"
@@ -63,19 +63,10 @@ var RootCmd = &cobra.Command{
6363
// Store updated context back to command
6464
cmd.SetContext(ctx)
6565

66-
// Remove stale goenv shim left over from v2.
67-
// v2's goenv-rehash bakes the Homebrew Cellar path into shims at creation
68-
// time (e.g. exec "/opt/homebrew/Cellar/goenv/2.2.38_1/libexec/goenv").
69-
// After upgrading to v3, the old shim may still point to a deleted Cellar
70-
// path, shadowing the real v3 binary. We only remove it if it contains
71-
// "libexec/goenv" — the v2 fingerprint — to avoid deleting anything unexpected.
72-
goenvShim := filepath.Join(cfg.ShimsDir(), "goenv")
73-
if data, err := os.ReadFile(goenvShim); err == nil {
74-
if strings.Contains(string(data), "libexec/goenv") {
75-
if err := os.Remove(goenvShim); err != nil {
76-
fmt.Fprintf(cmd.ErrOrStderr(), "warning: failed to remove stale v2 goenv shim %q: %v\n", goenvShim, err)
77-
}
78-
}
66+
// Remove stale goenv shim left over from v2 installations.
67+
// Uses helper function to handle both forward and backslash paths.
68+
if _, err := migration.RemoveStaleV2Shim(cfg.ShimsDir()); err != nil {
69+
fmt.Fprintf(cmd.ErrOrStderr(), "warning: %v\n", err)
7970
}
8071

8172
// Propagate output options

install.ps1

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,9 @@ $ErrorActionPreference = "Stop"
77
[Net.ServicePointManager]::SecurityProtocol = [Net.SecurityProtocolType]::Tls12
88

99
# Configuration
10-
$GOENV_ROOT = if ($env:GOENV_ROOT) { $env:GOENV_ROOT } else { Join-Path (if ($env:USERPROFILE) { $env:USERPROFILE } else { $HOME }) ".goenv" }
10+
# Use if/else instead of ?? operator for PowerShell 5.1 compatibility
11+
$defaultHome = if ($env:USERPROFILE) { $env:USERPROFILE } else { $HOME }
12+
$GOENV_ROOT = if ($env:GOENV_ROOT) { $env:GOENV_ROOT } else { Join-Path $defaultHome ".goenv" }
1113
$GITHUB_REPO = "go-nv/goenv"
1214
$INSTALL_DIR = Join-Path $GOENV_ROOT "bin"
1315

@@ -106,14 +108,20 @@ function Install-Binary {
106108

107109
# Remove stale goenv shim from v2 installations.
108110
# v2's goenv-rehash bakes the Cellar/libexec path into shims at creation time.
109-
# Only remove if it contains "libexec/goenv" — the v2 fingerprint.
110-
$staleShim = Join-Path (Join-Path $GOENV_ROOT "shims") "goenv"
111+
# Match both forward and backslashes to support all v2 installations.
112+
# Use nested Join-Path for PowerShell 5.1 compatibility (no 3-arg support).
113+
$shimsPath = Join-Path $GOENV_ROOT "shims"
114+
$staleShim = Join-Path $shimsPath "goenv"
111115
if (Test-Path $staleShim) {
112116
$shimContent = Get-Content $staleShim -Raw -ErrorAction SilentlyContinue
113-
if ($shimContent -and $shimContent -match "libexec/goenv") {
117+
if ($shimContent -and $shimContent -match "libexec[\\/]goenv") {
114118
Write-ColorOutput Yellow "Removing stale v2 goenv shim..."
115-
Remove-Item -Path $staleShim -Force -ErrorAction SilentlyContinue
116-
Write-ColorOutput Green "Stale shim removed"
119+
$removed = Remove-Item -Path $staleShim -Force -ErrorAction SilentlyContinue -PassThru
120+
if ($removed) {
121+
Write-ColorOutput Green "Stale shim removed"
122+
} else {
123+
Write-ColorOutput Yellow "Warning: Failed to remove stale v2 goenv shim"
124+
}
117125
}
118126
}
119127
}

install.sh

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -123,15 +123,17 @@ install_binary() {
123123
rm -rf "$tmp_dir"
124124

125125
# Remove stale goenv shim from v2 installations.
126-
# v2's goenv-rehash bakes the Homebrew Cellar path into shims at creation time
127-
# (e.g. exec "/opt/homebrew/Cellar/goenv/2.2.38_1/libexec/goenv"). After
128-
# upgrading to v3, the old shim shadows the real v3 binary. We only remove
129-
# it if it contains "libexec/goenv" — the v2 fingerprint.
130-
if [ -f "$GOENV_ROOT/shims/goenv" ] && grep -q 'libexec/goenv' "$GOENV_ROOT/shims/goenv" 2>/dev/null; then
131-
echo -e "${YELLOW}Removing stale v2 goenv shim...${NC}"
132-
rm -f "$GOENV_ROOT/shims/goenv"
133-
echo -e "${GREEN}✓ Stale shim removed${NC}"
134-
fi
126+
# v2's goenv-rehash bakes the Cellar/libexec path into shims at creation time
127+
# (e.g. exec "/opt/homebrew/Cellar/goenv/2.2.38_1/libexec/goenv"). After
128+
# upgrading to v3, the old shim shadows the real v3 binary. We only remove
129+
# it if it contains "libexec/goenv" or "libexec\goenv" — the v2 fingerprint.
130+
if [ -f "$GOENV_ROOT/shims/goenv" ] && grep -qE 'libexec[/\\]goenv' "$GOENV_ROOT/shims/goenv" 2>/dev/null; then
131+
echo -e "${YELLOW}Removing stale v2 goenv shim...${NC}"
132+
if rm -f "$GOENV_ROOT/shims/goenv" 2>/dev/null; then
133+
echo -e "${GREEN}✓ Stale shim removed${NC}"
134+
else
135+
echo -e "${YELLOW}⚠ Warning: Failed to remove stale v2 goenv shim${NC}"
136+
fi
135137

136138
echo -e "${GREEN}✓ goenv installed successfully!${NC}"
137139
}

internal/migration/v2shim.go

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
package migration
2+
3+
import (
4+
"fmt"
5+
"os"
6+
"path/filepath"
7+
"regexp"
8+
)
9+
10+
// V2ShimPattern matches v2's libexec/goenv path with both forward slashes and backslashes.
11+
// The pattern ensures we match "libexec/goenv" or "libexec\goenv" where "goenv" is the final
12+
// path component (followed by quote, space, or directory separator, but not a hyphen or letter).
13+
var V2ShimPattern = regexp.MustCompile(`libexec[\\/]goenv(["'\s/\\]|$)`)
14+
15+
// RemoveStaleV2Shim removes the stale goenv shim left over from v2 installations.
16+
// v2's goenv-rehash bakes the Homebrew Cellar path into shims at creation time
17+
// (e.g. exec "/opt/homebrew/Cellar/goenv/2.2.38_1/libexec/goenv" on macOS/Linux
18+
// or "C:\path\to\libexec\goenv" on Windows). After upgrading to v3, the old shim
19+
// may still point to a deleted path, shadowing the real v3 binary.
20+
//
21+
// We only remove the shim if it contains "libexec/goenv" or "libexec\goenv" —
22+
// the v2 fingerprint — to avoid deleting anything unexpected.
23+
//
24+
// Returns true if the shim was removed, false otherwise.
25+
// If removal fails, it returns an error that can be logged as a warning.
26+
func RemoveStaleV2Shim(shimsDir string) (bool, error) {
27+
goenvShim := filepath.Join(shimsDir, "goenv")
28+
29+
// Read the shim file
30+
data, err := os.ReadFile(goenvShim)
31+
if err != nil {
32+
// File doesn't exist or can't be read - nothing to remove
33+
return false, nil
34+
}
35+
36+
// Check if it contains the v2 fingerprint (supports both / and \)
37+
if !V2ShimPattern.Match(data) {
38+
// Not a v2 shim - leave it alone
39+
return false, nil
40+
}
41+
42+
// Remove the stale shim
43+
if err := os.Remove(goenvShim); err != nil {
44+
return false, fmt.Errorf("failed to remove stale v2 goenv shim %q: %w", goenvShim, err)
45+
}
46+
47+
return true, nil
48+
}

internal/migration/v2shim_test.go

Lines changed: 171 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,171 @@
1+
package migration
2+
3+
import (
4+
"os"
5+
"path/filepath"
6+
"testing"
7+
)
8+
9+
func TestRemoveStaleV2Shim(t *testing.T) {
10+
// Create a temporary directory for testing
11+
tmpDir, err := os.MkdirTemp("", "goenv-test-*")
12+
if err != nil {
13+
t.Fatalf("Failed to create temp dir: %v", err)
14+
}
15+
defer os.RemoveAll(tmpDir)
16+
17+
shimsDir := filepath.Join(tmpDir, "shims")
18+
if err := os.MkdirAll(shimsDir, 0755); err != nil {
19+
t.Fatalf("Failed to create shims dir: %v", err)
20+
}
21+
22+
t.Run("removes v2 shim with forward slash", func(t *testing.T) {
23+
// Create a v2 shim with forward slash
24+
shimPath := filepath.Join(shimsDir, "goenv")
25+
v2Content := `#!/usr/bin/env bash
26+
exec "/opt/homebrew/Cellar/goenv/2.2.38_1/libexec/goenv" "$@"
27+
`
28+
if err := os.WriteFile(shimPath, []byte(v2Content), 0755); err != nil {
29+
t.Fatalf("Failed to create shim: %v", err)
30+
}
31+
32+
removed, err := RemoveStaleV2Shim(shimsDir)
33+
if err != nil {
34+
t.Errorf("Unexpected error: %v", err)
35+
}
36+
if !removed {
37+
t.Error("Expected shim to be removed")
38+
}
39+
if _, err := os.Stat(shimPath); !os.IsNotExist(err) {
40+
t.Error("Shim file should not exist")
41+
}
42+
})
43+
44+
t.Run("removes v2 shim with backslash", func(t *testing.T) {
45+
// Create a v2 shim with backslash (Windows-style)
46+
shimPath := filepath.Join(shimsDir, "goenv")
47+
v2Content := `@echo off
48+
"C:\Program Files\goenv\libexec\goenv" %*
49+
`
50+
if err := os.WriteFile(shimPath, []byte(v2Content), 0755); err != nil {
51+
t.Fatalf("Failed to create shim: %v", err)
52+
}
53+
54+
removed, err := RemoveStaleV2Shim(shimsDir)
55+
if err != nil {
56+
t.Errorf("Unexpected error: %v", err)
57+
}
58+
if !removed {
59+
t.Error("Expected shim to be removed")
60+
}
61+
if _, err := os.Stat(shimPath); !os.IsNotExist(err) {
62+
t.Error("Shim file should not exist")
63+
}
64+
})
65+
66+
t.Run("does not remove non-v2 shim", func(t *testing.T) {
67+
// Create a non-v2 shim
68+
shimPath := filepath.Join(shimsDir, "goenv")
69+
v3Content := `#!/usr/bin/env bash
70+
# This is a v3 shim
71+
exec "goenv" "$@"
72+
`
73+
if err := os.WriteFile(shimPath, []byte(v3Content), 0755); err != nil {
74+
t.Fatalf("Failed to create shim: %v", err)
75+
}
76+
77+
removed, err := RemoveStaleV2Shim(shimsDir)
78+
if err != nil {
79+
t.Errorf("Unexpected error: %v", err)
80+
}
81+
if removed {
82+
t.Error("Expected shim to NOT be removed")
83+
}
84+
if _, err := os.Stat(shimPath); os.IsNotExist(err) {
85+
t.Error("Shim file should still exist")
86+
}
87+
})
88+
89+
t.Run("handles missing shim file", func(t *testing.T) {
90+
// Ensure no shim exists
91+
shimPath := filepath.Join(shimsDir, "goenv")
92+
os.Remove(shimPath)
93+
94+
removed, err := RemoveStaleV2Shim(shimsDir)
95+
if err != nil {
96+
t.Errorf("Unexpected error: %v", err)
97+
}
98+
if removed {
99+
t.Error("Expected no removal when file doesn't exist")
100+
}
101+
})
102+
103+
t.Run("returns error when removal fails", func(t *testing.T) {
104+
// This test is platform-dependent and may not work in all environments
105+
// Skip if we can't create a read-only directory
106+
shimPath := filepath.Join(shimsDir, "goenv")
107+
v2Content := `#!/usr/bin/env bash
108+
exec "/opt/homebrew/Cellar/goenv/2.2.38_1/libexec/goenv" "$@"
109+
`
110+
if err := os.WriteFile(shimPath, []byte(v2Content), 0755); err != nil {
111+
t.Fatalf("Failed to create shim: %v", err)
112+
}
113+
114+
// Make directory read-only (Unix-like systems)
115+
if err := os.Chmod(shimsDir, 0555); err != nil {
116+
t.Skip("Cannot test permission error on this platform")
117+
}
118+
defer os.Chmod(shimsDir, 0755) // Restore permissions
119+
120+
removed, err := RemoveStaleV2Shim(shimsDir)
121+
if err == nil {
122+
t.Error("Expected error when removal fails")
123+
}
124+
if removed {
125+
t.Error("Should not report as removed when error occurs")
126+
}
127+
})
128+
}
129+
130+
func TestV2ShimPattern(t *testing.T) {
131+
tests := []struct {
132+
name string
133+
content string
134+
matches bool
135+
}{
136+
{
137+
name: "forward slash",
138+
content: `exec "/opt/homebrew/Cellar/goenv/2.2.38_1/libexec/goenv" "$@"`,
139+
matches: true,
140+
},
141+
{
142+
name: "backslash",
143+
content: `"C:\Program Files\goenv\libexec\goenv" %*`,
144+
matches: true,
145+
},
146+
{
147+
name: "no match",
148+
content: `exec "goenv" "$@"`,
149+
matches: false,
150+
},
151+
{
152+
name: "libexec only",
153+
content: `exec "/usr/local/libexec/goenv-other" "$@"`,
154+
matches: false,
155+
},
156+
{
157+
name: "goenv only",
158+
content: `exec "/usr/local/bin/goenv" "$@"`,
159+
matches: false,
160+
},
161+
}
162+
163+
for _, tt := range tests {
164+
t.Run(tt.name, func(t *testing.T) {
165+
matches := V2ShimPattern.MatchString(tt.content)
166+
if matches != tt.matches {
167+
t.Errorf("Expected match=%v, got match=%v for content: %s", tt.matches, matches, tt.content)
168+
}
169+
})
170+
}
171+
}

scripts/swap/main.go

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,8 @@ import (
77
"os/exec"
88
"path/filepath"
99
"runtime"
10-
"strings"
1110

11+
"github.com/go-nv/goenv/internal/migration"
1212
"github.com/go-nv/goenv/internal/platform"
1313
"github.com/go-nv/goenv/internal/utils"
1414
)
@@ -409,21 +409,17 @@ Options:
409409
}
410410

411411
// Remove stale goenv shim from v2 that may shadow the new binary.
412-
// Only remove if it contains "libexec/goenv" — the v2 fingerprint.
412+
// Uses helper function to handle both forward and backslash paths.
413413
goenvRoot := os.Getenv("GOENV_ROOT")
414414
if goenvRoot == "" {
415415
homeDir, _ := os.UserHomeDir()
416416
goenvRoot = filepath.Join(homeDir, ".goenv")
417417
}
418-
staleShim := filepath.Join(goenvRoot, "shims", "goenv")
419-
if data, err := os.ReadFile(staleShim); err == nil {
420-
if strings.Contains(string(data), "libexec/goenv") {
421-
if err := os.Remove(staleShim); err == nil {
422-
success("Removed stale v2 goenv shim from " + staleShim)
423-
} else {
424-
warn(fmt.Sprintf("Could not remove stale shim %s: %v", staleShim, err))
425-
}
426-
}
418+
shimsDir := filepath.Join(goenvRoot, "shims")
419+
if removed, err := migration.RemoveStaleV2Shim(shimsDir); err != nil {
420+
warn(err.Error())
421+
} else if removed {
422+
success("Removed stale v2 goenv shim from " + filepath.Join(shimsDir, "goenv"))
427423
}
428424

429425
fmt.Println()

0 commit comments

Comments
 (0)