[PATCH 4 of 4 shelve-ext v3] shelve: make shelvestate use simplekeyvaluefile

Yuya Nishihara yuya at tcha.org
Wed Apr 12 11:03:28 EDT 2017


On Mon, 10 Apr 2017 16:28:36 -0700, Kostia Balytskyi wrote:
> # HG changeset patch
> # User Kostia Balytskyi <ikostia at fb.com>
> # Date 1491866434 25200
> #      Mon Apr 10 16:20:34 2017 -0700
> # Node ID 8e286a85816581cfa4ce44768482cb5722a88bb3
> # Parent  961539160565df5052d1c770788cacb2d76e9752
> shelve: make shelvestate use simplekeyvaluefile
> 
> Currently shelvestate uses line ordering to differentiate fields. This
> makes it hard for extensions to wrap shelve, since if two alternative
> versions of code add a new line, correct merging is going to be problematic.
> simplekeyvaluefile was introduced fot this purpose specifically.
> 
> After this patch:
> - shelve will always write a simplekeyvaluefile, unless 'shelve.oldstatefile'
> is on
> - unshelve will check the first line of the file and if it has '=' sign,
> will treat the file as a simplekeyvalue one. Otherwise, it will try to read
> an old-style statefile.
> 
> Test change is needed, because just a few lines below the place of change,
> test script does a direct manipulation of shelvestate file by removing a
> line in a particular position. Clearly, this relies on the fact that the
> file is  is position-based, not key-value.
> 
> diff --git a/hgext/shelve.py b/hgext/shelve.py
> --- a/hgext/shelve.py
> +++ b/hgext/shelve.py
> @@ -166,6 +166,10 @@ class shelvedstate(object):
>  
>      Handles saving and restoring a shelved state. Ensures that different
>      versions of a shelved state are possible and handles them appropriately.
> +
> +    By default, simplekeyvaluefile is used. The following config option
> +    allows one to enforce the old position-based state file to be used:
> +    shelve.oldstatefile
>      """
>      _version = 1

I think this is the version 2 of the state file.

And I don't think the config knob is good way to support old state file. If we
want to support writing old state files, we can write two separate files, like
mergestate. If not, we only need to switch parsers depending on the version
field, like histedit.

>      _filename = 'shelvedstate'
> @@ -175,6 +179,18 @@ class shelvedstate(object):
>      _noactivebook = ':no-active-bookmark'
>  
>      @classmethod
> +    def iskeyvaluebased(cls, repo):
> +        """Determine whether state file is simple lines or simplekeyvaluefile"""
> +        if repo.ui.configbool('shelve', 'oldstatefile', False):
> +            return True
> +        fp = repo.vfs(cls._filename)
> +        try:
> +            firstline = fp.readline()
> +            return '=' in firstline
> +        finally:
> +            fp.close()

Perhaps it's better to compare the version number instead of relying on
heuristic.

> +    @classmethod
>      def load(cls, repo):
>          # Order is important, because old shelvestate file uses it
>          # to detemine values of fields (i.g. version is on the first line,
> @@ -182,12 +198,15 @@ class shelvedstate(object):
>          keys = ['version', 'name', 'originalwctx', 'pendingctx', 'parents',
>                  'nodestoremove', 'branchtorestore', 'keep', 'activebook']
>          d = {}
> -        fp = repo.vfs(cls._filename)
> -        try:
> -            for key in keys:
> -                d[key] = fp.readline().strip()
> -        finally:
> -            fp.close()
> +        if cls.iskeyvaluebased(repo):
> +            d = scmutil.simplekeyvaluefile(repo.vfs, cls._filename).read()
> +        else:
> +            fp = repo.vfs(cls._filename)
> +            try:
> +                for key in keys:
> +                    d[key] = fp.readline().strip()
> +            finally:
> +                fp.close()
>  
>          # some basic syntactic verification and transformation
>          try:
> @@ -224,6 +243,22 @@ class shelvedstate(object):
>      @classmethod
>      def save(cls, repo, name, originalwctx, pendingctx, nodestoremove,
>               branchtorestore, keep=False, activebook=''):
> +        if not repo.ui.configbool('shelve', 'oldstatefile', False):
> +            info = {
> +                "version": str(cls._version),
> +                "name": name,
> +                "originalwctx": nodemod.hex(originalwctx.node()),
> +                "pendingctx": nodemod.hex(pendingctx.node()),
> +                "parents": ' '.join([nodemod.hex(p)
> +                                     for p in repo.dirstate.parents()]),
> +                "nodestoremove": ' '.join([nodemod.hex(n)
> +                                          for n in nodestoremove]),
> +                "branchtorestore": branchtorestore,
> +                "keep": cls._keep if keep else cls._nokeep,
> +                "activebook": activebook or cls._noactivebook
> +            }
> +            scmutil.simplekeyvaluefile(repo.vfs, cls._filename).write(info)
> +            return
>          fp = repo.vfs(cls._filename, 'wb')
>          fp.write('%i\n' % cls._version)
>          fp.write('%s\n' % name)

The shelvedstate class only provides load(), save() and clear(). They are all
storage functions. I guess new file format was supposed to be in new class.
Thoughts?


More information about the Mercurial-devel mailing list