Patchwork D9770: upgrade: don't create store backup if `--no-backup` is passed

login
register
mail settings
Submitter phabricator
Date Jan. 14, 2021, 12:54 p.m.
Message ID <differential-rev-PHID-DREV-ytw2n3bbmyntvc4notlh-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/48083/
State Superseded
Headers show

Comments

phabricator - Jan. 14, 2021, 12:54 p.m.
pulkit created this revision.
Herald added a reviewer: hg-reviewers.
Herald added a subscriber: mercurial-patches.

REVISION SUMMARY
  If the user explicitly mentioned that they don't need backup, then let's not
  create it.

REPOSITORY
  rHG Mercurial

BRANCH
  default

REVISION DETAIL
  https://phab.mercurial-scm.org/D9770

AFFECTED FILES
  mercurial/upgrade.py
  mercurial/upgrade_utils/actions.py
  mercurial/upgrade_utils/engine.py
  tests/test-upgrade-repo.t

CHANGE DETAILS




To: pulkit, #hg-reviewers
Cc: mercurial-patches, mercurial-devel

Patch

diff --git a/tests/test-upgrade-repo.t b/tests/test-upgrade-repo.t
--- a/tests/test-upgrade-repo.t
+++ b/tests/test-upgrade-repo.t
@@ -628,11 +628,9 @@ 
   data fully upgraded in a temporary repository
   marking source repository as being upgraded; clients will be unable to read from repository
   starting in-place swap of repository data
-  replaced files will be backed up at $TESTTMP/upgradegd/.hg/upgradebackup.* (glob)
   replacing store...
   store replacement complete; repository was inconsistent for * (glob)
   finalizing requirements file and making repository readable again
-  removing old repository content $TESTTMP/upgradegd/.hg/upgradebackup.* (glob)
   removing temporary repository $TESTTMP/upgradegd/.hg/upgrade.* (glob)
   $ ls -1 .hg/ | grep upgradebackup
   [1]
@@ -675,11 +673,9 @@ 
   data fully upgraded in a temporary repository
   marking source repository as being upgraded; clients will be unable to read from repository
   starting in-place swap of repository data
-  replaced files will be backed up at $TESTTMP/upgradegd/.hg/upgradebackup.* (glob)
   replacing store...
   store replacement complete; repository was inconsistent for *s (glob)
   finalizing requirements file and making repository readable again
-  removing old repository content $TESTTMP/upgradegd/.hg/upgradebackup.* (glob)
   removing temporary repository $TESTTMP/upgradegd/.hg/upgrade.* (glob)
 
 Check that the repo still works fine
@@ -755,11 +751,9 @@ 
   data fully upgraded in a temporary repository
   marking source repository as being upgraded; clients will be unable to read from repository
   starting in-place swap of repository data
-  replaced files will be backed up at $TESTTMP/upgradegd/.hg/upgradebackup.* (glob)
   replacing store...
   store replacement complete; repository was inconsistent for *s (glob)
   finalizing requirements file and making repository readable again
-  removing old repository content $TESTTMP/upgradegd/.hg/upgradebackup.* (glob)
   removing temporary repository $TESTTMP/upgradegd/.hg/upgrade.* (glob)
   $ hg verify
   checking changesets
@@ -806,11 +800,9 @@ 
   data fully upgraded in a temporary repository
   marking source repository as being upgraded; clients will be unable to read from repository
   starting in-place swap of repository data
-  replaced files will be backed up at $TESTTMP/upgradegd/.hg/upgradebackup.* (glob)
   replacing store...
   store replacement complete; repository was inconsistent for *s (glob)
   finalizing requirements file and making repository readable again
-  removing old repository content $TESTTMP/upgradegd/.hg/upgradebackup.* (glob)
   removing temporary repository $TESTTMP/upgradegd/.hg/upgrade.* (glob)
   $ hg verify
   checking changesets
@@ -857,11 +849,9 @@ 
   data fully upgraded in a temporary repository
   marking source repository as being upgraded; clients will be unable to read from repository
   starting in-place swap of repository data
-  replaced files will be backed up at $TESTTMP/upgradegd/.hg/upgradebackup.* (glob)
   replacing store...
   store replacement complete; repository was inconsistent for *s (glob)
   finalizing requirements file and making repository readable again
-  removing old repository content $TESTTMP/upgradegd/.hg/upgradebackup.* (glob)
   removing temporary repository $TESTTMP/upgradegd/.hg/upgrade.* (glob)
   $ hg verify
   checking changesets
@@ -915,11 +905,9 @@ 
   data fully upgraded in a temporary repository
   marking source repository as being upgraded; clients will be unable to read from repository
   starting in-place swap of repository data
-  replaced files will be backed up at $TESTTMP/upgradegd/.hg/upgradebackup.* (glob)
   replacing store...
   store replacement complete; repository was inconsistent for *s (glob)
   finalizing requirements file and making repository readable again
-  removing old repository content $TESTTMP/upgradegd/.hg/upgradebackup.* (glob)
   removing temporary repository $TESTTMP/upgradegd/.hg/upgrade.* (glob)
   $ hg verify
   checking changesets
@@ -974,11 +962,9 @@ 
   data fully upgraded in a temporary repository
   marking source repository as being upgraded; clients will be unable to read from repository
   starting in-place swap of repository data
-  replaced files will be backed up at $TESTTMP/upgradegd/.hg/upgradebackup.* (glob)
   replacing store...
   store replacement complete; repository was inconsistent for *s (glob)
   finalizing requirements file and making repository readable again
-  removing old repository content $TESTTMP/upgradegd/.hg/upgradebackup.* (glob)
   removing temporary repository $TESTTMP/upgradegd/.hg/upgrade.* (glob)
   $ hg verify
   checking changesets
diff --git a/mercurial/upgrade_utils/engine.py b/mercurial/upgrade_utils/engine.py
--- a/mercurial/upgrade_utils/engine.py
+++ b/mercurial/upgrade_utils/engine.py
@@ -412,33 +412,36 @@ 
     """
     # TODO: don't blindly rename everything in store
     # There can be upgrades where store is not touched at all
-    backupstorevfs = vfsmod.vfs(backupvfs.join(b'store'))
-    util.makedirs(backupstorevfs.base)
-    for path, kind, st in sorted(currentrepo.store.vfs.readdir(b'', stat=True)):
-        # Skip transaction related files.
-        if path.startswith(b'undo'):
-            continue
-        # Only copy regular files.
-        if kind != stat.S_IFREG:
-            continue
-        # Skip other skipped files.
-        if path in (b'lock',):
-            continue
-        src = currentrepo.store.rawvfs.join(path)
-        dst = backupstorevfs.join(path)
-        util.copyfile(src, dst, copystat=True)
-    if currentrepo.svfs.exists(b'data'):
-        util.copyfiles(
-            currentrepo.svfs.join(b'data'),
-            backupstorevfs.join(b'data'),
-            hardlink=False,
-        )
-    if currentrepo.svfs.exists(b'meta'):
-        util.copyfiles(
-            currentrepo.svfs.join(b'meta'),
-            backupstorevfs.join(b'meta'),
-            hardlink=False,
-        )
+    if upgrade_op.backup_store:
+        backupstorevfs = vfsmod.vfs(backupvfs.join(b'store'))
+        util.makedirs(backupstorevfs.base)
+        for path, kind, st in sorted(
+            currentrepo.store.vfs.readdir(b'', stat=True)
+        ):
+            # Skip transaction related files.
+            if path.startswith(b'undo'):
+                continue
+            # Only copy regular files.
+            if kind != stat.S_IFREG:
+                continue
+            # Skip other skipped files.
+            if path in (b'lock',):
+                continue
+            src = currentrepo.store.rawvfs.join(path)
+            dst = backupstorevfs.join(path)
+            util.copyfile(src, dst, copystat=True)
+        if currentrepo.svfs.exists(b'data'):
+            util.copyfiles(
+                currentrepo.svfs.join(b'data'),
+                backupstorevfs.join(b'data'),
+                hardlink=False,
+            )
+        if currentrepo.svfs.exists(b'meta'):
+            util.copyfiles(
+                currentrepo.svfs.join(b'meta'),
+                backupstorevfs.join(b'meta'),
+                hardlink=False,
+            )
 
     currentrepo.vfs.rmtree(b'store', forcibly=True)
     util.rename(upgradedrepo.spath, currentrepo.spath)
@@ -464,6 +467,8 @@ 
     """
     assert srcrepo.currentwlock()
     assert dstrepo.currentwlock()
+    backuppath = None
+    backupvfs = None
 
     ui.status(
         _(
@@ -492,11 +497,16 @@ 
 
     ui.status(_(b'data fully upgraded in a temporary repository\n'))
 
-    backuppath = pycompat.mkdtemp(prefix=b'upgradebackup.', dir=srcrepo.path)
-    backupvfs = vfsmod.vfs(backuppath)
+    if upgrade_op.backup_store:
+        backuppath = pycompat.mkdtemp(
+            prefix=b'upgradebackup.', dir=srcrepo.path
+        )
+        backupvfs = vfsmod.vfs(backuppath)
 
-    # Make a backup of requires file first, as it is the first to be modified.
-    util.copyfile(srcrepo.vfs.join(b'requires'), backupvfs.join(b'requires'))
+        # Make a backup of requires file first, as it is the first to be modified.
+        util.copyfile(
+            srcrepo.vfs.join(b'requires'), backupvfs.join(b'requires')
+        )
 
     # We install an arbitrary requirement that clients must not support
     # as a mechanism to lock out new clients during the data swap. This is
@@ -513,7 +523,8 @@ 
     )
 
     ui.status(_(b'starting in-place swap of repository data\n'))
-    ui.status(_(b'replaced files will be backed up at %s\n') % backuppath)
+    if upgrade_op.backup_store:
+        ui.status(_(b'replaced files will be backed up at %s\n') % backuppath)
 
     # Now swap in the new store directory. Doing it as a rename should make
     # the operation nearly instantaneous and atomic (at least in well-behaved
diff --git a/mercurial/upgrade_utils/actions.py b/mercurial/upgrade_utils/actions.py
--- a/mercurial/upgrade_utils/actions.py
+++ b/mercurial/upgrade_utils/actions.py
@@ -626,6 +626,7 @@ 
         upgrade_actions,
         removed_actions,
         revlogs_to_process,
+        backup_store,
     ):
         self.ui = ui
         self.new_requirements = new_requirements
@@ -670,6 +671,9 @@ 
             b're-delta-multibase' in self._upgrade_actions_names
         )
 
+        # should this operation create a backup of the store
+        self.backup_store = backup_store
+
     def _write_labeled(self, l, label):
         """
         Utility function to aid writing of a list under one label
diff --git a/mercurial/upgrade.py b/mercurial/upgrade.py
--- a/mercurial/upgrade.py
+++ b/mercurial/upgrade.py
@@ -118,6 +118,7 @@ 
         up_actions,
         removed_actions,
         revlogs,
+        backup,
     )
 
     if not run:
@@ -215,12 +216,6 @@ 
                 backuppath = upgrade_engine.upgrade(
                     ui, repo, dstrepo, upgrade_op
                 )
-            if not backup:
-                ui.status(
-                    _(b'removing old repository content %s\n') % backuppath
-                )
-                repo.vfs.rmtree(backuppath, forcibly=True)
-                backuppath = None
 
         finally:
             ui.status(_(b'removing temporary repository %s\n') % tmppath)