Patchwork [V3] strip: remove -b/--backup codepaths

login
register
mail settings
Submitter Jordi Gutiérrez Hermoso
Date July 24, 2014, 7:56 p.m.
Message ID <f11109bdbcf422f43ea7.1406231817@Iris>
Download mbox | patch
Permalink /patch/5197/
State Accepted
Commit 445472225ccd07c27323cab59c4a1bf728f76142
Headers show

Comments

Jordi Gutiérrez Hermoso - July 24, 2014, 7:56 p.m.
# HG changeset patch
# User Jordi Gutiérrez Hermoso <jordigh@octave.org>
# Date 1406228768 14400
#      Thu Jul 24 15:06:08 2014 -0400
# Node ID f11109bdbcf422f43ea74c1aca2bd1d858c0bbdc
# Parent  10abc3a5c6b21e1256c3e601fe4a950208e9293c
strip: remove -b/--backup codepaths

cset ba3bc6474bbf has removed this option. This commit just tidies the
code that was associated to it. It also fixes the internal calls to
the strip() function.

Before this change, any function that thought it would want as a final
safety to keep a partial backup bundle (bundling changes not linearly
related to the current change being stripped), had to explicitly pass
a backup="strip" option. With this change, these backups are always
kept in case of an exception and always removed if there is no
exception. Only full backups can be specified with backup=True or no
full backups with backup=False.
Pierre-Yves David - July 25, 2014, 9:23 a.m.
On 07/24/2014 09:56 PM, Jordi Gutiérrez Hermoso wrote:
> # HG changeset patch
> # User Jordi Gutiérrez Hermoso <jordigh@octave.org>
> # Date 1406228768 14400
> #      Thu Jul 24 15:06:08 2014 -0400
> # Node ID f11109bdbcf422f43ea74c1aca2bd1d858c0bbdc
> # Parent  10abc3a5c6b21e1256c3e601fe4a950208e9293c
> strip: remove -b/--backup codepaths
>
> cset ba3bc6474bbf has removed this option. This commit just tidies the
> code that was associated to it. It also fixes the internal calls to
> the strip() function.

I'm pretty sure this patch is introducing too much churn for no 
user-visible benefit to be accepted that far int he freeze. please 
resent in August.
Augie Fackler - Aug. 7, 2014, 5:02 p.m.
On Thu, Jul 24, 2014 at 03:56:57PM -0400, Jordi Gutiérrez Hermoso wrote:
> # HG changeset patch
> # User Jordi Gutiérrez Hermoso <jordigh@octave.org>
> # Date 1406228768 14400
> #      Thu Jul 24 15:06:08 2014 -0400
> # Node ID f11109bdbcf422f43ea74c1aca2bd1d858c0bbdc
> # Parent  10abc3a5c6b21e1256c3e601fe4a950208e9293c
> strip: remove -b/--backup codepaths

Went through the unloved patches file and queued this one too.

>
> cset ba3bc6474bbf has removed this option. This commit just tidies the
> code that was associated to it. It also fixes the internal calls to
> the strip() function.
>
> Before this change, any function that thought it would want as a final
> safety to keep a partial backup bundle (bundling changes not linearly
> related to the current change being stripped), had to explicitly pass
> a backup="strip" option. With this change, these backups are always
> kept in case of an exception and always removed if there is no
> exception. Only full backups can be specified with backup=True or no
> full backups with backup=False.
>
> diff --git a/hgext/mq.py b/hgext/mq.py
> --- a/hgext/mq.py
> +++ b/hgext/mq.py
> @@ -621,7 +621,7 @@ class queue(object):
>
>          # apply failed, strip away that rev and merge.
>          hg.clean(repo, head)
> -        strip(self.ui, repo, [n], update=False, backup='strip')
> +        strip(self.ui, repo, [n], update=False, backup=False)
>
>          ctx = repo[rev]
>          ret = hg.merge(repo, rev)
> @@ -1456,7 +1456,7 @@ class queue(object):
>              for patch in reversed(self.applied[start:end]):
>                  self.ui.status(_("popping %s\n") % patch.name)
>              del self.applied[start:end]
> -            strip(self.ui, repo, [rev], update=False, backup='strip')
> +            strip(self.ui, repo, [rev], update=False, backup=False)
>              for s, state in repo['.'].substate.items():
>                  repo['.'].sub(s).get(state)
>              if self.applied:
> @@ -1645,7 +1645,7 @@ class queue(object):
>                  repo.setparents(*cparents)
>                  self.applied.pop()
>                  self.applieddirty = True
> -                strip(self.ui, repo, [top], update=False, backup='strip')
> +                strip(self.ui, repo, [top], update=False, backup=False)
>              except: # re-raises
>                  repo.dirstate.invalidate()
>                  raise
> @@ -1842,7 +1842,7 @@ class queue(object):
>                      update = True
>                  else:
>                      update = False
> -                strip(self.ui, repo, [rev], update=update, backup='strip')
> +                strip(self.ui, repo, [rev], update=update, backup=False)
>          if qpp:
>              self.ui.warn(_("saved queue repository parents: %s %s\n") %
>                           (short(qpp[0]), short(qpp[1])))
> diff --git a/hgext/shelve.py b/hgext/shelve.py
> --- a/hgext/shelve.py
> +++ b/hgext/shelve.py
> @@ -388,7 +388,7 @@ def unshelveabort(ui, repo, state, opts)
>
>          mergefiles(ui, repo, state.wctx, state.pendingctx)
>
> -        repair.strip(ui, repo, state.stripnodes, backup='none', topic='shelve')
> +        repair.strip(ui, repo, state.stripnodes, backup=False, topic='shelve')
>          shelvedstate.clear(repo)
>          ui.warn(_("unshelve of '%s' aborted\n") % state.name)
>      finally:
> @@ -457,7 +457,7 @@ def unshelvecontinue(ui, repo, state, op
>          mergefiles(ui, repo, state.wctx, shelvectx)
>
>          state.stripnodes.append(shelvectx.node())
> -        repair.strip(ui, repo, state.stripnodes, backup='none', topic='shelve')
> +        repair.strip(ui, repo, state.stripnodes, backup=False, topic='shelve')
>          shelvedstate.clear(repo)
>          unshelvecleanup(ui, repo, state.name, opts)
>          ui.status(_("unshelve of '%s' complete\n") % state.name)
> diff --git a/hgext/strip.py b/hgext/strip.py
> --- a/hgext/strip.py
> +++ b/hgext/strip.py
> @@ -42,7 +42,7 @@ def checklocalchanges(repo, force=False,
>              raise util.Abort(_("local changed subrepos found" + excsuffix))
>      return m, a, r, d
>
> -def strip(ui, repo, revs, update=True, backup="all", force=None, bookmark=None):
> +def strip(ui, repo, revs, update=True, backup=True, force=None, bookmark=None):
>      wlock = lock = None
>      try:
>          wlock = repo.wlock()
> @@ -114,11 +114,9 @@ def stripcmd(ui, repo, *revs, **opts):
>
>      Return 0 on success.
>      """
> -    backup = 'all'
> -    if opts.get('backup'):
> -        backup = 'strip'
> -    elif opts.get('no_backup') or opts.get('nobackup'):
> -        backup = 'none'
> +    backup = True
> +    if opts.get('no_backup') or opts.get('nobackup'):
> +        backup = False
>
>      cl = repo.changelog
>      revs = list(revs) + opts.get('rev')
> diff --git a/mercurial/repair.py b/mercurial/repair.py
> --- a/mercurial/repair.py
> +++ b/mercurial/repair.py
> @@ -47,7 +47,13 @@ def _collectbrokencsets(repo, files, str
>
>      return s
>
> -def strip(ui, repo, nodelist, backup="all", topic='backup'):
> +def strip(ui, repo, nodelist, backup=True, topic='backup'):
> +
> +    # Simple way to maintain backwards compatibility for this
> +    # argument.
> +    if backup in ['none', 'strip']:
> +        backup = False
> +
>      repo = repo.unfiltered()
>      repo.destroying()
>
> @@ -58,8 +64,6 @@ def strip(ui, repo, nodelist, backup="al
>      striplist = [cl.rev(node) for node in nodelist]
>      striprev = min(striplist)
>
> -    keeppartialbundle = backup == 'strip'
> -
>      # Some revisions with rev > striprev may not be descendants of striprev.
>      # We have to find these revisions and put them in a bundle, so that
>      # we can restore them after the truncations.
> @@ -109,7 +113,7 @@ def strip(ui, repo, nodelist, backup="al
>      # create a changegroup for all the branches we need to keep
>      backupfile = None
>      vfs = repo.vfs
> -    if backup == "all":
> +    if backup:
>          backupfile = _bundle(repo, stripbases, cl.heads(), node, topic)
>          repo.ui.status(_("saved backup bundle to %s\n") %
>                         vfs.join(backupfile))
> @@ -118,7 +122,7 @@ def strip(ui, repo, nodelist, backup="al
>      if saveheads or savebases:
>          # do not compress partial bundle if we remove it from disk later
>          chgrpfile = _bundle(repo, savebases, saveheads, node, 'temp',
> -                            compress=keeppartialbundle)
> +                            compress=False)
>
>      mfst = repo.manifest
>
> @@ -156,8 +160,6 @@ def strip(ui, repo, nodelist, backup="al
>              if not repo.ui.verbose:
>                  repo.ui.popbuffer()
>              f.close()
> -            if not keeppartialbundle:
> -                vfs.unlink(chgrpfile)
>
>          # remove undo files
>          for undovfs, undofile in repo.undofiles():
> @@ -179,5 +181,9 @@ def strip(ui, repo, nodelist, backup="al
>              ui.warn(_("strip failed, partial bundle stored in '%s'\n")
>                      % vfs.join(chgrpfile))
>          raise
> +    else:
> +        if saveheads or savebases:
> +            # Remove partial backup only if there were no exceptions
> +            vfs.unlink(chgrpfile)
>
>      repo.destroyed()
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel

Patch

diff --git a/hgext/mq.py b/hgext/mq.py
--- a/hgext/mq.py
+++ b/hgext/mq.py
@@ -621,7 +621,7 @@  class queue(object):
 
         # apply failed, strip away that rev and merge.
         hg.clean(repo, head)
-        strip(self.ui, repo, [n], update=False, backup='strip')
+        strip(self.ui, repo, [n], update=False, backup=False)
 
         ctx = repo[rev]
         ret = hg.merge(repo, rev)
@@ -1456,7 +1456,7 @@  class queue(object):
             for patch in reversed(self.applied[start:end]):
                 self.ui.status(_("popping %s\n") % patch.name)
             del self.applied[start:end]
-            strip(self.ui, repo, [rev], update=False, backup='strip')
+            strip(self.ui, repo, [rev], update=False, backup=False)
             for s, state in repo['.'].substate.items():
                 repo['.'].sub(s).get(state)
             if self.applied:
@@ -1645,7 +1645,7 @@  class queue(object):
                 repo.setparents(*cparents)
                 self.applied.pop()
                 self.applieddirty = True
-                strip(self.ui, repo, [top], update=False, backup='strip')
+                strip(self.ui, repo, [top], update=False, backup=False)
             except: # re-raises
                 repo.dirstate.invalidate()
                 raise
@@ -1842,7 +1842,7 @@  class queue(object):
                     update = True
                 else:
                     update = False
-                strip(self.ui, repo, [rev], update=update, backup='strip')
+                strip(self.ui, repo, [rev], update=update, backup=False)
         if qpp:
             self.ui.warn(_("saved queue repository parents: %s %s\n") %
                          (short(qpp[0]), short(qpp[1])))
diff --git a/hgext/shelve.py b/hgext/shelve.py
--- a/hgext/shelve.py
+++ b/hgext/shelve.py
@@ -388,7 +388,7 @@  def unshelveabort(ui, repo, state, opts)
 
         mergefiles(ui, repo, state.wctx, state.pendingctx)
 
-        repair.strip(ui, repo, state.stripnodes, backup='none', topic='shelve')
+        repair.strip(ui, repo, state.stripnodes, backup=False, topic='shelve')
         shelvedstate.clear(repo)
         ui.warn(_("unshelve of '%s' aborted\n") % state.name)
     finally:
@@ -457,7 +457,7 @@  def unshelvecontinue(ui, repo, state, op
         mergefiles(ui, repo, state.wctx, shelvectx)
 
         state.stripnodes.append(shelvectx.node())
-        repair.strip(ui, repo, state.stripnodes, backup='none', topic='shelve')
+        repair.strip(ui, repo, state.stripnodes, backup=False, topic='shelve')
         shelvedstate.clear(repo)
         unshelvecleanup(ui, repo, state.name, opts)
         ui.status(_("unshelve of '%s' complete\n") % state.name)
diff --git a/hgext/strip.py b/hgext/strip.py
--- a/hgext/strip.py
+++ b/hgext/strip.py
@@ -42,7 +42,7 @@  def checklocalchanges(repo, force=False,
             raise util.Abort(_("local changed subrepos found" + excsuffix))
     return m, a, r, d
 
-def strip(ui, repo, revs, update=True, backup="all", force=None, bookmark=None):
+def strip(ui, repo, revs, update=True, backup=True, force=None, bookmark=None):
     wlock = lock = None
     try:
         wlock = repo.wlock()
@@ -114,11 +114,9 @@  def stripcmd(ui, repo, *revs, **opts):
 
     Return 0 on success.
     """
-    backup = 'all'
-    if opts.get('backup'):
-        backup = 'strip'
-    elif opts.get('no_backup') or opts.get('nobackup'):
-        backup = 'none'
+    backup = True
+    if opts.get('no_backup') or opts.get('nobackup'):
+        backup = False
 
     cl = repo.changelog
     revs = list(revs) + opts.get('rev')
diff --git a/mercurial/repair.py b/mercurial/repair.py
--- a/mercurial/repair.py
+++ b/mercurial/repair.py
@@ -47,7 +47,13 @@  def _collectbrokencsets(repo, files, str
 
     return s
 
-def strip(ui, repo, nodelist, backup="all", topic='backup'):
+def strip(ui, repo, nodelist, backup=True, topic='backup'):
+
+    # Simple way to maintain backwards compatibility for this
+    # argument.
+    if backup in ['none', 'strip']:
+        backup = False
+
     repo = repo.unfiltered()
     repo.destroying()
 
@@ -58,8 +64,6 @@  def strip(ui, repo, nodelist, backup="al
     striplist = [cl.rev(node) for node in nodelist]
     striprev = min(striplist)
 
-    keeppartialbundle = backup == 'strip'
-
     # Some revisions with rev > striprev may not be descendants of striprev.
     # We have to find these revisions and put them in a bundle, so that
     # we can restore them after the truncations.
@@ -109,7 +113,7 @@  def strip(ui, repo, nodelist, backup="al
     # create a changegroup for all the branches we need to keep
     backupfile = None
     vfs = repo.vfs
-    if backup == "all":
+    if backup:
         backupfile = _bundle(repo, stripbases, cl.heads(), node, topic)
         repo.ui.status(_("saved backup bundle to %s\n") %
                        vfs.join(backupfile))
@@ -118,7 +122,7 @@  def strip(ui, repo, nodelist, backup="al
     if saveheads or savebases:
         # do not compress partial bundle if we remove it from disk later
         chgrpfile = _bundle(repo, savebases, saveheads, node, 'temp',
-                            compress=keeppartialbundle)
+                            compress=False)
 
     mfst = repo.manifest
 
@@ -156,8 +160,6 @@  def strip(ui, repo, nodelist, backup="al
             if not repo.ui.verbose:
                 repo.ui.popbuffer()
             f.close()
-            if not keeppartialbundle:
-                vfs.unlink(chgrpfile)
 
         # remove undo files
         for undovfs, undofile in repo.undofiles():
@@ -179,5 +181,9 @@  def strip(ui, repo, nodelist, backup="al
             ui.warn(_("strip failed, partial bundle stored in '%s'\n")
                     % vfs.join(chgrpfile))
         raise
+    else:
+        if saveheads or savebases:
+            # Remove partial backup only if there were no exceptions
+            vfs.unlink(chgrpfile)
 
     repo.destroyed()