Skip to content

Commit 7649d2f

Browse files
authored
utils_test.popen: allow avoiding path modification, use sysconfig for portability (#9174)
1 parent 1c130e4 commit 7649d2f

File tree

2 files changed

+41
-4
lines changed

2 files changed

+41
-4
lines changed

distributed/tests/test_utils_test.py

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,13 @@
22

33
import asyncio
44
import logging
5+
import os
56
import pathlib
67
import signal
78
import socket
89
import subprocess
910
import sys
11+
import tempfile
1012
import textwrap
1113
import threading
1214
from contextlib import contextmanager
@@ -1069,3 +1071,15 @@ def test_captured_context_meter():
10691071
("a", "o", "u"): 6,
10701072
}
10711073
assert isinstance(metrics["foo", "s"], int)
1074+
1075+
1076+
def test_popen_file_not_found():
1077+
tmp_fd, tmp_path = tempfile.mkstemp(prefix="tmp-python")
1078+
os.close(tmp_fd)
1079+
os.remove(tmp_path)
1080+
with pytest.raises(
1081+
FileNotFoundError,
1082+
match=r"provide an absolute path to an existing installation to popen",
1083+
):
1084+
with popen([tmp_path]):
1085+
pass

distributed/utils_test.py

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
import ssl
1818
import subprocess
1919
import sys
20+
import sysconfig
2021
import tempfile
2122
import threading
2223
import warnings
@@ -1152,7 +1153,9 @@ def popen(
11521153
Parameters
11531154
----------
11541155
args: list[str]
1155-
Command line arguments
1156+
Command line arguments.
1157+
The first argument is expected to be an executable.
1158+
If it is not an absolute path, this function assumes it is in ``sysconfig.get_path("scripts")``.
11561159
capture_output: bool, default False
11571160
Set to True if you need to read output from the subprocess.
11581161
Stdout and stderr will both be piped to ``proc.stdout``.
@@ -1194,10 +1197,30 @@ def popen(
11941197
kwargs["creationflags"] = subprocess.CREATE_NEW_PROCESS_GROUP
11951198

11961199
args = list(args)
1197-
if sys.platform.startswith("win"):
1198-
args[0] = os.path.join(sys.prefix, "Scripts", args[0])
1200+
# avoid searching for executables... only accept absolute paths or look in this Python interpreter's default location for scripts
1201+
executable_path = args[0]
1202+
if not os.path.isabs(executable_path):
1203+
executable_path = os.path.join(sysconfig.get_path("scripts"), executable_path)
1204+
1205+
# On Windows, it's valid to start a process using only '{program-name}' and Windows will
1206+
# automatically find and execute '{program-name}.exe'.
1207+
#
1208+
# That allows e.g. `popen(["dask-worker"])` to work despite the installed file being called 'dask-worker.exe'.
1209+
#
1210+
# docs: https://learn.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-createprocessw
1211+
#
1212+
if WINDOWS:
1213+
executable_exists = os.path.isfile(executable_path) or os.path.isfile(
1214+
f"{executable_path}.exe"
1215+
)
11991216
else:
1200-
args[0] = os.path.join(sys.prefix, "bin", args[0])
1217+
executable_exists = os.path.isfile(executable_path)
1218+
if not executable_exists:
1219+
raise FileNotFoundError(
1220+
f"Could not find '{executable_path}'. To avoid this warning, provide an absolute path to an existing installation to popen()."
1221+
)
1222+
1223+
args[0] = executable_path
12011224
with subprocess.Popen(args, **kwargs) as proc:
12021225
try:
12031226
yield proc

0 commit comments

Comments
 (0)