From 45fc4c922558197d6f6e3eaa581255ad38a40335 Mon Sep 17 00:00:00 2001 From: achekerylla Date: Tue, 12 Mar 2024 13:17:39 -0700 Subject: [PATCH 1/8] QOL: Config run should handle shell paths with spaces --- invoke/runners.py | 3 +++ invoke/terminals.py | 37 +++++++++++++++++++++++++++++++++++++ tests/_util.py | 10 ++++++++++ tests/runners.py | 11 +++++++++++ tests/terminals.py | 10 +++++++++- 5 files changed, 70 insertions(+), 1 deletion(-) diff --git a/invoke/runners.py b/invoke/runners.py index f1c888f44..5c32fbbe2 100644 --- a/invoke/runners.py +++ b/invoke/runners.py @@ -50,6 +50,7 @@ character_buffered, ready_for_reading, bytes_to_read, + get_short_path_name, ) from .util import has_fileno, isatty, ExceptionHandlingThread @@ -415,6 +416,8 @@ def _setup(self, command: str, kwargs: Any) -> None: # Echo running command (wants to be early to be included in dry-run) if self.opts["echo"]: self.echo(command) + if WINDOWS: + self.opts["shell"] = get_short_path_name(self.opts["shell"]) # Prepare common result args. # TODO: I hate this. Needs a deeper separate think about tweaking # Runner.generate_result in a way that isn't literally just this same diff --git a/invoke/terminals.py b/invoke/terminals.py index 2694712fa..0026ffdca 100644 --- a/invoke/terminals.py +++ b/invoke/terminals.py @@ -34,8 +34,10 @@ Structure, c_ushort, windll, + wintypes, POINTER, byref, + create_unicode_buffer, ) from ctypes.wintypes import HANDLE, _COORD, _SMALL_RECT else: @@ -245,3 +247,38 @@ def bytes_to_read(input_: IO) -> int: fionread = fcntl.ioctl(input_, termios.FIONREAD, b" ") return int(struct.unpack("h", fionread)[0]) return 1 + + +def get_short_path_name(long_path: str) -> str: + """ + Get short path form for long path. + + Only applies to Windows-based systems; on Unix this is a pass-thru. + + .. note:: + This API converts path strings to the 8.3 format used by earlier + tools on the Windows platform. This format has no spaces. + + :param long_path: Long path such as `shutil.which()` results. + + :returns: `str` Short path form of the long path. + """ + if not WINDOWS: + return long_path + # Access `GetShortPathNameW()` function from `kernel32.dll`. + GetShortPathNameW = windll.kernel32.GetShortPathNameW + GetShortPathNameW.argtypes = [ + wintypes.LPCWSTR, + wintypes.LPWSTR, + wintypes.DWORD, + ] + GetShortPathNameW.restype = wintypes.DWORD + # Call function to get short path form. + buffer_size = 0 + while True: + buffer_array = create_unicode_buffer(buffer_size) + required_size = GetShortPathNameW(long_path, buffer_array, buffer_size) + if required_size > buffer_size: + buffer_size = required_size + else: + return buffer_array.value diff --git a/tests/_util.py b/tests/_util.py index 50a4c7fc9..576c3c00f 100644 --- a/tests/_util.py +++ b/tests/_util.py @@ -32,6 +32,16 @@ def wrapper(*args, **kwargs): return wrapper +def skip_if_posix(fn): + @wraps(fn) + def wrapper(*args, **kwargs): + if not WINDOWS: + skip() + return fn(*args, **kwargs) + + return wrapper + + @contextmanager def support_path(): sys.path.insert(0, support) diff --git a/tests/runners.py b/tests/runners.py index 94c63d8b3..e1776de8f 100644 --- a/tests/runners.py +++ b/tests/runners.py @@ -36,6 +36,7 @@ from _util import ( mock_subprocess, mock_pty, + skip_if_posix, skip_if_windows, _Dummy, _KeyboardInterruptingRunner, @@ -231,6 +232,16 @@ def may_be_configured(self): runner = self._runner(run={"shell": "/bin/tcsh"}) assert runner.run(_).shell == "/bin/tcsh" + @skip_if_posix + def may_be_configured_with_short_path_on_windows(self): + runner = self._runner(run={"shell": "C:\\Program Files\\PowerShell\\7\\pwsh.exe"}) + assert runner.run(_).shell == "C:\\PROGRA~1\\POWERS~1\\7\\pwsh.exe" + + @skip_if_windows + def may_be_configured_with_passthru_on_posix(self): + runner = self._runner(run={"shell": "/foo/bar/baz /bang"}) + assert runner.run(_).shell == "/foo/bar/baz /bang" + def kwarg_beats_config(self): runner = self._runner(run={"shell": "/bin/tcsh"}) assert runner.run(_, shell="/bin/zsh").shell == "/bin/zsh" diff --git a/tests/terminals.py b/tests/terminals.py index 96d095886..31fd81128 100644 --- a/tests/terminals.py +++ b/tests/terminals.py @@ -4,7 +4,7 @@ from unittest.mock import Mock, patch from pytest import skip, mark -from invoke.terminals import pty_size, bytes_to_read, WINDOWS +from invoke.terminals import pty_size, bytes_to_read, get_short_path_name, WINDOWS # Skip on Windows CI, it may blow up on one of these tests pytestmark = mark.skipif( @@ -76,3 +76,11 @@ def returns_FIONREAD_result_when_stream_is_a_tty(self): def returns_1_on_windows(self): skip() + + class get_short_path_name_: + def returns_passthru_on_posix(self): + short_path = "/foo/bar/baz" + assert get_short_path_name(short_path) == short_path + + def returns_shortpath_on_windows(self): + skip() From 73fabacac0e944447f75052af6fd8647b6e175f0 Mon Sep 17 00:00:00 2001 From: achekerylla Date: Tue, 12 Mar 2024 14:16:10 -0700 Subject: [PATCH 2/8] Fix blacken failures --- tests/runners.py | 4 +++- tests/terminals.py | 7 ++++++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/tests/runners.py b/tests/runners.py index e1776de8f..2cf197054 100644 --- a/tests/runners.py +++ b/tests/runners.py @@ -234,7 +234,9 @@ def may_be_configured(self): @skip_if_posix def may_be_configured_with_short_path_on_windows(self): - runner = self._runner(run={"shell": "C:\\Program Files\\PowerShell\\7\\pwsh.exe"}) + runner = self._runner( + run={"shell": "C:\\Program Files\\PowerShell\\7\\pwsh.exe"} + ) assert runner.run(_).shell == "C:\\PROGRA~1\\POWERS~1\\7\\pwsh.exe" @skip_if_windows diff --git a/tests/terminals.py b/tests/terminals.py index 31fd81128..890f89909 100644 --- a/tests/terminals.py +++ b/tests/terminals.py @@ -4,7 +4,12 @@ from unittest.mock import Mock, patch from pytest import skip, mark -from invoke.terminals import pty_size, bytes_to_read, get_short_path_name, WINDOWS +from invoke.terminals import ( + pty_size, + bytes_to_read, + get_short_path_name, + WINDOWS, +) # Skip on Windows CI, it may blow up on one of these tests pytestmark = mark.skipif( From b9d50e4903d87a74095658bff870091ba009e2ae Mon Sep 17 00:00:00 2001 From: achekerylla Date: Tue, 12 Mar 2024 14:40:42 -0700 Subject: [PATCH 3/8] Fix mypy windows type not defined errors --- invoke/terminals.py | 50 ++++++++++++++++++++++++++++----------------- 1 file changed, 31 insertions(+), 19 deletions(-) diff --git a/invoke/terminals.py b/invoke/terminals.py index 0026ffdca..301e5f313 100644 --- a/invoke/terminals.py +++ b/invoke/terminals.py @@ -249,6 +249,36 @@ def bytes_to_read(input_: IO) -> int: return 1 +if sys.platform == "win32": + + def _get_short_path_name(long_path: str) -> str: + # Access `GetShortPathNameW()` function from `kernel32.dll`. + GetShortPathNameW = windll.kernel32.GetShortPathNameW + GetShortPathNameW.argtypes = [ + wintypes.LPCWSTR, + wintypes.LPWSTR, + wintypes.DWORD, + ] + GetShortPathNameW.restype = wintypes.DWORD + # Call function to get short path form. + buffer_size = 0 + while True: + buffer_array = create_unicode_buffer(buffer_size) + required_size = GetShortPathNameW( + long_path, + buffer_array, + buffer_size, + ) + if required_size > buffer_size: + buffer_size = required_size + else: + return buffer_array.value + +else: + + def _get_short_path_name(long_path: str) -> str: + return long_path + def get_short_path_name(long_path: str) -> str: """ Get short path form for long path. @@ -263,22 +293,4 @@ def get_short_path_name(long_path: str) -> str: :returns: `str` Short path form of the long path. """ - if not WINDOWS: - return long_path - # Access `GetShortPathNameW()` function from `kernel32.dll`. - GetShortPathNameW = windll.kernel32.GetShortPathNameW - GetShortPathNameW.argtypes = [ - wintypes.LPCWSTR, - wintypes.LPWSTR, - wintypes.DWORD, - ] - GetShortPathNameW.restype = wintypes.DWORD - # Call function to get short path form. - buffer_size = 0 - while True: - buffer_array = create_unicode_buffer(buffer_size) - required_size = GetShortPathNameW(long_path, buffer_array, buffer_size) - if required_size > buffer_size: - buffer_size = required_size - else: - return buffer_array.value + return _get_short_path_name(long_path) From c030562d2b88d1b5617d1fbd4d1e5e121166e964 Mon Sep 17 00:00:00 2001 From: achekerylla Date: Tue, 12 Mar 2024 14:51:57 -0700 Subject: [PATCH 4/8] Fix another blacken error --- invoke/terminals.py | 1 + 1 file changed, 1 insertion(+) diff --git a/invoke/terminals.py b/invoke/terminals.py index 301e5f313..4c1de7fab 100644 --- a/invoke/terminals.py +++ b/invoke/terminals.py @@ -279,6 +279,7 @@ def _get_short_path_name(long_path: str) -> str: def _get_short_path_name(long_path: str) -> str: return long_path + def get_short_path_name(long_path: str) -> str: """ Get short path form for long path. From 3b637242dfe2d17096622d575381c2fbcdfc8de3 Mon Sep 17 00:00:00 2001 From: achekerylla Date: Tue, 12 Mar 2024 15:17:27 -0700 Subject: [PATCH 5/8] Move untestable code to shims --- .coveragerc | 4 +++- invoke/runners.py | 5 ++--- invoke/shims.py | 54 +++++++++++++++++++++++++++++++++++++++++++++ invoke/terminals.py | 50 ----------------------------------------- 4 files changed, 59 insertions(+), 54 deletions(-) create mode 100644 invoke/shims.py diff --git a/.coveragerc b/.coveragerc index 450dee107..eea441fb1 100644 --- a/.coveragerc +++ b/.coveragerc @@ -3,4 +3,6 @@ branch = True include = invoke/* tests/* -omit = invoke/vendor/* +omit = + invoke/shims.py + invoke/vendor/* diff --git a/invoke/runners.py b/invoke/runners.py index 5c32fbbe2..2e10080fb 100644 --- a/invoke/runners.py +++ b/invoke/runners.py @@ -50,8 +50,8 @@ character_buffered, ready_for_reading, bytes_to_read, - get_short_path_name, ) +from .shims import get_short_path_name from .util import has_fileno, isatty, ExceptionHandlingThread if TYPE_CHECKING: @@ -416,8 +416,7 @@ def _setup(self, command: str, kwargs: Any) -> None: # Echo running command (wants to be early to be included in dry-run) if self.opts["echo"]: self.echo(command) - if WINDOWS: - self.opts["shell"] = get_short_path_name(self.opts["shell"]) + self.opts["shell"] = get_short_path_name(self.opts["shell"]) # Prepare common result args. # TODO: I hate this. Needs a deeper separate think about tweaking # Runner.generate_result in a way that isn't literally just this same diff --git a/invoke/shims.py b/invoke/shims.py new file mode 100644 index 000000000..490f0342a --- /dev/null +++ b/invoke/shims.py @@ -0,0 +1,54 @@ +import sys + + +if sys.platform == "win32": + from ctypes import ( + windll, + wintypes, + create_unicode_buffer, + ) + + def _get_short_path_name(long_path: str) -> str: + # Access `GetShortPathNameW()` function from `kernel32.dll`. + GetShortPathNameW = windll.kernel32.GetShortPathNameW + GetShortPathNameW.argtypes = [ + wintypes.LPCWSTR, + wintypes.LPWSTR, + wintypes.DWORD, + ] + GetShortPathNameW.restype = wintypes.DWORD + # Call function to get short path form. + buffer_size = 0 + while True: + buffer_array = create_unicode_buffer(buffer_size) + required_size = GetShortPathNameW( + long_path, + buffer_array, + buffer_size, + ) + if required_size > buffer_size: + buffer_size = required_size + else: + return buffer_array.value + +else: + + def _get_short_path_name(long_path: str) -> str: + return long_path + + +def get_short_path_name(long_path: str) -> str: + """ + Get short path form for long path. + + Only applies to Windows-based systems; on Unix this is a pass-thru. + + .. note:: + This API converts path strings to the 8.3 format used by earlier + tools on the Windows platform. This format has no spaces. + + :param long_path: Long path such as `shutil.which()` results. + + :returns: `str` Short path form of the long path. + """ + return _get_short_path_name(long_path) diff --git a/invoke/terminals.py b/invoke/terminals.py index 4c1de7fab..2694712fa 100644 --- a/invoke/terminals.py +++ b/invoke/terminals.py @@ -34,10 +34,8 @@ Structure, c_ushort, windll, - wintypes, POINTER, byref, - create_unicode_buffer, ) from ctypes.wintypes import HANDLE, _COORD, _SMALL_RECT else: @@ -247,51 +245,3 @@ def bytes_to_read(input_: IO) -> int: fionread = fcntl.ioctl(input_, termios.FIONREAD, b" ") return int(struct.unpack("h", fionread)[0]) return 1 - - -if sys.platform == "win32": - - def _get_short_path_name(long_path: str) -> str: - # Access `GetShortPathNameW()` function from `kernel32.dll`. - GetShortPathNameW = windll.kernel32.GetShortPathNameW - GetShortPathNameW.argtypes = [ - wintypes.LPCWSTR, - wintypes.LPWSTR, - wintypes.DWORD, - ] - GetShortPathNameW.restype = wintypes.DWORD - # Call function to get short path form. - buffer_size = 0 - while True: - buffer_array = create_unicode_buffer(buffer_size) - required_size = GetShortPathNameW( - long_path, - buffer_array, - buffer_size, - ) - if required_size > buffer_size: - buffer_size = required_size - else: - return buffer_array.value - -else: - - def _get_short_path_name(long_path: str) -> str: - return long_path - - -def get_short_path_name(long_path: str) -> str: - """ - Get short path form for long path. - - Only applies to Windows-based systems; on Unix this is a pass-thru. - - .. note:: - This API converts path strings to the 8.3 format used by earlier - tools on the Windows platform. This format has no spaces. - - :param long_path: Long path such as `shutil.which()` results. - - :returns: `str` Short path form of the long path. - """ - return _get_short_path_name(long_path) From f0b26fc3a5bbd8c7cc0cc1df5597f32732f7b4b1 Mon Sep 17 00:00:00 2001 From: achekerylla Date: Tue, 12 Mar 2024 15:36:10 -0700 Subject: [PATCH 6/8] Move untestable code to shims again --- .coveragerc | 1 + tests/_shims.py | 33 +++++++++++++++++++++++++++++++++ tests/_util.py | 10 ---------- tests/runners.py | 2 +- tests/terminals.py | 2 +- 5 files changed, 36 insertions(+), 12 deletions(-) create mode 100644 tests/_shims.py diff --git a/.coveragerc b/.coveragerc index eea441fb1..8b485e140 100644 --- a/.coveragerc +++ b/.coveragerc @@ -6,3 +6,4 @@ include = omit = invoke/shims.py invoke/vendor/* + tests/_shims.py diff --git a/tests/_shims.py b/tests/_shims.py new file mode 100644 index 000000000..b1fddd52a --- /dev/null +++ b/tests/_shims.py @@ -0,0 +1,33 @@ +import sys + +from functools import wraps +from pytest import skip +from typing import Callable + + +if sys.platform == "win32": + + def _skip_if_posix(fn): + @wraps(fn) + def wrapper(*args, **kwargs): + return fn(*args, **kwargs) + + return wrapper + + +else: + + def _skip_if_posix(fn): + @wraps(fn) + def wrapper(*args, **kwargs): + skip() + return fn(*args, **kwargs) + + return wrapper + + +def skip_if_posix(fn: Callable) -> Callable: + """ + Skip test if posix. + """ + return _skip_if_posix(fn) diff --git a/tests/_util.py b/tests/_util.py index 576c3c00f..50a4c7fc9 100644 --- a/tests/_util.py +++ b/tests/_util.py @@ -32,16 +32,6 @@ def wrapper(*args, **kwargs): return wrapper -def skip_if_posix(fn): - @wraps(fn) - def wrapper(*args, **kwargs): - if not WINDOWS: - skip() - return fn(*args, **kwargs) - - return wrapper - - @contextmanager def support_path(): sys.path.insert(0, support) diff --git a/tests/runners.py b/tests/runners.py index 2cf197054..34380c636 100644 --- a/tests/runners.py +++ b/tests/runners.py @@ -33,10 +33,10 @@ from invoke.runners import default_encoding from invoke.terminals import WINDOWS +from _shims import skip_if_posix from _util import ( mock_subprocess, mock_pty, - skip_if_posix, skip_if_windows, _Dummy, _KeyboardInterruptingRunner, diff --git a/tests/terminals.py b/tests/terminals.py index 890f89909..3a989bdda 100644 --- a/tests/terminals.py +++ b/tests/terminals.py @@ -4,10 +4,10 @@ from unittest.mock import Mock, patch from pytest import skip, mark +from invoke.shims import get_short_path_name from invoke.terminals import ( pty_size, bytes_to_read, - get_short_path_name, WINDOWS, ) From c140d4e23a53955c27f3213ea39463646cb8931f Mon Sep 17 00:00:00 2001 From: achekerylla Date: Tue, 12 Mar 2024 16:10:50 -0700 Subject: [PATCH 7/8] Skip unrunnable test for windows --- .coveragerc | 1 - tests/_shims.py | 33 --------------------------------- tests/runners.py | 7 +------ 3 files changed, 1 insertion(+), 40 deletions(-) delete mode 100644 tests/_shims.py diff --git a/.coveragerc b/.coveragerc index 8b485e140..eea441fb1 100644 --- a/.coveragerc +++ b/.coveragerc @@ -6,4 +6,3 @@ include = omit = invoke/shims.py invoke/vendor/* - tests/_shims.py diff --git a/tests/_shims.py b/tests/_shims.py deleted file mode 100644 index b1fddd52a..000000000 --- a/tests/_shims.py +++ /dev/null @@ -1,33 +0,0 @@ -import sys - -from functools import wraps -from pytest import skip -from typing import Callable - - -if sys.platform == "win32": - - def _skip_if_posix(fn): - @wraps(fn) - def wrapper(*args, **kwargs): - return fn(*args, **kwargs) - - return wrapper - - -else: - - def _skip_if_posix(fn): - @wraps(fn) - def wrapper(*args, **kwargs): - skip() - return fn(*args, **kwargs) - - return wrapper - - -def skip_if_posix(fn: Callable) -> Callable: - """ - Skip test if posix. - """ - return _skip_if_posix(fn) diff --git a/tests/runners.py b/tests/runners.py index 34380c636..91dc47993 100644 --- a/tests/runners.py +++ b/tests/runners.py @@ -33,7 +33,6 @@ from invoke.runners import default_encoding from invoke.terminals import WINDOWS -from _shims import skip_if_posix from _util import ( mock_subprocess, mock_pty, @@ -232,12 +231,8 @@ def may_be_configured(self): runner = self._runner(run={"shell": "/bin/tcsh"}) assert runner.run(_).shell == "/bin/tcsh" - @skip_if_posix def may_be_configured_with_short_path_on_windows(self): - runner = self._runner( - run={"shell": "C:\\Program Files\\PowerShell\\7\\pwsh.exe"} - ) - assert runner.run(_).shell == "C:\\PROGRA~1\\POWERS~1\\7\\pwsh.exe" + skip() @skip_if_windows def may_be_configured_with_passthru_on_posix(self): From c1880d2bba7a56aa5ddee4565fe9df0e8f6a2904 Mon Sep 17 00:00:00 2001 From: achekerylla Date: Sun, 31 Mar 2024 11:41:06 -0700 Subject: [PATCH 8/8] PR Update: Restore coverage for shims --- .coveragerc | 4 +--- tests/terminals.py | 4 ++-- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/.coveragerc b/.coveragerc index eea441fb1..450dee107 100644 --- a/.coveragerc +++ b/.coveragerc @@ -3,6 +3,4 @@ branch = True include = invoke/* tests/* -omit = - invoke/shims.py - invoke/vendor/* +omit = invoke/vendor/* diff --git a/tests/terminals.py b/tests/terminals.py index 3a989bdda..d1cb9cece 100644 --- a/tests/terminals.py +++ b/tests/terminals.py @@ -6,9 +6,9 @@ from invoke.shims import get_short_path_name from invoke.terminals import ( - pty_size, - bytes_to_read, WINDOWS, + bytes_to_read, + pty_size, ) # Skip on Windows CI, it may blow up on one of these tests