From 1d0645e86f693f5f24ec9aea815504e13f1fee76 Mon Sep 17 00:00:00 2001 From: Chris Jerdonek Date: Mon, 11 Feb 2019 08:27:26 -0800 Subject: [PATCH] Add format_command_args() with tests. --- src/pip/_internal/utils/misc.py | 23 +++++++++++++++++------ src/pip/_internal/wheel.py | 6 +++--- tests/unit/test_utils.py | 19 +++++++++++++++---- tests/unit/test_wheel.py | 12 ++++++------ 4 files changed, 41 insertions(+), 19 deletions(-) diff --git a/src/pip/_internal/utils/misc.py b/src/pip/_internal/utils/misc.py index 3c49cf1da..3053ca4b2 100644 --- a/src/pip/_internal/utils/misc.py +++ b/src/pip/_internal/utils/misc.py @@ -650,6 +650,21 @@ def unpack_file( ) +def format_command_args(args): + # type: (List[str]) -> str + """ + Format command arguments for display. + """ + parts = [] + for arg in args: + if ' ' in arg or '\n' in arg or '"' in arg or "'" in arg: + arg = '"%s"' % arg.replace('"', '\\"') + parts.append(arg) + command = ' '.join(parts) + + return command + + def call_subprocess( cmd, # type: List[str] show_stdout=False, # type: bool @@ -699,12 +714,8 @@ def call_subprocess( else: stdout = subprocess.PIPE if command_desc is None: - cmd_parts = [] - for part in cmd: - if ' ' in part or '\n' in part or '"' in part or "'" in part: - part = '"%s"' % part.replace('"', '\\"') - cmd_parts.append(part) - command_desc = ' '.join(cmd_parts) + command_desc = format_command_args(cmd) + logger.debug("Running command %s", command_desc) env = os.environ.copy() if extra_environ: diff --git a/src/pip/_internal/wheel.py b/src/pip/_internal/wheel.py index 8cdd21b01..466688a6e 100644 --- a/src/pip/_internal/wheel.py +++ b/src/pip/_internal/wheel.py @@ -786,7 +786,7 @@ def should_use_ephemeral_cache( return True -def format_command( +def format_command_result( command_args, # type: List[str] command_output, # type: str ): @@ -825,7 +825,7 @@ def get_legacy_build_wheel_path( msg = ( 'Legacy build of wheel for {!r} created no files.\n' ).format(req.name) - msg += format_command(command_args, command_output) + msg += format_command_result(command_args, command_output) logger.warning(msg) return None @@ -834,7 +834,7 @@ def get_legacy_build_wheel_path( 'Legacy build of wheel for {!r} created more than one file.\n' 'Filenames (choosing first): {}\n' ).format(req.name, names) - msg += format_command(command_args, command_output) + msg += format_command_result(command_args, command_output) logger.warning(msg) return os.path.join(temp_dir, names[0]) diff --git a/tests/unit/test_utils.py b/tests/unit/test_utils.py index 2ceebd079..41b3b0e09 100644 --- a/tests/unit/test_utils.py +++ b/tests/unit/test_utils.py @@ -24,10 +24,11 @@ from pip._internal.utils.encoding import auto_decode from pip._internal.utils.glibc import check_glibc_version from pip._internal.utils.hashes import Hashes, MissingHashes from pip._internal.utils.misc import ( - call_subprocess, egg_link_path, ensure_dir, get_installed_distributions, - get_prog, make_vcs_requirement_url, normalize_path, redact_netloc, - redact_password_from_url, remove_auth_from_url, rmtree, - split_auth_from_netloc, untar_file, unzip_file, + call_subprocess, egg_link_path, ensure_dir, format_command_args, + get_installed_distributions, get_prog, make_vcs_requirement_url, + normalize_path, redact_netloc, redact_password_from_url, + remove_auth_from_url, rmtree, split_auth_from_netloc, untar_file, + unzip_file, ) from pip._internal.utils.packaging import check_dist_requires_python from pip._internal.utils.temp_dir import AdjacentTempDirectory, TempDirectory @@ -724,6 +725,16 @@ class TestGetProg(object): assert get_prog() == expected +@pytest.mark.parametrize('args, expected', [ + (['pip', 'list'], 'pip list'), + (['foo', 'space space', 'new\nline', 'double"quote', "single'quote"], + 'foo "space space" "new\nline" "double\\"quote" "single\'quote"'), +]) +def test_format_command_args(args, expected): + actual = format_command_args(args) + assert actual == expected + + def test_call_subprocess_works__no_keyword_arguments(): result = call_subprocess( [sys.executable, '-c', 'print("Hello")'], diff --git a/tests/unit/test_wheel.py b/tests/unit/test_wheel.py index 61e9aaa52..47121d0af 100644 --- a/tests/unit/test_wheel.py +++ b/tests/unit/test_wheel.py @@ -99,9 +99,9 @@ def test_should_use_ephemeral_cache__issue_6197( assert ephem_cache is expected -def test_format_command__INFO(caplog): +def test_format_command_result__INFO(caplog): caplog.set_level(logging.INFO) - actual = wheel.format_command( + actual = wheel.format_command_result( command_args=['arg1', 'arg2'], command_output='output line 1\noutput line 2\n', ) @@ -117,9 +117,9 @@ def test_format_command__INFO(caplog): # Test no trailing newline. 'output line 1\noutput line 2', ]) -def test_format_command__DEBUG(caplog, command_output): +def test_format_command_result__DEBUG(caplog, command_output): caplog.set_level(logging.DEBUG) - actual = wheel.format_command( + actual = wheel.format_command_result( command_args=['arg1', 'arg2'], command_output=command_output, ) @@ -133,9 +133,9 @@ def test_format_command__DEBUG(caplog, command_output): @pytest.mark.parametrize('log_level', ['DEBUG', 'INFO']) -def test_format_command__empty_output(caplog, log_level): +def test_format_command_result__empty_output(caplog, log_level): caplog.set_level(log_level) - actual = wheel.format_command( + actual = wheel.format_command_result( command_args=['arg1', 'arg2'], command_output='', )