Add test coverage for special characters in manifest and west commands

tests/conftest.py

@@ -14,6 +14,7 @@
 from pathlib import Path, PurePath
 
 import pytest
+import yaml
 
 from west.app import main
 
@@ -39,25 +40,26 @@
     remote: test-local
 
   remotes:
-    - name: test-local
-      url-base: THE_URL_BASE
+  - name: test-local
+    url-base: THE_URL_BASE
 
   projects:
-    - name: Kconfiglib
-      description: |
-        Kconfiglib is an implementation of
-        the Kconfig language written in Python.
-      revision: zephyr
-      path: subdir/Kconfiglib
-      groups:
-        - Kconfiglib-group
-      submodules: true
-    - name: tagged_repo
-      revision: v1.0
-    - name: net-tools
-      description: Networking tools.
-      clone-depth: 1
-      west-commands: scripts/west-commands.yml
+  - name: Kconfiglib
+    description: |
+      Kconfiglib is an implementation of
+      the Kconfig language written in Python.
+    revision: zephyr
+    path: subdir/Kconfiglib
+    groups:
+    - Kconfiglib-group
+    submodules: true
+  - name: tagged_repo
+    revision: v1.0
+  - name: net-tools
+    description: Networking tools.
+    clone-depth: 1
+    west-commands: scripts/west-commands.yml
+
   self:
     path: zephyr
 '''
@@ -69,6 +71,19 @@
 #
 
 
+@contextlib.contextmanager
+def yaml_editor(yaml_f: str | Path):
+    # Fail fast if not writable
+    with open(yaml_f, 'r+') as f:
+        pass
+    with open(yaml_f) as f:
+        mf = yaml.safe_load(f)
+    yield mf
+    # Overwrite file
+    with open(yaml_f, 'w') as f:
+        yaml.safe_dump(mf, f, sort_keys=False)
+
+
 @contextlib.contextmanager
 def tmp_west_topdir(path: str | Path):
     """
@@ -253,11 +268,11 @@ def _session_repos(tmp_path_factory):
             'qemu-script.sh': 'echo hello world net-tools\n',
             'scripts/west-commands.yml': textwrap.dedent('''\
                 west-commands:
-                  - file: scripts/test.py
-                    commands:
-                      - name: test-extension
-                        class: TestExtension
-                        help: test-extension-help
+                - file: scripts/test.py
+                  commands:
+                  - name: test-extension
+                    class: TestExtension
+                    help: test-extension-help
                 '''),
             'scripts/test.py': textwrap.dedent('''\
                 from west.commands import WestCommand

tests/test_extension_commands.py

@@ -2,9 +2,14 @@
 #
 # SPDX-License-Identifier: Apache-2.0
 
+import subprocess
 import textwrap
+from pathlib import Path
 
-from conftest import add_commit, cmd, cmd_raises
+import yaml
+from conftest import GIT, WINDOWS, add_commit, cmd, cmd_raises, yaml_editor
+
+# The west command "test-extension" comes from the "west_update_tmpdir" fixture in conftest.py
 
 
 def test_extension_commands_basic(west_update_tmpdir):
@@ -259,3 +264,81 @@ def do_run(self, args, unknown):
     assert 'first command' in ext_output
     ext_output = cmd('second')
     assert 'second command' in ext_output
+
+
+def test_extension_special_chars(west_update_tmpdir):
+    # Detect any unexpected changes in the way we've been handling backslashes and other
+    # special characters. Changes in how we handle such edge cases may or may not be desired
+    # (and this test may be updated accordingly), but we never want these changes to come as
+    # a surprise and we want to keep control over them.
+
+    ext_proj = 'net-tools'
+    ext_proj_p = Path(ext_proj)
+
+    # Rename scripts/test.py to something strange.
+    # The actual location is purposely different on Windows
+    weird_ext_py = r'scripts///win subdir\\\test.py'
+    with yaml_editor(ext_proj_p / 'scripts' / 'west-commands.yml') as cmds:
+        assert cmds["west-commands"][0]["file"] == 'scripts/test.py'
+        cmds["west-commands"][0]["file"] = weird_ext_py
+    if WINDOWS:
+        (ext_proj_p / 'scripts' / 'win subdir').mkdir()
+    (ext_proj_p / 'scripts' / 'test.py').rename(ext_proj_p / weird_ext_py)
+
+    # Just for the logs
+    subprocess.check_call([GIT, '-C', ext_proj, 'add', weird_ext_py])
+    print(cmd('diff --manifest'))
+
+    # Does the extension still work
+    ext_output = cmd('test-extension')
+    assert 'Testing test command 1' in ext_output
+
+    def yaml_get_proj(mf: dict, projname: str):
+        _l = [p for p in mf["manifest"]['projects'] if p["name"] == projname]
+        assert len(_l) == 1
+        return _l[0]
+
+    # Now also rename the project's 'scripts/west-commands.yml' to something strange
+    weird_cmds = r'scripts///win subdir\\\w-cmds.yml'
+    with yaml_editor('zephyr/west.yml') as _mf:
+        _ext_p_yml = yaml_get_proj(_mf, ext_proj)
+        assert _ext_p_yml["west-commands"] == 'scripts/west-commands.yml'
+        _ext_p_yml["west-commands"] = weird_cmds
+    (ext_proj_p / 'scripts' / 'west-commands.yml').rename(ext_proj_p / weird_cmds)
+
+    # Just for the logs
+    subprocess.check_call([GIT, '-C', ext_proj, 'add', weird_cmds])
+    print(cmd('diff --manifest'))
+
+    # Does the extension still work
+    ext_output = cmd('test-extension')
+    assert 'Testing test command 1' in ext_output
+
+    # Test how west-commands gets printed back in `west manifest --resolve`
+    resolved_mf = cmd('manifest --resolve')
+    resolved_mf = yaml.safe_load(resolved_mf)
+    ext_proj_yaml = yaml_get_proj(resolved_mf, ext_proj)
+    assert ext_proj_yaml["west-commands"] == weird_cmds
+
+    ######  self: west-commands #####
+
+    # self: west-commands: follows a slightly different code path.
+    # Move the extension away from the project and into self.
+    Path('zephyr', 'scripts').mkdir()
+    if WINDOWS:
+        Path('zephyr', 'scripts', 'win subdir').mkdir()
+    (ext_proj_p / weird_ext_py).rename(Path('zephyr', weird_ext_py))
+
+    (ext_proj_p / weird_cmds).rename(Path('zephyr', weird_cmds))
+
+    # The extension is now missing from ext_proj. That's OK, it's supported.
+    with yaml_editor('zephyr/west.yml') as _mf:
+        _mf["manifest"]["self"]["west-commands"] = weird_cmds
+
+    ext_output = cmd('test-extension')
+    assert 'Testing test command 1' in ext_output
+
+    # Test how west-commands gets printed back in `west manifest --resolve`
+    resolved_mf = cmd('manifest --resolve')
+    resolved_mf = yaml.safe_load(resolved_mf)
+    assert resolved_mf["manifest"]["self"]["west-commands"] == weird_cmds

tests/test_project.py

@@ -25,6 +25,7 @@
     create_repo,
     create_workspace,
     rev_parse,
+    yaml_editor,
 )
 
 from west.manifest import ImportFlag as MIF
@@ -157,17 +158,57 @@ def test_list_manifest(west_update_tmpdir):
     abspath = cmd('list -f {abspath} manifest').strip()
     posixpath = cmd('list -f {posixpath} manifest').strip()
     assert path == 'manifest_moved'
-    assert Path(abspath) == west_update_tmpdir / 'manifest_moved'
+    assert abspath == str(west_update_tmpdir / 'manifest_moved')
     assert posixpath == Path(west_update_tmpdir).as_posix() + '/manifest_moved'
 
     path = cmd('list --manifest-path-from-yaml -f {path} manifest').strip()
     abspath = cmd('list --manifest-path-from-yaml -f {abspath} manifest').strip()
     posixpath = cmd('list --manifest-path-from-yaml -f {posixpath} manifest').strip()
     assert path == 'zephyr'
-    assert Path(abspath) == Path(str(west_update_tmpdir / 'zephyr'))
+    assert abspath == str(west_update_tmpdir / 'zephyr')
     assert posixpath == Path(west_update_tmpdir).as_posix() + '/zephyr'
 
 
+def test_list_special_chars(west_update_tmpdir):
+    # Detect any unexpected changes in the way we've been handling backslashes and multiple
+    # slashes in paths. Changes in how we handle such edge cases may or may not be desired
+    # (and this test may be updated accordingly), but we never want these changes to come as
+    # a surprise and we want to keep control over them.
+
+    proj_name = 'net-tools2-stress-slashes'
+
+    with yaml_editor('zephyr/west.yml') as mf:
+        mf["manifest"]["projects"].append(
+            {
+                'name': proj_name,
+                'path': r'subdir///sub dir2\\\net-tools2',
+                'url': mf["manifest"]["remotes"][0]["url-base"] + '/net-tools',
+                'description': 'Test special chars in paths',
+            },
+        )
+    print(cmd('diff'))  # just logging
+
+    path = cmd('list -f {path} ' + proj_name).strip()
+    abspath = cmd('list -f {abspath} ' + proj_name).strip()
+    posixpath = cmd('list -f {posixpath} ' + proj_name).strip()
+
+    forward_rel_path = r'subdir/sub dir2/net-tools2' if WINDOWS else r'subdir/sub dir2\\\net-tools2'
+    native_rel_path = r'subdir\sub dir2\net-tools2' if WINDOWS else r'subdir/sub dir2\\\net-tools2'
+    assert path == forward_rel_path
+    assert abspath == str(west_update_tmpdir) + os.path.sep + native_rel_path
+    assert posixpath == Path(west_update_tmpdir).as_posix() + '/' + forward_rel_path
+
+    def yaml_get_proj(mf: dict, projname: str):
+        _l = [p for p in mf["manifest"]['projects'] if p["name"] == projname]
+        assert len(_l) == 1
+        return _l[0]
+
+    resolved_mf = cmd('manifest --resolve')
+    resolved_mf = yaml.safe_load(resolved_mf)
+    proj_yaml = yaml_get_proj(resolved_mf, proj_name)
+    assert proj_yaml["path"] == forward_rel_path
+
+
 def test_list_groups(west_init_tmpdir):
     with open('zephyr/west.yml', 'w') as f:
         f.write("""