This is an archive of the discontinued Mercurial Phabricator instance.

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

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
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.

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!