Skip to content

Commit 6de0c92

Browse files
AdamDrewsTRChronosMasterOfAllTimeCopilot
authored
Remove stale goenv shims from v2 installations during upgrade to v3 (#537)
* Remove stale goenv shims from v2 installations during upgrade to v3 * Apply suggestions from code review Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * consolidate duplicated logic into helper funcs and add tests * fix windows test failure --------- Co-authored-by: Stathi C. <efstathiosc@gmail.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: Efstathios Chouliaris <Efstathios.Chouliaris@thomsonreuters.com>
1 parent 34afd30 commit 6de0c92

6 files changed

Lines changed: 298 additions & 15 deletions

File tree

cmd/root.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"github.com/go-nv/goenv/internal/cmdutil"
99
"github.com/go-nv/goenv/internal/config"
1010
"github.com/go-nv/goenv/internal/manager"
11+
"github.com/go-nv/goenv/internal/migration"
1112
"github.com/go-nv/goenv/internal/utils"
1213
"github.com/go-nv/goenv/internal/vscode"
1314
"github.com/go-nv/goenv/internal/workflow"
@@ -62,6 +63,12 @@ var RootCmd = &cobra.Command{
6263
// Store updated context back to command
6364
cmd.SetContext(ctx)
6465

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)
70+
}
71+
6572
// Propagate output options
6673
utils.SetOutputOptions(NoColor, Plain)
6774
},

install.ps1

Lines changed: 40 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -3,19 +3,25 @@
33

44
$ErrorActionPreference = "Stop"
55

6+
# Ensure TLS 1.2 for GitHub API/downloads (older Windows defaults to TLS 1.0)
7+
[Net.ServicePointManager]::SecurityProtocol = [Net.SecurityProtocolType]::Tls12
8+
69
# Configuration
7-
$GOENV_ROOT = if ($env:GOENV_ROOT) { $env:GOENV_ROOT } 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" }
813
$GITHUB_REPO = "go-nv/goenv"
9-
$INSTALL_DIR = "$GOENV_ROOT\bin"
14+
$INSTALL_DIR = Join-Path $GOENV_ROOT "bin"
1015

11-
# Colors
12-
function Write-ColorOutput($ForegroundColor) {
13-
$fc = $host.UI.RawUI.ForegroundColor
14-
$host.UI.RawUI.ForegroundColor = $ForegroundColor
15-
if ($args) {
16-
Write-Output $args
17-
}
18-
$host.UI.RawUI.ForegroundColor = $fc
16+
# Colors — use Write-Host which works in non-interactive/piped contexts
17+
function Write-ColorOutput {
18+
param(
19+
[Parameter(Position=0)]
20+
[System.ConsoleColor]$ForegroundColor,
21+
[Parameter(Position=1, ValueFromRemainingArguments)]
22+
[string[]]$Message
23+
)
24+
Write-Host ($Message -join ' ') -ForegroundColor $ForegroundColor
1925
}
2026

2127
# Detect architecture
@@ -85,35 +91,54 @@ function Install-Binary {
8591
# Copy binary
8692
$binaryPath = Join-Path $tmpDir "goenv.exe"
8793
if (Test-Path $binaryPath) {
88-
Copy-Item -Path $binaryPath -Destination "$INSTALL_DIR\goenv.exe" -Force
94+
Copy-Item -Path $binaryPath -Destination (Join-Path $INSTALL_DIR "goenv.exe") -Force
8995
} else {
9096
throw "Binary not found in archive"
9197
}
9298

9399
# Copy completions if they exist
94100
$completionsPath = Join-Path $tmpDir "completions"
95101
if (Test-Path $completionsPath) {
96-
$targetCompletions = "$GOENV_ROOT\completions"
102+
$targetCompletions = Join-Path $GOENV_ROOT "completions"
97103
New-Item -ItemType Directory -Path $targetCompletions -Force | Out-Null
98104
Copy-Item -Path "$completionsPath\*" -Destination $targetCompletions -Recurse -Force -ErrorAction SilentlyContinue
99105
}
100106

101107
Write-ColorOutput Green "goenv installed successfully!"
108+
109+
# Remove stale goenv shim from v2 installations.
110+
# v2's goenv-rehash bakes the Cellar/libexec path into shims at creation time.
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"
115+
if (Test-Path $staleShim) {
116+
$shimContent = Get-Content $staleShim -Raw -ErrorAction SilentlyContinue
117+
if ($shimContent -and $shimContent -match "libexec[\\/]goenv") {
118+
Write-ColorOutput Yellow "Removing stale v2 goenv shim..."
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+
}
125+
}
126+
}
102127
}
103128
catch {
104129
Write-ColorOutput Red "Installation failed: $_"
105130
exit 1
106131
}
107132
finally {
108-
# Cleanup
133+
# Cleanup temp directory
109134
if (Test-Path $tmpDir) {
110135
Remove-Item -Path $tmpDir -Recurse -Force -ErrorAction SilentlyContinue
111136
}
112137
}
113138
}
114139

115140
# Auto-configure PowerShell profile
116-
function Setup-PowerShellProfile {
141+
function Initialize-PowerShellProfile {
117142
$profilePath = $PROFILE
118143

119144
# Create profile directory if it doesn't exist
@@ -184,7 +209,7 @@ function Main {
184209

185210
$version = Get-LatestVersion
186211
Install-Binary -Version $version -Arch $arch
187-
Setup-PowerShellProfile
212+
Initialize-PowerShellProfile
188213
Show-Instructions
189214
}
190215

install.sh

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,19 @@ install_binary() {
122122
# Cleanup
123123
rm -rf "$tmp_dir"
124124

125+
# Remove stale goenv shim from v2 installations.
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
137+
125138
echo -e "${GREEN}✓ goenv installed successfully!${NC}"
126139
}
127140

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

scripts/swap/main.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"path/filepath"
99
"runtime"
1010

11+
"github.com/go-nv/goenv/internal/migration"
1112
"github.com/go-nv/goenv/internal/platform"
1213
"github.com/go-nv/goenv/internal/utils"
1314
)
@@ -407,6 +408,20 @@ Options:
407408
swapGoenvBinary(target)
408409
}
409410

411+
// Remove stale goenv shim from v2 that may shadow the new binary.
412+
// Uses helper function to handle both forward and backslash paths.
413+
goenvRoot := os.Getenv("GOENV_ROOT")
414+
if goenvRoot == "" {
415+
homeDir, _ := os.UserHomeDir()
416+
goenvRoot = filepath.Join(homeDir, ".goenv")
417+
}
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"))
423+
}
424+
410425
fmt.Println()
411426
success("Switch successful!")
412427
warn("IMPORTANT: Reload your shell before testing:")

0 commit comments

Comments
 (0)