[PATCH 02 of 10 shelve-ext] shelve: add an ability to write json to a new type of shelve files

[Kostia Balytskyi]

On 11/29/16, 3:55 PM, "Jun Wu" <quark at fb.com> wrote:

    Excerpts from Kostia Balytskyi's message of 2016-11-29 07:22:56 -0800:
    > # HG changeset patch
    > # User Kostia Balytskyi <ikostia at fb.com>
    > # Date 1480426193 28800
    > #      Tue Nov 29 05:29:53 2016 -0800
    > # Node ID 37119e028c699d9fabd220086e08c754827e709f
    > # Parent  f6f0ab3f7b0ea0e05cfdcd7afd4994ea21988fd9
    > shelve: add an ability to write json to a new type of shelve files
    > 
    > Obsolescense-based shelve only needs metadata stored in .hg/shelved
    > and I think that this metadata should be stored in json for
    > potential extensibility purposes. JSON is not critical here, but
    > I want to avoid storing it in an unstructured text file where
    > order of lines determines their semantical meanings (as now
    > happens in .hg/shelvedstate. .hg/rebasestate and I suspect other
    > state files as well).
    > 
    > If we want, we can refactor it to something else later.
    > 
    > diff --git a/hgext/shelve.py b/hgext/shelve.py
    > --- a/hgext/shelve.py
    > +++ b/hgext/shelve.py
    > @@ -25,6 +25,7 @@ from __future__ import absolute_import
    >  import collections
    >  import errno
    >  import itertools
    > +import json
    
    I think we avoid using "import json" for some reason (encoding?). There is
    no "import json" in the code base yet.
    
    >  from mercurial.i18n import _
    >  from mercurial import (
    > @@ -62,7 +63,7 @@ testedwith = 'ships-with-hg-core'
    >  
    >  backupdir = 'shelve-backup'
    >  shelvedir = 'shelved'
    > -shelvefileextensions = ['hg', 'patch']
    > +shelvefileextensions = ['hg', 'patch', 'oshelve']
    >  # universal extension is present in all types of shelves
    >  patchextension = 'patch'
    >  
    > @@ -153,6 +154,26 @@ class shelvedfile(object):
    >          bundle2.writebundle(self.ui, cg, self.fname, btype, self.vfs,
    >                                  compression=compression)
    >  
    > +    def writejson(self, jsn):
    > +        fp = self.opener('wb')
    > +        try:
    > +            fp.write(json.dumps(jsn))
    > +        finally:
    > +            fp.close()
    > +
    > +    def readjson(self):
    > +        fp = self.opener()
    > +        contents = None
    > +        try:
    > +            contents = fp.read()
    > +        finally:
    > +            fp.close()
    > +        try:
    > +            jsn = json.loads(contents)
    > +        except (TypeError, ValueError):
    > +            raise error.abort(_('could not read obsolescense-based shelve'))
    
    error.Abort
    
    The method does not seem to be related to "obsolescense" just from the name
    and logic. So the error message would be inaccurate if callers use
    "writejson" to write other things.
    
    Given the fact "json" is a too generic name and we may replace it using
    other (lightweight) format in the future, it may be better to rename those
    methods to something more specific, and avoid exposing the "json" format
    to the caller, like:
    
        def writeobsinfo(self, node):
            ....
    
        def readobsinfo(self):
            ....
Renaming it makes sense, I will do that in a following patch. I did not get what to do with json though: should I try to replace it with some other format right away or is it fine to leave json for now?

    > +        return jsn
    > +
    >  class shelvedstate(object):
    >      """Handle persistence during unshelving operations.
    >