| Submitter | Pierre-Yves David |
|---|---|
| Date | Sept. 17, 2014, 2:28 a.m. |
| Message ID | <d5f9d5cb72e220cc8966.1410920911@marginatus.alto.octopoid.net> |
| Download | mbox | patch |
| Permalink | /patch/5840/ |
| State | Superseded |
| Headers | show |
Comments
On Tue, Sep 16, 2014 at 07:28:31PM -0700, Pierre-Yves David wrote: > # HG changeset patch > # User Pierre-Yves David <pierre-yves.david@fb.com> > # Date 1410525703 -7200 > # Fri Sep 12 14:41:43 2014 +0200 > # Node ID d5f9d5cb72e220cc89661dbe08afd9b13436c1df > # Parent 48791c2bea1ceda4e4f28bc11651e281d636ce1a > obsolete: gather all contents related to format version 0 in a single place > > All contents embrances documentation, constants and functions. The diff is confusing Should "embrances" have been "enhances"? I'm not quite sure what word you meant. > because it cannot understand what code is semantically moved around in this > grand migration. I assume "no functionality change, just code movement" is true? This might be a case where a class-as-namespace would make some amount of sense, as much as I hate myself for the proposal. > > diff --git a/mercurial/obsolete.py b/mercurial/obsolete.py > --- a/mercurial/obsolete.py > +++ b/mercurial/obsolete.py > @@ -61,29 +61,12 @@ Markers are stored in an append-only fil > > The file starts with a version header: > > - 1 unsigned byte: version number, starting at zero. > > - > -The header is followed by the markers. Each marker is made of: > - > -- 1 unsigned byte: number of new changesets "N", can be zero. > - > -- 1 unsigned 32-bits integer: metadata size "M" in bytes. > - > -- 1 byte: a bit field. It is reserved for flags used in common > - obsolete marker operations, to avoid repeated decoding of metadata > - entries. > - > -- 20 bytes: obsoleted changeset identifier. > - > -- N*20 bytes: new changesets identifiers. > - > -- M bytes: metadata as a sequence of nul-terminated strings. Each > - string contains a key and a value, separated by a colon ':', without > - additional encoding. Keys cannot contain '\0' or ':' and values > - cannot contain '\0'. > +The header is followed by the markers. Marker format depend of the version. See > +comment associated with each format for details. > > """ > import struct > import util, base85, node > import phases > @@ -96,17 +79,10 @@ from i18n import _ > > # the obsolete feature is not mature enough to be enabled by default. > # you have to rely on third party extension extension to enable this. > _enabled = False > > -# data used for parsing and writing > -_fm0version = 0 > -_fm0fixed = '>BIB20s' > -_fm0node = '20s' > -_fm0fsize = struct.calcsize(_fm0fixed) > -_fm0fnodesize = struct.calcsize(_fm0node) > - > ### obsolescence marker flag > > ## bumpedfix flag > # > # When a changeset A' succeed to a changeset A which became public, we call A' > @@ -135,28 +111,35 @@ from i18n import _ > # This flag mean that the successors express the changes between the public and > # bumped version and fix the situation, breaking the transitivity of > # "bumped" here. > bumpedfix = 1 > > -def _readmarkers(data): > - """Read and enumerate markers from raw data""" > - off = 0 > - diskversion = _unpack('>B', data[off:off + 1])[0] > - off += 1 > - if diskversion not in formats: > - raise util.Abort(_('parsing obsolete marker: unknown version %r') > - % diskversion) > - return diskversion, formats[diskversion][0](data, off) > - > -def encodemarkers(markers, addheader=False, version=_fm0version): > - # Kept separate from flushmarkers(), it will be reused for > - # markers exchange. > - encodeone = formats[version][1] > - if addheader: > - yield _pack('>B', _fm0version) > - for marker in markers: > - yield encodeone(marker) > +## Parsing and writing of version "0" > +# > +# The header is followed by the markers. Each marker is made of: > +# > +# - 1 unsigned byte: number of new changesets "N", can be zero. > +# > +# - 1 unsigned 32-bits integer: metadata size "M" in bytes. > +# > +# - 1 byte: a bit field. It is reserved for flags used in common > +# obsolete marker operations, to avoid repeated decoding of metadata > +# entries. > +# > +# - 20 bytes: obsoleted changeset identifier. > +# > +# - N*20 bytes: new changesets identifiers. > +# > +# - M bytes: metadata as a sequence of nul-terminated strings. Each > +# string contains a key and a value, separated by a colon ':', without > +# additional encoding. Keys cannot contain '\0' or ':' and values > +# cannot contain '\0'. > +_fm0version = 0 > +_fm0fixed = '>BIB20s' > +_fm0node = '20s' > +_fm0fsize = struct.calcsize(_fm0fixed) > +_fm0fnodesize = struct.calcsize(_fm0node) > > def _fm0readmarkers(data, off=0): > # Loop on markers > l = len(data) > while off + _fm0fsize <= l: > @@ -227,10 +210,30 @@ def _fm0encodeonemarker(marker): > > # mapping to read/write various marker formats > # <version> -> (decoder, encoder) > formats = {0: (_fm0readmarkers, _fm0encodeonemarker)} > > +def _readmarkers(data): > + """Read and enumerate markers from raw data""" > + off = 0 > + diskversion = _unpack('>B', data[off:off + 1])[0] > + off += 1 > + if diskversion not in formats: > + raise util.Abort(_('parsing obsolete marker: unknown version %r') > + % diskversion) > + return diskversion, formats[diskversion][0](data, off) > + > +def encodemarkers(markers, addheader=False, version=_fm0version): > + # Kept separate from flushmarkers(), it will be reused for > + # markers exchange. > + encodeone = formats[version][1] > + if addheader: > + yield _pack('>B', _fm0version) > + for marker in markers: > + yield encodeone(marker) > + > + > def encodemeta(meta): > """Return encoded metadata string to string mapping. > > Assume no ':' in key and no '\0' in both key and value.""" > for key, value in meta.iteritems(): > _______________________________________________ > Mercurial-devel mailing list > Mercurial-devel@selenic.com > http://selenic.com/mailman/listinfo/mercurial-devel
On 09/24/2014 08:21 AM, Augie Fackler wrote: > On Tue, Sep 16, 2014 at 07:28:31PM -0700, Pierre-Yves David wrote: >> # HG changeset patch >> # User Pierre-Yves David <pierre-yves.david@fb.com> >> # Date 1410525703 -7200 >> # Fri Sep 12 14:41:43 2014 +0200 >> # Node ID d5f9d5cb72e220cc89661dbe08afd9b13436c1df >> # Parent 48791c2bea1ceda4e4f28bc11651e281d636ce1a >> obsolete: gather all contents related to format version 0 in a single place >> >> All contents embrances documentation, constants and functions. The diff is confusing > > Should "embrances" have been "enhances"? I'm not quite sure what word you meant. Nope. I mean "all content" == documentation + constants + functions. Not sure what the word should be here. >> because it cannot understand what code is semantically moved around in this >> grand migration. > > I assume "no functionality change, just code movement" is true? Yop > This might be a case where a class-as-namespace would make some amount > of sense, as much as I hate myself for the proposal. Not sure. It is tempting but there is actually just a couple of function and an handful of constant. Not sure what the class would bring but the namespace. However the proposal makes sense and I tempted to do it. My plan it to discuss it with mpm in person next week.
On Wed, Sep 24, 2014 at 12:40 PM, Pierre-Yves David <pierre-yves.david@ens-lyon.org> wrote: >>> All contents embrances documentation, constants and functions. The diff >>> is confusing >> >> >> Should "embrances" have been "enhances"? I'm not quite sure what word you >> meant. > > > Nope. > > I mean "all content" == documentation + constants + functions. > > Not sure what the word should be here. All contents includes documentation, constants, and functions, so we gather all of those things into a single location. How's that? Is that what you meant?
On 09/24/2014 10:02 AM, Augie Fackler wrote: > On Wed, Sep 24, 2014 at 12:40 PM, Pierre-Yves David > <pierre-yves.david@ens-lyon.org> wrote: >>>> All contents embrances documentation, constants and functions. The diff >>>> is confusing >>> >>> >>> Should "embrances" have been "enhances"? I'm not quite sure what word you >>> meant. >> >> >> Nope. >> >> I mean "all content" == documentation + constants + functions. >> >> Not sure what the word should be here. > > > All contents includes documentation, constants, and functions, so we > gather all of those things into a single location. > > How's that? Is that what you meant? yop
Patch
diff --git a/mercurial/obsolete.py b/mercurial/obsolete.py --- a/mercurial/obsolete.py +++ b/mercurial/obsolete.py @@ -61,29 +61,12 @@ Markers are stored in an append-only fil The file starts with a version header: - 1 unsigned byte: version number, starting at zero. - -The header is followed by the markers. Each marker is made of: - -- 1 unsigned byte: number of new changesets "N", can be zero. - -- 1 unsigned 32-bits integer: metadata size "M" in bytes. - -- 1 byte: a bit field. It is reserved for flags used in common - obsolete marker operations, to avoid repeated decoding of metadata - entries. - -- 20 bytes: obsoleted changeset identifier. - -- N*20 bytes: new changesets identifiers. - -- M bytes: metadata as a sequence of nul-terminated strings. Each - string contains a key and a value, separated by a colon ':', without - additional encoding. Keys cannot contain '\0' or ':' and values - cannot contain '\0'. +The header is followed by the markers. Marker format depend of the version. See +comment associated with each format for details. """ import struct import util, base85, node import phases @@ -96,17 +79,10 @@ from i18n import _ # the obsolete feature is not mature enough to be enabled by default. # you have to rely on third party extension extension to enable this. _enabled = False -# data used for parsing and writing -_fm0version = 0 -_fm0fixed = '>BIB20s' -_fm0node = '20s' -_fm0fsize = struct.calcsize(_fm0fixed) -_fm0fnodesize = struct.calcsize(_fm0node) - ### obsolescence marker flag ## bumpedfix flag # # When a changeset A' succeed to a changeset A which became public, we call A' @@ -135,28 +111,35 @@ from i18n import _ # This flag mean that the successors express the changes between the public and # bumped version and fix the situation, breaking the transitivity of # "bumped" here. bumpedfix = 1 -def _readmarkers(data): - """Read and enumerate markers from raw data""" - off = 0 - diskversion = _unpack('>B', data[off:off + 1])[0] - off += 1 - if diskversion not in formats: - raise util.Abort(_('parsing obsolete marker: unknown version %r') - % diskversion) - return diskversion, formats[diskversion][0](data, off) - -def encodemarkers(markers, addheader=False, version=_fm0version): - # Kept separate from flushmarkers(), it will be reused for - # markers exchange. - encodeone = formats[version][1] - if addheader: - yield _pack('>B', _fm0version) - for marker in markers: - yield encodeone(marker) +## Parsing and writing of version "0" +# +# The header is followed by the markers. Each marker is made of: +# +# - 1 unsigned byte: number of new changesets "N", can be zero. +# +# - 1 unsigned 32-bits integer: metadata size "M" in bytes. +# +# - 1 byte: a bit field. It is reserved for flags used in common +# obsolete marker operations, to avoid repeated decoding of metadata +# entries. +# +# - 20 bytes: obsoleted changeset identifier. +# +# - N*20 bytes: new changesets identifiers. +# +# - M bytes: metadata as a sequence of nul-terminated strings. Each +# string contains a key and a value, separated by a colon ':', without +# additional encoding. Keys cannot contain '\0' or ':' and values +# cannot contain '\0'. +_fm0version = 0 +_fm0fixed = '>BIB20s' +_fm0node = '20s' +_fm0fsize = struct.calcsize(_fm0fixed) +_fm0fnodesize = struct.calcsize(_fm0node) def _fm0readmarkers(data, off=0): # Loop on markers l = len(data) while off + _fm0fsize <= l: @@ -227,10 +210,30 @@ def _fm0encodeonemarker(marker): # mapping to read/write various marker formats # <version> -> (decoder, encoder) formats = {0: (_fm0readmarkers, _fm0encodeonemarker)} +def _readmarkers(data): + """Read and enumerate markers from raw data""" + off = 0 + diskversion = _unpack('>B', data[off:off + 1])[0] + off += 1 + if diskversion not in formats: + raise util.Abort(_('parsing obsolete marker: unknown version %r') + % diskversion) + return diskversion, formats[diskversion][0](data, off) + +def encodemarkers(markers, addheader=False, version=_fm0version): + # Kept separate from flushmarkers(), it will be reused for + # markers exchange. + encodeone = formats[version][1] + if addheader: + yield _pack('>B', _fm0version) + for marker in markers: + yield encodeone(marker) + + def encodemeta(meta): """Return encoded metadata string to string mapping. Assume no ':' in key and no '\0' in both key and value.""" for key, value in meta.iteritems():