D3579: state: write the version number in plain text on top of state files
martinvonz added a comment. In https://phab.mercurial-scm.org/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! REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D3579 To: pulkit, #hg-reviewers, martinvonz Cc: yuja, martinvonz, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: D3579: state: write the version number in plain text on top of state files
> mercurial/state.py:65: undefined name 'iv' > mercurial/state.py:73: local variable 'version' is assigned to but never used Queued the fixes. ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: D3579: state: write the version number in plain text on top of state files
> -def save(self, data):
> +def save(self, version, data):
> """write all the state data stored to .hg/ 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-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D3579: state: write the version number in plain text on top of state files
yuja added a comment.
> - def save(self, data): +def save(self, version, data): """write all
the state data stored to .hg/ 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.
REPOSITORY
rHG Mercurial
REVISION DETAIL
https://phab.mercurial-scm.org/D3579
To: pulkit, #hg-reviewers, martinvonz
Cc: yuja, martinvonz, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D3579: state: write the version number in plain text on top of state files
This revision was automatically updated to reflect the committed changes.
Closed by commit rHGa0e4d654bceb: state: write the version number in plain text
on top of state files (authored by pulkit, committed by ).
REPOSITORY
rHG Mercurial
CHANGES SINCE LAST UPDATE
https://phab.mercurial-scm.org/D3579?vs=8787=8853
REVISION DETAIL
https://phab.mercurial-scm.org/D3579
AFFECTED FILES
mercurial/state.py
CHANGE DETAILS
diff --git a/mercurial/state.py b/mercurial/state.py
--- a/mercurial/state.py
+++ b/mercurial/state.py
@@ -22,6 +22,7 @@
from .thirdparty import cbor
from . import (
+error,
util,
)
@@ -51,18 +52,28 @@
"""read the existing state file and return a dict of data stored"""
return self._read()
-def save(self, data):
+def save(self, version, data):
"""write all the state data stored to .hg/ 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())
+except ValueError:
+raise error.ProgrammingError("unknown version of state file"
+ " found")
return cbor.load(fp)
def delete(self):
To: pulkit, #hg-reviewers, martinvonz
Cc: martinvonz, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D3579: state: write the version number in plain text on top of state files
martinvonz requested changes to this revision. martinvonz added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > state.py:60-62 > +try: > +iv = int(version) > +except ValueError: why not `if not isinstance(version, int):`? REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D3579 To: pulkit, #hg-reviewers, martinvonz Cc: martinvonz, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D3579: state: write the version number in plain text on top of state files
pulkit created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.
REVISION 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.
REPOSITORY
rHG Mercurial
REVISION DETAIL
https://phab.mercurial-scm.org/D3579
AFFECTED FILES
mercurial/state.py
CHANGE DETAILS
diff --git a/mercurial/state.py b/mercurial/state.py
--- a/mercurial/state.py
+++ b/mercurial/state.py
@@ -22,6 +22,7 @@
from .thirdparty import cbor
from . import (
+error,
util,
)
@@ -51,18 +52,30 @@
"""read the existing state file and return a dict of data stored"""
return self._read()
-def save(self, data):
+def save(self, version, data):
"""write all the state data stored to .hg/ file
we use third-party library cbor to serialize data to write in the file.
"""
+try:
+iv = int(version)
+except ValueError:
+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())
+except ValueError:
+raise error.ProgrammingError("unknown version of state file"
+ " found")
return cbor.load(fp)
def delete(self):
To: pulkit, #hg-reviewers
Cc: mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
