Skip to content

Commit

Permalink
Fix finding Python within virtualenv on Windows
Browse files Browse the repository at this point in the history
  • Loading branch information
denik committed Dec 18, 2024
1 parent 6b4b908 commit a1fc119
Show file tree
Hide file tree
Showing 5 changed files with 110 additions and 0 deletions.
3 changes: 3 additions & 0 deletions .github/workflows/push.yml
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,9 @@ jobs:
with:
python-version: '3.9'

- name: Install uv
uses: astral-sh/setup-uv@v4

- name: Set go env
run: |
echo "GOPATH=$(go env GOPATH)" >> $GITHUB_ENV
Expand Down
26 changes: 26 additions & 0 deletions internal/testutil/cmd.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
package testutil

import (
"bytes"
"os"
"os/exec"

"github.com/stretchr/testify/require"
)

func RunCommand(t TestingT, name string, args ...string) {
cmd := exec.Command(name, args...)
cmd.Stdout = os.Stdout
cmd.Stderr = os.Stderr
require.NoError(t, cmd.Run())
}

func CaptureCommandOutput(t TestingT, name string, args ...string) string {
cmd := exec.Command(name, args...)
var stdout bytes.Buffer
cmd.Stdout = &stdout
cmd.Stderr = os.Stderr
err := cmd.Run()
require.NoError(t, err)
return stdout.String()
}
22 changes: 22 additions & 0 deletions internal/testutil/env.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,3 +61,25 @@ func Chdir(t TestingT, dir string) string {

return wd
}

func InsertPathEntry(t TestingT, path string) {
var separator string
if runtime.GOOS == "windows" {
separator = ";"
} else {
separator = ":"
}

t.Setenv("PATH", path+separator+os.Getenv("PATH"))
}

func InsertVirtualenvInPath(t TestingT, venvPath string) {
if runtime.GOOS == "windows" {
// https://github.com/pypa/virtualenv/commit/993ba1316a83b760370f5a3872b3f5ef4dd904c1
venvPath = filepath.Join(venvPath, "Scripts")
} else {
venvPath = filepath.Join(venvPath, "bin")
}

InsertPathEntry(t, venvPath)
}
17 changes: 17 additions & 0 deletions libs/python/detect.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,32 @@ func DetectExecutable(ctx context.Context) (string, error) {
// the parent directory tree.
//
// See https://github.com/pyenv/pyenv#understanding-python-version-selection

// On Windows when virtualenv is created, the <env>/Scripts directory
// contains python.exe but no python3.exe. However, system python does have python3 entry
// and it is also added to PATH, so it is found first.
if runtime.GOOS == "windows" {
out, err := exec.LookPath("python.exe")
if err == nil && out != "" {
return out, nil
}
if err != nil && !errors.Is(err, exec.ErrNotFound) {
return "", err
}
}

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 && !errors.Is(err, exec.ErrNotFound) {
return "", err
}

if out != "" {
return out, nil
}

// otherwise, detect all interpreters and pick the least that satisfies
// minimal version requirements
all, err := DetectInterpreters(ctx)
Expand Down
42 changes: 42 additions & 0 deletions libs/python/uv_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
package python

import (
"context"
"path/filepath"
"strings"
"testing"

"github.com/databricks/cli/internal/testutil"
"github.com/stretchr/testify/assert"
)

func setupVenvWithUV(t testutil.TestingT, ctx context.Context, directory, pythonVersion string) {
venvName := testutil.RandomName("test-venv-")
testutil.RunCommand(t, "uv", "venv", venvName, "--python", pythonVersion, "--seed")

testutil.InsertVirtualenvInPath(t, filepath.Join(directory, venvName))

pythonExe, err := DetectExecutable(ctx)
assert.NoError(t, err)
assert.Contains(t, pythonExe, venvName)

actualVersion := testutil.CaptureCommandOutput(t, pythonExe, "--version")
expectVersion := "Python " + pythonVersion
assert.True(t, strings.HasPrefix(actualVersion, expectVersion), "Running %s --version: Expected %v, got %v", pythonExe, expectVersion, actualVersion)
}

func TestVenv(t *testing.T) {
// Test at least two version to ensure we capture a case where venv version does not match system one
for _, pythonVersion := range []string{"3.11", "3.12"} {
t.Run(pythonVersion, func(t *testing.T) {
testVenv(t, pythonVersion)
})
}
}

func testVenv(t *testing.T, pythonVersion string) {
ctx := context.Background()
tmpDir := t.TempDir()
testutil.Chdir(t, tmpDir)
setupVenvWithUV(t, ctx, tmpDir, pythonVersion)
}

0 comments on commit a1fc119

Please sign in to comment.