Patchwork [4,of,5,evolve-ext] strip: add the option for wrapping the strip command

login
register
mail settings
Submitter Durham Goode
Date March 20, 2015, 1:14 a.m.
Message ID <e93100523ef10fae3fce.1426814069@dev2000.prn2.facebook.com>
Download mbox | patch
Permalink /patch/8192/
State Accepted
Headers show

Comments

Durham Goode - March 20, 2015, 1:14 a.m.
# HG changeset patch
# User Durham Goode <durham@fb.com>
# Date 1426793511 25200
#      Thu Mar 19 12:31:51 2015 -0700
# Node ID e93100523ef10fae3fcee2bdeac0f676ed842ecd
# Parent  0a4343392b3358584bd16e7b7f15e9c2264c9c8b
strip: add the option for wrapping the strip command

Adds an experimental option for wrapping the existing strip command and
replacing its functionality with prune. It currently doesn't handle the --keep
case, but an upcoming patch will address that.
Jordi Gutiérrez Hermoso - March 20, 2015, 1:34 p.m.
On Thu, 2015-03-19 at 18:14 -0700, Durham Goode wrote:
> # HG changeset patch
> # User Durham Goode <durham@fb.com>
> # Date 1426793511 25200
> #      Thu Mar 19 12:31:51 2015 -0700
> # Node ID e93100523ef10fae3fcee2bdeac0f676ed842ecd
> # Parent  0a4343392b3358584bd16e7b7f15e9c2264c9c8b
> strip: add the option for wrapping the strip command
> 
> Adds an experimental option for wrapping the existing strip command and
> replacing its functionality with prune. It currently doesn't handle the --keep
> case, but an upcoming patch will address that.

Ehhhh, I'm not sure I want this.

Sometimes I really do want to strip, outright strip. It's ok, I'm a
big boy, I know what I'm doing. If I enable both the strip extension
and evolve, why would evolve decide that I don't want to strip,
against my explicit wishes?

I could see a case for Evolve aliasing strip to prune if the strip
extension isn't loaded, but not for acting against my explicit wishes.
Durham Goode - March 20, 2015, 5:32 p.m.
On 3/20/15 6:34 AM, Jordi Gutiérrez Hermoso wrote:
> On Thu, 2015-03-19 at 18:14 -0700, Durham Goode wrote:
>> # HG changeset patch
>> # User Durham Goode <durham@fb.com>
>> # Date 1426793511 25200
>> #      Thu Mar 19 12:31:51 2015 -0700
>> # Node ID e93100523ef10fae3fcee2bdeac0f676ed842ecd
>> # Parent  0a4343392b3358584bd16e7b7f15e9c2264c9c8b
>> strip: add the option for wrapping the strip command
>>
>> Adds an experimental option for wrapping the existing strip command and
>> replacing its functionality with prune. It currently doesn't handle the --keep
>> case, but an upcoming patch will address that.
> Ehhhh, I'm not sure I want this.
>
> Sometimes I really do want to strip, outright strip. It's ok, I'm a
> big boy, I know what I'm doing. If I enable both the strip extension
> and evolve, why would evolve decide that I don't want to strip,
> against my explicit wishes?
>
> I could see a case for Evolve aliasing strip to prune if the strip
> extension isn't loaded, but not for acting against my explicit wishes.
>
>
We're migrating our existing user's onto evolve and in their minds 
'strip' is how they make commits go away.  They don't care if they're 
actually stripped, or just hidden, or sent to the moon.  So from a user 
experience stand point, the first step of deploying evolve is doing it 
in a way that changes as little of the UI as possible, then we'll 
introduce the new concepts piece by piece.

The functionality I added here is behind a config, so it won't affect 
existing evolve users.  It also adds '--bundle' to the strip command so 
you can tell it 'yes, i really want these commits deleted and put in a 
bundle'.
Harvey Chapman - March 20, 2015, 6:12 p.m.
> On Mar 20, 2015, at 1:32 PM, Durham Goode <durham@fb.com> wrote:
> We're migrating our existing user's onto evolve and in their minds 'strip' is how they make commits go away.  They don't care if they're actually stripped, or just hidden, or sent to the moon.  So from a user experience stand point, the first step of deploying evolve is doing it in a way that changes as little of the UI as possible, then we'll introduce the new concepts piece by piece.

Why not…

$ hg strip options
The strip command is no more. Perhaps you meant:
hg prune equivalent options
$

That should retrain your users in a hurry while giving them a quick copy/paste solution. It also clues them into the fact that the method of making commits go away may have changed (if they’re interested).
Durham Goode - March 20, 2015, 6:25 p.m.
On 3/20/15 11:12 AM, Harvey Chapman wrote:
>> On Mar 20, 2015, at 1:32 PM, Durham Goode <durham@fb.com> wrote:
>> We're migrating our existing user's onto evolve and in their minds 'strip' is how they make commits go away.  They don't care if they're actually stripped, or just hidden, or sent to the moon.  So from a user experience stand point, the first step of deploying evolve is doing it in a way that changes as little of the UI as possible, then we'll introduce the new concepts piece by piece.
> Why not…
>
> $ hg strip options
> The strip command is no more. Perhaps you meant:
> hg prune equivalent options
> $
>
> That should retrain your users in a hurry while giving them a quick copy/paste solution. It also clues them into the fact that the method of making commits go away may have changed (if they’re interested).
A few reasons:

1) with thousands of users, even the slightest UI change always 
generates support load (and this isn't really slight)
2) doing this would require that we come to some final design on the 
prune command, which we're not ready for
3) automation (much of which our team doesn't own) already uses strip, 
so we're not going to bother migrating them yet since it's a lot of work 
with no real benefit

There are of course rebuttals to all those points, but the big picture 
is that our users are having a tough time with recovering commits in 
mercurial (bundles are not a great experience), and this is the shortest 
path to solving that problem and getting evolve deployed.  Then we can 
iterate from there.
Pierre-Yves David - March 20, 2015, 7:44 p.m.
On 03/20/2015 11:25 AM, Durham Goode wrote:
>
>
> On 3/20/15 11:12 AM, Harvey Chapman wrote:
>>> On Mar 20, 2015, at 1:32 PM, Durham Goode <durham@fb.com> wrote:
>>> We're migrating our existing user's onto evolve and in their minds
>>> 'strip' is how they make commits go away.  They don't care if they're
>>> actually stripped, or just hidden, or sent to the moon.  So from a
>>> user experience stand point, the first step of deploying evolve is
>>> doing it in a way that changes as little of the UI as possible, then
>>> we'll introduce the new concepts piece by piece.
>> Why not…
>>
>> $ hg strip options
>> The strip command is no more. Perhaps you meant:
>> hg prune equivalent options
>> $
>>
>> That should retrain your users in a hurry while giving them a quick
>> copy/paste solution. It also clues them into the fact that the method
>> of making commits go away may have changed (if they’re interested).
> A few reasons:
>
> 1) with thousands of users, even the slightest UI change always
> generates support load (and this isn't really slight)
> 2) doing this would require that we come to some final design on the
> prune command, which we're not ready for
> 3) automation (much of which our team doesn't own) already uses strip,
> so we're not going to bother migrating them yet since it's a lot of work
> with no real benefit
>
> There are of course rebuttals to all those points, but the big picture
> is that our users are having a tough time with recovering commits in
> mercurial (bundles are not a great experience), and this is the shortest
> path to solving that problem and getting evolve deployed.  Then we can
> iterate from there.

I'm +1 on that. The strip change is not suitable for core-ish behavior 
and will most probably not make it in the final UI. But that is why it 
is behind a config option, turned off by default. In some environment, 
like the fb one were user and tool have been heavily taught about `hg 
strip` we need a more smooth transition. Moving from actual strip to 
hidden changeset will be a good enough jump.

Having this modificiation to strip in evolve makes it much more easy to 
use the `hg prune` code base and will focus the dev effort toward 
improving `hg prune`.

So, do not panic, this is not a long term UI direction, just an useful 
helper to get more user and effort toward building the right UI.

Patch

diff --git a/hgext/evolve.py b/hgext/evolve.py
--- a/hgext/evolve.py
+++ b/hgext/evolve.py
@@ -2101,6 +2101,28 @@  def commitwrapper(orig, ui, repo, *arg, 
         if lock is not None:
             lock.release()
 
+@eh.wrapcommand('strip', extension='strip', opts=[
+    ('', 'bundle', None, _("delete the commit entirely and move it to a "
+        "backup bundle")),
+    ])
+def stripwrapper(orig, ui, repo, *args, **kwargs):
+    if (not ui.configbool('experimental', 'replacestrip') or
+        kwargs.get('bundle', False)):
+        return orig(ui, repo, *args, **kwargs)
+
+    if kwargs.get('force'):
+        ui.warn(_("warning: --force has no effect during strip with evolve "
+            "enabled\n"))
+    if kwargs.get('no_backup', False):
+        ui.warn(_("warning: --no-backup has no effect during strips with "
+            "evolve enabled\n"))
+
+    kwargs['descendants'] = True
+    kwargs['new'] = []
+    kwargs['succ'] = []
+    kwargs['biject'] = False
+    return cmdprune(ui, repo, *args, **kwargs)
+
 @command('^touch',
     [('r', 'rev', [], 'revision to update'),
      ('D', 'duplicate', False,
diff --git a/tests/test-prune.t b/tests/test-prune.t
--- a/tests/test-prune.t
+++ b/tests/test-prune.t
@@ -238,6 +238,20 @@  test hg prune --descendants
   9
   0
 
+test hg strip replacement
+
+  $ hg up 10
+  2 files updated, 0 files merged, 0 files removed, 0 files unresolved
+  $ mkcommit n1
+  $ mkcommit n2
+  $ hg --config extensions.strip= --config experimental.replacestrip=True strip -r .
+  1 changesets pruned
+  0 files updated, 0 files merged, 1 files removed, 0 files unresolved
+  working directory now at c7e58696a948
+  $ hg --config extensions.strip= --config experimental.replacestrip=True strip -r . --bundle
+  0 files updated, 0 files merged, 1 files removed, 0 files unresolved
+  saved backup bundle to $TESTTMP/repo/.hg/strip-backup/c7e58696a948-69ca36d3-backup.hg
+
 test hg prune -B bookmark
 yoinked from test-mq-strip.t