Patchwork [2,of,8,upgraderepo,V3] repair: implement requirements checking for upgrades

login
register
mail settings
Submitter Gregory Szorc
Date Dec. 19, 2016, 1:07 a.m.
Message ID <8c7cd8a43f9e9b230bc1.1482109678@ubuntu-vm-main>
Download mbox | patch
Permalink /patch/17957/
State Accepted
Headers show

Comments

Gregory Szorc - Dec. 19, 2016, 1:07 a.m.
# HG changeset patch
# User Gregory Szorc <gregory.szorc@gmail.com>
# Date 1482106614 28800
#      Sun Dec 18 16:16:54 2016 -0800
# Node ID 8c7cd8a43f9e9b230bc125e57b80de73eb0649b5
# Parent  94c7d28b32a1f15ebd67e4ef54114ec80b33243f
repair: implement requirements checking for upgrades

This commit introduces functionality for upgrading a repository in
place. The first part that's implemented is testing for upgrade
"compatibility." This is done by examining repository requirements.

There are 5 functions returning sets of requirements that control
upgrading. Why so many functions? Mainly to support extensions.
Functions are easier to monkeypatch than module variables.

Astute readers will see that we don't support "manifestv2" and
"treemanifest" requirements in the upgrade mechanism. I don't have
a great answer for why other than this is a complex set of patches
and I don't want to deal with the complexity of these experimental
features just yet. We can teach the upgrade mechanism about them
later, once the basic upgrade mechanism is in place.

This commit also introduces the "upgraderepo" function. This will be
our main routine for performing an in-place upgrade. Currently, it
just implements requirements checking. The structure of some code in
this function may look a bit weird (e.g. the inline function that is
only called once). But this will make sense after future commits.
Augie Fackler - Dec. 24, 2016, 8:16 p.m.
(+martinvonz, durham for manifest questions)

On Sun, Dec 18, 2016 at 05:07:58PM -0800, Gregory Szorc wrote:
> # HG changeset patch
> # User Gregory Szorc <gregory.szorc@gmail.com>
> # Date 1482106614 28800
> #      Sun Dec 18 16:16:54 2016 -0800
> # Node ID 8c7cd8a43f9e9b230bc125e57b80de73eb0649b5
> # Parent  94c7d28b32a1f15ebd67e4ef54114ec80b33243f
> repair: implement requirements checking for upgrades
>
> This commit introduces functionality for upgrading a repository in
> place. The first part that's implemented is testing for upgrade
> "compatibility." This is done by examining repository requirements.
>
> There are 5 functions returning sets of requirements that control
> upgrading. Why so many functions? Mainly to support extensions.
> Functions are easier to monkeypatch than module variables.
>
> Astute readers will see that we don't support "manifestv2" and
> "treemanifest" requirements in the upgrade mechanism. I don't have
> a great answer for why other than this is a complex set of patches
> and I don't want to deal with the complexity of these experimental
> features just yet. We can teach the upgrade mechanism about them
> later, once the basic upgrade mechanism is in place.

I believe manifestv2 has turned out to be a total dead end. We should
probably jettison that code and mark the capability string as obsolete
and not to be used in the future. Does anyone see a reason for me to
not do that?
Durham Goode - Jan. 6, 2017, 11:25 p.m.
On 12/24/16 12:16 PM, Augie Fackler wrote:
> (+martinvonz, durham for manifest questions)
>
> On Sun, Dec 18, 2016 at 05:07:58PM -0800, Gregory Szorc wrote:
>> # HG changeset patch
>> # User Gregory Szorc <gregory.szorc@gmail.com>
>> # Date 1482106614 28800
>> #      Sun Dec 18 16:16:54 2016 -0800
>> # Node ID 8c7cd8a43f9e9b230bc125e57b80de73eb0649b5
>> # Parent  94c7d28b32a1f15ebd67e4ef54114ec80b33243f
>> repair: implement requirements checking for upgrades
>>
>> This commit introduces functionality for upgrading a repository in
>> place. The first part that's implemented is testing for upgrade
>> "compatibility." This is done by examining repository requirements.
>>
>> There are 5 functions returning sets of requirements that control
>> upgrading. Why so many functions? Mainly to support extensions.
>> Functions are easier to monkeypatch than module variables.
>>
>> Astute readers will see that we don't support "manifestv2" and
>> "treemanifest" requirements in the upgrade mechanism. I don't have
>> a great answer for why other than this is a complex set of patches
>> and I don't want to deal with the complexity of these experimental
>> features just yet. We can teach the upgrade mechanism about them
>> later, once the basic upgrade mechanism is in place.
> I believe manifestv2 has turned out to be a total dead end. We should
> probably jettison that code and mark the capability string as obsolete
> and not to be used in the future. Does anyone see a reason for me to
> not do that?
Yea, manifestv2 is dead.  Martin had asked me to kill some of the code 
during my refactor and I just never got around to it.

Patch

diff --git a/mercurial/debugcommands.py b/mercurial/debugcommands.py
--- a/mercurial/debugcommands.py
+++ b/mercurial/debugcommands.py
@@ -34,6 +34,7 @@  from . import (
     localrepo,
     lock as lockmod,
     pycompat,
+    repair,
     revlog,
     scmutil,
     setdiscovery,
@@ -873,4 +874,4 @@  def debugupgraderepo(ui, repo, run=False
     should complete almost instantaneously and the chances of a consumer being
     unable to access the repository should be low.
     """
-    raise error.Abort(_('not yet implemented'))
+    return repair.upgraderepo(ui, repo, run=run, optimize=optimize)
diff --git a/mercurial/repair.py b/mercurial/repair.py
--- a/mercurial/repair.py
+++ b/mercurial/repair.py
@@ -360,3 +360,138 @@  def deleteobsmarkers(obsstore, indices):
         newobsstorefile.write(bytes)
     newobsstorefile.close()
     return n
+
+def upgraderequiredsourcerequirements(repo):
+    """Obtain requirements required to be present to upgrade a repo.
+
+    An upgrade will not be allowed if the repository doesn't have the
+    requirements returned by this function.
+    """
+    return set([
+        # Introduced in Mercurial 0.9.2.
+        'revlogv1',
+        # Introduced in Mercurial 0.9.2.
+        'store',
+    ])
+
+def upgradeblocksourcerequirements(repo):
+    """Obtain requirements that will prevent an upgrade from occurring.
+
+    An upgrade cannot be performed if the source repository contains a
+    requirements in the returned set.
+    """
+    return set([
+        # The upgrade code does not yet support these experimental features.
+        # This is an artificial limitation.
+        'manifestv2',
+        'treemanifest',
+        # This was a precursor to generaldelta and was never enabled by default.
+        # It should (hopefully) not exist in the wild.
+        'parentdelta',
+        # Upgrade should operate on the actual store, not the shared link.
+        'shared',
+    ])
+
+def upgradesupportremovedrequirements(repo):
+    """Obtain requirements that can be removed during an upgrade.
+
+    If an upgrade were to create a repository that dropped a requirement,
+    the dropped requirement must appear in the returned set for the upgrade
+    to be allowed.
+    """
+    return set()
+
+def upgradesupporteddestrequirements(repo):
+    """Obtain requirements that upgrade supports in the destination.
+
+    If the result of the upgrade would create requirements not in this set,
+    the upgrade is disallowed.
+
+    Extensions should monkeypatch this to add their custom requirements.
+    """
+    return set([
+        'dotencode',
+        'fncache',
+        'generaldelta',
+        'revlogv1',
+        'store',
+    ])
+
+def upgradeallowednewrequirements(repo):
+    """Obtain requirements that can be added to a repository during upgrade.
+
+    This is used to disallow proposed requirements from being added when
+    they weren't present before.
+
+    We use a list of allowed requirement additions instead of a list of known
+    bad additions because the whitelist approach is safer and will prevent
+    future, unknown requirements from accidentally being added.
+    """
+    return set([
+        'dotencode',
+        'fncache',
+        'generaldelta',
+    ])
+
+def upgraderepo(ui, repo, run=False, optimize=None):
+    """Upgrade a repository in place."""
+    # Avoid cycle: cmdutil -> repair -> localrepo -> cmdutil
+    from . import localrepo
+
+    repo = repo.unfiltered()
+
+    # Ensure the repository can be upgraded.
+    missingreqs = upgraderequiredsourcerequirements(repo) - repo.requirements
+    if missingreqs:
+        raise error.Abort(_('cannot upgrade repository; requirement '
+                            'missing: %s') % _(', ').join(sorted(missingreqs)))
+
+    blockedreqs = upgradeblocksourcerequirements(repo) & repo.requirements
+    if blockedreqs:
+        raise error.Abort(_('cannot upgrade repository; unsupported source '
+                            'requirement: %s') %
+                          _(', ').join(sorted(blockedreqs)))
+
+    # FUTURE there is potentially a need to control the wanted requirements via
+    # command arguments or via an extension hook point.
+    newreqs = localrepo.newreporequirements(repo)
+
+    noremovereqs = (repo.requirements - newreqs -
+                   upgradesupportremovedrequirements(repo))
+    if noremovereqs:
+        raise error.Abort(_('cannot upgrade repository; requirement would be '
+                            'removed: %s') % _(', ').join(sorted(noremovereqs)))
+
+    noaddreqs = (newreqs - repo.requirements -
+                 upgradeallowednewrequirements(repo))
+    if noaddreqs:
+        raise error.Abort(_('cannot upgrade repository; do not support adding '
+                            'requirement: %s') %
+                          _(', ').join(sorted(noaddreqs)))
+
+    unsupportedreqs = newreqs - upgradesupporteddestrequirements(repo)
+    if unsupportedreqs:
+        raise error.Abort(_('cannot upgrade repository; do not support '
+                            'destination requirement: %s') %
+                          _(', ').join(sorted(unsupportedreqs)))
+
+    def printrequirements():
+        ui.write(_('requirements\n'))
+        ui.write(_('   preserved: %s\n') %
+                 _(', ').join(sorted(newreqs & repo.requirements)))
+
+        if repo.requirements - newreqs:
+            ui.write(_('   removed: %s\n') %
+                     _(', ').join(sorted(repo.requirements - newreqs)))
+
+        if newreqs - repo.requirements:
+            ui.write(_('   added: %s\n') %
+                     _(', ').join(sorted(newreqs - repo.requirements)))
+
+        ui.write('\n')
+
+    if not run:
+        ui.write(_('performing an upgrade with "--run" will make the following '
+                   'changes:\n\n'))
+
+        printrequirements()
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
@@ -1,5 +1,51 @@ 
-  $ hg init empty
-  $ cd empty
-  $ hg debugupgraderepo
-  abort: not yet implemented
+  $ cat >> $HGRCPATH << EOF
+  > [extensions]
+  > share =
+  > EOF
+
+store and revlogv1 are required in source
+
+  $ hg --config format.usestore=false init no-store
+  $ hg -R no-store debugupgraderepo
+  abort: cannot upgrade repository; requirement missing: store
+  [255]
+
+  $ hg init no-revlogv1
+  $ cat > no-revlogv1/.hg/requires << EOF
+  > dotencode
+  > fncache
+  > generaldelta
+  > store
+  > EOF
+
+  $ hg -R no-revlogv1 debugupgraderepo
+  abort: cannot upgrade repository; requirement missing: revlogv1
   [255]
+
+Cannot upgrade shared repositories
+
+  $ hg init share-parent
+  $ hg -q share share-parent share-child
+
+  $ hg -R share-child debugupgraderepo
+  abort: cannot upgrade repository; unsupported source requirement: shared
+  [255]
+
+Do not yet support upgrading manifestv2 and treemanifest repos
+
+  $ hg --config experimental.manifestv2=true init manifestv2
+  $ hg -R manifestv2 debugupgraderepo
+  abort: cannot upgrade repository; unsupported source requirement: manifestv2
+  [255]
+
+  $ hg --config experimental.treemanifest=true init treemanifest
+  $ hg -R treemanifest debugupgraderepo
+  abort: cannot upgrade repository; unsupported source requirement: treemanifest
+  [255]
+
+Cannot add manifestv2 or treemanifest requirement during upgrade
+
+  $ hg init disallowaddedreq
+  $ hg -R disallowaddedreq --config experimental.manifestv2=true --config experimental.treemanifest=true debugupgraderepo
+  abort: cannot upgrade repository; do not support adding requirement: manifestv2, treemanifest
+  [255]