Skip to content

Commit d548477

Browse files
committed
fix(python): use ensurepip for reliable pip installation on Windows
- Replace get-pip.py with ensurepip --default-pip --upgrade as primary method - Reorder Install() to configure pip before creating shims - Make ._pth file modification best-effort (for python.org embeddable) - Keep get-pip.py as fallback for edge cases - Add unit tests for enableSitePackages and shim configuration This fixes pip installation failures on Windows where: 1. python.org embeddable packages don't include pip 2. python-build-standalone has pip but pip.exe has broken embedded paths Running ensurepip post-installation creates working pip executables with correct paths regardless of the distribution source. Fixes #201
1 parent 4339f80 commit d548477

2 files changed

Lines changed: 183 additions & 23 deletions

File tree

src/runtimes/python/provider.go

Lines changed: 43 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -102,22 +102,23 @@ func determineSourceDir(extractDir string) string {
102102
return extractDir
103103
}
104104

105-
// installPipIfNeeded installs pip on Windows or shows success message on Unix
105+
// installPipIfNeeded ensures pip is properly installed and accessible.
106+
// On Windows, pip may be missing (python.org embeddable) or have broken
107+
// executables (python-build-standalone with embedded build paths).
108+
// Running ensurepip --default-pip --upgrade creates working pip executables.
106109
func (p *Provider) installPipIfNeeded(version string) {
107110
if goruntime.GOOS == constants.OSWindows {
108-
// Windows embeddable packages need pip installed
109-
pipSpinner := ui.NewSpinner("Installing pip...")
111+
pipSpinner := ui.NewSpinner("Configuring pip...")
110112
pipSpinner.Start()
111113
if err := p.installPip(version); err != nil {
112-
pipSpinner.Warning("Failed to install pip")
113-
ui.Info("To install pip manually:")
114-
ui.Info(" 1. Download: %s", p.getPipURL(version))
115-
ui.Info(" 2. Run: python get-pip.py")
114+
pipSpinner.Warning("Failed to configure pip")
115+
ui.Info("To install pip manually, run:")
116+
ui.Info(" python -m ensurepip --default-pip --upgrade")
116117
} else {
117-
pipSpinner.Success("pip installed successfully")
118+
pipSpinner.Success("pip configured successfully")
118119
}
119120
} else {
120-
// python-build-standalone includes pip
121+
// python-build-standalone includes pip on Unix
121122
ui.Success("pip included")
122123
}
123124
}
@@ -169,7 +170,10 @@ func (p *Provider) Install(version string) error {
169170
return fmt.Errorf("failed to move to install location: %w", err)
170171
}
171172

172-
// Create shims
173+
// Install/configure pip first (so executables exist before creating shims)
174+
p.installPipIfNeeded(version)
175+
176+
// Create shims (after pip is installed, all executables now exist)
173177
shimSpinner := ui.NewSpinner("Creating shims...")
174178
shimSpinner.Start()
175179
if err := p.createShims(); err != nil {
@@ -181,9 +185,6 @@ func (p *Provider) Install(version string) error {
181185
ui.Success("Python v%s installed successfully", version)
182186
ui.Info("Location: %s", installPath)
183187

184-
// Install/verify pip
185-
p.installPipIfNeeded(version)
186-
187188
return nil
188189
}
189190

@@ -222,7 +223,13 @@ func (p *Provider) createShims() error {
222223
return manager.CreateShims(shimNames)
223224
}
224225

225-
// installPip installs pip for Windows embeddable Python packages
226+
// installPip ensures pip is properly installed with working executables.
227+
// This handles two scenarios:
228+
// 1. python.org embeddable packages: pip is not included, needs ensurepip
229+
// 2. python-build-standalone: pip module exists but pip.exe has broken paths
230+
//
231+
// Running "python -m ensurepip --default-pip --upgrade" handles both cases
232+
// by (re)installing pip and creating working pip/pip3/pipX.Y executables.
226233
func (p *Provider) installPip(version string) error {
227234
pythonPath, err := p.ExecutablePath(version)
228235
if err != nil {
@@ -231,25 +238,38 @@ func (p *Provider) installPip(version string) error {
231238

232239
installPath := config.RuntimeVersionPath("python", version)
233240

234-
// For embeddable packages, we need to:
235-
// 1. Modify python311._pth to enable site-packages
236-
// 2. Download and run get-pip.py
237-
238-
// Step 1: Enable site-packages by uncommenting the import site line
241+
// For python.org embeddable packages, enable site-packages first.
242+
// This file doesn't exist in python-build-standalone, so errors are ignored.
239243
pthFile := filepath.Join(installPath, fmt.Sprintf("python%s._pth", strings.Join(strings.Split(version, ".")[:2], "")))
240-
if err := p.enableSitePackages(pthFile); err != nil {
241-
return fmt.Errorf("failed to enable site-packages: %w", err)
244+
_ = p.enableSitePackages(pthFile) // Best effort - ignore errors
245+
246+
// Run ensurepip to install/reinstall pip with working executables.
247+
// --default-pip: creates pip.exe in addition to pipX.exe and pipX.Y.exe
248+
// --upgrade: reinstalls even if pip module already exists (fixes broken executables)
249+
cmd := exec.Command(pythonPath, "-m", "ensurepip", "--default-pip", "--upgrade")
250+
cmd.Dir = installPath
251+
output, err := cmd.CombinedOutput()
252+
if err != nil {
253+
ui.Debug("ensurepip failed: %v\nOutput: %s", err, string(output))
254+
// Fall back to get-pip.py for older Python versions or edge cases
255+
return p.installPipWithGetPip(version, pythonPath, installPath)
242256
}
243257

244-
// Step 2: Download get-pip.py (use version-specific URL for older Python)
258+
return nil
259+
}
260+
261+
// installPipWithGetPip is a fallback method that downloads and runs get-pip.py.
262+
// Used when ensurepip fails (e.g., ensurepip module missing or corrupted).
263+
func (p *Provider) installPipWithGetPip(version, pythonPath, installPath string) error {
264+
ui.Debug("Falling back to get-pip.py")
265+
245266
getPipURL := p.getPipURL(version)
246267
getPipPath := filepath.Join(installPath, "get-pip.py")
247268
if err := download.File(getPipURL, getPipPath); err != nil {
248269
return fmt.Errorf("failed to download get-pip.py: %w", err)
249270
}
250271
defer func() { _ = os.Remove(getPipPath) }()
251272

252-
// Step 3: Run get-pip.py
253273
cmd := exec.Command(pythonPath, getPipPath)
254274
cmd.Dir = installPath
255275
output, err := cmd.CombinedOutput()

src/runtimes/python/provider_test.go

Lines changed: 140 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
package python
22

33
import (
4+
"os"
5+
"path/filepath"
6+
"strings"
47
"testing"
58

69
"github.com/CodingWithCalvin/dtvem.cli/src/internal/runtime"
@@ -143,3 +146,140 @@ func TestPythonProvider_GetPipURL(t *testing.T) {
143146
})
144147
}
145148
}
149+
150+
// TestPythonProvider_EnableSitePackages tests the ._pth file modification
151+
func TestPythonProvider_EnableSitePackages(t *testing.T) {
152+
provider := NewProvider()
153+
154+
t.Run("returns error for non-existent file", func(t *testing.T) {
155+
err := provider.enableSitePackages("/nonexistent/path/python311._pth")
156+
if err == nil {
157+
t.Error("enableSitePackages() should return error for non-existent file")
158+
}
159+
})
160+
161+
t.Run("uncomments import site line", func(t *testing.T) {
162+
// Create a temp file with commented import site
163+
tempDir := t.TempDir()
164+
pthFile := filepath.Join(tempDir, "python311._pth")
165+
content := "python311.zip\n.\n#import site\n"
166+
if err := os.WriteFile(pthFile, []byte(content), 0644); err != nil {
167+
t.Fatalf("Failed to create test file: %v", err)
168+
}
169+
170+
err := provider.enableSitePackages(pthFile)
171+
if err != nil {
172+
t.Fatalf("enableSitePackages() returned error: %v", err)
173+
}
174+
175+
// Read and verify
176+
result, err := os.ReadFile(pthFile)
177+
if err != nil {
178+
t.Fatalf("Failed to read result file: %v", err)
179+
}
180+
181+
if !strings.Contains(string(result), "import site") {
182+
t.Error("Result should contain 'import site'")
183+
}
184+
if strings.Contains(string(result), "#import site") {
185+
t.Error("Result should not contain commented '#import site'")
186+
}
187+
})
188+
189+
t.Run("adds import site if missing", func(t *testing.T) {
190+
// Create a temp file without import site
191+
tempDir := t.TempDir()
192+
pthFile := filepath.Join(tempDir, "python311._pth")
193+
content := "python311.zip\n.\n"
194+
if err := os.WriteFile(pthFile, []byte(content), 0644); err != nil {
195+
t.Fatalf("Failed to create test file: %v", err)
196+
}
197+
198+
err := provider.enableSitePackages(pthFile)
199+
if err != nil {
200+
t.Fatalf("enableSitePackages() returned error: %v", err)
201+
}
202+
203+
// Read and verify
204+
result, err := os.ReadFile(pthFile)
205+
if err != nil {
206+
t.Fatalf("Failed to read result file: %v", err)
207+
}
208+
209+
if !strings.Contains(string(result), "import site") {
210+
t.Error("Result should contain 'import site'")
211+
}
212+
})
213+
214+
t.Run("preserves already uncommented import site", func(t *testing.T) {
215+
// Create a temp file with already uncommented import site
216+
tempDir := t.TempDir()
217+
pthFile := filepath.Join(tempDir, "python311._pth")
218+
content := "python311.zip\n.\nimport site\n"
219+
if err := os.WriteFile(pthFile, []byte(content), 0644); err != nil {
220+
t.Fatalf("Failed to create test file: %v", err)
221+
}
222+
223+
err := provider.enableSitePackages(pthFile)
224+
if err != nil {
225+
t.Fatalf("enableSitePackages() returned error: %v", err)
226+
}
227+
228+
// Read and verify - should still have import site, not duplicated
229+
result, err := os.ReadFile(pthFile)
230+
if err != nil {
231+
t.Fatalf("Failed to read result file: %v", err)
232+
}
233+
234+
count := strings.Count(string(result), "import site")
235+
if count != 1 {
236+
t.Errorf("Expected exactly 1 'import site' line, got %d", count)
237+
}
238+
})
239+
}
240+
241+
// TestPythonProvider_Shims tests the shim configuration
242+
func TestPythonProvider_Shims(t *testing.T) {
243+
provider := NewProvider()
244+
245+
shims := provider.Shims()
246+
247+
t.Run("includes python shim", func(t *testing.T) {
248+
found := false
249+
for _, s := range shims {
250+
if s == "python" {
251+
found = true
252+
break
253+
}
254+
}
255+
if !found {
256+
t.Error("Shims() should include 'python'")
257+
}
258+
})
259+
260+
t.Run("includes pip shim", func(t *testing.T) {
261+
found := false
262+
for _, s := range shims {
263+
if s == "pip" {
264+
found = true
265+
break
266+
}
267+
}
268+
if !found {
269+
t.Error("Shims() should include 'pip'")
270+
}
271+
})
272+
273+
t.Run("includes pip3 shim", func(t *testing.T) {
274+
found := false
275+
for _, s := range shims {
276+
if s == "pip3" {
277+
found = true
278+
break
279+
}
280+
}
281+
if !found {
282+
t.Error("Shims() should include 'pip3'")
283+
}
284+
})
285+
}

0 commit comments

Comments
 (0)