Patchwork D3716: ui: add an unsafeoperation context manager that can block SIGINT

login
register
mail settings
Submitter phabricator
Date June 12, 2018, 9:31 p.m.
Message ID <eea36025ca8fb9c1b70ab265a4875daf@localhost.localdomain>
Download mbox | patch
Permalink /patch/32095/
State Not Applicable
Headers show

Comments

phabricator - June 12, 2018, 9:31 p.m.
durin42 updated this revision to Diff 9033.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D3716?vs=9023&id=9033

REVISION DETAIL
  https://phab.mercurial-scm.org/D3716

AFFECTED FILES
  mercurial/configitems.py
  mercurial/ui.py

CHANGE DETAILS




To: durin42, #hg-reviewers
Cc: martinvonz, mercurial-devel
Yuya Nishihara - June 14, 2018, 3:14 p.m.
LGTM by a non-googler.

> +    @contextlib.contextmanager
> +    def unsafeoperation(self):
> +        """Mark an operation as unsafe.

Nit: the word "unsafe" seems too obscure. Maybe it could be "uninterruptible"
or "noninterruptible"?

> +        try:
> +            self._oldsiginthandler = signal.getsignal(signal.SIGINT)
> +            signal.signal(signal.SIGINT, disabledsiginthandler)

`signal.signal()` may raise ValueError if it's called from sub thread.

> --- a/mercurial/configitems.py
> +++ b/mercurial/configitems.py
> @@ -560,6 +560,17 @@
>  coreconfigitem('experimental', 'mergedriver',
>      default=None,
>  )
> +coreconfigitem('experimental', 'nointerrupt', default=False)
> +_nointmsg = """
> +==========================
> +Interrupting Mercurial may leave your repo in a bad state.
> +If you really want to interrupt your current command, press
> +CTRL-C again.
> +==========================
> +""".strip()
> +coreconfigitem('experimental', 'nointerrupt-message', default=_nointmsg)

Nit: perhaps we'll need to move the default message to ui.py to make it
translatable.

FWIW, I've added a similar function to lock.py at d77c3b023393. My version
would be too strict to block user interrupt casually, but maybe we can
factor out a utility function for both of these use cases?
phabricator - June 14, 2018, 3:35 p.m.
yuja added a comment.


  LGTM by a non-googler.
  
  > +    @contextlib.contextmanager
  >  +    def unsafeoperation(self):
  >  +        """Mark an operation as unsafe.
  
  Nit: the word "unsafe" seems too obscure. Maybe it could be "uninterruptible"
  or "noninterruptible"?
  
  > +        try:
  >  +            self._oldsiginthandler = signal.getsignal(signal.SIGINT)
  >  +            signal.signal(signal.SIGINT, disabledsiginthandler)
  
  `signal.signal()` may raise ValueError if it's called from sub thread.
  
  > - a/mercurial/configitems.py +++ b/mercurial/configitems.py @@ -560,6 +560,17 @@ coreconfigitem('experimental', 'mergedriver', default=None, ) +coreconfigitem('experimental', 'nointerrupt', default=False) +_nointmsg = """ +========================== +Interrupting Mercurial may leave your repo in a bad state. +If you really want to interrupt your current command, press +CTRL-C again. +========================== +""".strip() +coreconfigitem('experimental', 'nointerrupt-message', default=_nointmsg)
  
  Nit: perhaps we'll need to move the default message to ui.py to make it
  translatable.
  
  FWIW, I've added a similar function to lock.py at https://phab.mercurial-scm.org/rHGd77c3b02339308f8f3abda27947cf9387aa1cbc8. My version
  would be too strict to block user interrupt casually, but maybe we can
  factor out a utility function for both of these use cases?

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D3716

To: durin42, #hg-reviewers
Cc: yuja, martinvonz, mercurial-devel

Patch

diff --git a/mercurial/ui.py b/mercurial/ui.py
--- a/mercurial/ui.py
+++ b/mercurial/ui.py
@@ -224,6 +224,7 @@ 
         self._colormode = None
         self._terminfoparams = {}
         self._styles = {}
+        self._oldsiginthandler = None
 
         if src:
             self.fout = src.fout
@@ -334,6 +335,40 @@ 
             self._blockedtimes[key + '_blocked'] += \
                 (util.timer() - starttime) * 1000
 
+    @contextlib.contextmanager
+    def unsafeoperation(self):
+        """Mark an operation as unsafe.
+
+        Most operations on a repository are safe to interrupt, but a
+        few are risky (for example repair.strip). This context manager
+        lets you advise Mercurial that something risky is happening so
+        that control-C etc can be blocked if desired.
+        """
+        enabled = self.configbool('experimental', 'nointerrupt')
+        if (enabled and
+            self.configbool('experimental', 'nointerrupt-interactiveonly')):
+            enabled = self.interactive()
+        if self._oldsiginthandler is not None or not enabled:
+            # if nointerrupt support is turned off, the process isn't
+            # interactive, or we're already in an unsafeoperation
+            # block, do nothing.
+            yield
+            return
+
+        def disabledsiginthandler(*args):
+            self.warn(self.config('experimental', 'nointerrupt-message') + '\n')
+            signal.signal(signal.SIGINT, self._oldsiginthandler)
+            self._oldsiginthandler = None
+
+        try:
+            self._oldsiginthandler = signal.getsignal(signal.SIGINT)
+            signal.signal(signal.SIGINT, disabledsiginthandler)
+            yield
+        finally:
+            if self._oldsiginthandler is not None:
+                signal.signal(signal.SIGINT, self._oldsiginthandler)
+            self._oldsiginthandler = None
+
     def formatter(self, topic, opts):
         return formatter.formatter(self, self, topic, opts)
 
diff --git a/mercurial/configitems.py b/mercurial/configitems.py
--- a/mercurial/configitems.py
+++ b/mercurial/configitems.py
@@ -560,6 +560,17 @@ 
 coreconfigitem('experimental', 'mergedriver',
     default=None,
 )
+coreconfigitem('experimental', 'nointerrupt', default=False)
+_nointmsg = """
+==========================
+Interrupting Mercurial may leave your repo in a bad state.
+If you really want to interrupt your current command, press
+CTRL-C again.
+==========================
+""".strip()
+coreconfigitem('experimental', 'nointerrupt-message', default=_nointmsg)
+coreconfigitem('experimental', 'nointerrupt-interactiveonly', default=True)
+
 coreconfigitem('experimental', 'obsmarkers-exchange-debug',
     default=False,
 )