Skip to content

Commit c2d6bdb

Browse files
nfxnfx
committed
shuffle some code around
1 parent aea6d13 commit c2d6bdb

File tree

6 files changed

+116
-82
lines changed

6 files changed

+116
-82
lines changed

‎libs/python/detect_unix_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import (
99
"github.com/stretchr/testify/assert"
1010
)
1111

12-
func TestDetectsViaWhich(t *testing.T) {
12+
func TestDetectsViaPathLookup(t *testing.T) {
1313
ctx := context.Background()
1414
py, err := DetectExecutable(ctx)
1515
assert.NoError(t, err)

‎libs/python/detect_win_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import (
99
"github.com/stretchr/testify/assert"
1010
)
1111

12-
func TestDetectsViaWhich(t *testing.T) {
12+
func TestDetectsViaPathLookup(t *testing.T) {
1313
ctx := context.Background()
1414
py, err := DetectExecutable(ctx)
1515
assert.NoError(t, err)

‎libs/python/interpreters.go‎

Lines changed: 102 additions & 77 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,13 @@ import (
1616
"golang.org/x/mod/semver"
1717
)
1818

19+
var ErrNoPythonInterpreters = errors.New("no python3 interpreters found")
20+
21+
const officialMswinPython = "(Python Official) https://python.org/downloads/windows"
22+
const microsoftStorePython = "(Microsoft Store) https://apps.microsoft.com/store/search?publisher=Python%20Software%20Foundation"
23+
24+
const worldWriteable = 0o002
25+
1926
type Interpreter struct {
2027
Version string
2128
Path string
@@ -46,17 +53,64 @@ func (a allInterpreters) AtLeast(minimalVersion string) (*Interpreter, error) {
4653
return nil, fmt.Errorf("cannot find Python greater or equal to %s", canonicalMinimalVersion)
4754
}
4855

49-
var ErrNoPythonInterpreters = errors.New("no python3 interpreters found")
50-
51-
const officialMswinPython = "(Python Official) https://www.python.org/downloads/windows"
52-
const microsoftStorePython = "(Microsoft Store) https://apps.microsoft.com/store/search?publisher=Python%20Software%20Foundation"
53-
54-
const worldWriteable = 0o002
55-
5656
func DetectInterpreters(ctx context.Context) (allInterpreters, error) {
5757
found := allInterpreters{}
58-
paths := strings.Split(os.Getenv("PATH"), string(os.PathListSeparator))
5958
seen := map[string]bool{}
59+
executables, err := pythonicExecutablesFromPathEnvironment(ctx)
60+
if err != nil {
61+
return nil, err
62+
}
63+
log.Debugf(ctx, "found %d potential alternative Python versions in $PATH", len(executables))
64+
for _, resolved := range executables {
65+
if seen[resolved] {
66+
continue
67+
}
68+
seen[resolved] = true
69+
// probe the binary version by executing it, like `python --version`
70+
// and parsing the output.
71+
//
72+
// Keep in mind, that mswin installations get python.exe and pythonw.exe,
73+
// which are slightly different: see https://stackoverflow.com/a/30313091
74+
out, err := process.Background(ctx, []string{resolved, "--version"})
75+
var processErr *process.ProcessError
76+
if errors.As(err, &processErr) {
77+
log.Debugf(ctx, "failed to check version for %s: %s", resolved, processErr.Err)
78+
continue
79+
}
80+
if err != nil {
81+
log.Debugf(ctx, "failed to check version for %s: %s", resolved, err)
82+
continue
83+
}
84+
version := validPythonVersion(ctx, resolved, out)
85+
if version == "" {
86+
continue
87+
}
88+
found = append(found, Interpreter{
89+
Version: version,
90+
Path: resolved,
91+
})
92+
}
93+
if runtime.GOOS == "windows" && len(found) == 0 {
94+
return nil, fmt.Errorf("%w. Install them from %s or %s and restart the shell",
95+
ErrNoPythonInterpreters, officialMswinPython, microsoftStorePython)
96+
}
97+
if len(found) == 0 {
98+
return nil, ErrNoPythonInterpreters
99+
}
100+
sort.Slice(found, func(i, j int) bool {
101+
a := found[i].Version
102+
b := found[j].Version
103+
cmp := semver.Compare(a, b)
104+
if cmp != 0 {
105+
return cmp < 0
106+
}
107+
return a < b
108+
})
109+
return found, nil
110+
}
111+
112+
func pythonicExecutablesFromPathEnvironment(ctx context.Context) (out []string, err error) {
113+
paths := strings.Split(os.Getenv("PATH"), string(os.PathListSeparator))
60114
for _, prefix := range paths {
61115
info, err := os.Stat(prefix)
62116
if errors.Is(err, fs.ErrNotExist) {
@@ -74,8 +128,7 @@ func DetectInterpreters(ctx context.Context) (allInterpreters, error) {
74128
continue
75129
}
76130
perm := info.Mode().Perm()
77-
xx := runtime.GOOS
78-
if xx != "windows" && perm&worldWriteable != 0 {
131+
if runtime.GOOS != "windows" && perm&worldWriteable != 0 {
79132
// we try not to run any python binary that sits in a writable folder by all users.
80133
// this is mainly to avoid breaking the security model on a multi-user system.
81134
// If the PATH is pointing somewhere untrusted it is the user fault, but we can
@@ -101,7 +154,6 @@ func DetectInterpreters(ctx context.Context) (allInterpreters, error) {
101154
// skip python3-config, python3.10-config, etc
102155
continue
103156
}
104-
105157
// If Python3 is installed on Windows through GUI installer app that was
106158
// downloaded from https://python.org/downloads/windows, it may appear
107159
// in $PATH as `python`, even though it means Python 2.7 in all other
@@ -117,75 +169,48 @@ func DetectInterpreters(ctx context.Context) (allInterpreters, error) {
117169
log.Debugf(ctx, "cannot resolve symlink for %s: %s", bin, resolved)
118170
continue
119171
}
120-
if seen[resolved] {
121-
continue
122-
}
123-
seen[resolved] = true
172+
out = append(out, resolved)
173+
}
174+
}
175+
return out, nil
176+
}
124177

125-
// probe the binary version by executing it, like `python --version`
126-
// and parsing the output.
127-
//
128-
// Keep in mind, that mswin installations get python.exe and pythonw.exe,
129-
// which are slightly different: see https://stackoverflow.com/a/30313091
130-
out, err := process.Background(ctx, []string{resolved, "--version"})
131-
var processErr *process.ProcessError
132-
if errors.As(err, &processErr) {
133-
log.Debugf(ctx, "failed to check version for %s: %s", resolved, processErr.Err)
134-
continue
135-
}
136-
if err != nil {
137-
log.Debugf(ctx, "failed to check version for %s: %s", resolved, err)
138-
continue
139-
}
178+
func validPythonVersion(ctx context.Context, resolved, out string) string {
179+
out = strings.TrimSpace(out)
180+
log.Debugf(ctx, "%s --version: %s", resolved, out)
140181

141-
words := strings.Split(strings.TrimSpace(out), " ")
142-
// The Python distribution from the Windows Store is available in $PATH as `python.exe`
143-
// and `python3.exe`, even though it symlinks to a real file packaged with some versions of Windows:
144-
// /c/Program Files/WindowsApps/Microsoft.DesktopAppInstaller_.../AppInstallerPythonRedirector.exe.
145-
// Executing the `python` command from this distribution opens the Windows Store, allowing users to
146-
// download and install Python. Once installed, it replaces the `python.exe` and `python3.exe`` stub
147-
// with the genuine Python executable. Additionally, once user installs from the main installer at
148-
// https://python.org/downloads/windows, it does not replace this stub.
149-
//
150-
// However, a drawback is that if this initial stub is run with any command line arguments, it quietly
151-
// fails to execute. According to https://github.com/databrickslabs/ucx/issues/281, it can be
152-
// detected by seeing just the "Python" output without any version info from the `python --version`
153-
// command execution.
154-
//
155-
// See https://github.com/pypa/packaging-problems/issues/379
156-
// See https://bugs.python.org/issue41327
157-
if len(words) < 2 {
158-
continue
159-
}
160-
if words[0] != "Python" {
161-
continue
162-
}
163-
lastWord := words[len(words)-1]
164-
version := semver.Canonical("v" + lastWord)
165-
if version == "" {
166-
continue
167-
}
168-
found = append(found, Interpreter{
169-
Version: version,
170-
Path: resolved,
171-
})
172-
}
182+
words := strings.Split(out, " ")
183+
// The Python distribution from the Windows Store is available in $PATH as `python.exe`
184+
// and `python3.exe`, even though it symlinks to a real file packaged with some versions of Windows:
185+
// /c/Program Files/WindowsApps/Microsoft.DesktopAppInstaller_.../AppInstallerPythonRedirector.exe.
186+
// Executing the `python` command from this distribution opens the Windows Store, allowing users to
187+
// download and install Python. Once installed, it replaces the `python.exe` and `python3.exe`` stub
188+
// with the genuine Python executable. Additionally, once user installs from the main installer at
189+
// https://python.org/downloads/windows, it does not replace this stub.
190+
//
191+
// However, a drawback is that if this initial stub is run with any command line arguments, it quietly
192+
// fails to execute. According to https://github.com/databrickslabs/ucx/issues/281, it can be
193+
// detected by seeing just the "Python" output without any version info from the `python --version`
194+
// command execution.
195+
//
196+
// See https://github.com/pypa/packaging-problems/issues/379
197+
// See https://bugs.python.org/issue41327
198+
if len(words) < 2 {
199+
log.Debugf(ctx, "%s --version: stub from Windows Store", resolved)
200+
return ""
173201
}
174-
if runtime.GOOS == "windows" && len(found) == 0 {
175-
return nil, fmt.Errorf("%w. Install them from %s or %s and restart the shell",
176-
ErrNoPythonInterpreters, officialMswinPython, microsoftStorePython)
202+
203+
if words[0] != "Python" {
204+
log.Debugf(ctx, "%s --version: not a Python", resolved)
205+
return ""
177206
}
178-
if len(found) == 0 {
179-
return nil, ErrNoPythonInterpreters
207+
208+
lastWord := words[len(words)-1]
209+
version := semver.Canonical("v" + lastWord)
210+
if version == "" {
211+
log.Debugf(ctx, "%s --version: invalid SemVer: %s", resolved, lastWord)
212+
return ""
180213
}
181-
sort.Slice(found, func(i, j int) bool {
182-
a := found[i].Version
183-
b := found[j].Version
184-
cmp := semver.Compare(a, b)
185-
if cmp != 0 {
186-
return cmp < 0
187-
}
188-
return a < b
189-
})
190-
return found, nil
214+
215+
return version
191216
}

‎libs/python/interpreters_unix_test.go‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ package python
55
import (
66
"context"
77
"os"
8+
"os/exec"
89
"path/filepath"
910
"testing"
1011

@@ -38,11 +39,15 @@ func TestFilteringInterpreters(t *testing.T) {
3839
raw, err := os.ReadFile("testdata/world-writeable/python8.4")
3940
assert.NoError(t, err)
4041

41-
binary := filepath.Join(rogueBin, "python8.4")
42-
err = os.WriteFile(binary, raw, 00777)
42+
injectedBinary := filepath.Join(rogueBin, "python8.4")
43+
err = os.WriteFile(injectedBinary, raw, 00777)
4344
assert.NoError(t, err)
4445

45-
t.Setenv("PATH", "testdata/other-binaries-filtered"+string(os.PathListSeparator)+rogueBin)
46+
t.Setenv("PATH", "testdata/other-binaries-filtered:"+rogueBin)
47+
48+
roguePath, err := exec.LookPath("python8.4")
49+
assert.NoError(t, err)
50+
assert.Equal(t, injectedBinary, roguePath)
4651

4752
ctx := context.Background()
4853
all, err := DetectInterpreters(ctx)

‎libs/python/interpreters_win_test.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,4 +24,5 @@ func TestNoInterpretersFound(t *testing.T) {
2424
ctx := context.Background()
2525
_, err := DetectInterpreters(ctx)
2626
assert.ErrorIs(t, err, ErrNoPythonInterpreters)
27+
assert.ErrorContains(t, err, "python.org/downloads")
2728
}

‎libs/python/testdata/other-binaries-filtered/python3.10.100‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
#!/bin/sh
2+
3+
echo "Python 3.a.b"

0 commit comments

Comments
 (0)