-
Notifications
You must be signed in to change notification settings - Fork 59
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
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.
- Loading branch information
Showing
7 changed files
with
176 additions
and
12 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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) | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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{})) | ||
| } |