fix(ebuild): escape ninja-special characters in generated build paths

CHANGELOG.md

@@ -3,6 +3,13 @@
 ## [Unreleased]
 
 ### Fixed
+- **A path containing a space produced a silently wrong `build.ninja`.** Paths
+  were written into build statements unescaped, but Ninja ends the output list
+  at the first unescaped `:` and splits on unescaped spaces. A build directory
+  under `C:\Users\Jane Doe\` parsed into four targets instead of one, so
+  `ebuild build` failed with `expected build command name` or built the wrong
+  thing. `$`, spaces and `:` are now escaped in generated build statements
+  (`ebuild/build/ninja_backend.py`).
 - **`NinjaBackend` could not generate anything.** `_object_path()` is called by
   both `_write_ninja()` and `_write_compile_commands()`, but the method itself
   was dropped in a merge, so every `generate()` raised

ebuild/build/ninja_backend.py

@@ -31,42 +31,23 @@ class PackagePaths:
               "-fno-pie", "-fno-PIE"}
 
 
-def _ninja_path(path) -> str:
-    """Escape *path* for use in a Ninja build statement.
+def escape_ninja_path(path) -> str:
+    """Escape a path for use inside a Ninja build statement.
 
-    Ninja splits build statements on unescaped spaces and colons, so a Windows
-    absolute path writes a drive letter that Ninja reads as the output/rule
-    separator:
+    Ninja's lexer ends the output list at the first unescaped ``:`` and splits
+    on unescaped spaces, so a Windows drive letter or a directory containing a
+    space does not raise an error -- it silently parses into several wrong
+    targets. ``C:\\Users\\Jane Doe\\build\\app.o`` becomes four targets
+    (``C``, ``...\\Jane``, ``Doe``, ``build\\app.o``) instead of one.
 
-        build C:\\...\\main.o: cc main.c
-              ^ "expected build command name"
+    ``$`` is replaced first: the space and colon replacements introduce ``$``
+    characters that must not be escaped again.
 
-    `$` is escaped first so the escapes introduced below are not re-escaped.
-    Only build statements need this; variable values (cflags, ldflags) are read
-    to end of line and must not be escaped, or the flags reach the compiler
-    mangled.
+    Only the manifest needs this. Ninja shell-quotes ``$in``/``$out`` itself
+    when it builds the command line, so an escaped path still reaches the
+    compiler as a single argument.
     """
-    text = str(path)
-    return text.replace("$", "$$").replace(":", "$:").replace(" ", "$ ")
-
-
-def _ninja_path(path) -> str:
-    """Escape *path* for use in a Ninja build statement.
-
-    Ninja splits build statements on unescaped spaces and colons, so a Windows
-    absolute path writes a drive letter that Ninja reads as the output/rule
-    separator:
-
-        build C:\\...\\main.o: cc main.c
-              ^ "expected build command name"
-
-    `$` is escaped first so the escapes introduced below are not re-escaped.
-    Only build statements need this; variable values (cflags, ldflags) are read
-    to end of line and must not be escaped, or the flags reach the compiler
-    mangled.
-    """
-    text = str(path)
-    return text.replace("$", "$$").replace(":", "$:").replace(" ", "$ ")
+    return str(path).replace("$", "$$").replace(" ", "$ ").replace(":", "$:")
 
 
 class NinjaBackend:
@@ -207,10 +188,10 @@ def _write_ninja(self) -> None:
 
             obj_files = []
             for src in target.sources:
-                obj = str(self._object_path(target, src))
+                obj = escape_ninja_path(self._object_path(target, src))
                 obj_files.append(obj)
                 lines.append(
-                    f"build {_ninja_path(obj)}: cc {_ninja_path(src)}"
+                    f"build {obj}: cc {escape_ninja_path(src)}"
                 )
                 if cflags:
                     lines.append(f"  cflags = {' '.join(cflags)}")
@@ -232,10 +213,12 @@ def _write_ninja(self) -> None:
                 for dep_name in target.depends:
                     for dep_target in self.config.targets:
                         if dep_target.name == dep_name and dep_target.target_type == "static_library":
-                            dep_archives.append(str(self.build_dir / f"lib{dep_name}.a"))
+                            dep_archives.append(
+                                escape_ninja_path(self.build_dir / f"lib{dep_name}.a")
+                            )
 
                 link_inputs = obj_files + dep_archives
-                out = str(self.build_dir / target.name)
+                out = escape_ninja_path(self.build_dir / target.name)
                 lines.append(
                     f"build {_ninja_path(out)}: link " f"{' '.join(_ninja_path(x) for x in link_inputs)}"
                 )
@@ -252,7 +235,7 @@ def _write_ninja(self) -> None:
                     ext = ".dylib"
                 else:
                     ext = ".so"
-                out = str(self.build_dir / f"lib{target.name}{ext}")
+                out = escape_ninja_path(self.build_dir / f"lib{target.name}{ext}")
 
                 if target.target_type == "static_library":
                     lines.append(f"build {_ninja_path(out)}: ar_rule " f"{' '.join(_ninja_path(x) for x in obj_files)}")

tests/unit/test_ninja_backend.py

@@ -13,7 +13,7 @@
 
 import pytest
 
-from ebuild.build.ninja_backend import NinjaBackend
+from ebuild.build.ninja_backend import NinjaBackend, escape_ninja_path
 from ebuild.build.toolchain import ResolvedToolchain
 from ebuild.core.config import ProjectConfig, TargetConfig
 
@@ -244,5 +244,117 @@ def test_compile_commands_distinguishes_shared_source_entries(self, tmp_path):
         assert len(set(objects)) == 2, f"entries name the same object: {objects}"
 
 
+@pytest.mark.ebuild
+class TestNinjaPathEscaping:
+    """Paths in build statements must be escaped for Ninja's lexer.
+
+    Ninja ends the output list at the first unescaped ``:`` and splits on
+    unescaped spaces. Neither is an error -- a Windows drive letter or a
+    directory with a space parses into several wrong targets instead of one
+    right one, so the manifest either fails to build the requested target or
+    fails much later with a confusing message.
+    """
+
+    def test_escapes_space_colon_and_dollar(self):
+        assert escape_ninja_path("a b") == "a$ b"
+        assert escape_ninja_path("C:/x") == "C$:/x"
+        assert escape_ninja_path("a$b") == "a$$b"
+
+    def test_dollar_is_escaped_before_the_others(self):
+        """Order matters. Escaping the space first would leave a ``$`` that
+        the dollar pass then doubles, yielding ``$$ `` -- a literal dollar
+        followed by a separator rather than an escaped space."""
+        assert escape_ninja_path("a b:c") == "a$ b$:c"
+
+    def test_leaves_ordinary_paths_untouched(self):
+        assert escape_ninja_path("src/main.c") == "src/main.c"
+
+    @staticmethod
+    def _config_in(build_dir, source_dir):
+        return ProjectConfig(
+            name="spacey",
+            version="1.0.0",
+            targets=[
+                TargetConfig(
+                    name="app",
+                    target_type="executable",
+                    sources=["main.c"],
+                )
+            ],
+            source_dir=source_dir,
+        )
+
+    def test_build_dir_with_a_space_is_escaped_in_the_manifest(self, tmp_path):
+        build_dir = tmp_path / "dir with space" / "_build"
+        NinjaBackend(
+            self._config_in(build_dir, tmp_path), build_dir, ResolvedToolchain()
+        ).generate()
+
+        manifest = (build_dir / "build.ninja").read_text(encoding="utf-8")
+        build_lines = [ln for ln in manifest.splitlines() if ln.startswith("build ")]
+        assert build_lines
+
+        for line in build_lines:
+            # The path is escaped, so no raw space survives before the rule
+            # separator, and the drive colon does not terminate the outputs.
+            outputs = line[len("build "):].split(": ", 1)[0]
+            assert " " not in outputs.replace("$ ", ""), line
+            assert "$ " in outputs, line
+
+    def test_ninja_parses_a_spacey_path_as_one_target(self, tmp_path):
+        """The check that matters: real ninja must resolve one target, not four.
+
+        A string assertion cannot catch this -- an unescaped path parses
+        cleanly and is simply wrong.
+        """
+        pytest.importorskip("ninja", reason="ninja package not installed")
+
+        source_dir = tmp_path / "dir with space"
+        source_dir.mkdir()
+        (source_dir / "main.c").write_text("int main(void){return 0;}\n", encoding="utf-8")
+
+        build_dir = source_dir / "_build"
+        NinjaBackend(
+            self._config_in(build_dir, source_dir), build_dir, ResolvedToolchain()
+        ).generate()
+
+        result = subprocess.run(
+            [
+                sys.executable, "-m", "ninja",
+                "-f", str(build_dir / "build.ninja"),
+                "-t", "targets", "all",
+            ],
+            cwd=str(source_dir),
+            capture_output=True,
+            text=True,
+        )
+        assert result.returncode == 0, (
+            f"ninja rejected the manifest:\n{result.stdout}\n{result.stderr}"
+        )
+
+        # One compile edge and one link edge -- not one per path fragment.
+        targets = [ln for ln in result.stdout.strip().splitlines() if ln.strip()]
+        assert len(targets) == 2, f"expected 2 targets, got {targets}"
+        assert all("dir with space" in t for t in targets), targets
+
+    def test_escaping_does_not_leak_into_compile_commands(self, tmp_path):
+        """compile_commands.json is JSON consumed by clang tooling, not a
+        Ninja manifest. A ``$:`` or ``$ `` there would be a corrupt path."""
+        source_dir = tmp_path / "dir with space"
+        source_dir.mkdir()
+        build_dir = source_dir / "_build"
+        NinjaBackend(
+            self._config_in(build_dir, source_dir), build_dir, ResolvedToolchain()
+        ).generate()
+
+        cc_data = json.loads(
+            (build_dir / "compile_commands.json").read_text(encoding="utf-8")
+        )
+        assert cc_data
+        for entry in cc_data:
+            assert "$:" not in entry["command"]
+            assert "$ " not in entry["command"]
+
+
 if __name__ == "__main__":
     unittest.main()