This is an archive of the discontinued Mercurial Phabricator instance.

Differential D3579

state: write the version number in plain text on top of state files
ClosedPublic

Authored by pulkit on May 18 2018, 7:18 AM.

Details

Reviewers
martinvonz
Group Reviewers
hg-reviewers
Commits
rHGa0e4d654bceb: state: write the version number in plain text on top of state files
Summary

We will soon be using CBOR format to write the data in state files. But we
should not write the version number of the state files in CBOR format and we
should rather write it in plain text because in future we can change the format
of state files and we should be able to parse the version number of state file
without requiring to understand a certain format.

This will help us in making sure we have a good compatibility story with other
versions of state files.

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

pulkit created this revision.May 18 2018, 7:18 AM
martinvonz requested changes to this revision.May 18 2018, 5:12 PM
martinvonz added a subscriber: martinvonz.
martinvonz added inline comments.
mercurial/state.py
60–62

why not if not isinstance(version, int):?

This revision now requires changes to proceed.May 18 2018, 5:12 PM
pulkit updated this revision to Diff 8787.May 19 2018, 10:52 AM
martinvonz accepted this revision.May 21 2018, 12:00 PM
This revision is now accepted and ready to land.May 21 2018, 12:00 PM
Closed by commit rHGa0e4d654bceb: state: write the version number in plain text on top of state files (authored by pulkit). · Explain WhyMay 21 2018, 1:40 PM
This revision was automatically updated to reflect the committed changes.
yuja added a subscriber: yuja.May 22 2018, 7:13 AM
  • def save(self, data):

+ def save(self, version, data):

"""write all the state data stored to .hg/<filename> file
we use third-party library cbor to serialize data to write in the file.
"""

+ if not isinstance(version, int):
+ raise error.ProgrammingError("version of state file should be"
+ " an integer")
+

with self._repo.vfs(self.fname, 'wb', atomictemp=True) as fp:

+ fp.write('%d\n' % iv)

        cbor.dump(self.opts, fp, canonical=True)
def _read(self):
    """reads the state file and returns a dictionary which contain
    data in the same format as it was before storing"""
    with self._repo.vfs(self.fname, 'rb') as fp:

+ try:
+ version = int(fp.readline())

mercurial/state.py:65: undefined name 'iv'
mercurial/state.py:73: local variable 'version' is assigned to but never used

+ except ValueError:
+ raise error.ProgrammingError("unknown version of state file"
+ " found")

Perhaps this should be a CorruptedState error. We don't know whether the
state file is good until reading it.

yuja added a comment.May 22 2018, 8:13 AM

mercurial/state.py:65: undefined name 'iv'
mercurial/state.py:73: local variable 'version' is assigned to but never used

Queued the fixes.

martinvonz added a comment.May 22 2018, 11:26 AM
In D3579#57298, @yuja wrote:

mercurial/state.py:65: undefined name 'iv'
mercurial/state.py:73: local variable 'version' is assigned to but never used

Queued the fixes.

Thanks!

Revision Contents
Changeset List

PathPackages
Mmercurial/state.py (13 lines)

Diff 8787

mercurial/state.py

to serialize and deserialize data. to serialize and deserialize data.
""" """
from __future__ import absolute_import from __future__ import absolute_import
from .thirdparty import cbor from .thirdparty import cbor
from . import ( from . import (
error,
util, util,
) )
class cmdstate(object): class cmdstate(object):
"""a wrapper class to store the state of commands like `rebase`, `graft`, """a wrapper class to store the state of commands like `rebase`, `graft`,
`histedit`, `shelve` etc. Extensions can also use this to write state files. `histedit`, `shelve` etc. Extensions can also use this to write state files.
All the data for the state is stored in the form of key-value pairs in a All the data for the state is stored in the form of key-value pairs in a
""" """
self._repo = repo self._repo = repo
self.fname = fname self.fname = fname
def read(self): def read(self):
"""read the existing state file and return a dict of data stored""" """read the existing state file and return a dict of data stored"""
return self._read() return self._read()
def save(self, data): def save(self, version, data):
"""write all the state data stored to .hg/<filename> file """write all the state data stored to .hg/<filename> file
we use third-party library cbor to serialize data to write in the file. we use third-party library cbor to serialize data to write in the file.
""" """
if not isinstance(version, int):
raise error.ProgrammingError("version of state file should be"
" an integer")
martinvonzUnsubmitted
Not Done

why not if not isinstance(version, int):?

martinvonz: why not `if not isinstance(version, int):`?
with self._repo.vfs(self.fname, 'wb', atomictemp=True) as fp: with self._repo.vfs(self.fname, 'wb', atomictemp=True) as fp:
fp.write('%d\n' % iv)
cbor.dump(self.opts, fp, canonical=True) cbor.dump(self.opts, fp, canonical=True)
def _read(self): def _read(self):
"""reads the state file and returns a dictionary which contain """reads the state file and returns a dictionary which contain
data in the same format as it was before storing""" data in the same format as it was before storing"""
with self._repo.vfs(self.fname, 'rb') as fp: with self._repo.vfs(self.fname, 'rb') as fp:
try:
version = int(fp.readline())
except ValueError:
raise error.ProgrammingError("unknown version of state file"
" found")
return cbor.load(fp) return cbor.load(fp)
def delete(self): def delete(self):
"""drop the state file if exists""" """drop the state file if exists"""
util.unlinkpath(self._repo.vfs.join(self.fname), ignoremissing=True) util.unlinkpath(self._repo.vfs.join(self.fname), ignoremissing=True)
def exists(self): def exists(self):
"""check whether the state file exists or not""" """check whether the state file exists or not"""
return self._repo.vfs.exists(self.fname) return self._repo.vfs.exists(self.fname)