some comments

Author: nfx

libs/python/detect.go

@@ -2,32 +2,23 @@ package python
 
 import (
 	"context"
-	"runtime"
-	"strings"
-
-	"github.com/databricks/cli/libs/process"
+	"errors"
+	"os/exec"
 )
 
-var detectPythonBinaryName = "python3"
-
 func DetectExecutable(ctx context.Context) (string, error) {
 	// TODO: add a shortcut if .python-version file is detected somewhere in
 	// the parent directory tree.
 	//
 	// See https://github.com/pyenv/pyenv#understanding-python-version-selection
-	detector := "which"
-	if runtime.GOOS == "windows" {
-		detector = "where.exe"
-	}
-	out, err := process.Background(ctx, []string{detector, detectPythonBinaryName})
+	out, err := exec.LookPath("python3")
 	// most of the OS'es have python3 in $PATH, but for those which don't,
 	// we perform the latest version lookup
-	if err != nil && !strings.Contains(err.Error(), "not found") {
+	if err != nil && !errors.Is(err, exec.ErrNotFound) {
 		return "", err
 	}
 	if out != "" {
-		res := strings.Split(out, "\n")
-		return strings.TrimSpace(res[0]), nil
+		return out, nil
 	}
 	// otherwise, detect all interpreters and pick the least that satisfies
 	// minimal version requirements
@@ -39,5 +30,5 @@ func DetectExecutable(ctx context.Context) (string, error) {
 	if err != nil {
 		return "", err
 	}
-	return interpreter.Binary, nil
+	return interpreter.Path, nil
 }

libs/python/interpreters.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"io/fs"
 	"os"
 	"path/filepath"
 	"sort"
@@ -16,11 +17,11 @@ import (
 
 type Interpreter struct {
 	Version string
-	Binary  string
+	Path    string
 }
 
 func (i Interpreter) String() string {
-	return fmt.Sprintf("%s (%s)", i.Version, i.Binary)
+	return fmt.Sprintf("%s (%s)", i.Version, i.Path)
 }
 
 type allInterpreters []Interpreter
@@ -52,10 +53,14 @@ func DetectInterpreters(ctx context.Context) (allInterpreters, error) {
 	seen := map[string]bool{}
 	for _, prefix := range paths {
 		entries, err := os.ReadDir(prefix)
-		if os.IsNotExist(err) {
+		if errors.Is(err, fs.ErrNotExist) {
 			// some directories in $PATH may not exist
 			continue
 		}
+		if errors.Is(err, fs.ErrPermission) {
+			// some directories we cannot list
+			continue
+		}
 		if err != nil {
 			return nil, fmt.Errorf("listing %s: %w", prefix, err)
 		}
@@ -92,12 +97,11 @@ func DetectInterpreters(ctx context.Context) (allInterpreters, error) {
 			// and parsing the output.
 			out, err := process.Background(ctx, []string{resolved, "--version"})
 			if err != nil {
-				// TODO: skip-and-log or return?
 				log.Debugf(ctx, "failed to check version for %s: %s", resolved, err)
 				continue
 			}
 
-			words := strings.Split(out, " ")
+			words := strings.Split(strings.TrimSpace(out), " ")
 			// The Python distribution from the Windows Store is available in $PATH as `python.exe`
 			// and `python3.exe`, even though it symlinks to a real file packaged with some versions of Windows:
 			// /c/Program Files/WindowsApps/Microsoft.DesktopAppInstaller_.../AppInstallerPythonRedirector.exe.
@@ -126,7 +130,7 @@ func DetectInterpreters(ctx context.Context) (allInterpreters, error) {
 			}
 			found = append(found, Interpreter{
 				Version: version,
-				Binary:  resolved,
+				Path:    resolved,
 			})
 		}
 	}

libs/python/interpreters_test.go

@@ -34,9 +34,9 @@ func TestFilteringInterpreters(t *testing.T) {
 	assert.Len(t, all, 3)
 	assert.Equal(t, "v2.7.18", all[0].Version)
 	assert.Equal(t, "v3.10.5", all[1].Version)
-	assert.Equal(t, "testdata/other-binaries-filtered/python3.10", all[1].Binary)
+	assert.Equal(t, "testdata/other-binaries-filtered/python3.10", all[1].Path)
 	assert.Equal(t, "v3.11.4", all[2].Version)
-	assert.Equal(t, "testdata/other-binaries-filtered/real-python3.11.4", all[2].Binary)
+	assert.Equal(t, "testdata/other-binaries-filtered/real-python3.11.4", all[2].Path)
 }
 
 func TestInterpretersAtLeastInvalidSemver(t *testing.T) {
@@ -59,7 +59,7 @@ func TestInterpretersAtLeast(t *testing.T) {
 
 	interpreter, err := all.AtLeast("3.10")
 	assert.NoError(t, err)
-	assert.Equal(t, "testdata/other-binaries-filtered/python3.10", interpreter.Binary)
+	assert.Equal(t, "testdata/other-binaries-filtered/python3.10", interpreter.Path)
 }
 
 func TestInterpretersAtLeastNotSatisfied(t *testing.T) {