RE: [PATCH 3 of 4 shelve-ext v5] shelve: refactor shelvestate loading

2017-05-13 Thread Kostia Balytskyi 


> -Original Message-
> From: Yuya Nishihara [mailto:you...@gmail.com] On Behalf Of Yuya
> Nishihara
> Sent: Saturday, 13 May, 2017 11:36
> To: Kostia Balytskyi <ikos...@fb.com>
> Cc: mercurial-devel@mercurial-scm.org
> Subject: Re: [PATCH 3 of 4 shelve-ext v5] shelve: refactor shelvestate loading
> 
> On Thu, 11 May 2017 14:43:26 -0700, Kostia Balytskyi wrote:
> > # HG changeset patch
> > # User Kostia Balytskyi <ikos...@fb.com> # Date 1494520029 25200
> > #  Thu May 11 09:27:09 2017 -0700
> > # Node ID 9170d8e0460bfe9ae4ea79394786cf2cac686fcb
> > # Parent  b79f27451d7af524c07b89a983cf750bd96292c5
> > shelve: refactor shelvestate loading
> 
> > +# some basic syntactic verification and transformation
> > +try:
> > +d['version'] = int(d['version'])
> > +if d['version'] != cls._version:
> > +raise error.Abort(_('this version of shelve is 
> > incompatible '
> > +'with the version used in this repo'))
> > +d['originalwctx'] = nodemod.bin(d['originalwctx'])
> > +d['pendingctx'] = nodemod.bin(d['pendingctx'])
> > +d['parents'] = [nodemod.bin(h)
> > +for h in d['parents'].split(' ')]
> > +d['nodestoremove'] = [nodemod.bin(h)
> > +  for h in d['nodestoremove'].split(' ')]
> > +except (ValueError, TypeError, KeyError) as err:
> > +raise error.CorruptedState(str(err))
> > +
> >  try:
> >  obj = cls()
> > -obj.name = name
> > -obj.wctx = repo[wctx]
> > -obj.pendingctx = repo[pendingctx]
> > -obj.parents = parents
> > -obj.nodestoremove = nodestoremove
> > -obj.branchtorestore = branchtorestore
> > -obj.keep = keep
> > +obj.name = d.get('name')
> 
> Still d.get() here seems not appropriate. Does the shelve work if state.name
> is None?
Will change.
> 
> > +obj.wctx = repo[d.get('originalwctx')]
> > +obj.pendingctx = repo[d.get('pendingctx')]
> > +obj.parents = d.get('parents')
> > +obj.nodestoremove = d.get('nodestoremove')
> > +obj.branchtorestore = d.get('branchtorestore')
> > +obj.keep = d.get('keep') == cls._keep
> >  obj.activebookmark = ''
> > -if activebook != cls._noactivebook:
> > -obj.activebookmark = activebook
> > +if d.get('activebook', '') != cls._noactivebook:
> > +obj.activebookmark = d.get('activebook')
> 
> Maybe obj.activebookmark = d.get('activebook', '') ?
We also need to be able to distinguish cases when d['acivebook'] == 
cls._noactivebook.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH 3 of 4 shelve-ext v5] shelve: refactor shelvestate loading

2017-05-13 Thread Yuya Nishihara 
On Thu, 11 May 2017 14:43:26 -0700, Kostia Balytskyi wrote:
> # HG changeset patch
> # User Kostia Balytskyi 
> # Date 1494520029 25200
> #  Thu May 11 09:27:09 2017 -0700
> # Node ID 9170d8e0460bfe9ae4ea79394786cf2cac686fcb
> # Parent  b79f27451d7af524c07b89a983cf750bd96292c5
> shelve: refactor shelvestate loading

> +# some basic syntactic verification and transformation
> +try:
> +d['version'] = int(d['version'])
> +if d['version'] != cls._version:
> +raise error.Abort(_('this version of shelve is incompatible '
> +'with the version used in this repo'))
> +d['originalwctx'] = nodemod.bin(d['originalwctx'])
> +d['pendingctx'] = nodemod.bin(d['pendingctx'])
> +d['parents'] = [nodemod.bin(h)
> +for h in d['parents'].split(' ')]
> +d['nodestoremove'] = [nodemod.bin(h)
> +  for h in d['nodestoremove'].split(' ')]
> +except (ValueError, TypeError, KeyError) as err:
> +raise error.CorruptedState(str(err))
> +
>  try:
>  obj = cls()
> -obj.name = name
> -obj.wctx = repo[wctx]
> -obj.pendingctx = repo[pendingctx]
> -obj.parents = parents
> -obj.nodestoremove = nodestoremove
> -obj.branchtorestore = branchtorestore
> -obj.keep = keep
> +obj.name = d.get('name')

Still d.get() here seems not appropriate. Does the shelve work if state.name
is None?

> +obj.wctx = repo[d.get('originalwctx')]
> +obj.pendingctx = repo[d.get('pendingctx')]
> +obj.parents = d.get('parents')
> +obj.nodestoremove = d.get('nodestoremove')
> +obj.branchtorestore = d.get('branchtorestore')
> +obj.keep = d.get('keep') == cls._keep
>  obj.activebookmark = ''
> -if activebook != cls._noactivebook:
> -obj.activebookmark = activebook
> +if d.get('activebook', '') != cls._noactivebook:
> +obj.activebookmark = d.get('activebook')

Maybe obj.activebookmark = d.get('activebook', '') ?
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

[PATCH 3 of 4 shelve-ext v5] shelve: refactor shelvestate loading

2017-05-11 Thread Kostia Balytskyi 
# HG changeset patch
# User Kostia Balytskyi 
# Date 1494520029 25200
#  Thu May 11 09:27:09 2017 -0700
# Node ID 9170d8e0460bfe9ae4ea79394786cf2cac686fcb
# Parent  b79f27451d7af524c07b89a983cf750bd96292c5
shelve: refactor shelvestate loading

This is a preparatory patch which separates file reading from the
minimal validation we have (like turning version into int and
checking that this version is supported). The purpose of this patch
is to be able to read statefile form simplekeyvaluefile, which is
implemented in the following patch.

diff --git a/hgext/shelve.py b/hgext/shelve.py
--- a/hgext/shelve.py
+++ b/hgext/shelve.py
@@ -176,38 +176,46 @@ class shelvedstate(object):
 
 @classmethod
 def load(cls, repo):
+# Order is important, because old shelvestate file uses it
+# to detemine values of fields (i.g. version is on the first line,
+# name is on the second and so forth). Please do not change.
+keys = ['version', 'name', 'originalwctx', 'pendingctx', 'parents',
+'nodestoremove', 'branchtorestore', 'keep', 'activebook']
+d = {}
 fp = repo.vfs(cls._filename)
 try:
-version = int(fp.readline().strip())
-
-if version != cls._version:
-raise error.Abort(_('this version of shelve is incompatible '
-   'with the version used in this repo'))
-name = fp.readline().strip()
-wctx = nodemod.bin(fp.readline().strip())
-pendingctx = nodemod.bin(fp.readline().strip())
-parents = [nodemod.bin(h) for h in fp.readline().split()]
-nodestoremove = [nodemod.bin(h) for h in fp.readline().split()]
-branchtorestore = fp.readline().strip()
-keep = fp.readline().strip() == cls._keep
-activebook = fp.readline().strip()
-except (ValueError, TypeError) as err:
-raise error.CorruptedState(str(err))
+for key in keys:
+d[key] = fp.readline().strip()
 finally:
 fp.close()
 
+# some basic syntactic verification and transformation
+try:
+d['version'] = int(d['version'])
+if d['version'] != cls._version:
+raise error.Abort(_('this version of shelve is incompatible '
+'with the version used in this repo'))
+d['originalwctx'] = nodemod.bin(d['originalwctx'])
+d['pendingctx'] = nodemod.bin(d['pendingctx'])
+d['parents'] = [nodemod.bin(h)
+for h in d['parents'].split(' ')]
+d['nodestoremove'] = [nodemod.bin(h)
+  for h in d['nodestoremove'].split(' ')]
+except (ValueError, TypeError, KeyError) as err:
+raise error.CorruptedState(str(err))
+
 try:
 obj = cls()
-obj.name = name
-obj.wctx = repo[wctx]
-obj.pendingctx = repo[pendingctx]
-obj.parents = parents
-obj.nodestoremove = nodestoremove
-obj.branchtorestore = branchtorestore
-obj.keep = keep
+obj.name = d.get('name')
+obj.wctx = repo[d.get('originalwctx')]
+obj.pendingctx = repo[d.get('pendingctx')]
+obj.parents = d.get('parents')
+obj.nodestoremove = d.get('nodestoremove')
+obj.branchtorestore = d.get('branchtorestore')
+obj.keep = d.get('keep') == cls._keep
 obj.activebookmark = ''
-if activebook != cls._noactivebook:
-obj.activebookmark = activebook
+if d.get('activebook', '') != cls._noactivebook:
+obj.activebookmark = d.get('activebook')
 except error.RepoLookupError as err:
 raise error.CorruptedState(str(err))
 
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel