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.

Download Raw Diff

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

Herald added a reviewer: hg-reviewers. · View Herald TranscriptMay 18 2018, 7:18 AM

Herald added a subscriber: mercurial-devel. · View Herald Transcript

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.

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.

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!

Revision Contents
Changeset List

			Path	Packages
M			mercurial/state.py (15 lines)

Diff	ID	Description	Created	Lint	Unit
Base		Base
Diff 1	8734		May 18 2018, 7:18 AM	★	★
Diff 2	8787		May 19 2018, 10:51 AM	★	★
Diff 3	8853	rHGa0e4d654bceb40c1d68aaa293181ac1d9052a0d1	May 18 2018, 6:58 AM	★	★

Commit	Parents	Author	Summary	Date
		Pulkit Goyal		May 18 2018, 6:58 AM

Status	Author	Revision
Closed	pulkit	D3579 state: write the version number in plain text on top of state files
Closed	pulkit	D3578 state: set canonical=True to write deterministically
Closed	pulkit	D3572 state: don't have a dict like interface for cmdstate class
Closed	pulkit	D2591 state: import the file to write state files from evolve extension
Closed	pulkit	D2752 cbor: add a __init__.py to top level cbor module
Closed	pulkit	D2751 cbor: remove unrequired files and fix core test-check*
Closed	pulkit	D2750 thirdparty: start vendoring cbor python library

Diff 8734

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.
	"""			"""
				try:
				iv = int(version)
				except ValueError:
				martinvonzUnsubmitted Not Done why not `if not isinstance(version, int):`? martinvonz: why not `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:			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)