[PATCH 4 of 4 shelve-ext v3] shelve: make shelvestate use simplekeyvaluefile

[Yuya Nishihara]

On Sat, 15 Apr 2017 17:43:22 +0000, Kostia Balytskyi wrote:
> > -----Original Message-----
> > From: Yuya Nishihara [mailto:youjah at gmail.com] On Behalf Of Yuya
> > Nishihara
> > Sent: Saturday, 15 April, 2017 09:02
> > To: Kostia Balytskyi <ikostia at fb.com>
> > Cc: mercurial-devel at mercurial-scm.org
> > Subject: Re: [PATCH 4 of 4 shelve-ext v3] shelve: make shelvestate use
> > simplekeyvaluefile
> > 
> > On Fri, 14 Apr 2017 16:15:01 +0000, Kostia Balytskyi wrote:
> > > > -----Original Message-----
> > > > From: Yuya Nishihara [mailto:youjah at gmail.com] On Behalf Of Yuya
> > > > Nishihara
> > > > Sent: Friday, 14 April, 2017 12:16
> > > > To: Kostia Balytskyi <ikostia at fb.com>
> > > > Cc: mercurial-devel at mercurial-scm.org
> > > > Subject: Re: [PATCH 4 of 4 shelve-ext v3] shelve: make shelvestate
> > > > use simplekeyvaluefile
> > > >
> > > > On Thu, 13 Apr 2017 21:28:52 +0000, Kostia Balytskyi wrote:
> > > > > > > Comparing versions (if I understand you correctly) wouldn't
> > > > > > > work, see
> > > > > > below.
> > > > > >
> > > > > > I couldn't get why new format can't be version 2. The old format had
> > '1\n'
> > > > > > for future extension. This patch introduces new format, so using
> > > > > > '2\n' makes sense to me.
> > > > > >
> > > > > > The current simplekeyvaluefile API would make this a bit harder,
> > > > > > but we can extract functions to serialize and deserialize dict
> > > > > > from/to file
> > > > object.
> > > > > I do not understand what 'extract functions ...' means in this
> > > > > context? Can
> > > > you explain?
> > > >
> > > > We can move deserialization function (e.g. 'loadkv(fp: file) -> dict'
> > > > or 'loadkv(s: str) -> dict') from simplekeyvaluefile.read() for example.
> > >
> > > I don't like this either. If we go this way, there's no point in having
> > simplekeyvalue file at all.
> > 
> > Why is there no point? If I understand correctly, we want to switch to the
> > simplekeyvalue file format so that we can easily add new fields.
> By "no point" I meant that I don't want to break the simplekeyvalue promise, e.g. don't want to write non-key-value data.

Then, how about changing the filename? I prefer using a version header '%d\n'
for the maximum backward/forward compatibility, but you don't want to put it
into the simplekeyvalue file. This probably means a simplekeyvalue file must
always be in the simplekeyvalue format.

Since old clients expect the first line can be parsed as an int, we'll need
two files, e.g. 'shelvedstate' says '2\n', and 'shelvedstate2' contains data
in the simplekeyvalue format.