Fix system tool

pkg/tools/base_tool.go

@@ -352,11 +352,29 @@ func (b *BaseTool) SetupHomeEnvironment(version string, cfg config.ToolConfig, e
 		return nil
 	}
 
+	// If binPath is empty, this might be a system tool already in PATH
+	// Try to get the HOME directory from the system tool detection
+	if binPath == "" && UseSystemTool(b.toolName) {
+		homeEnvVar := getSystemToolHomeEnvVar(b.toolName)
+		if homeEnvVar != "" && homeEnvVar == envVarName {
+			// Try to get the system tool home directory
+			if homeDir, _, err := getSystemToolHome(b.toolName, b.GetBinaryName(), homeEnvVar); err == nil {
+				envVars[envVarName] = homeDir
+				util.LogVerbose("Set %s=%s for system %s %s", envVarName, homeDir, b.toolName, version)
+				return nil
+			}
+		}
+		// If we can't determine HOME, that's okay - the tool might work from PATH alone
+		return nil
+	}
+
 	// *_HOME should point to the installation directory, not the bin directory
-	if strings.HasSuffix(binPath, "/bin") {
-		homeDir := strings.TrimSuffix(binPath, "/bin")
-		envVars[envVarName] = homeDir
-		util.LogVerbose("Set %s=%s for %s %s", envVarName, homeDir, b.toolName, version)
+	if binPath != "" {
+		if strings.HasSuffix(binPath, "/bin") || strings.HasSuffix(binPath, "\\bin") {
+			homeDir := strings.TrimSuffix(strings.TrimSuffix(binPath, "/bin"), "\\bin")
+			envVars[envVarName] = homeDir
+			util.LogVerbose("Set %s=%s for %s %s", envVarName, homeDir, b.toolName, version)
+		}
 	}
 
 	return nil
@@ -644,11 +662,21 @@ func (b *BaseTool) ListInstalledVersions(distribution string) []InstalledVersion
 // StandardIsInstalled provides standard installation check for tools
 func (b *BaseTool) StandardIsInstalled(versionSpec string, cfg config.ToolConfig, getPath func(string, config.ToolConfig) (string, error)) bool {
 	if UseSystemTool(b.toolName) {
+		// Try primary binary name in PATH first
 		if _, err := exec.LookPath(b.GetBinaryName()); err == nil {
 			util.LogVerbose("System %s is available in PATH (MVX_USE_SYSTEM_%s=true)", b.toolName, strings.ToUpper(b.toolName))
 			return true
 		}
 
+		// If not in PATH, try to get HOME environment variable (which can derive from HOME env var or PATH)
+		homeEnvVar := getSystemToolHomeEnvVar(b.toolName)
+		if homeEnvVar != "" {
+			if _, _, err := getSystemToolHome(b.toolName, b.GetBinaryName(), homeEnvVar); err == nil {
+				util.LogVerbose("System %s is available via %s (MVX_USE_SYSTEM_%s=true)", b.toolName, homeEnvVar, strings.ToUpper(b.toolName))
+				return true
+			}
+		}
+
 		util.LogVerbose("System %s not available: not found in environment variables or PATH", b.toolName)
 		return false
 	}
@@ -795,14 +823,26 @@ func (b *BaseTool) StandardGetPath(version string, cfg config.ToolConfig, getIns
 	}
 	// Check if we should use system tool instead of mvx-managed tool
 	if UseSystemTool(b.toolName) {
-		// Try primary binary name in PATH
+		// Try to find system tool - first check if it's in PATH
 		if _, err := exec.LookPath(b.GetBinaryName()); err == nil {
 			util.LogVerbose("Using system %s from PATH (MVX_USE_SYSTEM_%s=true)", b.toolName, strings.ToUpper(b.toolName))
 			b.setCachedPath(cacheKey, "", nil)
 			return "", nil
 		}
 
-		systemErr := SystemToolError(b.toolName, fmt.Errorf("MVX_USE_SYSTEM_%s=true but system %s not available", strings.ToUpper(b.toolName), b.toolName))
+		// If not in PATH, try to get HOME environment variable and use its bin directory
+		homeEnvVar := getSystemToolHomeEnvVar(b.toolName)
+		if homeEnvVar != "" {
+			_, binDir, err := getSystemToolHome(b.toolName, b.GetBinaryName(), homeEnvVar)
+			if err == nil {
+				util.LogVerbose("Using system %s from %s/bin (MVX_USE_SYSTEM_%s=true, adding to PATH)", b.toolName, homeEnvVar, strings.ToUpper(b.toolName))
+				b.setCachedPath(cacheKey, binDir, nil)
+				return binDir, nil
+			}
+		}
+
+		// Neither in PATH nor HOME available
+		systemErr := SystemToolError(b.toolName, fmt.Errorf("MVX_USE_SYSTEM_%s=true but system %s not available (not in PATH and %s not set or invalid)", strings.ToUpper(b.toolName), b.toolName, homeEnvVar))
 		b.setCachedPath(cacheKey, "", systemErr)
 		return "", systemErr
 	}

pkg/tools/java.go

@@ -31,23 +31,6 @@ type DiscoDistribution struct {
 	Available    bool   `json:"available"`
 }
 
-// getSystemJavaHome returns the system JAVA_HOME if available and valid
-func getSystemJavaHome() (string, error) {
-	javaHome := os.Getenv(EnvJavaHome)
-	if javaHome == "" {
-		return "", SystemToolError(ToolJava, fmt.Errorf("%s environment variable not set", EnvJavaHome))
-	}
-
-	// Check if JAVA_HOME points to a valid Java installation
-	javaExe := filepath.Join(javaHome, "bin", getJavaBinaryName())
-
-	if _, err := os.Stat(javaExe); err != nil {
-		return "", SystemToolError(ToolJava, fmt.Errorf("Java executable not found at %s: %w", javaExe, err))
-	}
-
-	return javaHome, nil
-}
-
 // getSystemJavaVersion returns the version of the system Java installation
 func getSystemJavaVersion(javaHome string) (string, error) {
 	javaExe := filepath.Join(javaHome, "bin", getJavaBinaryName())
@@ -260,17 +243,6 @@ func (j *JavaTool) getDownloadURLWithChecksum(version, distribution string) (str
 
 // IsInstalled checks if the specified version is installed
 func (j *JavaTool) IsInstalled(version string, cfg config.ToolConfig) bool {
-	if UseSystemTool(j.toolName) {
-		// Try primary binary name in PATH
-		if _, err := exec.LookPath(j.GetBinaryName()); err == nil {
-			util.LogVerbose("System %s is available in PATH (MVX_USE_SYSTEM_%s=true)", j.toolName, strings.ToUpper(j.toolName))
-			return true
-		}
-
-		util.LogVerbose("System %s not available: not found in environment variables or PATH", j.toolName)
-		return false
-	}
-
 	// Resolve the full version string
 	distribution := cfg.Distribution
 	if distribution == "" {
@@ -282,7 +254,7 @@ func (j *JavaTool) IsInstalled(version string, cfg config.ToolConfig) bool {
 		return false
 	}
 
-	// Use standardized installation check with Java-specific environment variables
+	// Use standardized installation check (includes system tool detection)
 	return j.StandardIsInstalled(fullVersion, cfg, j.GetPath)
 }
 
@@ -312,8 +284,9 @@ func (j *JavaTool) getJavaHomeUncached(version string, cfg config.ToolConfig) (s
 
 	// If using system Java, return system JAVA_HOME if available (no version compatibility check)
 	if UseSystemTool(ToolJava) {
-		if systemJavaHome, err := getSystemJavaHome(); err == nil {
-			util.LogVerbose("Using system Java from %s: %s (MVX_USE_SYSTEM_JAVA=true)", EnvJavaHome, systemJavaHome)
+		homeEnvVar := getSystemToolHomeEnvVar(ToolJava)
+		if systemJavaHome, _, err := getSystemToolHome(ToolJava, j.GetBinaryName(), homeEnvVar); err == nil {
+			util.LogVerbose("Using system Java from %s: %s (MVX_USE_SYSTEM_JAVA=true)", homeEnvVar, systemJavaHome)
 			return systemJavaHome, nil
 		} else {
 			return "", EnvironmentError(ToolJava, version, fmt.Errorf("MVX_USE_SYSTEM_JAVA=true but system Java not available: %w", err))
@@ -383,7 +356,7 @@ func (j *JavaTool) GetBinaryName() string {
 
 // GetPath returns the binary path for the specified version (for PATH management)
 func (j *JavaTool) GetPath(version string, cfg config.ToolConfig) (string, error) {
-	// Use standardized path resolution with Java-specific environment variables
+	// Use standardized path resolution (includes system tool detection)
 	return j.StandardGetPath(version, cfg, j.getInstalledPath)
 }
 
@@ -496,7 +469,7 @@ func (j *JavaTool) GetDisplayName() string {
 
 // SetupEnvironment sets up Java-specific environment variables (implements EnvironmentProvider)
 func (j *JavaTool) SetupEnvironment(version string, cfg config.ToolConfig, envManager *EnvironmentManager) error {
-	// Convert EnvironmentManager to map for the existing helper
+	// Use the standard SetupHomeEnvironment which works for both system and mvx-managed tools
 	envVars := envManager.ToMap()
 	err := j.SetupHomeEnvironment(version, cfg, envVars, EnvJavaHome, j.GetPath)
 	// Update the environment manager with any changes

pkg/tools/java_test.go

@@ -78,18 +78,30 @@ func TestJavaSystemDetector(t *testing.T) {
 		}
 	}()
 
-	// Test when JAVA_HOME is not set
+	// Test when JAVA_HOME is not set and java is not in PATH
 	os.Unsetenv("JAVA_HOME")
-	_, err := getSystemJavaHome()
-	if err == nil {
-		t.Error("getSystemJavaHome() should return error when JAVA_HOME is not set")
+	// Note: This test might pass if java is in PATH, which is fine
+	// We're testing that the generic function handles the case gracefully
+	javaBinaryName := BinaryJava
+	if runtime.GOOS == "windows" {
+		javaBinaryName = BinaryJava + ".exe"
+	}
+	_, _, err := getSystemToolHome(ToolJava, javaBinaryName, EnvJavaHome)
+	if err != nil {
+		// This is expected if java is not in PATH and JAVA_HOME is not set
+		t.Logf("getSystemToolHome() returned error as expected: %v", err)
 	}
 
 	// Test when JAVA_HOME points to non-existent directory
 	os.Setenv("JAVA_HOME", "/non/existent/path")
-	_, err = getSystemJavaHome()
-	if err == nil {
-		t.Error("getSystemJavaHome() should return error when JAVA_HOME points to non-existent directory")
+	_, _, err = getSystemToolHome(ToolJava, javaBinaryName, EnvJavaHome)
+	// Note: getSystemToolHome() will fall back to PATH if JAVA_HOME is invalid
+	// So this test will only fail if java is also not in PATH
+	// This is the expected behavior - we want to be permissive and use PATH as fallback
+	if err != nil {
+		t.Logf("getSystemToolHome() returned error (java not in PATH): %v", err)
+	} else {
+		t.Logf("getSystemToolHome() succeeded by falling back to PATH despite invalid JAVA_HOME")
 	}
 }
 

pkg/tools/manager.go

@@ -772,13 +772,6 @@ func (m *Manager) SetupEnvironment(cfg *config.Config) (map[string]string, error
 
 	// Add tool-specific environment variables and PATH entries
 	for toolName, toolConfig := range cfg.Tools {
-		// Check if user wants to use system tool instead
-		systemEnvVar := fmt.Sprintf("MVX_USE_SYSTEM_%s", strings.ToUpper(toolName))
-		if os.Getenv(systemEnvVar) == "true" {
-			util.LogVerbose("Skipping %s environment setup: %s=true (using system tool)", toolName, systemEnvVar)
-			continue
-		}
-
 		// Resolve version
 		resolvedVersion, err := m.resolveVersion(toolName, toolConfig)
 		if err != nil {
@@ -789,7 +782,7 @@ func (m *Manager) SetupEnvironment(cfg *config.Config) (map[string]string, error
 		resolvedConfig := toolConfig
 		resolvedConfig.Version = resolvedVersion
 
-		// Check if installed (using cache)
+		// Check if installed (using cache) - this works for both system and mvx-managed tools
 		if !m.isToolInstalled(toolName, resolvedVersion, resolvedConfig) {
 			util.LogVerbose("Skipping tool %s: not installed", toolName)
 			continue // Skip uninstalled tools
@@ -802,18 +795,24 @@ func (m *Manager) SetupEnvironment(cfg *config.Config) (map[string]string, error
 			continue
 		}
 
-		// Get tool path and add to PATH
+		// Get tool path and add to PATH (returns empty string for system tools already in PATH)
 		toolPath, err := tool.GetPath(resolvedVersion, resolvedConfig)
 		if err != nil {
 			util.LogVerbose("Skipping tool %s: failed to get path: %v", toolName, err)
 			continue
 		}
 
-		// Add tool bin directory to PATH
-		envManager.AddToPath(toolPath)
-		util.LogVerbose("Added %s bin path to PATH: %s", toolName, toolPath)
+		// Add tool bin directory to PATH (only if path is not empty)
+		// For system tools already in PATH, toolPath will be empty and we skip adding it
+		if toolPath != "" {
+			envManager.AddToPath(toolPath)
+			util.LogVerbose("Added %s bin path to PATH: %s", toolName, toolPath)
+		} else {
+			util.LogVerbose("System %s is in PATH, not adding to PATH", toolName)
+		}
 
 		// Set tool-specific environment variables (HOME directories, etc.)
+		// This is important for system tools too - they may need HOME variables set
 		if envProvider, ok := tool.(EnvironmentProvider); ok {
 			if err := envProvider.SetupEnvironment(resolvedVersion, resolvedConfig, envManager); err != nil {
 				util.LogVerbose("Failed to setup environment for %s %s: %v", toolName, resolvedVersion, err)

pkg/tools/system.go

@@ -4,9 +4,21 @@ import (
 	"fmt"
 	"os"
 	"os/exec"
+	"path/filepath"
 	"strings"
+
+	"github.com/gnodet/mvx/pkg/util"
 )
 
+// toolHomeEnvVars maps tool names to their HOME environment variable names
+var toolHomeEnvVars = map[string]string{
+	ToolJava:  EnvJavaHome,
+	ToolMaven: EnvMavenHome,
+	ToolMvnd:  EnvMvndHome,
+	ToolNode:  EnvNodeHome,
+	ToolGo:    EnvGoRoot, // Go uses GOROOT instead of GO_HOME
+}
+
 // UseSystemTool checks if a system tool should be used instead of downloading
 // by checking the MVX_USE_SYSTEM_<TOOL> environment variable
 func UseSystemTool(toolName string) bool {
@@ -19,6 +31,61 @@ func getSystemToolEnvVar(toolName string) string {
 	return fmt.Sprintf("MVX_USE_SYSTEM_%s", strings.ToUpper(toolName))
 }
 
+// getSystemToolHomeEnvVar returns the HOME environment variable name for a tool, if any
+func getSystemToolHomeEnvVar(toolName string) string {
+	return toolHomeEnvVars[toolName]
+}
+
+// getSystemToolHome returns the system tool home directory if available and valid
+// It first checks the HOME environment variable (e.g., JAVA_HOME, MAVEN_HOME), then falls back to finding the tool in PATH
+// Returns the home directory and bin directory if found, or an error if not available
+func getSystemToolHome(toolName, binaryName, homeEnvVar string) (homeDir string, binDir string, err error) {
+	// First, try HOME environment variable if specified
+	if homeEnvVar != "" {
+		homeDir = os.Getenv(homeEnvVar)
+		if homeDir != "" {
+			// Check if HOME points to a valid installation
+			binDirPath := filepath.Join(homeDir, "bin")
+			binaryPath := filepath.Join(binDirPath, binaryName)
+			if _, err := os.Stat(binaryPath); err == nil {
+				util.LogVerbose("Found system %s via %s=%s", toolName, homeEnvVar, homeDir)
+				return homeDir, binDirPath, nil
+			}
+			// HOME is set but doesn't point to a valid installation
+			util.LogVerbose("%s is set to %s but %s executable not found at %s, trying PATH", homeEnvVar, homeDir, binaryName, binaryPath)
+		}
+	}
+
+	// Fallback: try to find tool in PATH and derive home directory from it
+	binaryPath, err := exec.LookPath(binaryName)
+	if err != nil {
+		if homeEnvVar != "" {
+			return "", "", SystemToolError(toolName, fmt.Errorf("%s not found: %s environment variable not set or invalid and %s not found in PATH", toolName, homeEnvVar, binaryName))
+		}
+		return "", "", SystemToolError(toolName, fmt.Errorf("%s not found in PATH", binaryName))
+	}
+
+	// Resolve the absolute path (follow symlinks)
+	absPath, err := filepath.EvalSymlinks(binaryPath)
+	if err != nil {
+		absPath = binaryPath
+	}
+
+	// Derive home directory from binary path
+	// Binary is typically at $HOME/bin/binary (or bin/binary.exe on Windows)
+	binDirPath := filepath.Dir(absPath)
+	if filepath.Base(binDirPath) != "bin" {
+		// Some tools might be installed directly without a bin directory structure
+		// In this case, we can't derive a home directory, but we can still use the binary
+		util.LogVerbose("Found %s at %s but not in a 'bin' directory, using directory as-is", toolName, absPath)
+		return filepath.Dir(binDirPath), binDirPath, nil
+	}
+
+	homeDir = filepath.Dir(binDirPath)
+	util.LogVerbose("Derived %s home directory=%s from binary at %s", toolName, homeDir, absPath)
+	return homeDir, binDirPath, nil
+}
+
 // SystemToolInfo contains information about a detected system tool
 type SystemToolInfo struct {
 	Path    string // Full path to the tool executable