Skip to content

Commit 688c47e

Browse files
fix(shims): Preserve system PATH case on Windows in goenv exec (#561)
* fix(shims): Preserve system PATH case on Windows in goenv exec (#557) Windows environment variables are case-insensitive but os.Environ() preserves the original casing (e.g., Path, PATH, path). Previously, setEnvVar() and prependToPath() used case-sensitive string matching, causing duplicate environment variables when the casing didn't match exactly. This caused PATH to be overwritten instead of prepended, breaking CGO builds when gcc was in the system PATH but not found after goenv exec. Changes: - Added findEnvVar() helper that uses case-insensitive matching on Windows - Updated setEnvVar() to use findEnvVar and preserve original casing - Updated prependToPath() to use findEnvVar and preserve original casing - Added comprehensive unit tests including regression test for issue #557 Fixes #557 * [AB#557] docs: Add optional cross-platform testing with nektos/act Added instructions for developers who want to run cross-platform CI tests locally using nektos/act before pushing to GitHub. Also fixed PATH separator detection in prependToPath() to handle cross- platform test scenarios where Unix-style paths (with :) are tested on Windows runners (which default to ; separator). Changes: - Added nektos/act installation and usage guide to agent instructions - Updated when-to-use guidance to cover all OS-specific code, not just shims - Fixed prependToPath() to detect separator from existing PATH value - Updated repo memory with cross-platform testing guidance This is optional - CI always runs full cross-platform tests automatically. * [AB#557] style: Apply automated formatting - Format markdown sections with proper spacing - Align test struct fields - Clean up whitespace in exec.go * fix(test): Use count check for colon separator detection Fixed Windows test failures where drive letter colons (C:) were being incorrectly detected as path separators. Changed separator detection from: - strings.Contains(currentPath, ":") to: - strings.Count(currentPath, ":") > 1 This distinguishes between: - Drive letters: C:\Windows\System32 (one colon) - Path separators: /usr/bin:/usr/local/bin (multiple colons) Fixes test failures: - TestPrependToPath/prepend_to_existing_path_-_Windows_(lowercase) - TestPrependToPath/prepend_to_existing_PaTh_-_Windows_(mixed_case) * fix(shims): improve separator detection to handle single-colon Unix paths Previous logic failed when Unix paths had exactly 1 colon (e.g., /usr/bin:/bin) because the check was 'count > 1'. This caused Windows CI tests to incorrectly use semicolon as separator when testing Unix-style paths. New logic: - Checks for semicolons first (Windows-style) - Counts colons but excludes drive letter colons (position 1) - Any remaining colons indicate Unix-style separator - Falls back to OS default separator if neither detected Fixes Windows CI test failure for 'prepend to existing PATH - Unix' case. Issue #557
1 parent ea61d60 commit 688c47e

4 files changed

Lines changed: 400 additions & 16 deletions

File tree

.github/copilot-instructions.md

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -315,6 +315,53 @@ Before submitting a PR:
315315
4. ✅ No race conditions: `go test -race ./...` (done by `make test`)
316316
5. ✅ Windows compatibility validated (for shim/path/shell changes)
317317

318+
### Local Cross-Platform Testing (Optional)
319+
320+
**For developers who want to test against Windows/other platforms locally**, you can use [nektos/act](https://github.com/nektos/act) to run GitHub Actions workflows on your machine:
321+
322+
**Installation**:
323+
324+
```bash
325+
# macOS
326+
brew install act
327+
328+
# Linux
329+
curl https://raw.githubusercontent.com/nektos/act/master/install.sh | sudo bash
330+
331+
# Windows
332+
choco install act-cli
333+
```
334+
335+
**Usage**:
336+
337+
```bash
338+
# Run all CI tests (includes Windows)
339+
act
340+
341+
# Run specific job (e.g., Windows tests only)
342+
act -j test-windows
343+
344+
# Run with specific platform
345+
act --platform ubuntu-latest=catthehacker/ubuntu:act-latest
346+
347+
# Dry run to see what would execute
348+
act --dryrun
349+
```
350+
351+
**Note**:
352+
353+
- Windows containers require Docker Desktop with Windows containers enabled
354+
- For Windows-specific tests, you may need larger runners: `act -P windows-latest=-self-hosted`
355+
- CI will always run full cross-platform tests automatically when you push
356+
357+
**When to use local cross-platform testing**:
358+
359+
- ✅ Testing platform-specific code (Windows batch files, Unix shell scripts, path handling)
360+
- ✅ Verifying OS-specific features (environment variables, file permissions, executables)
361+
- ✅ Testing changes to shims, install scripts, or platform detection
362+
- ✅ Debugging cross-platform CI failures locally
363+
- ❌ Not needed for most changes (CI handles it automatically)
364+
318365
### Common Testing Commands
319366

320367
```bash

AGENTS.md

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,49 @@ Before submitting:
220220
4. ✅ Windows compatibility validated (if applicable)
221221
5. ✅ Coverage maintained or improved
222222

223+
### Local Cross-Platform Testing (Optional)
224+
225+
**For developers who want to test against Windows/other platforms locally**, you can use [nektos/act](https://github.com/nektos/act) to run GitHub Actions workflows on your machine:
226+
227+
**Installation**:
228+
```bash
229+
# macOS
230+
brew install act
231+
232+
# Linux
233+
curl https://raw.githubusercontent.com/nektos/act/master/install.sh | sudo bash
234+
235+
# Windows
236+
choco install act-cli
237+
```
238+
239+
**Usage**:
240+
```bash
241+
# Run all CI tests (includes Windows)
242+
act
243+
244+
# Run specific job (e.g., Windows tests only)
245+
act -j test-windows
246+
247+
# Run with specific platform
248+
act --platform ubuntu-latest=catthehacker/ubuntu:act-latest
249+
250+
# Dry run to see what would execute
251+
act --dryrun
252+
```
253+
254+
**Note**:
255+
- Windows containers require Docker Desktop with Windows containers enabled
256+
- For Windows-specific tests, you may need larger runners: `act -P windows-latest=-self-hosted`
257+
- CI will always run full cross-platform tests automatically when you push
258+
259+
**When to use local cross-platform testing**:
260+
- ✅ Testing platform-specific code (Windows batch files, Unix shell scripts, path handling)
261+
- ✅ Verifying OS-specific features (environment variables, file permissions, executables)
262+
- ✅ Testing changes to shims, install scripts, or platform detection
263+
- ✅ Debugging cross-platform CI failures locally
264+
- ❌ Not needed for most changes (CI handles it automatically)
265+
223266
### Quick Commands
224267

225268
```bash

cmd/shims/exec.go

Lines changed: 67 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -374,16 +374,44 @@ func runRehashSilent(cfg *config.Config, env *utils.GoenvEnvironment) error {
374374
return shimMgr.Rehash()
375375
}
376376

377-
// setEnvVar sets or updates an environment variable
378-
func setEnvVar(env []string, key, value string) []string {
379-
prefix := key + "="
377+
// findEnvVar finds an environment variable in the env slice using case-insensitive matching on Windows.
378+
// Returns the index and the actual key name (with original casing) if found, or -1 and empty string if not found.
379+
// On Unix systems, matching is case-sensitive.
380+
func findEnvVar(env []string, key string) (int, string) {
380381
for i, envVar := range env {
381-
if strings.HasPrefix(envVar, prefix) {
382-
env[i] = prefix + value
383-
return env
382+
// Find the equals sign to separate key from value
383+
eqIdx := strings.Index(envVar, "=")
384+
if eqIdx == -1 {
385+
continue
386+
}
387+
actualKey := envVar[:eqIdx]
388+
389+
// On Windows, environment variables are case-insensitive
390+
if utils.IsWindows() {
391+
if strings.EqualFold(actualKey, key) {
392+
return i, actualKey
393+
}
394+
} else {
395+
// On Unix, case-sensitive match
396+
if actualKey == key {
397+
return i, actualKey
398+
}
384399
}
385400
}
386-
return append(env, prefix+value)
401+
return -1, ""
402+
}
403+
404+
// setEnvVar sets or updates an environment variable.
405+
// On Windows, this handles case-insensitive environment variable names to prevent
406+
// creating duplicate entries (e.g., both "Path=" and "PATH=").
407+
func setEnvVar(env []string, key, value string) []string {
408+
if idx, actualKey := findEnvVar(env, key); idx != -1 {
409+
// Update existing variable, preserving original key casing
410+
env[idx] = actualKey + "=" + value
411+
return env
412+
}
413+
// Variable not found, add it with the requested key
414+
return append(env, key+"="+value)
387415
}
388416

389417
// buildCacheSuffix constructs a cache directory suffix that includes ABI variants.
@@ -392,17 +420,40 @@ func buildCacheSuffix(goBinaryPath, goos, goarch string, env []string) string {
392420
return cache.BuildCacheSuffix(goBinaryPath, goos, goarch, env)
393421
}
394422

395-
// prependToPath prepends a directory to the PATH environment variable
423+
// prependToPath prepends a directory to the PATH environment variable.
424+
// On Windows, this handles case-insensitive PATH matching to prevent creating duplicate
425+
// PATH entries (e.g., both "Path=" and "PATH="), which would cause the original system
426+
// PATH to be lost.
396427
func prependToPath(env []string, dir string) []string {
397-
const pathPrefix = "PATH="
398-
for i, envVar := range env {
399-
if strings.HasPrefix(envVar, pathPrefix) {
400-
currentPath := envVar[len(pathPrefix):]
401-
newPath := dir + string(os.PathListSeparator) + currentPath
402-
env[i] = pathPrefix + newPath
403-
return env
428+
if idx, actualKey := findEnvVar(env, "PATH"); idx != -1 {
429+
// Found PATH, extract current value and prepend new directory
430+
currentPath := env[idx][len(actualKey)+1:] // +1 for the "=" sign
431+
432+
// Detect separator from existing PATH to handle cross-platform scenarios
433+
// (e.g., testing Unix-style paths on Windows)
434+
sep := string(os.PathListSeparator) // default to OS separator
435+
if strings.Contains(currentPath, ";") {
436+
// Semicolon found - Windows-style separator
437+
sep = ";"
438+
} else {
439+
// Check for colons, but exclude Windows drive letter colons (e.g., "C:")
440+
colonCount := strings.Count(currentPath, ":")
441+
if len(currentPath) >= 2 && currentPath[1] == ':' {
442+
// Has a drive letter at the start, subtract it from the count
443+
colonCount--
444+
}
445+
if colonCount > 0 {
446+
// Has colons that are path separators (Unix-style)
447+
sep = ":"
448+
}
449+
// Otherwise, use OS default (already set)
404450
}
451+
452+
newPath := dir + sep + currentPath
453+
// Preserve original key casing (e.g., "Path" on Windows, "PATH" on Unix)
454+
env[idx] = actualKey + "=" + newPath
455+
return env
405456
}
406457
// PATH not found, add it
407-
return append(env, pathPrefix+dir)
458+
return append(env, "PATH="+dir)
408459
}

0 commit comments

Comments
 (0)