Patchwork [2,of,2] dispatch: handle late KeyboardInterrupt occurred in run()

login
register
mail settings
Submitter Yuya Nishihara
Date July 13, 2020, 12:40 p.m.
Message ID <96904994ee794243dd57.1594644046@mimosa>
Download mbox | patch
Permalink /patch/46718/
State Accepted
Headers show

Comments

Yuya Nishihara - July 13, 2020, 12:40 p.m.
# HG changeset patch
# User Yuya Nishihara <yuya@tcha.org>
# Date 1594642460 -32400
#      Mon Jul 13 21:14:20 2020 +0900
# Node ID 96904994ee794243dd57881e5eb24540d7c2aaf2
# Parent  39c2578fd3da0e2d526b9a4107e25c833e6be050
dispatch: handle late KeyboardInterrupt occurred in run()

User can press Ctrl+C while flushing streams in dispatch.run(). In such case,
I think exiting with 255 is better than printing Python traceback and exiting
with 1.
Manuel Jacob - July 14, 2020, 5:43 a.m.
On 2020-07-13 14:40, Yuya Nishihara wrote:
> # HG changeset patch
> # User Yuya Nishihara <yuya@tcha.org>
> # Date 1594642460 -32400
> #      Mon Jul 13 21:14:20 2020 +0900
> # Node ID 96904994ee794243dd57881e5eb24540d7c2aaf2
> # Parent  39c2578fd3da0e2d526b9a4107e25c833e6be050
> dispatch: handle late KeyboardInterrupt occurred in run()
> 
> User can press Ctrl+C while flushing streams in dispatch.run(). In such 
> case,
> I think exiting with 255 is better than printing Python traceback and 
> exiting
> with 1.

Wouldn’t it be better if we preserved the original status if there was 
any? See also the thoughts in 48fe4f56a3b4.

> diff --git a/mercurial/dispatch.py b/mercurial/dispatch.py
> --- a/mercurial/dispatch.py
> +++ b/mercurial/dispatch.py
> @@ -140,8 +140,10 @@ def run():
>                  status = -1
> 
>          _silencestdio()
> -    finally:
> -        pass
> +    except KeyboardInterrupt:
> +        # Catch early/late KeyboardInterrupt as last ditch. Here 
> nothing will
> +        # be printed to console to avoid another 
> IOError/KeyboardInterrupt.
> +        status = -1
>      sys.exit(status & 255)
> 
> 
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Yuya Nishihara - July 14, 2020, 9:39 a.m.
On Tue, 14 Jul 2020 07:43:53 +0200, Manuel Jacob wrote:
> On 2020-07-13 14:40, Yuya Nishihara wrote:
> > # HG changeset patch
> > # User Yuya Nishihara <yuya@tcha.org>
> > # Date 1594642460 -32400
> > #      Mon Jul 13 21:14:20 2020 +0900
> > # Node ID 96904994ee794243dd57881e5eb24540d7c2aaf2
> > # Parent  39c2578fd3da0e2d526b9a4107e25c833e6be050
> > dispatch: handle late KeyboardInterrupt occurred in run()
> > 
> > User can press Ctrl+C while flushing streams in dispatch.run(). In such 
> > case,
> > I think exiting with 255 is better than printing Python traceback and 
> > exiting
> > with 1.
> 
> Wouldn’t it be better if we preserved the original status if there was 
> any? See also the thoughts in 48fe4f56a3b4.

That might be, but let's copy the existing IOError behavior. I think exiting
with 255 is valid since some data or error message might be lost because of
KeyboardInterrupt.
Augie Fackler - July 15, 2020, 2:41 p.m.
On Mon, Jul 13, 2020 at 09:40:46PM +0900, Yuya Nishihara wrote:
> # HG changeset patch
> # User Yuya Nishihara <yuya@tcha.org>
> # Date 1594642460 -32400
> #      Mon Jul 13 21:14:20 2020 +0900
> # Node ID 96904994ee794243dd57881e5eb24540d7c2aaf2
> # Parent  39c2578fd3da0e2d526b9a4107e25c833e6be050
> dispatch: handle late KeyboardInterrupt occurred in run()

queued, thanks

Patch

diff --git a/mercurial/dispatch.py b/mercurial/dispatch.py
--- a/mercurial/dispatch.py
+++ b/mercurial/dispatch.py
@@ -140,8 +140,10 @@  def run():
                 status = -1
 
         _silencestdio()
-    finally:
-        pass
+    except KeyboardInterrupt:
+        # Catch early/late KeyboardInterrupt as last ditch. Here nothing will
+        # be printed to console to avoid another IOError/KeyboardInterrupt.
+        status = -1
     sys.exit(status & 255)