From 4d61fe3e56cd5b9201c0ec70a4979fb6c609c155 Mon Sep 17 00:00:00 2001 From: Steve Dower Date: Wed, 3 Aug 2022 00:50:04 +0100 Subject: [PATCH 1/4] gh-94399: Restore PATH search behaviour of py.exe launcher --- Doc/using/windows.rst | 5 ++ Lib/test/test_launcher.py | 17 +++- ...2-08-03-00-49-46.gh-issue-94399.KvxHc0.rst | 3 + PC/launcher2.c | 80 +++++++++++++++++++ 4 files changed, 104 insertions(+), 1 deletion(-) create mode 100644 Misc/NEWS.d/next/Windows/2022-08-03-00-49-46.gh-issue-94399.KvxHc0.rst diff --git a/Doc/using/windows.rst b/Doc/using/windows.rst index 2db73c8c5b9c2c..a289435a2f51db 100644 --- a/Doc/using/windows.rst +++ b/Doc/using/windows.rst @@ -868,6 +868,11 @@ The ``/usr/bin/env`` form of shebang line has one further special property. Before looking for installed Python interpreters, this form will search the executable :envvar:`PATH` for a Python executable. This corresponds to the behaviour of the Unix ``env`` program, which performs a :envvar:`PATH` search. +If an executable matching the first argument after the ``env`` command cannot +be found, it will be handled as described below. Additionally, the environment +variable :envvar:`PYLAUNCHER_NO_SEARCH_PATH` may be set (to any value) to skip +this additional search. + Arguments in shebang lines -------------------------- diff --git a/Lib/test/test_launcher.py b/Lib/test/test_launcher.py index c8ea4f62170eac..b523b7cb48db33 100644 --- a/Lib/test/test_launcher.py +++ b/Lib/test/test_launcher.py @@ -194,9 +194,10 @@ def run_py(self, args, env=None, allow_fail=False, expect_returncode=0, argv=Non ignore = {"VIRTUAL_ENV", "PY_PYTHON", "PY_PYTHON2", "PY_PYTHON3"} env = { **{k.upper(): v for k, v in os.environ.items() if k.upper() not in ignore}, - **{k.upper(): v for k, v in (env or {}).items()}, + "PYLAUNCHER_NO_SEARCH_PATH": "1", "PYLAUNCHER_DEBUG": "1", "PYLAUNCHER_DRYRUN": "1", + **{k.upper(): v for k, v in (env or {}).items()}, } if not argv: argv = [self.py_exe, *args] @@ -551,6 +552,20 @@ def test_py_shebang_short_argv0(self): self.assertEqual("3.100", data["SearchInfo.tag"]) self.assertEqual(f'X.Y.exe -prearg "{script}" -postarg', data["stdout"].strip()) + def test_search_path(self): + stem = Path(sys.executable).stem + with self.py_ini(TEST_PY_COMMANDS): + with self.script(f"#! /usr/bin/env {stem} -prearg") as script: + data = self.run_py([script, "-postarg"], env={"PYLAUNCHER_NO_SEARCH_PATH": ""}) + self.assertEqual(f"{sys.executable} -prearg {script} -postarg", data["stdout"].strip()) + + def test_recursive_search_path(self): + with self.py_ini(TEST_PY_COMMANDS): + with self.script("#! /usr/bin/env py") as script: + data = self.run_py([script], env={"PYLAUNCHER_NO_SEARCH_PATH": ""}) + # The recursive search is ignored and we get normal "py" behavior + self.assertEqual(f"X.Y.exe {script}", data["stdout"].strip()) + def test_install(self): data = self.run_py(["-V:3.10"], env={"PYLAUNCHER_ALWAYS_INSTALL": "1"}, expect_returncode=111) cmd = data["stdout"].strip() diff --git a/Misc/NEWS.d/next/Windows/2022-08-03-00-49-46.gh-issue-94399.KvxHc0.rst b/Misc/NEWS.d/next/Windows/2022-08-03-00-49-46.gh-issue-94399.KvxHc0.rst new file mode 100644 index 00000000000000..a49e99ca266526 --- /dev/null +++ b/Misc/NEWS.d/next/Windows/2022-08-03-00-49-46.gh-issue-94399.KvxHc0.rst @@ -0,0 +1,3 @@ +Restores the behaviour of :ref:`launcher` for ``/usr/bin/env`` shebang +lines, which will now search :envvar:`PATH` for an executable matching the +given command. If none is found, the usual search process is used. diff --git a/PC/launcher2.c b/PC/launcher2.c index 033218ee2f408e..6006fb332cd617 100644 --- a/PC/launcher2.c +++ b/PC/launcher2.c @@ -36,6 +36,7 @@ #define RC_DUPLICATE_ITEM 110 #define RC_INSTALLING 111 #define RC_NO_PYTHON_AT_ALL 112 +#define RC_NO_SHEBANG 113 static FILE * log_fp = NULL; @@ -750,6 +751,78 @@ _shebangStartsWith(const wchar_t *buffer, int bufferLength, const wchar_t *prefi } +int +searchPath(SearchInfo *search, const wchar_t *shebang, int shebangLength) +{ + if (isEnvVarSet(L"PYLAUNCHER_NO_SEARCH_PATH")) { + return RC_NO_SHEBANG; + } + + wchar_t *command; + if (!_shebangStartsWith(shebang, shebangLength, L"/usr/bin/env ", &command)) { + return RC_NO_SHEBANG; + } + + wchar_t filename[MAXLEN]; + int lastDot = 0; + int commandLength = 0; + while (commandLength < MAXLEN && command[commandLength] && !isspace(command[commandLength])) { + if (command[commandLength] == L'.') { + lastDot = commandLength; + } + filename[commandLength] = command[commandLength]; + commandLength += 1; + } + + if (!commandLength || commandLength == MAXLEN) { + return RC_BAD_VIRTUAL_PATH; + } + + filename[commandLength] = L'\0'; + + const wchar_t *ext = L".exe"; + // If the command already has an extension, we do not want to add it again + if (!lastDot || _comparePath(&filename[lastDot], -1, ext, -1)) { + if (wcscat_s(filename, MAXLEN, L".exe")) { + return RC_BAD_VIRTUAL_PATH; + } + } + + wchar_t buffer[MAXLEN]; + SetSearchPathMode(BASE_SEARCH_PATH_ENABLE_SAFE_SEARCHMODE); + int n = SearchPathW(NULL, filename, NULL, MAXLEN, buffer, NULL); + if (!n) { + if (GetLastError() == ERROR_FILE_NOT_FOUND) { + // If we didn't find it on PATH, let normal handling take over + return RC_NO_SHEBANG; + } + // Other errors should cause us to break + winerror(0, L"Failed to find %s on PATH\n", filename); + return RC_BAD_VIRTUAL_PATH; + } + + // Check that we aren't going to call ourselves again + // If we are, pretend there was no shebang and let normal handling take over + if (GetModuleFileNameW(NULL, filename, MAXLEN) && + 0 == _comparePath(filename, -1, buffer, -1)) { + debug(L"# ignoring recursive shebang command\n"); + return RC_NO_SHEBANG; + } + + wchar_t *buf = allocSearchInfoBuffer(search, n + 1); + if (!buf || wcscpy_s(buf, n + 1, buffer)) { + return RC_NO_MEMORY; + } + + search->executablePath = buf; + search->executableArgs = &command[commandLength]; + search->executableArgsLength = shebangLength - commandLength; + debug(L"# Found %s on PATH\n", buf); + + return 0; +} + + int _readIni(const wchar_t *section, const wchar_t *settingName, wchar_t *buffer, int bufferLength) { @@ -885,6 +958,12 @@ checkShebang(SearchInfo *search) } debug(L"Shebang: %s\n", shebang); + // Handle shebangs that we should search PATH for + exitCode = searchPath(search, shebang, shebangLength); + if (exitCode != RC_NO_SHEBANG) { + return exitCode; + } + // Handle some known, case-sensitive shebang templates const wchar_t *command; int commandLength; @@ -895,6 +974,7 @@ checkShebang(SearchInfo *search) L"", NULL }; + for (const wchar_t **tmpl = shebangTemplates; *tmpl; ++tmpl) { if (_shebangStartsWith(shebang, shebangLength, *tmpl, &command)) { commandLength = 0; From f373dd732500cc919ee2f33b9d18e83881d869de Mon Sep 17 00:00:00 2001 From: Steve Dower Date: Wed, 3 Aug 2022 00:54:09 +0100 Subject: [PATCH 2/4] Add additional test --- Lib/test/test_launcher.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/Lib/test/test_launcher.py b/Lib/test/test_launcher.py index b523b7cb48db33..b2b3e3831da286 100644 --- a/Lib/test/test_launcher.py +++ b/Lib/test/test_launcher.py @@ -559,6 +559,14 @@ def test_search_path(self): data = self.run_py([script, "-postarg"], env={"PYLAUNCHER_NO_SEARCH_PATH": ""}) self.assertEqual(f"{sys.executable} -prearg {script} -postarg", data["stdout"].strip()) + def test_search_path_exe(self): + # Leave the .exe on the name to ensure we don't add it a second time + name = Path(sys.executable).name + with self.py_ini(TEST_PY_COMMANDS): + with self.script(f"#! /usr/bin/env {name} -prearg") as script: + data = self.run_py([script, "-postarg"], env={"PYLAUNCHER_NO_SEARCH_PATH": ""}) + self.assertEqual(f"{sys.executable} -prearg {script} -postarg", data["stdout"].strip()) + def test_recursive_search_path(self): with self.py_ini(TEST_PY_COMMANDS): with self.script("#! /usr/bin/env py") as script: From 37af09560b5dfcbf67d9a58f42d2236d621fd3d0 Mon Sep 17 00:00:00 2001 From: Steve Dower Date: Wed, 3 Aug 2022 21:28:53 +0100 Subject: [PATCH 3/4] Only search PATH, and handle longer filenames in shebangs --- Lib/test/test_launcher.py | 38 ++++++++++++++++++++++++++------------ PC/launcher2.c | 33 +++++++++++++++++++++++++++++---- 2 files changed, 55 insertions(+), 16 deletions(-) diff --git a/Lib/test/test_launcher.py b/Lib/test/test_launcher.py index b2b3e3831da286..835a51e3b6e011 100644 --- a/Lib/test/test_launcher.py +++ b/Lib/test/test_launcher.py @@ -187,6 +187,11 @@ def find_py(cls): ) return py_exe + def get_py_exe(self): + if not self.py_exe: + self.py_exe = self.find_py() + return self.py_exe + def run_py(self, args, env=None, allow_fail=False, expect_returncode=0, argv=None): if not self.py_exe: self.py_exe = self.find_py() @@ -194,7 +199,6 @@ def run_py(self, args, env=None, allow_fail=False, expect_returncode=0, argv=Non ignore = {"VIRTUAL_ENV", "PY_PYTHON", "PY_PYTHON2", "PY_PYTHON3"} env = { **{k.upper(): v for k, v in os.environ.items() if k.upper() not in ignore}, - "PYLAUNCHER_NO_SEARCH_PATH": "1", "PYLAUNCHER_DEBUG": "1", "PYLAUNCHER_DRYRUN": "1", **{k.upper(): v for k, v in (env or {}).items()}, @@ -497,7 +501,7 @@ def test_virtualenv_with_env(self): def test_py_shebang(self): with self.py_ini(TEST_PY_COMMANDS): - with self.script("#! /usr/bin/env python -prearg") as script: + with self.script("#! /usr/bin/python -prearg") as script: data = self.run_py([script, "-postarg"]) self.assertEqual("PythonTestSuite", data["SearchInfo.company"]) self.assertEqual("3.100", data["SearchInfo.tag"]) @@ -505,7 +509,7 @@ def test_py_shebang(self): def test_py2_shebang(self): with self.py_ini(TEST_PY_COMMANDS): - with self.script("#! /usr/bin/env python2 -prearg") as script: + with self.script("#! /usr/bin/python2 -prearg") as script: data = self.run_py([script, "-postarg"]) self.assertEqual("PythonTestSuite", data["SearchInfo.company"]) self.assertEqual("3.100-32", data["SearchInfo.tag"]) @@ -513,7 +517,7 @@ def test_py2_shebang(self): def test_py3_shebang(self): with self.py_ini(TEST_PY_COMMANDS): - with self.script("#! /usr/bin/env python3 -prearg") as script: + with self.script("#! /usr/bin/python3 -prearg") as script: data = self.run_py([script, "-postarg"]) self.assertEqual("PythonTestSuite", data["SearchInfo.company"]) self.assertEqual("3.100-arm64", data["SearchInfo.tag"]) @@ -521,7 +525,7 @@ def test_py3_shebang(self): def test_py_shebang_nl(self): with self.py_ini(TEST_PY_COMMANDS): - with self.script("#! /usr/bin/env python -prearg\n") as script: + with self.script("#! /usr/bin/python -prearg\n") as script: data = self.run_py([script, "-postarg"]) self.assertEqual("PythonTestSuite", data["SearchInfo.company"]) self.assertEqual("3.100", data["SearchInfo.tag"]) @@ -529,7 +533,7 @@ def test_py_shebang_nl(self): def test_py2_shebang_nl(self): with self.py_ini(TEST_PY_COMMANDS): - with self.script("#! /usr/bin/env python2 -prearg\n") as script: + with self.script("#! /usr/bin/python2 -prearg\n") as script: data = self.run_py([script, "-postarg"]) self.assertEqual("PythonTestSuite", data["SearchInfo.company"]) self.assertEqual("3.100-32", data["SearchInfo.tag"]) @@ -537,7 +541,7 @@ def test_py2_shebang_nl(self): def test_py3_shebang_nl(self): with self.py_ini(TEST_PY_COMMANDS): - with self.script("#! /usr/bin/env python3 -prearg\n") as script: + with self.script("#! /usr/bin/python3 -prearg\n") as script: data = self.run_py([script, "-postarg"]) self.assertEqual("PythonTestSuite", data["SearchInfo.company"]) self.assertEqual("3.100-arm64", data["SearchInfo.tag"]) @@ -545,7 +549,7 @@ def test_py3_shebang_nl(self): def test_py_shebang_short_argv0(self): with self.py_ini(TEST_PY_COMMANDS): - with self.script("#! /usr/bin/env python -prearg") as script: + with self.script("#! /usr/bin/python -prearg") as script: # Override argv to only pass "py.exe" as the command data = self.run_py([script, "-postarg"], argv=f'"py.exe" "{script}" -postarg') self.assertEqual("PythonTestSuite", data["SearchInfo.company"]) @@ -556,7 +560,10 @@ def test_search_path(self): stem = Path(sys.executable).stem with self.py_ini(TEST_PY_COMMANDS): with self.script(f"#! /usr/bin/env {stem} -prearg") as script: - data = self.run_py([script, "-postarg"], env={"PYLAUNCHER_NO_SEARCH_PATH": ""}) + data = self.run_py( + [script, "-postarg"], + env={"PATH": f"{Path(sys.executable).parent};{os.getenv('PATH')}"}, + ) self.assertEqual(f"{sys.executable} -prearg {script} -postarg", data["stdout"].strip()) def test_search_path_exe(self): @@ -564,13 +571,20 @@ def test_search_path_exe(self): name = Path(sys.executable).name with self.py_ini(TEST_PY_COMMANDS): with self.script(f"#! /usr/bin/env {name} -prearg") as script: - data = self.run_py([script, "-postarg"], env={"PYLAUNCHER_NO_SEARCH_PATH": ""}) + data = self.run_py( + [script, "-postarg"], + env={"PATH": f"{Path(sys.executable).parent};{os.getenv('PATH')}"}, + ) self.assertEqual(f"{sys.executable} -prearg {script} -postarg", data["stdout"].strip()) def test_recursive_search_path(self): + stem = self.get_py_exe().stem with self.py_ini(TEST_PY_COMMANDS): - with self.script("#! /usr/bin/env py") as script: - data = self.run_py([script], env={"PYLAUNCHER_NO_SEARCH_PATH": ""}) + with self.script(f"#! /usr/bin/env {stem}") as script: + data = self.run_py( + [script], + env={"PATH": f"{self.get_py_exe().parent};{os.getenv('PATH')}"}, + ) # The recursive search is ignored and we get normal "py" behavior self.assertEqual(f"X.Y.exe {script}", data["stdout"].strip()) diff --git a/PC/launcher2.c b/PC/launcher2.c index 6006fb332cd617..1f3c796a8f8cca 100644 --- a/PC/launcher2.c +++ b/PC/launcher2.c @@ -788,11 +788,21 @@ searchPath(SearchInfo *search, const wchar_t *shebang, int shebangLength) } } + wchar_t pathVariable[MAXLEN]; + int n = GetEnvironmentVariableW(L"PATH", pathVariable, MAXLEN); + if (!n) { + if (GetLastError() == ERROR_ENVVAR_NOT_FOUND) { + return RC_NO_SHEBANG; + } + winerror(0, L"Failed to read PATH\n", filename); + return RC_INTERNAL_ERROR; + } + wchar_t buffer[MAXLEN]; - SetSearchPathMode(BASE_SEARCH_PATH_ENABLE_SAFE_SEARCHMODE); - int n = SearchPathW(NULL, filename, NULL, MAXLEN, buffer, NULL); + n = SearchPathW(pathVariable, filename, NULL, MAXLEN, buffer, NULL); if (!n) { if (GetLastError() == ERROR_FILE_NOT_FOUND) { + debug(L"# Did not find %s on PATH\n", filename); // If we didn't find it on PATH, let normal handling take over return RC_NO_SHEBANG; } @@ -988,8 +998,23 @@ checkShebang(SearchInfo *search) debug(L"# Treating shebang command '%.*s' as %s\n", commandLength, command, search->executablePath); } else if (_shebangStartsWith(command, commandLength, L"python", NULL)) { - search->tag = &command[6]; - search->tagLength = commandLength - 6; + int tagOffset = 6; + // If we're looking for 'python_d', well, we'll settle for a + // release build, but need to skip the _d to make sure we get + // the right version. + if (_startsWith(&command[tagOffset], commandLength - tagOffset, L"_d", -1)) { + tagOffset += 2; + } + search->tag = &command[tagOffset]; + search->tagLength = commandLength - tagOffset; + // If we had 'python3.12.exe' then we want to strip the suffix + // off of the tag + if (search->tagLength > 4) { + const wchar_t *suffix = &search->tag[search->tagLength - 4]; + if (0 == _comparePath(suffix, 4, L".exe", -1)) { + search->tagLength -= 4; + } + } search->oldStyleTag = true; search->executableArgs = &command[commandLength]; search->executableArgsLength = shebangLength - commandLength; From f90b95e99b66cdc78d08eaccecb82a2f77dfb7ab Mon Sep 17 00:00:00 2001 From: Steve Dower Date: Wed, 3 Aug 2022 21:33:29 +0100 Subject: [PATCH 4/4] _d suffix would be at the end --- PC/launcher2.c | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/PC/launcher2.c b/PC/launcher2.c index 1f3c796a8f8cca..a5dfd25f7d52f0 100644 --- a/PC/launcher2.c +++ b/PC/launcher2.c @@ -998,15 +998,8 @@ checkShebang(SearchInfo *search) debug(L"# Treating shebang command '%.*s' as %s\n", commandLength, command, search->executablePath); } else if (_shebangStartsWith(command, commandLength, L"python", NULL)) { - int tagOffset = 6; - // If we're looking for 'python_d', well, we'll settle for a - // release build, but need to skip the _d to make sure we get - // the right version. - if (_startsWith(&command[tagOffset], commandLength - tagOffset, L"_d", -1)) { - tagOffset += 2; - } - search->tag = &command[tagOffset]; - search->tagLength = commandLength - tagOffset; + search->tag = &command[6]; + search->tagLength = commandLength - 6; // If we had 'python3.12.exe' then we want to strip the suffix // off of the tag if (search->tagLength > 4) { @@ -1015,6 +1008,14 @@ checkShebang(SearchInfo *search) search->tagLength -= 4; } } + // If we had 'python3_d' then we want to strip the '_d' (any + // '.exe' is already gone) + if (search->tagLength > 2) { + const wchar_t *suffix = &search->tag[search->tagLength - 2]; + if (0 == _comparePath(suffix, 2, L"_d", -1)) { + search->tagLength -= 2; + } + } search->oldStyleTag = true; search->executableArgs = &command[commandLength]; search->executableArgsLength = shebangLength - commandLength;