Quote paths in editable installs

HISTORY.rst

@@ -2,6 +2,10 @@
 
 Release History
 ===============
+0.2.11b2
+++++++++
+* Quote paths when running editable installs (``pip install -e <path>``) and the pytest runner so that paths containing spaces (e.g. OneDrive folders) no longer break ``azdev extension add``, ``azdev setup``, code generation, and ``azdev test``. (#550, #415)
+
 0.2.11b1
 ++++++++
 * Extract extension metadata generation logic and decouple from ``wheel==0.30.0``; read wheel ``METADATA`` via ``pkginfo`` instead of the legacy ``metadata.json`` artifact. Drops the ``wheel==0.30.0`` and ``setuptools==70.0.0`` pins. (#521)

azdev/__init__.py

@@ -4,4 +4,4 @@
 # license information.
 # -----------------------------------------------------------------------------
 
-__VERSION__ = '0.2.11b1'
+__VERSION__ = '0.2.11b2'

azdev/operations/code_gen.py

@@ -14,7 +14,7 @@
 
 from azdev.utilities import (
     pip_cmd, display, heading, COMMAND_MODULE_PREFIX, EXTENSION_PREFIX, get_cli_repo_path, get_ext_repo_paths,
-    find_files)
+    find_files, quote_arg)
 
 logger = get_logger(__name__)
 
@@ -300,7 +300,7 @@ def _create_package(prefix, repo_path, is_ext, name='test', display_name=None, d
 
     if is_ext:
         result = pip_cmd(
-            'install -e {} {}'.format(new_package_path, _PIP_EDITABLE_OPTS),
+            'install -e {} {}'.format(quote_arg(new_package_path), _PIP_EDITABLE_OPTS),
             "Installing `{}{}`...".format(prefix, name),
         )
         if result.error:

azdev/operations/extensions/__init__.py

@@ -16,7 +16,7 @@
 
 from azdev.utilities import (
     cmd, py_cmd, pip_cmd, display, get_ext_repo_paths, find_files, get_azure_config, get_azdev_config,
-    get_azure_config_dir, require_azure_cli, heading, subheading, EXTENSION_PREFIX)
+    get_azure_config_dir, require_azure_cli, heading, subheading, quote_arg, EXTENSION_PREFIX)
 from .version_upgrade import VersionUpgradeMod
 
 logger = get_logger(__name__)
@@ -73,7 +73,7 @@ def add_extension(extensions):
 
     for path in paths_to_add:
         result = pip_cmd(
-            'install -e {} {}'.format(path, _PIP_EDITABLE_OPTS),
+            'install -e {} {}'.format(quote_arg(path), _PIP_EDITABLE_OPTS),
             "Adding extension '{}'...".format(path),
         )
         if result.error:

azdev/operations/setup.py

@@ -17,7 +17,7 @@
 from azdev.params import Flag
 from azdev.utilities import (
     display, heading, subheading, pip_cmd, CommandError, find_file,
-    get_azdev_config_dir, get_azdev_config, require_virtual_env, get_azure_config)
+    get_azdev_config_dir, get_azdev_config, require_virtual_env, get_azure_config, quote_arg)
 
 logger = get_logger(__name__)
 
@@ -51,7 +51,8 @@ def _install_extensions(ext_paths):
 
     # install specified extensions
     for path in ext_paths or []:
-        result = pip_cmd('install -e {} {}'.format(path, _PIP_EDITABLE_OPTS), "Adding extension '{}'...".format(path))
+        result = pip_cmd('install -e {} {}'.format(quote_arg(path), _PIP_EDITABLE_OPTS),
+                         "Adding extension '{}'...".format(path))
         if result.error:
             raise result.error  # pylint: disable=raising-bad-type
 
@@ -77,13 +78,13 @@ def _install_cli(cli_path, deps=None):
     privates_dir = os.path.join(cli_path, "privates")
     if os.path.isdir(privates_dir) and os.listdir(privates_dir):
         whl_list = " ".join(
-            [os.path.join(privates_dir, f) for f in os.listdir(privates_dir)]
+            [quote_arg(os.path.join(privates_dir, f)) for f in os.listdir(privates_dir)]
         )
         pip_cmd("install {}".format(whl_list), "Installing private whl files...")
 
     # install general requirements
     pip_cmd(
-        "install -r {}".format(os.path.join(cli_path, "requirements.txt")),
+        "install -r {}".format(quote_arg(os.path.join(cli_path, "requirements.txt"))),
         "Installing `requirements.txt`..."
     )
 
@@ -92,53 +93,56 @@ def _install_cli(cli_path, deps=None):
         # Resolve dependencies from setup.py files.
         # command modules have dependency on azure-cli-core so install this first
         pip_cmd(
-            "install -e {} {}".format(os.path.join(cli_src, 'azure-cli-telemetry'), _PIP_EDITABLE_OPTS),
+            "install -e {} {}".format(quote_arg(os.path.join(cli_src, 'azure-cli-telemetry')), _PIP_EDITABLE_OPTS),
             "Installing `azure-cli-telemetry`..."
         )
         pip_cmd(
-            "install -e {} {}".format(os.path.join(cli_src, 'azure-cli-core'), _PIP_EDITABLE_OPTS),
+            "install -e {} {}".format(quote_arg(os.path.join(cli_src, 'azure-cli-core')), _PIP_EDITABLE_OPTS),
             "Installing `azure-cli-core`..."
         )
 
         # azure cli has dependencies on the above packages so install this one last
         pip_cmd(
-            "install -e {} {}".format(os.path.join(cli_src, 'azure-cli'), _PIP_EDITABLE_OPTS),
+            "install -e {} {}".format(quote_arg(os.path.join(cli_src, 'azure-cli')), _PIP_EDITABLE_OPTS),
             "Installing `azure-cli`..."
         )
 
         pip_cmd(
-            "install -e {} {}".format(os.path.join(cli_src, 'azure-cli-testsdk'), _PIP_EDITABLE_OPTS),
+            "install -e {} {}".format(quote_arg(os.path.join(cli_src, 'azure-cli-testsdk')), _PIP_EDITABLE_OPTS),
             "Installing `azure-cli-testsdk`..."
         )
     else:
         # First install packages without dependencies,
         # then resolve dependencies from requirements.*.txt file.
         pip_cmd(
-            "install -e {} --no-deps {}".format(os.path.join(cli_src, 'azure-cli-telemetry'), _PIP_EDITABLE_OPTS),
+            "install -e {} --no-deps {}".format(
+                quote_arg(os.path.join(cli_src, 'azure-cli-telemetry')), _PIP_EDITABLE_OPTS),
             "Installing `azure-cli-telemetry`..."
         )
         pip_cmd(
-            "install -e {} --no-deps {}".format(os.path.join(cli_src, 'azure-cli-core'), _PIP_EDITABLE_OPTS),
+            "install -e {} --no-deps {}".format(
+                quote_arg(os.path.join(cli_src, 'azure-cli-core')), _PIP_EDITABLE_OPTS),
             "Installing `azure-cli-core`..."
         )
 
         pip_cmd(
-            "install -e {} --no-deps {}".format(os.path.join(cli_src, 'azure-cli'), _PIP_EDITABLE_OPTS),
+            "install -e {} --no-deps {}".format(
+                quote_arg(os.path.join(cli_src, 'azure-cli')), _PIP_EDITABLE_OPTS),
             "Installing `azure-cli`..."
         )
 
         # The dependencies of testsdk are not in requirements.txt as this package is not needed by the
         # azure-cli package for running commands.
         # Here we need to install with dependencies for azdev test.
         pip_cmd(
-            "install -e {} {}".format(os.path.join(cli_src, 'azure-cli-testsdk'), _PIP_EDITABLE_OPTS),
+            "install -e {} {}".format(quote_arg(os.path.join(cli_src, 'azure-cli-testsdk')), _PIP_EDITABLE_OPTS),
             "Installing `azure-cli-testsdk`..."
         )
         import platform
         system = platform.system()
         req_file = 'requirements.py3.{}.txt'.format(system)
         pip_cmd(
-            "install -r {}".format(os.path.join(cli_src, 'azure-cli', req_file)),
+            "install -r {}".format(quote_arg(os.path.join(cli_src, 'azure-cli', req_file))),
             "Installing `{}`...".format(req_file)
         )
 

azdev/operations/tests/test_editable_install_quoting.py

@@ -0,0 +1,61 @@
+# -----------------------------------------------------------------------------
+# Copyright (c) Microsoft Corporation. All rights reserved.
+# Licensed under the MIT License. See License.txt in the project root for
+# license information.
+# -----------------------------------------------------------------------------
+
+import shlex
+import unittest
+from unittest.mock import patch, MagicMock
+
+from azdev.utilities import quote_arg
+
+
+class TestQuoteArg(unittest.TestCase):
+    """Regression coverage for GitHub issue #550: editable installs must survive
+    paths containing spaces (e.g. OneDrive folders)."""
+
+    def test_posix_quoting_roundtrips_through_shlex(self):
+        path = '/home/user/Azure Powershell/src/ssh'
+        with patch('azdev.utilities.IS_WINDOWS', False):
+            quoted = quote_arg(path)
+        # POSIX command runners split with shlex.split, so the quoted form must
+        # round-trip back to the original single argument.
+        self.assertEqual(shlex.split('install -e {}'.format(quoted)), ['install', '-e', path])
+
+    def test_windows_quoting_wraps_spaces(self):
+        path = r'C:\Users\me\Azure Powershell\src\ssh'
+        with patch('azdev.utilities.IS_WINDOWS', True):
+            quoted = quote_arg(path)
+        # On Windows the string is passed verbatim to CreateProcess, which treats a
+        # space-containing path as multiple arguments unless it is double-quoted.
+        self.assertTrue(quoted.startswith('"') and quoted.endswith('"'))
+        self.assertIn(path, quoted)
+
+    def test_no_spaces_is_unchanged_on_posix(self):
+        path = '/home/user/src/ssh'
+        with patch('azdev.utilities.IS_WINDOWS', False):
+            self.assertEqual(quote_arg(path), path)
+
+
+class TestEditableInstallUsesQuotedPath(unittest.TestCase):
+
+    @patch('azdev.operations.extensions._invalidate_command_index')
+    @patch('azdev.operations.extensions.pip_cmd')
+    @patch('azdev.operations.extensions.find_files',
+           return_value=['/repo/Azure Powershell/src/my-ext/setup.py'])
+    @patch('azdev.operations.extensions.get_ext_repo_paths', return_value=['/repo'])
+    def test_add_extension_quotes_whitespace_path(self, _mock_paths, _mock_find, mock_pip, _mock_invalidate):
+        mock_pip.return_value = MagicMock(error=None)
+
+        from azdev.operations.extensions import add_extension
+        add_extension(['my-ext'])
+
+        # The path interpolated into the pip command must be quoted, not bare.
+        command = mock_pip.call_args[0][0]
+        self.assertNotIn('install -e /repo/Azure Powershell/src/my-ext ', command)
+        self.assertIn(quote_arg('/repo/Azure Powershell/src/my-ext'), command)
+
+
+if __name__ == '__main__':
+    unittest.main()

azdev/operations/testtool/pytest_runner.py

@@ -9,7 +9,7 @@
 
 from knack.log import get_logger
 
-from azdev.utilities import call
+from azdev.utilities import call, quote_arg
 
 
 def get_test_runner(parallel, log_path, last_failed, no_exit_first, mark):
@@ -18,18 +18,19 @@ def _run(test_paths, pytest_args):
 
         logger = get_logger(__name__)
 
+        quoted_log_path = quote_arg(log_path)
         if os.name == 'posix':
-            arguments = ['-x', '-v', '--forked', '-p no:warnings', '--log-level=WARN', '--junit-xml', log_path]
+            arguments = ['-x', '-v', '--forked', '-p no:warnings', '--log-level=WARN', '--junit-xml', quoted_log_path]
         else:
-            arguments = ['-x', '-v', '-p no:warnings', '--log-level=WARN', '--junit-xml', log_path]
+            arguments = ['-x', '-v', '-p no:warnings', '--log-level=WARN', '--junit-xml', quoted_log_path]
 
         if no_exit_first:
             arguments.remove('-x')
 
         if mark:
             arguments.append('-m "{}"'.format(mark))
 
-        arguments.extend(test_paths)
+        arguments.extend(quote_arg(test_path) for test_path in test_paths)
         if parallel:
             arguments += ['-n', 'auto']
         if last_failed:

azdev/utilities/__init__.py

@@ -15,6 +15,7 @@
     cmd,
     py_cmd,
     pip_cmd,
+    quote_arg,
     CommandError
 )
 from .const import (
@@ -71,6 +72,7 @@
     'cmd',
     'py_cmd',
     'pip_cmd',
+    'quote_arg',
     'CommandError',
     'test_cmd',
     'get_env_path',

azdev/utilities/command.py

@@ -15,6 +15,26 @@
 logger = get_logger(__name__)
 
 
+def quote_arg(arg):
+    """ Quote a single command-line argument so it survives the shell/argv parsing
+    performed by the command runners in this module.
+
+    On Windows, command strings are passed verbatim to ``subprocess`` (and ultimately
+    ``CreateProcess``), so Windows-style quoting via ``subprocess.list2cmdline`` is used.
+    On POSIX, command strings are split with ``shlex.split``, so POSIX-style quoting via
+    ``shlex.quote`` is used. This makes paths containing spaces (e.g. OneDrive folders)
+    safe to interpolate into command strings.
+
+    :param arg: The argument to quote.
+    :returns: (str) the quoted argument.
+    """
+    from azdev.utilities import IS_WINDOWS
+    arg = str(arg)
+    if IS_WINDOWS:
+        return subprocess.list2cmdline([arg])
+    return shlex.quote(arg)
+
+
 class CommandError(Exception):
 
     def __init__(self, output, exit_code, command):