Good suggestions, integrated them. :)

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?

I agree with @yuja that we should move this to `util.py` or one of its

siblings and rename it to uninterruptable or some such.

procutil.py would be a better place.

That being said, signal handlers are process global. And we do need to
maintain persistent state so things don't get out of whack if we have
overlapping calls, potentially from multiple threads (e.g. in the case
of hgweb). So maybe ui.py - or even a global variable in that module -
is the proper place for such state.

I think ui-level blocking flag is fine. We won't have to care about
overlapped requests from threads because signal.signal() just doesn't
work in sub threads.

What I had in mind was something like the following:

class ui:
    @contextmanager
    def uninterruptable(self):
        if already blocked or not enabled:
            yield
            return
        with procutil.uninterruptable(warn=self.warn, onlysigint=True,
                                      allowdoublesigint=True):
            set blocked
            yield
            clear blocked

In D3716#58934, @indygreg wrote:

I agree with @yuja that we should move this to util.py or one of its siblings and rename it to uninterruptable or some such.
That being said, signal handlers are process global. And we do need to maintain persistent state so things don't get out of whack if we have overlapping calls, potentially from multiple threads (e.g. in the case of hgweb). So maybe ui.py - or even a global variable in that module - is the proper place for such state.
Also, I wonder if we shouldn't delay signal handling instead of ignoring it. e.g. our new signal handler would print that the signal was received and then raise KeyboardInterrupt at context manager exit time. Otherwise, the handling of SIGINT is timing dependent and doesn't consistently result in an aborted process.

I like this idea. I think bazel does something similar, I know blaze does: one ctrl-c says something like "Shutting down...", three will do a rather forceful shutdown (not kill-9 forceful, but it won't wait for it to get to a convenient spot). Something like that would work well here imho.

I'm not in love with uninterruptible as a name (I think unsafeoperation is more explicit) but I don't feel strongly. The rest of the suggestions I liked, so the series is updated and ready for another look.

+ @contextlib.contextmanager
+ def uninterruptable(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._uninterruptible or not enabled:
+ # if nointerrupt support is turned off, the process isn't
+ # interactive, or we're already in an uninterruptable
+ # block, do nothing.
+ yield
+ return
+ def warn():
+ self.warn(_("shutting down cleanly\n"))
+ self.warn(
+ _("press ^C again to terminate immediately (dangerous)\n"))

Missed return True ?

Other that that, the patch looks good to me.

Do we need a different strategy for Windows, or just eat the error? (See the unsupported signal message)

https://buildbot.mercurial-scm.org/builders/Win7%20x86_64%20hg%20tests/builds/808/steps/run-tests.py%20%28python%202.7.13%29/logs/stdio

Do we need a different strategy for Windows, or just eat the error? (See the unsupported signal message)
https://buildbot.mercurial-scm.org/builders/Win7%20x86_64%20hg%20tests/builds/808/steps/run-tests.py%20%28python%202.7.13%29/logs/stdio

I'm not sure. We could probably just not support this on windows for now, or at least disable the test on Windows?

In D3716#61963, @durin42 wrote:

Do we need a different strategy for Windows, or just eat the error? (See the unsupported signal message)
https://buildbot.mercurial-scm.org/builders/Win7%20x86_64%20hg%20tests/builds/808/steps/run-tests.py%20%28python%202.7.13%29/logs/stdio

I'm not sure. We could probably just not support this on windows for now, or at least disable the test on Windows?

I’m ok with disabling the test as long as it doesn’t explode on Windows without turning the experimental knobs. It sounded like future would would depend on this, and I wasn’t sure if that landed yet.

Do we need a different strategy for Windows, or just eat the error? (See the unsupported signal message)
https://buildbot.mercurial-scm.org/builders/Win7%20x86_64%20hg%20tests/builds/808/steps/run-tests.py%20%28python%202.7.13%29/logs/stdio

I'm not sure. We could probably just not support this on windows for now, or at least disable the test on Windows?

I’m ok with disabling the test as long as it doesn’t explode on Windows without turning the experimental knobs. It sounded like future would would depend on this, and I wasn’t sure if that landed yet.

We’ve already made some use of it in core - anything that calls strip will hit this context manager, as will the (experimental) narrow extension.