diff --git a/pkg/tools/base_tool.go b/pkg/tools/base_tool.go index 5faf1f6..269b5a1 100644 --- a/pkg/tools/base_tool.go +++ b/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 } diff --git a/pkg/tools/java.go b/pkg/tools/java.go index ddbf246..13ce3c5 100644 --- a/pkg/tools/java.go +++ b/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 diff --git a/pkg/tools/java_test.go b/pkg/tools/java_test.go index 43a9db8..5838da8 100644 --- a/pkg/tools/java_test.go +++ b/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") } } diff --git a/pkg/tools/manager.go b/pkg/tools/manager.go index a9b4a8b..cd7867f 100644 --- a/pkg/tools/manager.go +++ b/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) diff --git a/pkg/tools/system.go b/pkg/tools/system.go index 237dda9..4f7effe 100644 --- a/pkg/tools/system.go +++ b/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_ 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