[PATCH censor RFC] changegroup: emit full-replacement deltas if either revision is censored

Michael Edgar adgar at google.com
Wed Feb 18 13:05:53 CST 2015


On Fri, Feb 13, 2015 at 12:43 PM, Augie Fackler <raf at durin42.com> wrote:

> On Wed, Feb 11, 2015 at 03:51:56PM -0500, Mike Edgar wrote:
> > # HG changeset patch
> > # User Mike Edgar <adgar at google.com>
> > # Date 1421896172 18000
> > #      Wed Jan 21 22:09:32 2015 -0500
> > # Node ID a770441a605cfcbdc757a02d3847c1e8880e29e4
> > # Parent  1b3b6d629d6c1f8544db349707e47293965b94c0
> > changegroup: emit full-replacement deltas if either revision is censored
> >
> > To ensure that exchanged deltas in the presence of censored revisions can
> > always be applied to the recipient repository, the deltas must replace
> the
> > entire base text. To make this restriction reasonably enforceable, the
> delta
> > must do so with a single patch operation.
> >
> > For background and broader design of the censorship feature, see:
> > http://mercurial.selenic.com/wiki/CensorPlan
> >
> > diff -r 1b3b6d629d6c -r a770441a605c mercurial/changegroup.py
> > --- a/mercurial/changegroup.py        Wed Jan 21 17:11:37 2015 -0500
> > +++ b/mercurial/changegroup.py        Wed Jan 21 22:09:32 2015 -0500
> > @@ -481,7 +481,17 @@
> >          base = self.deltaparent(revlog, rev, p1, p2, prev)
> >
> >          prefix = ''
> > -        if base == nullrev:
> > +        if revlog.iscensored(base) or revlog.iscensored(rev):
> > +            try:
> > +                delta = revlog.revision(rev)
> > +            except error.CensoredNodeError, e:
> > +                delta = e.metadata
>
> So the metadata of a CensoredNodeError is actually the delta? Could we
> name it delta instead?


It's actually the tombstone data as fulltext. I'll rename the "metadata"
attribute to be "tombstone" in v2.

As for "delta" - that local variable name is currently a bit of a misnomer.
In some cases, the entire changegroup delta ends up in the "delta"
variable, but in others, it's split across "prefix" and "delta" which are
yielded separately. I don't believe the separate yielding at this layer
would make a performance difference ("prefix" is either 0 or 12 bytes long)
and I'm sure it isn't a correctness detail.

I can prepare a separate change which refactors this, eliminating the
"prefix" variable and ensuring that "delta" actually always contains a
valid delta. That might make the new censorship changes more obviously
correct. What do you think?


>
> > +            if base == nullrev:
> > +                prefix = mdiff.trivialdiffheader(len(delta))
> > +            else:
> > +                baselen = revlog.rawsize(base)
> > +                prefix = mdiff.replacediffheader(baselen, len(delta))
> > +        elif base == nullrev:
> >              delta = revlog.revision(node)
> >              prefix = mdiff.trivialdiffheader(len(delta))
> >          else:
> > diff -r 1b3b6d629d6c -r a770441a605c mercurial/error.py
> > --- a/mercurial/error.py      Wed Jan 21 17:11:37 2015 -0500
> > +++ b/mercurial/error.py      Wed Jan 21 22:09:32 2015 -0500
> > @@ -138,9 +138,10 @@
> >  class CensoredNodeError(RevlogError):
> >      """error raised when content verification fails on a censored
> node"""
>
> This docstring might be polite and document that the CensoredNodeError
> includes the delta that we refused to apply because it's for a
> censored node. (Don't just use my wording though, unless that actually
> was clear.)


Great point, thanks - will include this in v2.


>
> >
> > -    def __init__(self, filename, node):
> > +    def __init__(self, filename, node, metadata):
> >          from node import short
> >          RevlogError.__init__(self, '%s:%s' % (filename, short(node)))
> > +        self.metadata = metadata
> >
> >  class CensoredBaseError(RevlogError):
> >      """error raised when a delta is rejected because its base is
> censored
> > diff -r 1b3b6d629d6c -r a770441a605c mercurial/filelog.py
> > --- a/mercurial/filelog.py    Wed Jan 21 17:11:37 2015 -0500
> > +++ b/mercurial/filelog.py    Wed Jan 21 22:09:32 2015 -0500
> > @@ -101,7 +101,7 @@
> >              super(filelog, self).checkhash(text, p1, p2, node, rev=rev)
> >          except error.RevlogError:
> >              if _censoredtext(text):
> > -                raise error.CensoredNodeError(self.indexfile, node)
> > +                raise error.CensoredNodeError(self.indexfile, node,
> text)
> >              raise
> >
> >      def iscensored(self, rev):
> > _______________________________________________
> > Mercurial-devel mailing list
> > Mercurial-devel at selenic.com
> > http://selenic.com/mailman/listinfo/mercurial-devel
>



-- 
Michael Edgar | Software Engineer | adgar at google.com | 518-496-6958
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://selenic.com/pipermail/mercurial-devel/attachments/20150218/817a8e04/attachment.html>


More information about the Mercurial-devel mailing list