Closed Bug 1273556 Opened 5 years ago Closed 5 years ago
[mozlint] Handle SIGINT better and still display partial results
MozReview Request: Bug 1273556 - [mozlint] Better SIGINT handling, return partial results on Ctrl-C, r?jgraham
58 bytes, text/x-review-board-request
The SIGINT (KeyboardInterrrupt) handling in mozlint is currently broken. A python bug is causing SIGINT to get swallowed, making it impossible to Ctrl-C |mach lint|. While fixing this, I came up with a better idea. Get the parent process to ignore SIGINT, and handle it cleanly in the child processes. This way, we can still collect and display whatever results are already finished. If the external linter is implemented properly (i.e results are streamed), we can even return partial results from a specific linter.
Currently a bug in python (https://bugs.python.org/issue8296) is preventing a KeyboardInterrupt from reaching the parent process, meaning we can't kill the process with SIGINT. There is a workaround to this bug, but instead I decided to ignore SIGINT in the parent process completely. Now, each child process is responsible for handling SIGINT on its own. Since child processes should all shutdown relatively quickly anyway, this effectively also ends the parent process. The benefit of doing it this way is that each child process can return the results they have collected to date. So when a developer hits Ctrl-C, they'll still see some (but not all) formatted lint output. The downside is that a poorly implemented external linter could block the parent process from exiting quickly, but if this happens we should just fix the linter. Review commit: https://reviewboard.mozilla.org/r/53310/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/53310/
Attachment #8753473 - Flags: review?(james)
Assignee: nobody → ahalberstadt
Status: NEW → ASSIGNED
Comment on attachment 8753473 [details] MozReview Request: Bug 1273556 - [mozlint] Better SIGINT handling, return partial results on Ctrl-C, r?jgraham https://reviewboard.mozilla.org/r/53310/#review50250 I guess. The lack of any timeout in the parent process causing it to forcibly shut down if the child processes don't return after a SIGINT makes me rather nervious though. ::: python/mozlint/mozlint/types.py:42 (Diff revision 1) > if self.batch: > return self._lint(paths, linter, **lintargs) > > errors =  > + try: > - for p in paths: > + for p in paths: Tabs
Good point, filed bug 1273914.
https://reviewboard.mozilla.org/r/53310/#review50250 > Tabs Hm, I don't see any tabs here?
https://reviewboard.mozilla.org/r/53310/#review50250 > Hm, I don't see any tabs here? Oh, i misunderstood reviewboard's notation it seems.
You need to log in before you can comment on or make changes to this bug.