Patchwork D2945: state: add a magicheader to each state file

login
register
mail settings
Submitter phabricator
Date March 26, 2018, 5:09 p.m.
Message ID <differential-rev-PHID-DREV-rpwwhkz4oke5q7u4kdvs-req@phab.mercurial-scm.org>
Download mbox | patch
Permalink /patch/29872/
State New
Headers show

Comments

phabricator - March 26, 2018, 5:09 p.m.
pulkit created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  This patch adds a magic header to each state file written using CBOR format so
  that we can distinguish between old state files and new one even if old state
  files get successfully parsed by cbor.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D2945

AFFECTED FILES
  mercurial/state.py

CHANGE DETAILS




To: pulkit, #hg-reviewers
Cc: mercurial-devel
phabricator - March 27, 2018, 12:58 p.m.
yuja added a comment.


  I think the magic has to vary depending on the current state file format.
  The state file must be backward/forward compatible with the older/newer formats.
  
  If the current magic is `2\n` (and is parsed as `int(f.readline())`) for example, the
  CBOR preamble would have to be `3\n`. Otherwise, old hg client cant determine
  whether the state file is corrupted or written by newer client.
  
  FWIW, last time I reviewed `simplekeyvaluefile`, I insisted that a parser should
  take a file stream instead of a filename, so the caller can handle compatibility thingy.
  
    with open("statefile", "rb") as fp:
        try:
            version = int(fp.readline())
        except ValueError:
            raise some nicer exception
        if version == 2:
            return readold(fp)
        elif version == 3:
            return readcbor(fp)

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D2945

To: pulkit, #hg-reviewers
Cc: yuja, mercurial-devel
phabricator - March 28, 2018, 11:05 a.m.
pulkit added a comment.


  In https://phab.mercurial-scm.org/D2945#47831, @yuja wrote:
  
  > I think the magic has to vary depending on the current state file format.
  >  The state file must be backward/forward compatible with the older/newer formats.
  >
  > If the current magic is `2\n` (and is parsed as `int(f.readline())`) for example, the
  >  CBOR preamble would have to be `3\n`. Otherwise, old hg client cant determine
  >  whether the state file is corrupted or written by newer client.
  
  
  There are some old state files, which don't have a version header on top of them like graftstate. I need suggestion on how to handle them.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D2945

To: pulkit, #hg-reviewers
Cc: yuja, mercurial-devel
phabricator - March 28, 2018, 1:39 p.m.
yuja added a comment.


  > There are some old state files, which don't have a version header on top
  >  of them like graftstate. I need suggestion on how to handle them.
  
  In that case, we would have to either do some heuristic (like histedit), or maintain
  old/new state files (like mergestate.) Perhaps graft can do the latter?

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D2945

To: pulkit, #hg-reviewers
Cc: yuja, mercurial-devel

Patch

diff --git a/mercurial/state.py b/mercurial/state.py
--- a/mercurial/state.py
+++ b/mercurial/state.py
@@ -51,6 +51,7 @@ 
             self.opts = {}
         else:
             self.opts = opts
+        self.opts['magicheader'] = 'HGStateFile'
 
     def __nonzero__(self):
         return self.exists()