Fix finding Python within virtualenv on Windows (#2034)

## Changes
Simplify logic for selecting Python to run when calculating default whl
build command: "python" on Windows and "python3" everywhere.

Python installers from python.org do not install python3.exe. In
virtualenv there is no python3.exe.

## Tests
Added new unit tests to create real venv with uv and simulate activation
by prepending venv/bin to PATH.
This commit is contained in:
Denis Bilenko 2024-12-20 08:45:32 +01:00 committed by GitHub
parent 07fff20eff
commit 2fee243586
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 176 additions and 12 deletions

View File

@ -40,6 +40,9 @@ jobs:
with: with:
python-version: '3.9' python-version: '3.9'
- name: Install uv
uses: astral-sh/setup-uv@v4
- name: Set go env - name: Set go env
run: | run: |
echo "GOPATH=$(go env GOPATH)" >> $GITHUB_ENV echo "GOPATH=$(go env GOPATH)" >> $GITHUB_ENV

View File

@ -16,12 +16,6 @@ type infer struct {
func (m *infer) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { func (m *infer) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
artifact := b.Config.Artifacts[m.name] artifact := b.Config.Artifacts[m.name]
// TODO use python.DetectVEnvExecutable once bundle has a way to specify venv path
py, err := python.DetectExecutable(ctx)
if err != nil {
return diag.FromErr(err)
}
// Note: using --build-number (build tag) flag does not help with re-installing // Note: using --build-number (build tag) flag does not help with re-installing
// libraries on all-purpose clusters. The reason is that `pip` ignoring build tag // libraries on all-purpose clusters. The reason is that `pip` ignoring build tag
// when upgrading the library and only look at wheel version. // when upgrading the library and only look at wheel version.
@ -36,7 +30,9 @@ func (m *infer) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
// version=datetime.datetime.utcnow().strftime("%Y%m%d.%H%M%S"), // version=datetime.datetime.utcnow().strftime("%Y%m%d.%H%M%S"),
// ... // ...
//) //)
artifact.BuildCommand = fmt.Sprintf(`"%s" setup.py bdist_wheel`, py)
py := python.GetExecutable()
artifact.BuildCommand = fmt.Sprintf(`%s setup.py bdist_wheel`, py)
return nil return nil
} }

View File

@ -541,7 +541,7 @@ func TestLoadDiagnosticsFile_nonExistent(t *testing.T) {
func TestInterpreterPath(t *testing.T) { func TestInterpreterPath(t *testing.T) {
if runtime.GOOS == "windows" { if runtime.GOOS == "windows" {
assert.Equal(t, "venv\\Scripts\\python3.exe", interpreterPath("venv")) assert.Equal(t, "venv\\Scripts\\python.exe", interpreterPath("venv"))
} else { } else {
assert.Equal(t, "venv/bin/python3", interpreterPath("venv")) assert.Equal(t, "venv/bin/python3", interpreterPath("venv"))
} }
@ -673,7 +673,7 @@ func withFakeVEnv(t *testing.T, venvPath string) {
func interpreterPath(venvPath string) string { func interpreterPath(venvPath string) string {
if runtime.GOOS == "windows" { if runtime.GOOS == "windows" {
return filepath.Join(venvPath, "Scripts", "python3.exe") return filepath.Join(venvPath, "Scripts", "python.exe")
} else { } else {
return filepath.Join(venvPath, "bin", "python3") return filepath.Join(venvPath, "bin", "python3")
} }

View File

@ -11,6 +11,19 @@ import (
"runtime" "runtime"
) )
// GetExecutable gets appropriate python binary name for the platform
func GetExecutable() string {
// On Windows when virtualenv is created, the <env>/Scripts directory
// contains python.exe but no python3.exe.
// Most installers (e.g. the ones from python.org) only install python.exe and not python3.exe
if runtime.GOOS == "windows" {
return "python"
} else {
return "python3"
}
}
// DetectExecutable looks up the path to the python3 executable from the PATH // DetectExecutable looks up the path to the python3 executable from the PATH
// environment variable. // environment variable.
// //
@ -25,7 +38,9 @@ func DetectExecutable(ctx context.Context) (string, error) {
// the parent directory tree. // the parent directory tree.
// //
// See https://github.com/pyenv/pyenv#understanding-python-version-selection // See https://github.com/pyenv/pyenv#understanding-python-version-selection
out, err := exec.LookPath("python3")
out, err := exec.LookPath(GetExecutable())
// most of the OS'es have python3 in $PATH, but for those which don't, // most of the OS'es have python3 in $PATH, but for those which don't,
// we perform the latest version lookup // we perform the latest version lookup
if err != nil && !errors.Is(err, exec.ErrNotFound) { if err != nil && !errors.Is(err, exec.ErrNotFound) {
@ -54,7 +69,7 @@ func DetectExecutable(ctx context.Context) (string, error) {
func DetectVEnvExecutable(venvPath string) (string, error) { func DetectVEnvExecutable(venvPath string) (string, error) {
interpreterPath := filepath.Join(venvPath, "bin", "python3") interpreterPath := filepath.Join(venvPath, "bin", "python3")
if runtime.GOOS == "windows" { if runtime.GOOS == "windows" {
interpreterPath = filepath.Join(venvPath, "Scripts", "python3.exe") interpreterPath = filepath.Join(venvPath, "Scripts", "python.exe")
} }
if _, err := os.Stat(interpreterPath); err != nil { if _, err := os.Stat(interpreterPath); err != nil {

View File

@ -39,7 +39,7 @@ func TestDetectVEnvExecutable_badLayout(t *testing.T) {
func interpreterPath(venvPath string) string { func interpreterPath(venvPath string) string {
if runtime.GOOS == "windows" { if runtime.GOOS == "windows" {
return filepath.Join(venvPath, "Scripts", "python3.exe") return filepath.Join(venvPath, "Scripts", "python.exe")
} else { } else {
return filepath.Join(venvPath, "bin", "python3") return filepath.Join(venvPath, "bin", "python3")
} }

View File

@ -0,0 +1,107 @@
package pythontest
import (
"context"
"errors"
"fmt"
"os"
"os/exec"
"path/filepath"
"runtime"
"strings"
"testing"
"github.com/databricks/cli/internal/testutil"
"github.com/stretchr/testify/require"
)
type VenvOpts struct {
// input
PythonVersion string
skipVersionCheck bool
// input/output
Dir string
Name string
// output:
// Absolute path to venv
EnvPath string
// Absolute path to venv/bin or venv/Scripts, depending on OS
BinPath string
// Absolute path to python binary
PythonExe string
}
func CreatePythonEnv(opts *VenvOpts) error {
if opts == nil || opts.PythonVersion == "" {
return errors.New("PythonVersion must be provided")
}
if opts.Name == "" {
opts.Name = testutil.RandomName("test-venv-")
}
cmd := exec.Command("uv", "venv", opts.Name, "--python", opts.PythonVersion, "--seed", "-q")
cmd.Stdout = os.Stdout
cmd.Stderr = os.Stderr
cmd.Dir = opts.Dir
err := cmd.Run()
if err != nil {
return err
}
opts.EnvPath, err = filepath.Abs(filepath.Join(opts.Dir, opts.Name))
if err != nil {
return err
}
_, err = os.Stat(opts.EnvPath)
if err != nil {
return fmt.Errorf("cannot stat EnvPath %s: %s", opts.EnvPath, err)
}
if runtime.GOOS == "windows" {
// https://github.com/pypa/virtualenv/commit/993ba1316a83b760370f5a3872b3f5ef4dd904c1
opts.BinPath = filepath.Join(opts.EnvPath, "Scripts")
opts.PythonExe = filepath.Join(opts.BinPath, "python.exe")
} else {
opts.BinPath = filepath.Join(opts.EnvPath, "bin")
opts.PythonExe = filepath.Join(opts.BinPath, "python3")
}
_, err = os.Stat(opts.BinPath)
if err != nil {
return fmt.Errorf("cannot stat BinPath %s: %s", opts.BinPath, err)
}
_, err = os.Stat(opts.PythonExe)
if err != nil {
return fmt.Errorf("cannot stat PythonExe %s: %s", opts.PythonExe, err)
}
if !opts.skipVersionCheck {
cmd := exec.Command(opts.PythonExe, "--version")
out, err := cmd.CombinedOutput()
if err != nil {
return fmt.Errorf("Failed to run %s --version: %s", opts.PythonExe, err)
}
outString := string(out)
expectVersion := "Python " + opts.PythonVersion
if !strings.HasPrefix(outString, expectVersion) {
return fmt.Errorf("Unexpected output from %s --version: %v (expected %v)", opts.PythonExe, outString, expectVersion)
}
}
return nil
}
func RequireActivatedPythonEnv(t *testing.T, ctx context.Context, opts *VenvOpts) {
err := CreatePythonEnv(opts)
require.NoError(t, err)
require.DirExists(t, opts.BinPath)
newPath := fmt.Sprintf("%s%c%s", opts.BinPath, os.PathListSeparator, os.Getenv("PATH"))
t.Setenv("PATH", newPath)
}

View File

@ -0,0 +1,43 @@
package pythontest
import (
"context"
"os/exec"
"path/filepath"
"testing"
"github.com/databricks/cli/libs/python"
"github.com/stretchr/testify/require"
)
func TestVenvSuccess(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) {
ctx := context.Background()
dir := t.TempDir()
opts := VenvOpts{
PythonVersion: pythonVersion,
Dir: dir,
}
RequireActivatedPythonEnv(t, ctx, &opts)
require.DirExists(t, opts.EnvPath)
require.DirExists(t, opts.BinPath)
require.FileExists(t, opts.PythonExe)
pythonExe, err := exec.LookPath(python.GetExecutable())
require.NoError(t, err)
require.Equal(t, filepath.Dir(pythonExe), filepath.Dir(opts.PythonExe))
require.FileExists(t, pythonExe)
})
}
}
func TestWrongVersion(t *testing.T) {
require.Error(t, CreatePythonEnv(&VenvOpts{PythonVersion: "4.0"}))
}
func TestMissingVersion(t *testing.T) {
require.Error(t, CreatePythonEnv(nil))
require.Error(t, CreatePythonEnv(&VenvOpts{}))
}