Patchwork [v2] rollback: add a config knob for entirely disabling the command

login
register
mail settings
Submitter Augie Fackler
Date May 4, 2016, 1:41 a.m.
Message ID <fccaae31a23fee6a82df.1462326116@139.17.16.172.in-addr.arpa>
Download mbox | patch
Permalink /patch/14876/
State Superseded
Headers show

Comments

Augie Fackler - May 4, 2016, 1:41 a.m.
# HG changeset patch
# User Augie Fackler <augie@google.com>
# Date 1462307605 14400
#      Tue May 03 16:33:25 2016 -0400
# Node ID fccaae31a23fee6a82df37d6163613741e7a257c
# Parent  8eba4cdcfd810d80785c94d770a442c5bd1f9d0f
rollback: add a config knob for entirely disabling the command

This is of pretty high value for organizations that used to use p4 (as
an example), since `p4 rollback` is what we call `hg backout`.
Gregory Szorc - May 4, 2016, 6:02 a.m.
'hg rollback' is already marked as deprecated. Can we rename it (debugrollback?), remove it entirely, require a config option to enable it, or move it to an extension?

> On May 3, 2016, at 18:41, Augie Fackler <raf@durin42.com> wrote:
> 
> # HG changeset patch
> # User Augie Fackler <augie@google.com>
> # Date 1462307605 14400
> #      Tue May 03 16:33:25 2016 -0400
> # Node ID fccaae31a23fee6a82df37d6163613741e7a257c
> # Parent  8eba4cdcfd810d80785c94d770a442c5bd1f9d0f
> rollback: add a config knob for entirely disabling the command
> 
> This is of pretty high value for organizations that used to use p4 (as
> an example), since `p4 rollback` is what we call `hg backout`.
> 
> diff --git a/mercurial/commands.py b/mercurial/commands.py
> --- a/mercurial/commands.py
> +++ b/mercurial/commands.py
> @@ -6377,6 +6377,11 @@ def rollback(ui, repo, **opts):
>       commit transaction if it isn't checked out. Use --force to
>       override this protection.
> 
> +      The rollback command can be entirely disabled using the
> +      `ui.norollback` configuration setting. If you're here because
> +      you want to use rollback and it's disabled, you can re-enable
> +      the command by setting `ui.norollback` to `true`.
> +
>     This command is not intended for use on public repositories. Once
>     changes are visible for pull by other users, rolling a transaction
>     back locally is ineffective (someone else may already have pulled
> @@ -6386,6 +6391,9 @@ def rollback(ui, repo, **opts):
> 
>     Returns 0 on success, 1 if no rollback data is available.
>     """
> +    if ui.configbool('ui', 'norollback', False):
> +        raise error.Abort(_('rollback is disabled because it is unsafe'),
> +                          hint=('see `hg help -v rollback` for information'))
>     return repo.rollback(dryrun=opts.get('dry_run'),
>                          force=opts.get('force'))
> 
> diff --git a/tests/test-rollback.t b/tests/test-rollback.t
> --- a/tests/test-rollback.t
> +++ b/tests/test-rollback.t
> @@ -196,3 +196,15 @@ corrupt journal test
>   checking files
>   1 files, 2 changesets, 2 total revisions
> 
> +rollback disabled by config
> +  $ cat >> $HGRCPATH <<EOF
> +  > [ui]
> +  > norollback = true
> +  > EOF
> +  $ echo narf >> pinky-sayings.txt
> +  $ hg add pinky-sayings.txt
> +  $ hg ci -m 'First one.'
> +  $ hg rollback
> +  abort: rollback is disabled because it is unsafe
> +  (see `hg help -v rollback` for information)
> +  [255]
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Sean Farley - May 4, 2016, 10:18 p.m.
Gregory Szorc <gregory.szorc@gmail.com> writes:

> On May 4, 2016, at 06:56, Kevin Bullock <kbullock+mercurial@ringworld.org> wrote:
>
>>> On May 4, 2016, at 01:02, Gregory Szorc <gregory.szorc@gmail.com> wrote:
>>> 
>>> 'hg rollback' is already marked as deprecated. Can we rename it (debugrollback?), remove it entirely, require a config option to enable it, or move it to an extension?
>> 
>> Remember that "deprecated" in Mercurial doesn't mean "slated for removal", it means "not recommended or supported but will continue to work as it did". I think having a knob to disable it is the best we can do.
>
> I know. It's just that this command is a massive footgun that when fired can lead to permanent data loss. That violates a core principle of VCS tools: don't lose my data. I feel that's a bigger concern than BC.

Yeah, I agree with Greg here. We have the (BC) flag for things that are
really bad like this. We've even done it before in the past with 'strip
-b' (a flag that was confused for '--branch' but actually meant 'do not
create a backup file).
Johan Björk - May 4, 2016, 10:20 p.m.
Did the ui.progressive patch ever get accepted? If so this sounds like a
perfect candidate.

/Johan

On Wednesday, May 4, 2016, Sean Farley <sean@farley.io> wrote:

>
> Gregory Szorc <gregory.szorc@gmail.com <javascript:;>> writes:
>
> > On May 4, 2016, at 06:56, Kevin Bullock <
> kbullock+mercurial@ringworld.org <javascript:;>> wrote:
> >
> >>> On May 4, 2016, at 01:02, Gregory Szorc <gregory.szorc@gmail.com
> <javascript:;>> wrote:
> >>>
> >>> 'hg rollback' is already marked as deprecated. Can we rename it
> (debugrollback?), remove it entirely, require a config option to enable it,
> or move it to an extension?
> >>
> >> Remember that "deprecated" in Mercurial doesn't mean "slated for
> removal", it means "not recommended or supported but will continue to work
> as it did". I think having a knob to disable it is the best we can do.
> >
> > I know. It's just that this command is a massive footgun that when fired
> can lead to permanent data loss. That violates a core principle of VCS
> tools: don't lose my data. I feel that's a bigger concern than BC.
>
> Yeah, I agree with Greg here. We have the (BC) flag for things that are
> really bad like this. We've even done it before in the past with 'strip
> -b' (a flag that was confused for '--branch' but actually meant 'do not
> create a backup file).
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org <javascript:;>
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
Sean Farley - May 4, 2016, 10:32 p.m.
Johan Björk <jbjoerk@gmail.com> writes:

> Did the ui.progressive patch ever get accepted? If so this sounds like a
> perfect candidate.

It's a work in progress but, yes, this is why Augie is at the very least
making a flag for it.

Patch

diff --git a/mercurial/commands.py b/mercurial/commands.py
--- a/mercurial/commands.py
+++ b/mercurial/commands.py
@@ -6377,6 +6377,11 @@  def rollback(ui, repo, **opts):
       commit transaction if it isn't checked out. Use --force to
       override this protection.
 
+      The rollback command can be entirely disabled using the
+      `ui.norollback` configuration setting. If you're here because
+      you want to use rollback and it's disabled, you can re-enable
+      the command by setting `ui.norollback` to `true`.
+
     This command is not intended for use on public repositories. Once
     changes are visible for pull by other users, rolling a transaction
     back locally is ineffective (someone else may already have pulled
@@ -6386,6 +6391,9 @@  def rollback(ui, repo, **opts):
 
     Returns 0 on success, 1 if no rollback data is available.
     """
+    if ui.configbool('ui', 'norollback', False):
+        raise error.Abort(_('rollback is disabled because it is unsafe'),
+                          hint=('see `hg help -v rollback` for information'))
     return repo.rollback(dryrun=opts.get('dry_run'),
                          force=opts.get('force'))
 
diff --git a/tests/test-rollback.t b/tests/test-rollback.t
--- a/tests/test-rollback.t
+++ b/tests/test-rollback.t
@@ -196,3 +196,15 @@  corrupt journal test
   checking files
   1 files, 2 changesets, 2 total revisions
 
+rollback disabled by config
+  $ cat >> $HGRCPATH <<EOF
+  > [ui]
+  > norollback = true
+  > EOF
+  $ echo narf >> pinky-sayings.txt
+  $ hg add pinky-sayings.txt
+  $ hg ci -m 'First one.'
+  $ hg rollback
+  abort: rollback is disabled because it is unsafe
+  (see `hg help -v rollback` for information)
+  [255]