On 3/29/17 2:18 PM, Kostia Balytskyi wrote:
# HG changeset patch
# User Kostia Balytskyi <ikos...@fb.com>
# Date 1490790691 25200
#      Wed Mar 29 05:31:31 2017 -0700
# Node ID 0953ca0a71a0e1f05078b0f3c255459e1ba56a4e
# Parent  9ce0a366a6316d1cffc2a875e7dc6b4a3227c851
shelve: add some forwards compatibility to shelve operations

This patch handles cases where someone enabled an obs-based
shelve in their repo, then disabled it while having made some
shelves. Approach of this patch is described below:
- assume this is what .hg/shelved looks like:
   - sh1.patch (created 1s ago)
   - sh1.oshelve (created 1s ago)
Would it make sense to have a "compatibility mode" where we also create the bundle, so that unshelving is actually backwards-compatible (if this option is enabled)?

   - sh2.patch (created 2s ago)
   - sh2.hg (created 2s ago)
- when obs-based shelve is enabled, 'hg shelve --list' shows both
   sh1 and s2 and is able to unshelve both. 'hg unshelbe' (without --name)

s/unshelbe/unshelve

   will unshelve sh1.
- when obs-based shelve is disabled, 'hg shelve --list' only shows
   sh2, prints a warning that it found an obs-based shelve and is
   only able to unshelve a traditional shelve. 'hg unshelve'
   (without --name) will unshelve sh2.

diff --git a/hgext/shelve.py b/hgext/shelve.py
--- a/hgext/shelve.py
+++ b/hgext/shelve.py
@@ -66,9 +66,15 @@ testedwith = 'ships-with-hg-core'
backupdir = 'shelve-backup'
  shelvedir = 'shelved'
-shelvefileextensions = ['hg', 'patch', 'oshelve']
  # universal extension is present in all types of shelves
  patchextension = 'patch'
+# extension used for bundle-based traditional shelves
+traditionalextension = 'hg'
+# extension used for obs-based shelves
+obsshelveextension = 'oshelve'
+shelvefileextensions = [traditionalextension,
+                        patchextension,
+                        obsshelveextension]
# we never need the user, so we use a
  # generic user for all shelve operations
@@ -530,7 +536,12 @@ def deletecmd(ui, repo, pats):
              raise error.Abort(_("shelved change '%s' not found") % name)
def listshelves(repo):
-    """return all shelves in repo as list of (time, filename)"""
+    """return a tuple of (allshelves, obsshelvespresent)
+    where
+    'allshelves' is a list of (time, filename) for shelves available
+                in current repo configuration
+    'obsshelvespresent' is a bool indicating whether repo has
+                        any obs-based shelves"""
      try:
          names = repo.vfs.readdir(shelvedir)
      except OSError as err:
@@ -538,13 +549,22 @@ def listshelves(repo):
              raise
          return []
      info = []
+    obsshelvedisabled = not isobsshelve(repo, repo.ui)
+    obsshelvespresent = False
      for (name, _type) in names:
          pfx, sfx = name.rsplit('.', 1)
+        if sfx == obsshelveextension:
+            obsshelvespresent = True
          if not pfx or sfx != patchextension:
              continue
+        traditionalfpath = repo.vfs.join(shelvedir,
+                                         pfx + '.' + traditionalextension)
+        if obsshelvedisabled and not repo.vfs.exists(traditionalfpath):
+            # this is not a traditional shelve
+            continue
          st = shelvedfile(repo, name).stat()
          info.append((st.st_mtime, shelvedfile(repo, pfx).filename()))
-    return sorted(info, reverse=True)
+    return sorted(info, reverse=True), obsshelvespresent
def listcmd(ui, repo, pats, opts):
      """subcommand that displays the list of shelves"""
@@ -554,7 +574,13 @@ def listcmd(ui, repo, pats, opts):
          width = ui.termwidth()
      namelabel = 'shelve.newest'
      ui.pager('shelve')
-    for mtime, name in listshelves(repo):
+    availableshelves, obsshelvespresent = listshelves(repo)
+    if (obsshelvespresent and not isobsshelve(repo, repo.ui) and
+        ui.configbool('shelve', 'showobswarning', True)):
New configs need to be documented.

+        ui.warn(_('warning: obsolescense-based shelve is disabled, but '
+                  'obs-based shelve files are in the repo\n'
+                  '(hint: set experimental.obsshelve=on in your hgrc)\n'))
+    for mtime, name in availableshelves :
          sname = util.split(name)[1]
          if pats and sname not in pats:
              continue
@@ -952,7 +978,7 @@ def _dounshelve(ui, repo, *shelved, **op
      elif len(shelved) > 1:
          raise error.Abort(_('can only unshelve one change at a time'))
      elif not shelved:
-        shelved = listshelves(repo)
+        shelved, __ = listshelves(repo)
          if not shelved:
              raise error.Abort(_('no shelved changes to apply!'))
          basename = util.split(shelved[0][1])[1]


I like the idea of warning users in this case -- thanks for thinking about this case. I think that a backwards-compat mode might be a nice option to allow safely testing this feature before diving in with both feet. I know that would be a bit slower because of the additional bundle, but if its configurable people can choose whether to pay that cost.

Overall, this series direction looks good to me but there are enough nitpicks and important questions I'm going to mark as "changes requested" in patchwork.
_______________________________________________
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Reply via email to