[PATCH censor RFC] revlog: addgroup checks if incoming deltas add censored revs, sets flag bit

[Pierre-Yves David]

On 03/09/2015 08:23 PM, Michael Edgar wrote:
> On Mon, Mar 9, 2015 at 10:37 PM, Pierre-Yves David
> <pierre-yves.david at ens-lyon.org <mailto:pierre-yves.david at ens-lyon.org>>
> wrote:
>
>
>
>     On 03/09/2015 12:56 PM, Mike Edgar wrote:
>
>         # HG changeset patch
>         # User Mike Edgar <adgar at google.com <mailto:adgar at google.com>>
>         # Date 1421266568 18000
>         #      Wed Jan 14 15:16:08 2015 -0500
>         # Node ID 816f69e12902d419421bb148705f7b__c65e0f7774
>         # Parent  02d7b5cd373bbb4e8263dad9bfbf9c__4c3b0e4e3a
>         revlog: addgroup checks if incoming deltas add censored revs,
>         sets flag bit
>
>         A censored revision stored in a revlog should have the censored
>         revlog index
>         flag bit set. This implies we must know if a revision is
>         censored before we
>         add it to the revlog. When adding revisions from exchanged
>         deltas, we would
>         prefer to determine this flag without decoding every single full
>         text.
>
>         This change introduces a heuristic based on assumptions around
>         the Mercurial
>         delta format and filelog metadata. Since deltas which produce a
>         censored
>         revision must be full-replacement deltas, we can read the
>         delta's first bytes
>         to check the filelog metadata. Since "censored" is the
>         alphabetically first
>         filelog metadata key, censored filelog revisions have a
>         well-known prefix we
>         can look for.
>
>         For more on the design and background of the censorship feature,
>         see:
>         http://mercurial.selenic.com/__wiki/CensorPlan
>         <http://mercurial.selenic.com/wiki/CensorPlan>
>
>
>
>     This approach looks like like a recipe for disaster. (expecially the
>     one relying on "censored" being the first entry by alphabetical order.
>
>
> Agreed it's suboptimal. Perhaps I could parse all the metadata,
> line-by-line, instead? That might make this a precise approach and not
> just a heuristic.

The final approach needs to be resistent to metadata extension. parsing 
all of them seems to fit this needs.

>     What do you miss in the protocol to makes it happen ? as far as I
>     understand the censored feature requires bundle2 to be used. Bundle2
>     is not the default yet so you have a windows for protocol improvement.
>
>
> Bundle2 is not required, and in fact supporting older clients which
> don't have bundle2 support is an explicit design goal.

Really do you have a pointer to something explaining how the feature 
works for older client (I'm curious now). And I'm a bit surprised that 
-you- care about older client.

> What's missing is
> changegroups don't transfer the "flags" of a revision (since those flags
> were unused until the censorship design).
>
> A changegroup v3 could add a 16-bit flags field which obviates this
> heuristic for new clients and servers,

If you can give a look at such changegroup v3, now is the right time.

> but supporting mercurial back to 1.x is in scope for censorship.

<puzzled look>

We should probably have and IRC/VC chat.

-- 
Pierre-Yves David