<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Feb 13, 2015 at 12:43 PM, Augie Fackler <span dir="ltr"><<a href="mailto:raf@durin42.com" target="_blank" class="cremed">raf@durin42.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Wed, Feb 11, 2015 at 03:51:56PM -0500, Mike Edgar wrote:<br>
> # HG changeset patch<br>
> # User Mike Edgar <<a href="mailto:adgar@google.com" class="cremed">adgar@google.com</a>><br>
> # Date 1421896172 18000<br>
> #      Wed Jan 21 22:09:32 2015 -0500<br>
> # Node ID a770441a605cfcbdc757a02d3847c1e8880e29e4<br>
> # Parent  1b3b6d629d6c1f8544db349707e47293965b94c0<br>
> changegroup: emit full-replacement deltas if either revision is censored<br>
><br>
> To ensure that exchanged deltas in the presence of censored revisions can<br>
> always be applied to the recipient repository, the deltas must replace the<br>
> entire base text. To make this restriction reasonably enforceable, the delta<br>
> must do so with a single patch operation.<br>
><br>
> For background and broader design of the censorship feature, see:<br>
> <a href="http://mercurial.selenic.com/wiki/CensorPlan" target="_blank" class="cremed">http://mercurial.selenic.com/wiki/CensorPlan</a><br>
><br>
> diff -r 1b3b6d629d6c -r a770441a605c mercurial/changegroup.py<br>
> --- a/mercurial/changegroup.py        Wed Jan 21 17:11:37 2015 -0500<br>
> +++ b/mercurial/changegroup.py        Wed Jan 21 22:09:32 2015 -0500<br>
> @@ -481,7 +481,17 @@<br>
>          base = self.deltaparent(revlog, rev, p1, p2, prev)<br>
><br>
>          prefix = ''<br>
> -        if base == nullrev:<br>
> +        if revlog.iscensored(base) or revlog.iscensored(rev):<br>
> +            try:<br>
> +                delta = revlog.revision(rev)<br>
> +            except error.CensoredNodeError, e:<br>
> +                delta = e.metadata<br>
<br>
</div></div>So the metadata of a CensoredNodeError is actually the delta? Could we<br>
name it delta instead?</blockquote><div><br></div><div>It's actually the tombstone data as fulltext. I'll rename the "metadata" attribute to be "tombstone" in v2.</div><div><br></div><div>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.</div><div><br></div><div>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?</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class=""><br>
> +            if base == nullrev:<br>
> +                prefix = mdiff.trivialdiffheader(len(delta))<br>
> +            else:<br>
> +                baselen = revlog.rawsize(base)<br>
> +                prefix = mdiff.replacediffheader(baselen, len(delta))<br>
> +        elif base == nullrev:<br>
>              delta = revlog.revision(node)<br>
>              prefix = mdiff.trivialdiffheader(len(delta))<br>
>          else:<br>
> diff -r 1b3b6d629d6c -r a770441a605c mercurial/error.py<br>
> --- a/mercurial/error.py      Wed Jan 21 17:11:37 2015 -0500<br>
> +++ b/mercurial/error.py      Wed Jan 21 22:09:32 2015 -0500<br>
> @@ -138,9 +138,10 @@<br>
>  class CensoredNodeError(RevlogError):<br>
>      """error raised when content verification fails on a censored node"""<br>
<br>
</span>This docstring might be polite and document that the CensoredNodeError<br>
includes the delta that we refused to apply because it's for a<br>
censored node. (Don't just use my wording though, unless that actually<br>
was clear.)</blockquote><div><br></div><div>Great point, thanks - will include this in v2.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class=""><br>
><br>
> -    def __init__(self, filename, node):<br>
> +    def __init__(self, filename, node, metadata):<br>
>          from node import short<br>
>          RevlogError.__init__(self, '%s:%s' % (filename, short(node)))<br>
> +        self.metadata = metadata<br>
><br>
>  class CensoredBaseError(RevlogError):<br>
>      """error raised when a delta is rejected because its base is censored<br>
> diff -r 1b3b6d629d6c -r a770441a605c mercurial/filelog.py<br>
> --- a/mercurial/filelog.py    Wed Jan 21 17:11:37 2015 -0500<br>
> +++ b/mercurial/filelog.py    Wed Jan 21 22:09:32 2015 -0500<br>
> @@ -101,7 +101,7 @@<br>
>              super(filelog, self).checkhash(text, p1, p2, node, rev=rev)<br>
>          except error.RevlogError:<br>
>              if _censoredtext(text):<br>
> -                raise error.CensoredNodeError(self.indexfile, node)<br>
> +                raise error.CensoredNodeError(self.indexfile, node, text)<br>
>              raise<br>
><br>
>      def iscensored(self, rev):<br>
</span>> _______________________________________________<br>
> Mercurial-devel mailing list<br>
> <a href="mailto:Mercurial-devel@selenic.com" class="cremed">Mercurial-devel@selenic.com</a><br>
> <a href="http://selenic.com/mailman/listinfo/mercurial-devel" target="_blank" class="cremed">http://selenic.com/mailman/listinfo/mercurial-devel</a><br>
</blockquote></div><br><br clear="all"><div><br></div>-- <br><div class="gmail_signature"><div style="line-height:1.5em;padding-top:10px;margin-top:10px;color:rgb(85,85,85);font-family:sans-serif"><span style="border-top-width:2px;border-right-width:0px;border-bottom-width:0px;border-left-width:0px;border-top-style:solid;border-right-style:solid;border-bottom-style:solid;border-left-style:solid;border-top-color:rgb(213,15,37);border-right-color:rgb(213,15,37);border-bottom-color:rgb(213,15,37);border-left-color:rgb(213,15,37);padding-top:2px;margin-top:2px">Michael Edgar |</span><span style="border-top-width:2px;border-right-width:0px;border-bottom-width:0px;border-left-width:0px;border-top-style:solid;border-right-style:solid;border-bottom-style:solid;border-left-style:solid;border-top-color:rgb(51,105,232);border-right-color:rgb(51,105,232);border-bottom-color:rgb(51,105,232);border-left-color:rgb(51,105,232);padding-top:2px;margin-top:2px"> Software Engineer |</span><span style="border-top-width:2px;border-right-width:0px;border-bottom-width:0px;border-left-width:0px;border-top-style:solid;border-right-style:solid;border-bottom-style:solid;border-left-style:solid;border-top-color:rgb(0,153,57);border-right-color:rgb(0,153,57);border-bottom-color:rgb(0,153,57);border-left-color:rgb(0,153,57);padding-top:2px;margin-top:2px"> <a href="mailto:adgar@google.com" target="_blank" class="cremed">adgar@google.com</a> |</span><span style="border-top-width:2px;border-right-width:0px;border-bottom-width:0px;border-left-width:0px;border-top-style:solid;border-right-style:solid;border-bottom-style:solid;border-left-style:solid;border-top-color:rgb(238,178,17);border-right-color:rgb(238,178,17);border-bottom-color:rgb(238,178,17);border-left-color:rgb(238,178,17);padding-top:2px;margin-top:2px"> 518-496-6958</span></div><font face="Times" size="3"><br></font></div>
</div></div>