[PATCH 1 of 7] revlog: use raw content when building delta

Ryan McElroy rm at fb.com
Wed Mar 29 08:39:10 EDT 2017


On 3/28/17 8:49 AM, Jun Wu wrote:
> # HG changeset patch
> # User Jun Wu <quark at fb.com>
> # Date 1490682886 25200
> #      Mon Mar 27 23:34:46 2017 -0700
> # Node ID 1e84f9bd4385a8f95ac1ec15dee14c723071ab34
> # Parent  1ed57a7dd904f8b79f79ecb4ea6fe1871e7af740
> revlog: use raw content when building delta
>
> Using external content provided by flagprocessor when building revlog delta
> is wrong, because deltas are applied to raw contents in revlog.

I believe you since you're mucking around in this area, but it's not 
"obviously true" to me (as an outsider looking in) that you would never 
want any flags processed before applying a delta. Can you please add a 
comment in the code explaining why this is the case?

NOTE: after reviewing the rest of the patch series and seeing more of 
this code, I see why this is "obvious" to someone familiar with the code 
-- but I still think that a comment would be useful for people reading 
this code in the future. I'd suggest some thing like this (feel free to 
reuse this or write something different with your own flair):

# Patches work on raw deltas, so ensure that we get the raw revlog data.
# Throughout the rest of the code in this class, we always process flags 
after
# building up the delta chain to produce the raw fulltext. Failing to do 
this
# in the right order leads to terrible corruption bugs and great sadness.

>
> This patch fixes the above issue by adding "raw=True". There are other
> issues about "raw". A test will be added later after all issues are fixed,
> to reduce churn on the test file.
>
> diff --git a/mercurial/revlog.py b/mercurial/revlog.py
> --- a/mercurial/revlog.py
> +++ b/mercurial/revlog.py
> @@ -1629,5 +1629,5 @@ class revlog(object):
>                       else:
>                           fh = dfh
> -                    ptext = self.revision(self.node(rev), _df=fh)
> +                    ptext = self.revision(self.node(rev), _df=fh, raw=True)
>                       delta = mdiff.textdiff(ptext, t)
>               header, data = self.compress(delta)
>



More information about the Mercurial-devel mailing list