From 5c5459b47e43a68e1dee22d4fbcf3557edd05c27 Mon Sep 17 00:00:00 2001 From: Scott K Logan Date: Sat, 17 Feb 2024 23:38:50 -0600 Subject: [PATCH 1/4] Re-work colcon_core.command.get_prog_name This function's purpose is to handle these special cases of argv[0]: * Invoked using python -m ... * Invoked using a path to the executable even though the executable is on the PATH This change enhances the path comparison to support normalization of that path, Windows long path prefixes, and also the easy-install behavior on Windows where argv[0] has no extension. Yet to be properly handled is invocation using python -c ... --- colcon_core/command.py | 33 +++++++++++++++++++++++++++------ 1 file changed, 27 insertions(+), 6 deletions(-) diff --git a/colcon_core/command.py b/colcon_core/command.py index 07dddaf1c..76ec82f59 100644 --- a/colcon_core/command.py +++ b/colcon_core/command.py @@ -249,17 +249,38 @@ def _parse_optional(self, arg_string): return parser +def _normalize_exe_path(path_str): + if path_str is None: + return None + path_str = os.path.normpath(path_str) + if sys.platform == 'win32' and path_str.startswith('\\\\?\\'): + path_str = path_str[4:] + return Path.cwd() / Path(path_str) + + def get_prog_name(): """Get the prog name used for the argparse parser.""" - prog = sys.argv[0] - basename = os.path.basename(prog) + prog = _normalize_exe_path(sys.argv[0]) + basename = prog.name if basename == '__main__.py': # use the module name in case the script was invoked with python -m ... - prog = os.path.basename(os.path.dirname(prog)) - elif shutil.which(basename) == prog: - # use basename only if it is on the PATH + prog = prog.parent.name + elif basename == '-c': + # the script was invoked with python -c ... prog = basename - return prog + else: + default_prog = _normalize_exe_path(shutil.which(basename)) + if prog == default_prog: + # use basename only if it is on the PATH + prog = basename + elif ( + sys.platform == 'win32' and + default_prog and + prog == default_prog.with_suffix('') + ): + # easy-install on Windows drops the .exe on argv[0] + prog = basename + return str(prog) class CustomFormatter(argparse.RawDescriptionHelpFormatter): From c174d0166493a9c80d451b92f2caae4784e3c6a9 Mon Sep 17 00:00:00 2001 From: Scott K Logan Date: Thu, 25 Apr 2024 15:53:07 -0500 Subject: [PATCH 2/4] Re-work to use os.path.samefile --- colcon_core/command.py | 41 ++++++++++++++++------------------------- 1 file changed, 16 insertions(+), 25 deletions(-) diff --git a/colcon_core/command.py b/colcon_core/command.py index 76ec82f59..c3030d4ca 100644 --- a/colcon_core/command.py +++ b/colcon_core/command.py @@ -249,38 +249,29 @@ def _parse_optional(self, arg_string): return parser -def _normalize_exe_path(path_str): - if path_str is None: - return None - path_str = os.path.normpath(path_str) - if sys.platform == 'win32' and path_str.startswith('\\\\?\\'): - path_str = path_str[4:] - return Path.cwd() / Path(path_str) - - def get_prog_name(): """Get the prog name used for the argparse parser.""" - prog = _normalize_exe_path(sys.argv[0]) - basename = prog.name + prog = sys.argv[0] + basename = os.path.basename(prog) if basename == '__main__.py': # use the module name in case the script was invoked with python -m ... - prog = prog.parent.name - elif basename == '-c': - # the script was invoked with python -c ... - prog = basename + prog = os.path.basename(os.path.dirname(prog)) else: - default_prog = _normalize_exe_path(shutil.which(basename)) - if prog == default_prog: - # use basename only if it is on the PATH - prog = basename - elif ( + default_prog = shutil.which(basename) or '' + default_ext = os.path.splitext(default_prog)[1] + real_prog = prog + if ( sys.platform == 'win32' and - default_prog and - prog == default_prog.with_suffix('') + os.path.splitext(real_prog)[1] != default_ext ): - # easy-install on Windows drops the .exe on argv[0] - prog = basename - return str(prog) + real_prog += default_ext + try: + if os.path.samefile(default_prog, real_prog): + # use basename only if it is on the PATH + prog = basename + except FileNotFoundError: + pass + return prog class CustomFormatter(argparse.RawDescriptionHelpFormatter): From 4d99d015de883b6b57dd825f96bbf67834e52640 Mon Sep 17 00:00:00 2001 From: Scott K Logan Date: Fri, 26 Apr 2024 09:42:40 -0500 Subject: [PATCH 3/4] Add comments --- colcon_core/command.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/colcon_core/command.py b/colcon_core/command.py index b223b97d8..c855b3512 100644 --- a/colcon_core/command.py +++ b/colcon_core/command.py @@ -274,8 +274,15 @@ def get_prog_name(): sys.platform == 'win32' and os.path.splitext(real_prog)[1] != default_ext ): + # On Windows, setuptools entry points drop the file extension from + # argv[0], but shutil.which does not. If the two don't end in the + # same extension, try appending the shutil extension for a better + # chance at matching. real_prog += default_ext try: + # The os.path.samefile requires that both files exist on disk, but + # has the advantage of working around symlinks, UNC-style paths, + # DOS 8.3 path atoms, and path normalization. if os.path.samefile(default_prog, real_prog): # use basename only if it is on the PATH prog = basename From d2553e6e873e74a2b3e784a8ffcf765c35934d69 Mon Sep 17 00:00:00 2001 From: Scott K Logan Date: Fri, 31 May 2024 17:34:56 -0500 Subject: [PATCH 4/4] Add tests, fix a bug --- colcon_core/command.py | 2 +- test/spell_check.words | 1 + test/test_command.py | 86 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 88 insertions(+), 1 deletion(-) diff --git a/colcon_core/command.py b/colcon_core/command.py index c855b3512..b4e605edd 100644 --- a/colcon_core/command.py +++ b/colcon_core/command.py @@ -286,7 +286,7 @@ def get_prog_name(): if os.path.samefile(default_prog, real_prog): # use basename only if it is on the PATH prog = basename - except FileNotFoundError: + except (FileNotFoundError, NotADirectoryError): pass return prog diff --git a/test/spell_check.words b/test/spell_check.words index 83548464c..284a0c162 100644 --- a/test/spell_check.words +++ b/test/spell_check.words @@ -113,6 +113,7 @@ setuptools shlex sigint sitecustomize +skipif sloretz stacklevel staticmethod diff --git a/test/test_command.py b/test/test_command.py index d2aca2d2c..c8c6b6e9e 100644 --- a/test/test_command.py +++ b/test/test_command.py @@ -1,15 +1,18 @@ # Copyright 2016-2018 Dirk Thomas # Licensed under the Apache License, Version 2.0 +import os import shutil import signal import sys from tempfile import mkdtemp +from tempfile import TemporaryDirectory from unittest.mock import Mock from unittest.mock import patch from colcon_core.command import CommandContext from colcon_core.command import create_parser +from colcon_core.command import get_prog_name from colcon_core.command import main from colcon_core.command import verb_main from colcon_core.environment_variable import EnvironmentVariable @@ -151,3 +154,86 @@ def test_verb_main(): assert logger.error.call_args[0][0].startswith( 'command_name verb_name: custom error message\n') assert 'Exception: custom error message' in logger.error.call_args[0][0] + + +def test_prog_name_module(): + argv = [os.path.join('foo', 'bar', '__main__.py')] + with patch('colcon_core.command.sys.argv', argv): + # prog should be the module containing __main__.py + assert get_prog_name() == 'bar' + + +def test_prog_name_on_path(): + # use __file__ since we know it exists + argv = [__file__] + with patch('colcon_core.command.sys.argv', argv): + with patch( + 'colcon_core.command.shutil.which', + return_value=__file__ + ): + # prog should be shortened to the basename + assert get_prog_name() == 'test_command.py' + + +def test_prog_name_not_on_path(): + # use __file__ since we know it exists + argv = [__file__] + with patch('colcon_core.command.sys.argv', argv): + with patch('colcon_core.command.shutil.which', return_value=None): + # prog should remain unchanged + assert get_prog_name() == __file__ + + +def test_prog_name_different_on_path(): + # use __file__ since we know it exists + argv = [__file__] + with patch('colcon_core.command.sys.argv', argv): + with patch( + 'colcon_core.command.shutil.which', + return_value=sys.executable + ): + # prog should remain unchanged + assert get_prog_name() == __file__ + + +def test_prog_name_not_a_file(): + # pick some file that doesn't actually exist on disk + no_such_file = os.path.join(__file__, 'foobar') + argv = [no_such_file] + with patch('colcon_core.command.sys.argv', argv): + with patch( + 'colcon_core.command.shutil.which', + return_value=no_such_file + ): + # prog should remain unchanged + assert get_prog_name() == no_such_file + + +@pytest.mark.skipif(sys.platform == 'win32', reason='Symlinks not supported.') +def test_prog_name_symlink(): + # use __file__ since we know it exists + with TemporaryDirectory(prefix='test_colcon_') as temp_dir: + linked_file = os.path.join(temp_dir, 'test_command.py') + os.symlink(__file__, linked_file) + + argv = [linked_file] + with patch('colcon_core.command.sys.argv', argv): + with patch( + 'colcon_core.command.shutil.which', + return_value=__file__ + ): + # prog should be shortened to the basename + assert get_prog_name() == 'test_command.py' + + +@pytest.mark.skipif(sys.platform != 'win32', reason='Only valid on Windows.') +def test_prog_name_easy_install(): + # use __file__ since we know it exists + argv = [__file__[:-3]] + with patch('colcon_core.command.sys.argv', argv): + with patch( + 'colcon_core.command.shutil.which', + return_value=__file__ + ): + # prog should be shortened to the basename + assert get_prog_name() == 'test_command'