[PATCH 09 of 10] repair: copy non-revlog store files during upgrade

[Augie Fackler]

On Sat, Nov 05, 2016 at 09:40:25PM -0700, Gregory Szorc wrote:
> # HG changeset patch
> # User Gregory Szorc <gregory.szorc at gmail.com>
> # Date 1478392019 25200
> #      Sat Nov 05 17:26:59 2016 -0700
> # Node ID 3d4dd237b705479f8c7475b821ae719893381fa8
> # Parent  d2261c558ca9639fb81c182de15d75151cbad0f9
> repair: copy non-revlog store files during upgrade
>
> The store contains more than just revlogs. This patch teaches the
> upgrade code to copy regular files as well.
>
> As the test changes demonstrate, the phaseroots file is now copied.
>
> diff --git a/mercurial/repair.py b/mercurial/repair.py
> --- a/mercurial/repair.py
> +++ b/mercurial/repair.py
> @@ -10,6 +10,7 @@ from __future__ import absolute_import
>
>  import errno
>  import hashlib
> +import stat
>  import tempfile
>  import time
>
> @@ -622,6 +623,39 @@ def _copyrevlogs(ui, srcrepo, dstrepo, t
>                 'across %d revlogs and %d revisions\n') % (
>               dstsize, dstsize - srcsize, rlcount, revcount))
>
> +def _upgradefilterstorefile(srcrepo, dstrepo, requirements, path, mode, st):
> +    """Determine whether to copy a store file during upgrade.
> +
> +    This function is called when migrating store files from ``srcrepo`` to
> +    ``dstrepo`` as part of upgrading a repository. The function receives
> +    the ``requirements`` for ``dstrepo``, the relative ``path`` of the
> +    store file being examined, its ``ST_MODE`` file type, and a full
> +    stat data structure.

This args list is pretty substantial. Can I interest you in adopting
the style used at
https://www.mercurial-scm.org/repo/hg/file/tip/mercurial/manifest.py#l526
or something similar that would give a more bullet-list-like
experience for enumerating the arguments and their types and meanings?

> +
> +    Function should return ``True`` if the file is to be copied.
> +    """
> +    # Skip revlogs.
> +    if path.endswith(('.i', '.d')):
> +        return False
> +    # Skip transaction related files.
> +    if path.startswith('undo'):
> +        return False
> +    # Only copy regular files.
> +    if mode != stat.S_IFREG:
> +        return False
> +    # Skip other skipped files.
> +    if path in ('lock', 'fncache'):
> +        return False
> +
> +    return True

I'm unclear which way we should bias. This is probably better than not
copying anything we don't recognize though.

This appears to only be files inside store. What about localtags and
bookmarks? I suppose it doesn't matter, since this is a store-level
upgrade that shouldn't change hashes.

> +
> +def _upgradefinishdatamigration(ui, srcrepo, dstrepo, requirements):
> +    """Hook point for extensions to perform additional actions during upgrade.
> +
> +    This function is called after revlogs and store files have been copied but
> +    before the new store is swapped into the original location.
> +    """
> +
>  def _upgraderepo(ui, srcrepo, dstrepo, requirements):
>      """Do the low-level work of upgrading a repository.
>
> @@ -641,7 +675,18 @@ def _upgraderepo(ui, srcrepo, dstrepo, r
>      with dstrepo.transaction('upgrade') as tr:
>          _copyrevlogs(ui, srcrepo, dstrepo, tr)
>
> -        # TODO copy non-revlog store files
> +    # Now copy other files in the store directory.
> +    for p, kind, st in srcrepo.store.vfs.readdir('', stat=True):
> +        if not _upgradefilterstorefile(srcrepo, dstrepo, requirements,
> +                                       p, kind, st):
> +            continue
> +
> +        srcrepo.ui.write(_('copying %s\n') % p)
> +        src = srcrepo.store.vfs.join(p)
> +        dst = dstrepo.store.vfs.join(p)
> +        util.copyfile(src, dst, copystat=True)
> +
> +    _upgradefinishdatamigration(ui, srcrepo, dstrepo, requirements)
>
>      ui.write(_('data fully migrated to temporary 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
> @@ -149,6 +149,7 @@ Upgrading a repository to generaldelta w
>    migrating manifests...
>    migrating changelog...
>    revlogs migration complete; wrote 917 bytes (delta 0 bytes) across 5 revlogs and 9 revisions
> +  copying phaseroots
>    data fully migrated to temporary repository
>    starting in-place swap of repository data
>    (clients may error or see inconsistent repository data until this operation completes)
> @@ -182,6 +183,7 @@ store directory has files we expect
>    00manifest.i
>    data
>    fncache
> +  phaseroots
>    undo
>    undo.backupfiles
>    undo.phaseroots
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel