Patchwork [04,of,10] repair: identify repository deficiencies

login
register
mail settings
Submitter Gregory Szorc
Date Nov. 6, 2016, 4:40 a.m.
Message ID <7518e68e2f8276e85fb6.1478407220@ubuntu-vm-main>
Download mbox | patch
Permalink /patch/17366/
State Changes Requested
Headers show

Comments

Gregory Szorc - Nov. 6, 2016, 4:40 a.m.
# HG changeset patch
# User Gregory Szorc <gregory.szorc@gmail.com>
# Date 1478382332 25200
#      Sat Nov 05 14:45:32 2016 -0700
# Node ID 7518e68e2f8276e85fb68174b3055a9dd16c665d
# Parent  9daec9c7adabe8c84cf2c01fc938e010ee4884d6
repair: identify repository deficiencies

A command that upgrades repositories but doesn't say what it is doing
or why it is doing it is less helpful than a command that does. So,
we start the implementation of repository upgrades by introducing
detection of sub-optimal repository state. Deficiencies with the
existing repository are now printed at the beginning of command
execution.

The added function returns a set of planned upgrade actions. This
variable will be used by subsequent patches.
timeless - Nov. 6, 2016, 10:03 a.m.
Gregory Szorc wrote:
> A command that upgrades repositories but doesn't say what it is doing

shouldn't it still mention that it isn't doing the work?
Gregory Szorc - Nov. 6, 2016, 5:38 p.m.
On Sun, Nov 6, 2016 at 2:03 AM, timeless <timeless@gmail.com> wrote:

> Gregory Szorc wrote:
> > A command that upgrades repositories but doesn't say what it is doing
>
> shouldn't it still mention that it isn't doing the work?
>

Good question. I'm very open to UX improvements for this command. It is
debug functionality (for now), so I didn't spend as much energy on the UX
as I could have.

FWIW, I do have aspirations for command flags to control upgrade options. I
also thought it might be useful for something in non-debug land (`hg
summary`?) to expose informative "your repo isn't optimally stored"
messages. That's partially the reason "identify deficiencies" and
"determine final actions" are separate. (The main reason is putting it all
in 1 function will make it difficult for extensions to monkeypatch.)
Augie Fackler - Nov. 21, 2016, 8:45 p.m.
On Sat, Nov 05, 2016 at 09:40:20PM -0700, Gregory Szorc wrote:
> # HG changeset patch
> # User Gregory Szorc <gregory.szorc@gmail.com>
> # Date 1478382332 25200
> #      Sat Nov 05 14:45:32 2016 -0700
> # Node ID 7518e68e2f8276e85fb68174b3055a9dd16c665d
> # Parent  9daec9c7adabe8c84cf2c01fc938e010ee4884d6
> repair: identify repository deficiencies

Mostly happy so far, some string bikeshedding below. Tried to elide
irrelevant bits of the message so they'll be easy to spot.

>
> diff --git a/mercurial/commands.py b/mercurial/commands.py
> --- a/mercurial/commands.py
> +++ b/mercurial/commands.py
> @@ -3756,7 +3756,7 @@ def debugupgraderepo(ui, repo, **opts):
>
>      At times during the upgrade, the repository may not be readable.
>      """
> -    raise error.Abort(_('not yet implemented'))
> +    return repair.upgraderepo(ui, repo, dryrun=opts.get('dry_run'))
>
>  @command('debugwalk', walkopts, _('[OPTION]... [FILE]...'), inferrepo=True)
>  def debugwalk(ui, repo, *pats, **opts):
> diff --git a/mercurial/repair.py b/mercurial/repair.py
> --- a/mercurial/repair.py
> +++ b/mercurial/repair.py
> @@ -360,3 +360,57 @@ def deleteobsmarkers(obsstore, indices):
>          newobsstorefile.write(bytes)
>      newobsstorefile.close()
>      return n
> +
> +def upgradefinddeficiencies(repo):
> +    """Obtain deficiencies with the existing repo and planned actions to fix.
> +
> +    Returns a list of strings that will be printed to the user and a set

[...]

> +
> +    if 'generaldelta' not in repo.requirements:
> +        l.append(_('not using generaldelta storage; repository is larger '
> +                   'and slower than it could be, pulling from '
> +                   'generaldelta repositories will be slow'))

Perhaps word as "pulling from modern servers may be slow" - I'm not
sure that "generaldelta repositories" is really a helpful string to
print to normal humans.

> +        actions.add('generaldelta')
> +
> +    cl = repo.changelog
> +    for rev in cl:
> +        chainbase = cl.chainbase(rev)
> +        if chainbase != rev:
> +            l.append(_('changelog using delta chains; changelog reading '
> +                       'is slower than it could be'))

"changelog is storing deltas; changelog reading is slower than it could be"

(chains are irrelevant here, what matters is that we're trying to
store deltas at all)

> +            actions.add('removecldeltachain')
> +            break
> +
> +    return l, actions
> +
> +def upgraderepo(ui, repo, dryrun=False):
> +    """Upgrade a repository in place."""
> +    repo = repo.unfiltered()
> +    deficiencies, actions = upgradefinddeficiencies(repo)
> +
> +    if deficiencies:
> +        ui.write(_('the following deficiencies with the existing repository '
> +                   'have been identified:\n\n'))
> +
> +        for d in deficiencies:
> +            ui.write('* %s\n' % d)
> +    else:
> +        ui.write(_('no obvious deficiencies found in existing repository\n'))

[...]
timeless - Nov. 23, 2016, 4:26 a.m.
Augie Fackler wrote:
> Perhaps word as "pulling from modern servers may be slow" - I'm not
> sure that "generaldelta repositories" is really a helpful string to
> print to normal humans.

> "changelog is storing deltas; changelog reading is slower than it could be"

I'm +1 to these suggestions / justifications.

Patch

diff --git a/mercurial/commands.py b/mercurial/commands.py
--- a/mercurial/commands.py
+++ b/mercurial/commands.py
@@ -3756,7 +3756,7 @@  def debugupgraderepo(ui, repo, **opts):
 
     At times during the upgrade, the repository may not be readable.
     """
-    raise error.Abort(_('not yet implemented'))
+    return repair.upgraderepo(ui, repo, dryrun=opts.get('dry_run'))
 
 @command('debugwalk', walkopts, _('[OPTION]... [FILE]...'), inferrepo=True)
 def debugwalk(ui, repo, *pats, **opts):
diff --git a/mercurial/repair.py b/mercurial/repair.py
--- a/mercurial/repair.py
+++ b/mercurial/repair.py
@@ -360,3 +360,57 @@  def deleteobsmarkers(obsstore, indices):
         newobsstorefile.write(bytes)
     newobsstorefile.close()
     return n
+
+def upgradefinddeficiencies(repo):
+    """Obtain deficiencies with the existing repo and planned actions to fix.
+
+    Returns a list of strings that will be printed to the user and a set
+    of machine-readable actions that will be acted on later.
+    """
+    l = []
+    actions = set()
+
+    # We could detect lack of revlogv1 and store here, but they were added
+    # in 0.9.2 and we don't support upgrading repos without these
+    # requirements, so let's not bother.
+
+    if 'fncache' not in repo.requirements:
+        l.append(_('not using fncache; long and reserved filenames '
+                   'may not work correctly'))
+        actions.add('fncache')
+
+    if 'dotencode' not in repo.requirements:
+        l.append(_('not using dotencode; filenames beginning with '
+                   'a period or space may not work correctly'))
+        actions.add('dotencode')
+
+    if 'generaldelta' not in repo.requirements:
+        l.append(_('not using generaldelta storage; repository is larger '
+                   'and slower than it could be, pulling from '
+                   'generaldelta repositories will be slow'))
+        actions.add('generaldelta')
+
+    cl = repo.changelog
+    for rev in cl:
+        chainbase = cl.chainbase(rev)
+        if chainbase != rev:
+            l.append(_('changelog using delta chains; changelog reading '
+                       'is slower than it could be'))
+            actions.add('removecldeltachain')
+            break
+
+    return l, actions
+
+def upgraderepo(ui, repo, dryrun=False):
+    """Upgrade a repository in place."""
+    repo = repo.unfiltered()
+    deficiencies, actions = upgradefinddeficiencies(repo)
+
+    if deficiencies:
+        ui.write(_('the following deficiencies with the existing repository '
+                   'have been identified:\n\n'))
+
+        for d in deficiencies:
+            ui.write('* %s\n' % d)
+    else:
+        ui.write(_('no obvious deficiencies found in existing repository\n'))
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,18 @@ 
   $ hg init empty
   $ cd empty
   $ hg debugupgraderepo
-  abort: not yet implemented
-  [255]
+  no obvious deficiencies found in existing repository
+
+Various sub-optimal detections work
+
+  $ cat > .hg/requires << EOF
+  > revlogv1
+  > store
+  > EOF
+
+  $ hg debugupgraderepo
+  the following deficiencies with the existing repository have been identified:
+  
+  * not using fncache; long and reserved filenames may not work correctly
+  * not using dotencode; filenames beginning with a period or space may not work correctly
+  * not using generaldelta storage; repository is larger and slower than it could be, pulling from generaldelta repositories will be slow