-
Notifications
You must be signed in to change notification settings - Fork 13.6k
[clang-tidy][run-clang-tidy] Fix minor shutdown noise #105724
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@llvm/pr-subscribers-clang-tools-extra @llvm/pr-subscribers-clang-tidy Author: Nicolas van Kempen (nicovank) ChangesOn my new machine, the script outputs some shutdown noise:
This fixes it. Also remove an unused typing import. Full diff: https://github.com/llvm/llvm-project/pull/105724.diff 1 Files Affected:
diff --git a/clang-tools-extra/clang-tidy/tool/run-clang-tidy.py b/clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
index 48401ba5ea42a9..30d3afc0173050 100755
--- a/clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
+++ b/clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
@@ -49,7 +49,7 @@
import time
import traceback
from types import ModuleType
-from typing import Any, Awaitable, Callable, List, Optional, Tuple, TypeVar
+from typing import Any, Awaitable, Callable, List, Optional, TypeVar
yaml: Optional[ModuleType] = None
@@ -623,4 +623,7 @@ async def main() -> None:
if __name__ == "__main__":
- asyncio.run(main())
+ try:
+ asyncio.run(main())
+ except KeyboardInterrupt:
+ pass
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because the commit with asyncio is in llvm-19, could you please add a release note for this fix?
@PiotrZSL wdyt about backporting this? 19.1.0 may be too late, but for 19.2.0 then
ca3b844
to
84535cd
Compare
On my new machine, the script outputs some shutdown noise: ``` Ctrl-C detected, goodbye. Traceback (most recent call last): File "/home/nvankempen/llvm-project/./clang-tools-extra/clang-tidy/tool/run-clang-tidy.py", line 626, in <module> asyncio.run(main()) File "/usr/lib/python3.10/asyncio/runners.py", line 44, in run return loop.run_until_complete(main) File "/usr/lib/python3.10/asyncio/base_events.py", line 636, in run_until_complete self.run_forever() File "/usr/lib/python3.10/asyncio/base_events.py", line 603, in run_forever self._run_once() File "/usr/lib/python3.10/asyncio/base_events.py", line 1871, in _run_once event_list = self._selector.select(timeout) File "/usr/lib/python3.10/selectors.py", line 469, in select fd_event_list = self._selector.poll(timeout, max_ev) KeyboardInterrupt ``` This fixes it.
84535cd
to
f8c596c
Compare
Added release note, rebased to trigger CI again. FWIW this only happens on interrupt, not normal execution. If you think this should be backported, let me know if there is anything I can help with. |
I'm going to go ahead and merge this. IMO this doesn't need backporting because 1. it only happens on abnormal exit where users probably wouldn't be surprised to see a stacktrace anyway, and 2. there is no regression really (check prior to asyncio change also had some noise after Ctrl+C message). But if someone thinks otherwise I'll stamp it 👍. |
On my new machine, the script outputs some shutdown noise:
This fixes it. Also remove an unused typing import.
Relevant documentation: https://docs.python.org/3/library/asyncio-runner.html#handling-keyboard-interruption