Repository navigation
Conversation
There was a problem hiding this comment.
Code Review
This pull request updates scripts/update_lit_checks.py to properly handle and escape paths containing spaces (such as %s, %S, and %t) when executing shell commands, using platform-appropriate quoting (shlex.quote or subprocess.list2cmdline). It also updates the foreach parsing logic to support arguments with spaces and introduces a new unit test suite in test/unit/test_update_lit_checks_paths.py to verify these changes across various scenarios. There are no review comments, and I have no additional feedback to provide.
tlively
left a comment
There was a problem hiding this comment.
Do we have lit tests with spaces where this matters? If not, it might be better to just add a TODO comment and save the complexity of making this more robust for now.
update_lit_checks.pyexpanded%s/%S/%tbefore parsingforeachanddidn't quote them for the shell, so paths with spaces broke the updater.
This parses
foreachfirst and quotes expanded paths.Added
test/unit/test_update_lit_checks_paths.py(real updater +wasm-optin a directory with spaces): 4 of 5 variants fail before the fix, all pass
after. Tested on Ubuntu (WSL) only.