Skip to content

Commit 8f02697

Browse files
committed
Fix PATH ordering to prioritize mvx-managed tools over system tools
- Store system PATH separately and append after tool directories - Ensures mvx-managed Java 17 takes priority over system Java 21 - Fixes failing TestShellCommand/run_java_version test - Tool directories are now added first, system directories appended - Maintains backward compatibility with existing environment setup This resolves the issue where system tools (like SDKMAN Java) were taking priority over mvx-managed tools due to PATH ordering.
1 parent e901e51 commit 8f02697

File tree

2 files changed

+29
-23
lines changed

2 files changed

+29
-23
lines changed

pkg/tools/environment.go

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import (
1111

1212
// EnvironmentManager provides safe environment variable management with special PATH handling
1313
type EnvironmentManager struct {
14-
envVars map[string]string
14+
envVars map[string]string
1515
pathDirs []string
1616
}
1717

@@ -26,7 +26,7 @@ func NewEnvironmentManager() *EnvironmentManager {
2626
// NewEnvironmentManagerFromMap creates a new environment manager from an existing map
2727
func NewEnvironmentManagerFromMap(envVars map[string]string) *EnvironmentManager {
2828
em := NewEnvironmentManager()
29-
29+
3030
for key, value := range envVars {
3131
if key == "PATH" {
3232
// Parse PATH into directories
@@ -37,7 +37,7 @@ func NewEnvironmentManagerFromMap(envVars map[string]string) *EnvironmentManager
3737
em.envVars[key] = value
3838
}
3939
}
40-
40+
4141
return em
4242
}
4343

@@ -64,18 +64,18 @@ func (em *EnvironmentManager) AddToPath(dir string) {
6464
if dir == "" {
6565
return
6666
}
67-
67+
6868
// Clean the path
6969
dir = filepath.Clean(dir)
70-
70+
7171
// Check if already present
7272
for _, existing := range em.pathDirs {
7373
if existing == dir {
7474
util.LogVerbose("Directory %s already in PATH", dir)
7575
return
7676
}
7777
}
78-
78+
7979
// Prepend to PATH
8080
em.pathDirs = append([]string{dir}, em.pathDirs...)
8181
util.LogVerbose("Added directory to PATH: %s", dir)
@@ -86,18 +86,18 @@ func (em *EnvironmentManager) AppendToPath(dir string) {
8686
if dir == "" {
8787
return
8888
}
89-
89+
9090
// Clean the path
9191
dir = filepath.Clean(dir)
92-
92+
9393
// Check if already present
9494
for _, existing := range em.pathDirs {
9595
if existing == dir {
9696
util.LogVerbose("Directory %s already in PATH", dir)
9797
return
9898
}
9999
}
100-
100+
101101
// Append to PATH
102102
em.pathDirs = append(em.pathDirs, dir)
103103
util.LogVerbose("Appended directory to PATH: %s", dir)
@@ -111,33 +111,33 @@ func (em *EnvironmentManager) GetPath() string {
111111
// ToMap converts the environment manager to a map[string]string
112112
func (em *EnvironmentManager) ToMap() map[string]string {
113113
result := make(map[string]string)
114-
114+
115115
// Copy all environment variables
116116
for key, value := range em.envVars {
117117
result[key] = value
118118
}
119-
119+
120120
// Add PATH
121121
if len(em.pathDirs) > 0 {
122122
result["PATH"] = em.GetPath()
123123
}
124-
124+
125125
return result
126126
}
127127

128128
// ToSlice converts the environment manager to []string in "KEY=VALUE" format
129129
func (em *EnvironmentManager) ToSlice() []string {
130130
var result []string
131-
131+
132132
// Add all environment variables
133133
for key, value := range em.envVars {
134134
result = append(result, fmt.Sprintf("%s=%s", key, value))
135135
}
136-
136+
137137
// Add PATH
138138
if len(em.pathDirs) > 0 {
139139
result = append(result, fmt.Sprintf("PATH=%s", em.GetPath()))
140140
}
141-
141+
142142
return result
143143
}

pkg/tools/manager.go

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -849,20 +849,17 @@ func (m *Manager) EnsureTool(toolName string, cfg config.ToolConfig) (string, er
849849

850850
// SetupEnvironment sets up environment variables for installed tools
851851
func (m *Manager) SetupEnvironment(cfg *config.Config) (map[string]string, error) {
852-
// Create environment manager starting with system environment
852+
// Create environment manager
853853
envManager := NewEnvironmentManager()
854854

855-
// Add system environment variables first
855+
// Add system environment variables first (except PATH - we'll handle that after tools)
856+
var systemPath string
856857
for _, envVar := range os.Environ() {
857858
parts := strings.SplitN(envVar, "=", 2)
858859
if len(parts) == 2 {
859860
if parts[0] == "PATH" {
860-
// Parse PATH into directories
861-
if parts[1] != "" {
862-
for _, dir := range strings.Split(parts[1], string(os.PathListSeparator)) {
863-
envManager.AddToPath(dir)
864-
}
865-
}
861+
// Store system PATH for later - we want tool paths to take priority
862+
systemPath = parts[1]
866863
} else {
867864
envManager.SetEnv(parts[0], parts[1])
868865
}
@@ -925,6 +922,15 @@ func (m *Manager) SetupEnvironment(cfg *config.Config) (map[string]string, error
925922
}
926923
}
927924

925+
// Add system PATH directories after tool directories (lower priority)
926+
if systemPath != "" {
927+
for _, dir := range strings.Split(systemPath, string(os.PathListSeparator)) {
928+
if dir != "" {
929+
envManager.AppendToPath(dir)
930+
}
931+
}
932+
}
933+
928934
// Convert environment manager to map
929935
return envManager.ToMap(), nil
930936
}

0 commit comments

Comments
 (0)