Share the argv quoting core between path and token quoting

esphome/build_gen/arduino8266.py

@@ -269,33 +269,41 @@ def _e(value) -> str:
     return str(value).replace("$", "$$").replace(":", "$:").replace(" ", "$ ")
 
 
-def _q(value) -> str:
-    """Quote a path for use inside a ninja command line (shell/CreateProcess).
+def _quote_arg(tok: str) -> str:
+    """Wrap a token in double quotes with the Windows argv rule.
 
-    ``$`` doubles so ninja passes it through literally instead of expanding
-    an (empty) ninja variable.
+    Same escaping rule as ``subprocess.list2cmdline``: a backslash run
+    doubles only immediately before a quote (or the closing quote), and the
+    quote itself is escaped. POSIX sh parses the result identically for
+    backslashes and quotes. ``$`` must already be doubled for ninja.
     """
-    return '"' + str(value).replace("$", "$$") + '"'
+    quoted = re.sub(r'(\\*)"', lambda m: m.group(1) * 2 + '\\"', tok)
+    quoted = re.sub(r"(\\+)\Z", lambda m: m.group(1) * 2, quoted)
+    return f'"{quoted}"'
+
+
+def _q(value) -> str:
+    """Force-quote a path for the ninja command line (shell/CreateProcess)."""
+    return _quote_arg(str(value).replace("$", "$$"))
 
 
 _NEEDS_QUOTE = re.compile(r'[\s"\']')
 
 
 def _shell_token(tok: str) -> str:
-    """Quote a lexed token for the ninja command line; ``_q`` is for paths.
+    """Quote a lexed token only when needed; ``_q`` force-quotes paths.
 
     Lexing strips the quoting a user wrote (``-DX="a b"`` becomes the single
     token ``-DX=a b``); re-quote on the way out so the compiler receives the
-    same argv element SCons would pass under PlatformIO. Uses the Windows
-    argv quoting rule, which POSIX sh parses identically inside double
-    quotes: a backslash run doubles only immediately before a quote.
+    same argv element SCons would pass under PlatformIO. After ninja
+    un-doubles ``$$``, sh still expands ``$VAR`` while CreateProcess passes
+    it literally -- the same divergence SCons-under-sh has, so this stays
+    PlatformIO parity.
     """
     tok = tok.replace("$", "$$")  # ninja would expand a bare $ to nothing
     if not _NEEDS_QUOTE.search(tok):
         return tok
-    quoted = re.sub(r'(\\*)"', lambda m: m.group(1) * 2 + '\\"', tok)
-    quoted = re.sub(r"(\\+)\Z", lambda m: m.group(1) * 2, quoted)
-    return f'"{quoted}"'
+    return _quote_arg(tok)
 
 
 def _defines_flags(
@@ -333,7 +341,7 @@ def _unflag_tokens() -> set[str]:
 
 
 def _project_flags(
-    unflags: set[str],
+    unflags: set[str] | None = None,
 ) -> tuple[list[str], list[str], list[Path], list[str]]:
     """Split the ESPHome build flags into compile, linker, -L, and -l lists.
 
@@ -342,6 +350,8 @@ def _project_flags(
     ``build_unflags`` matches individual tokens (``-Os`` inside ``-Os -g3``).
     Lexed tokens are re-quoted at emission via ``_shell_token``.
     """
+    if unflags is None:
+        unflags = _unflag_tokens()
     compile_flags: list[str] = []
     link_flags: list[str] = []
     lib_dirs: list[Path] = []

tests/unit_tests/build_gen/test_arduino8266.py

@@ -582,9 +582,7 @@ def test_project_flags_trailing_bare_linker_flag_warns(
     caplog: pytest.LogCaptureFixture,
 ) -> None:
     _set_flags("-l")
-    compile_flags, link_flags, lib_dirs, libs = arduino8266._project_flags(
-        arduino8266._unflag_tokens()
-    )
+    compile_flags, link_flags, lib_dirs, libs = arduino8266._project_flags()
     assert "Ignoring trailing '-l'" in caplog.text
     assert not libs
     assert not lib_dirs
@@ -594,9 +592,7 @@ def test_project_flags_trailing_bare_linker_flag_warns(
 
 def test_project_flags_lexed_entry_scatters_non_linker_tokens() -> None:
     _set_flags("-L /d -Wl,-Map=m stray")
-    compile_flags, link_flags, lib_dirs, libs = arduino8266._project_flags(
-        arduino8266._unflag_tokens()
-    )
+    compile_flags, link_flags, lib_dirs, libs = arduino8266._project_flags()
     assert lib_dirs == [Path("/d")]
     assert link_flags == ["-Wl,-Map=m"]
     assert "stray" in compile_flags
@@ -616,9 +612,7 @@ def test_flag_defines_lexes_multi_token_entries() -> None:
 def test_project_flags_lexes_every_entry() -> None:
     """A linker flag anywhere in an entry reaches the link line (PIO parity)."""
     _set_flags("-DFOO=1 -lbar")
-    compile_flags, _link, _dirs, libs = arduino8266._project_flags(
-        arduino8266._unflag_tokens()
-    )
+    compile_flags, _link, _dirs, libs = arduino8266._project_flags()
     assert libs == ["bar"]
     assert "-DFOO=1" in compile_flags
 
@@ -627,9 +621,7 @@ def test_project_flags_unflags_match_tokens() -> None:
     """build_unflags removes a token embedded in a multi-token entry."""
     _set_flags("-Os -g3")
     CORE.build_unflags = {"-Os"}
-    compile_flags, _link, _dirs, _libs = arduino8266._project_flags(
-        arduino8266._unflag_tokens()
-    )
+    compile_flags, _link, _dirs, _libs = arduino8266._project_flags()
     assert "-g3" in compile_flags
     assert "-Os" not in compile_flags
 
@@ -637,9 +629,7 @@ def test_project_flags_unflags_match_tokens() -> None:
 def test_project_flags_requotes_lexed_defines() -> None:
     """A quoted spaced value stays one compiler argument after lex/emit."""
     _set_flags('-DGREETING="hello world"')
-    compile_flags, _link, _dirs, _libs = arduino8266._project_flags(
-        arduino8266._unflag_tokens()
-    )
+    compile_flags, _link, _dirs, _libs = arduino8266._project_flags()
     # shlex folds the quotes (as PIO's ParseFlags does); _shell_token
     # re-quotes the spaced token so the shell passes one argv element
     assert compile_flags == ['"-DGREETING=hello world"']

tests/unit_tests/test_arduino8266_framework.py

@@ -310,7 +310,7 @@ def test_check_and_install_returns_paths(tmp_path: Path) -> None:
     # The layout checks cover the directories write_project needs, including
     # the bundled libraries/ tree
     fw_expect = mock_install.call_args_list[0].kwargs["expect"]
-    assert set(fw_expect) == {"cores/esp8266", "tools/sdk", "libraries"}
+    assert fw_expect == ("cores/esp8266", "tools/sdk", "libraries")
     assert mock_install.call_args_list[1].kwargs["expect"] == ("bin",)