Skip to content

Commit 4cf9557

Browse files
ChronosMasterOfAllTimeChronosMasterOfAllTime
committed
Fix doctor system go detection and goenv shimming
1 parent 6867f40 commit 4cf9557

4 files changed

Lines changed: 187 additions & 29 deletions

File tree

‎cmd/diagnostics/doctor.go‎

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1145,18 +1145,44 @@ func checkProfileSourcingIssues(cfg *config.Config) checkResult {
11451145
func checkSystemGoVersion(cfg *config.Config, mgr *manager.Manager) checkResult {
11461146
// Check if current version is system
11471147
currentVersion, _, err := mgr.GetCurrentVersion()
1148-
if err != nil || currentVersion != manager.SystemVersion {
1149-
// Not using system Go, skip check
1148+
usingSystemGo := err == nil && currentVersion == manager.SystemVersion
1149+
1150+
// Check if system Go exists in PATH (outside goenv directories)
1151+
hasSystemGo := mgr.HasSystemGo()
1152+
1153+
if !usingSystemGo && !hasSystemGo {
1154+
// Not using system Go and no system Go found
1155+
return checkResult{
1156+
id: "system-go-version",
1157+
name: "System Go version",
1158+
status: StatusOK,
1159+
message: "No system Go installation detected (using goenv-managed version)",
1160+
}
1161+
}
1162+
1163+
if !usingSystemGo && hasSystemGo {
1164+
// System Go exists but not using it - provide informative message
1165+
systemVersion, err := mgr.GetSystemGoVersion()
1166+
if err != nil {
1167+
return checkResult{
1168+
id: "system-go-version",
1169+
name: "System Go version",
1170+
status: StatusOK,
1171+
message: "System Go detected in PATH but using goenv-managed version",
1172+
advice: "To switch to system Go: goenv use system --global",
1173+
}
1174+
}
11501175
return checkResult{
11511176
id: "system-go-version",
11521177
name: "System Go version",
11531178
status: StatusOK,
1154-
message: "Not using system Go (using goenv-managed version)",
1179+
message: fmt.Sprintf("System Go %s detected but not active (using goenv-managed version)", systemVersion),
1180+
advice: "To switch to system Go: goenv use system --global",
11551181
}
11561182
}
11571183

11581184
// Using system Go - check if it exists and get version
1159-
if !mgr.HasSystemGo() {
1185+
if !hasSystemGo {
11601186
return checkResult{
11611187
id: "system-go-version",
11621188
name: "System Go version",

‎cmd/diagnostics/doctor_test.go‎

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1390,3 +1390,112 @@ func TestCheckObsoleteEnvVars(t *testing.T) {
13901390
})
13911391
}
13921392
}
1393+
1394+
func TestCheckSystemGoVersion(t *testing.T) {
1395+
tests := []struct {
1396+
name string
1397+
currentVersion string
1398+
hasSystemGo bool
1399+
systemVersion string
1400+
expectedStatus Status
1401+
shouldContain string
1402+
shouldAdvice string
1403+
}{
1404+
{
1405+
name: "Using goenv version, no system Go",
1406+
currentVersion: "1.23.2",
1407+
hasSystemGo: false,
1408+
expectedStatus: StatusOK,
1409+
shouldContain: "No system Go installation detected",
1410+
},
1411+
{
1412+
name: "Using goenv version, system Go exists",
1413+
currentVersion: "1.23.2",
1414+
hasSystemGo: true,
1415+
systemVersion: "1.26.2",
1416+
expectedStatus: StatusOK,
1417+
shouldContain: "System Go 1.26.2 detected but not active",
1418+
shouldAdvice: "goenv use system --global",
1419+
},
1420+
{
1421+
name: "Using system Go, it exists",
1422+
currentVersion: "system",
1423+
hasSystemGo: true,
1424+
systemVersion: "1.26.2",
1425+
expectedStatus: StatusOK,
1426+
shouldContain: "Using system Go 1.26.2",
1427+
},
1428+
{
1429+
name: "Using system Go, but not found",
1430+
currentVersion: "system",
1431+
hasSystemGo: false,
1432+
expectedStatus: StatusError,
1433+
shouldContain: "System Go is set as current version but not found",
1434+
},
1435+
}
1436+
1437+
for _, tt := range tests {
1438+
t.Run(tt.name, func(t *testing.T) {
1439+
// Use cmdtest for proper environment isolation
1440+
testRoot, cleanup := cmdtest.SetupTestEnv(t)
1441+
defer cleanup()
1442+
1443+
cfg := &config.Config{Root: testRoot}
1444+
1445+
// Setup current version if not "system"
1446+
if tt.currentVersion != "" && tt.currentVersion != "system" {
1447+
cmdtest.CreateTestVersion(t, testRoot, tt.currentVersion)
1448+
1449+
// Create version file
1450+
versionFile := filepath.Join(testRoot, "version")
1451+
err := os.WriteFile(versionFile, []byte(tt.currentVersion), 0o644)
1452+
require.NoError(t, err)
1453+
} else if tt.currentVersion == "system" {
1454+
// Set GOENV_VERSION to system
1455+
t.Setenv(utils.GoenvEnvVarVersion.String(), "system")
1456+
}
1457+
1458+
// Setup system Go if needed
1459+
if tt.hasSystemGo {
1460+
systemGoDir := t.TempDir()
1461+
err := utils.EnsureDir(filepath.Join(systemGoDir, "bin"))
1462+
require.NoError(t, err)
1463+
1464+
// Create mock go binary
1465+
goBinary := filepath.Join(systemGoDir, "bin", "go")
1466+
if utils.IsWindows() {
1467+
goBinary += ".exe"
1468+
}
1469+
1470+
// Create a script that outputs version
1471+
versionScript := "#!/bin/bash\necho 'go version go" + tt.systemVersion + " darwin/arm64'\n"
1472+
if utils.IsWindows() {
1473+
versionScript = "@echo off\r\necho go version go" + tt.systemVersion + " windows/amd64\r\n"
1474+
}
1475+
err = os.WriteFile(goBinary, []byte(versionScript), 0o755)
1476+
require.NoError(t, err)
1477+
1478+
// Add system Go to PATH
1479+
oldPath := os.Getenv(utils.EnvVarPath)
1480+
newPath := oldPath + string(os.PathListSeparator) + filepath.Join(systemGoDir, "bin")
1481+
t.Setenv(utils.EnvVarPath, newPath)
1482+
}
1483+
1484+
// Create manager
1485+
mgr := manager.NewManager(cfg, nil)
1486+
1487+
// Run check
1488+
result := checkSystemGoVersion(cfg, mgr)
1489+
1490+
// Verify result
1491+
assert.Equal(t, "system-go-version", result.id)
1492+
assert.Equal(t, "System Go version", result.name)
1493+
assert.Equal(t, tt.expectedStatus, result.status, "Status mismatch: %s", result.message)
1494+
assert.Contains(t, result.message, tt.shouldContain, "Message: %s", result.message)
1495+
1496+
if tt.shouldAdvice != "" {
1497+
assert.Contains(t, result.advice, tt.shouldAdvice, "Advice: %s", result.advice)
1498+
}
1499+
})
1500+
}
1501+
}

‎cmd/shell/init.go‎

Lines changed: 44 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -235,13 +235,20 @@ func renderInitScript(shell shellutil.ShellType, cfg *config.Config, noRehash bo
235235
builder.WriteString("if test -e $GOENV_RC_FILE\n")
236236
builder.WriteString(" source $GOENV_RC_FILE\n")
237237
builder.WriteString("end\n")
238-
builder.WriteString("if not contains $GOENV_ROOT/shims $PATH\n")
239-
builder.WriteString(" if test \"$GOENV_PATH_ORDER\" = \"back\"\n")
240-
builder.WriteString(" set -gx PATH $PATH $GOENV_ROOT/shims\n")
241-
builder.WriteString(" else\n")
242-
builder.WriteString(" set -gx PATH $GOENV_ROOT/shims $PATH\n")
238+
builder.WriteString("# Remove shims from PATH if present (to re-add in correct position)\n")
239+
builder.WriteString("set -l new_path\n")
240+
builder.WriteString("for p in $PATH\n")
241+
builder.WriteString(" if test \"$p\" != \"$GOENV_ROOT/shims\"\n")
242+
builder.WriteString(" set new_path $new_path $p\n")
243243
builder.WriteString(" end\n")
244244
builder.WriteString("end\n")
245+
builder.WriteString("set -gx PATH $new_path\n")
246+
builder.WriteString("# Add shims directory to PATH in correct position\n")
247+
builder.WriteString("if test \"$GOENV_PATH_ORDER\" = \"back\"\n")
248+
builder.WriteString(" set -gx PATH $PATH $GOENV_ROOT/shims\n")
249+
builder.WriteString("else\n")
250+
builder.WriteString(" set -gx PATH $GOENV_ROOT/shims $PATH\n")
251+
builder.WriteString("end\n")
245252
case shellutil.ShellTypePowerShell:
246253
fmt.Fprintf(&builder, "$env:GOENV_SHELL = \"%s\"\n", shell)
247254
fmt.Fprintf(&builder, "$env:GOENV_ROOT = \"%s\"\n", cfg.Root)
@@ -252,12 +259,13 @@ func renderInitScript(shell shellutil.ShellType, cfg *config.Config, noRehash bo
252259
builder.WriteString(" . $env:GOENV_RC_FILE\n")
253260
builder.WriteString("}\n")
254261
shimsDir := filepath.Join(cfg.Root, "shims")
255-
builder.WriteString(fmt.Sprintf("if ($env:PATH -notlike '*%s*') {\n", shimsDir))
256-
builder.WriteString(" if ($env:GOENV_PATH_ORDER -eq 'back') {\n")
257-
builder.WriteString(fmt.Sprintf(" $env:PATH = \"$env:PATH;%s\"\n", shimsDir))
258-
builder.WriteString(" } else {\n")
259-
builder.WriteString(fmt.Sprintf(" $env:PATH = \"%s;$env:PATH\"\n", shimsDir))
260-
builder.WriteString(" }\n")
262+
builder.WriteString("# Remove shims from PATH if present (to re-add in correct position)\n")
263+
builder.WriteString(fmt.Sprintf("$env:PATH = ($env:PATH -split ';' | Where-Object { $_ -ne '%s' }) -join ';'\n", shimsDir))
264+
builder.WriteString("# Add shims directory to PATH in correct position\n")
265+
builder.WriteString("if ($env:GOENV_PATH_ORDER -eq 'back') {\n")
266+
builder.WriteString(fmt.Sprintf(" $env:PATH = \"$env:PATH;%s\"\n", shimsDir))
267+
builder.WriteString("} else {\n")
268+
builder.WriteString(fmt.Sprintf(" $env:PATH = \"%s;$env:PATH\"\n", shimsDir))
261269
builder.WriteString("}\n")
262270
case shellutil.ShellTypeCmd:
263271
fmt.Fprintf(&builder, "SET GOENV_SHELL=%s\n", shell)
@@ -269,12 +277,14 @@ func renderInitScript(shell shellutil.ShellType, cfg *config.Config, noRehash bo
269277
builder.WriteString(" CALL \"%GOENV_RC_FILE%\"\n")
270278
builder.WriteString(")\n")
271279
shimsDir := filepath.Join(cfg.Root, "shims")
272-
builder.WriteString(fmt.Sprintf("IF \"%%PATH:%s=%%\" == \"%%PATH%%\" (\n", shimsDir))
273-
builder.WriteString(" IF \"%GOENV_PATH_ORDER%\" == \"back\" (\n")
274-
builder.WriteString(fmt.Sprintf(" SET PATH=%%PATH%%;%s\n", shimsDir))
275-
builder.WriteString(" ) ELSE (\n")
276-
builder.WriteString(fmt.Sprintf(" SET PATH=%s;%%PATH%%\n", shimsDir))
277-
builder.WriteString(" )\n")
280+
builder.WriteString("REM Remove shims from PATH if present (to re-add in correct position)\n")
281+
builder.WriteString(fmt.Sprintf("CALL SET PATH=%%PATH:%s;=%%\n", shimsDir))
282+
builder.WriteString(fmt.Sprintf("CALL SET PATH=%%PATH:;%s=%%\n", shimsDir))
283+
builder.WriteString("REM Add shims directory to PATH in correct position\n")
284+
builder.WriteString("IF \"%GOENV_PATH_ORDER%\" == \"back\" (\n")
285+
builder.WriteString(fmt.Sprintf(" SET PATH=%%PATH%%;%s\n", shimsDir))
286+
builder.WriteString(") ELSE (\n")
287+
builder.WriteString(fmt.Sprintf(" SET PATH=%s;%%PATH%%\n", shimsDir))
278288
builder.WriteString(")\n")
279289
default:
280290
fmt.Fprintf(&builder, "export GOENV_SHELL=%s\n", shell)
@@ -298,12 +308,23 @@ func renderInitScript(shell shellutil.ShellType, cfg *config.Config, noRehash bo
298308
builder.WriteString("done <<< \"$PATH\"\n")
299309
builder.WriteString("export PATH=\"$_NEW_PATH\"\n")
300310
builder.WriteString("unset _NEW_PATH\n")
301-
builder.WriteString("if [ \"${PATH#*$GOENV_ROOT/shims}\" = \"${PATH}\" ]; then\n")
302-
builder.WriteString(" if [ \"${GOENV_PATH_ORDER:-front}\" = \"back\" ] ; then\n")
303-
builder.WriteString(" export PATH=\"${PATH}:${GOENV_ROOT}/shims\"\n")
304-
builder.WriteString(" else\n")
305-
builder.WriteString(" export PATH=\"${GOENV_ROOT}/shims:${PATH}\"\n")
306-
builder.WriteString(" fi\n")
311+
builder.WriteString("# Remove shims from PATH if present (to re-add in correct position)\n")
312+
builder.WriteString("_NEW_PATH=\"\"\n")
313+
builder.WriteString("while IFS=: read -ra PATHARRAY; do\n")
314+
builder.WriteString(" for i in \"${PATHARRAY[@]}\"; do\n")
315+
builder.WriteString(" case \"$i\" in\n")
316+
builder.WriteString(" \"$GOENV_ROOT\"/shims) ;;\n")
317+
builder.WriteString(" *) _NEW_PATH=\"${_NEW_PATH}${_NEW_PATH:+:}$i\" ;;\n")
318+
builder.WriteString(" esac\n")
319+
builder.WriteString(" done\n")
320+
builder.WriteString("done <<< \"$PATH\"\n")
321+
builder.WriteString("export PATH=\"$_NEW_PATH\"\n")
322+
builder.WriteString("unset _NEW_PATH\n")
323+
builder.WriteString("# Add shims directory to PATH in correct position\n")
324+
builder.WriteString("if [ \"${GOENV_PATH_ORDER:-front}\" = \"back\" ] ; then\n")
325+
builder.WriteString(" export PATH=\"${PATH}:${GOENV_ROOT}/shims\"\n")
326+
builder.WriteString("else\n")
327+
builder.WriteString(" export PATH=\"${GOENV_ROOT}/shims:${PATH}\"\n")
307328
builder.WriteString("fi\n")
308329
}
309330

‎cmd/shell/init_test.go‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,8 @@ func TestInitCommand(t *testing.T) {
112112
"GOENV_RC_FILE=\"${HOME}/.goenvrc\"",
113113
"if [ -e \"${GOENV_RC_FILE:-}\" ]; then",
114114
"source \"${GOENV_RC_FILE}\"",
115-
"if [ \"${PATH#*$GOENV_ROOT/shims}\" = \"${PATH}\" ]; then",
115+
"# Remove shims from PATH if present (to re-add in correct position)",
116+
"# Add shims directory to PATH in correct position",
116117
"export PATH=\"${GOENV_ROOT}/shims:${PATH}\"",
117118
"export PATH=\"${PATH}:${GOENV_ROOT}/shims\"",
118119
"command goenv sh-rehash --only-manage-paths 2>/dev/null",
@@ -145,7 +146,8 @@ func TestInitCommand(t *testing.T) {
145146
"set GOENV_RC_FILE $HOME/.goenvrc",
146147
"if test -e $GOENV_RC_FILE",
147148
"source $GOENV_RC_FILE",
148-
"if not contains $GOENV_ROOT/shims $PATH",
149+
"# Remove shims from PATH if present (to re-add in correct position)",
150+
"# Add shims directory to PATH in correct position",
149151
"set -gx PATH $GOENV_ROOT/shims $PATH",
150152
"command goenv sh-rehash --only-manage-paths 2>/dev/null",
151153
"function goenv",

0 commit comments

Comments
 (0)