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

login
register
mail settings
Submitter Jun Wu
Date March 28, 2017, 7:49 a.m.
Message ID <1e84f9bd4385a8f95ac1.1490687342@localhost.localdomain>
Download mbox | patch
Permalink /patch/19772/
State Changes Requested
Headers show

Comments

Jun Wu - March 28, 2017, 7:49 a.m.
# HG changeset patch
# User Jun Wu <quark@fb.com>
# Date 1490682886 25200
#      Mon Mar 27 23:34:46 2017 -0700
# Node ID 1e84f9bd4385a8f95ac1ec15dee14c723071ab34
# Parent  1ed57a7dd904f8b79f79ecb4ea6fe1871e7af740
# Available At https://bitbucket.org/quark-zju/hg-draft
#              hg pull https://bitbucket.org/quark-zju/hg-draft -r 1e84f9bd4385
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.

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.
Ryan McElroy - March 29, 2017, 12:39 p.m.
On 3/28/17 8:49 AM, Jun Wu wrote:
> # HG changeset patch
> # User Jun Wu <quark@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)
>

Patch

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)