Patchwork revlog: use raw revision for rawsize

login
register
mail settings
Submitter Jun Wu
Date March 29, 2017, 3:31 a.m.
Message ID <5c6ebe1cd99e9557b61e.1490758287@localhost.localdomain>
Download mbox | patch
Permalink /patch/19809/
State Accepted
Headers show

Comments

Jun Wu - March 29, 2017, 3:31 a.m.
# HG changeset patch
# User Jun Wu <quark@fb.com>
# Date 1490758201 25200
#      Tue Mar 28 20:30:01 2017 -0700
# Node ID 5c6ebe1cd99e9557b61ef51ed88e335ac87df8da
# Parent  31073077267b5e330925d48098dd6e0fd28bdd8e
# Available At https://bitbucket.org/quark-zju/hg-draft
#              hg pull https://bitbucket.org/quark-zju/hg-draft -r 5c6ebe1cd99e
revlog: use raw revision for rawsize
Ryan McElroy - March 29, 2017, 10:30 a.m.
On 3/29/17 4:31 AM, Jun Wu wrote:
> # HG changeset patch
> # User Jun Wu <quark@fb.com>
> # Date 1490758201 25200
> #      Tue Mar 28 20:30:01 2017 -0700
> # Node ID 5c6ebe1cd99e9557b61ef51ed88e335ac87df8da
> # Parent  31073077267b5e330925d48098dd6e0fd28bdd8e
> revlog: use raw revision for rawsize
>
> diff --git a/mercurial/revlog.py b/mercurial/revlog.py
> --- a/mercurial/revlog.py
> +++ b/mercurial/revlog.py
> @@ -439,5 +439,5 @@ class revlog(object):
>               return l
>   
> -        t = self.revision(self.node(rev))
> +        t = self.revision(self.node(rev), raw=True)
>           return len(t)
>       size = rawsize
>

Having looked at the code, this is inside the "rawsize" function so this 
is an obviously correct fix. I have marked pre-reviewed in patchwork.

Is there anywhere this gets exposed today? Maybe a test would be nice in 
the future.
Jun Wu - March 29, 2017, 6:12 p.m.
Excerpts from Ryan McElroy's message of 2017-03-29 11:30:50 +0100:
> On 3/29/17 4:31 AM, Jun Wu wrote:
> > # HG changeset patch
> > # User Jun Wu <quark@fb.com>
> > # Date 1490758201 25200
> > #      Tue Mar 28 20:30:01 2017 -0700
> > # Node ID 5c6ebe1cd99e9557b61ef51ed88e335ac87df8da
> > # Parent  31073077267b5e330925d48098dd6e0fd28bdd8e
> > revlog: use raw revision for rawsize
> >
> > diff --git a/mercurial/revlog.py b/mercurial/revlog.py
> > --- a/mercurial/revlog.py
> > +++ b/mercurial/revlog.py
> > @@ -439,5 +439,5 @@ class revlog(object):
> >               return l
> >   
> > -        t = self.revision(self.node(rev))
> > +        t = self.revision(self.node(rev), raw=True)
> >           return len(t)
> >       size = rawsize
> >
> 
> Having looked at the code, this is inside the "rawsize" function so this 
> is an obviously correct fix. I have marked pre-reviewed in patchwork.
> 
> Is there anywhere this gets exposed today? Maybe a test would be nice in 
> the future.

Since the index will handle most cases, it seems not easy to get exposed
unless index returns 0.
Yuya Nishihara - March 31, 2017, 1:49 p.m.
On Wed, 29 Mar 2017 11:30:50 +0100, Ryan McElroy wrote:
> On 3/29/17 4:31 AM, Jun Wu wrote:
> > # HG changeset patch
> > # User Jun Wu <quark@fb.com>
> > # Date 1490758201 25200
> > #      Tue Mar 28 20:30:01 2017 -0700
> > # Node ID 5c6ebe1cd99e9557b61ef51ed88e335ac87df8da
> > # Parent  31073077267b5e330925d48098dd6e0fd28bdd8e
> > revlog: use raw revision for rawsize
> >
> > diff --git a/mercurial/revlog.py b/mercurial/revlog.py
> > --- a/mercurial/revlog.py
> > +++ b/mercurial/revlog.py
> > @@ -439,5 +439,5 @@ class revlog(object):
> >               return l
> >   
> > -        t = self.revision(self.node(rev))
> > +        t = self.revision(self.node(rev), raw=True)
> >           return len(t)
> >       size = rawsize
> >
> 
> Having looked at the code, this is inside the "rawsize" function so this 
> is an obviously correct fix. I have marked pre-reviewed in patchwork.

Yeah sounds good, but I'm not pretty sure if the "raw" of rawsize() is related
to flagprocessor's. It was added at 56a7721ee3ec to bypass filelog.size().

This should be queued by someone who has more knowledge.
Jun Wu - March 31, 2017, 5:16 p.m.
Excerpts from Yuya Nishihara's message of 2017-03-31 22:49:54 +0900:
> On Wed, 29 Mar 2017 11:30:50 +0100, Ryan McElroy wrote:
> > On 3/29/17 4:31 AM, Jun Wu wrote:
> > > # HG changeset patch
> > > # User Jun Wu <quark@fb.com>
> > > # Date 1490758201 25200
> > > #      Tue Mar 28 20:30:01 2017 -0700
> > > # Node ID 5c6ebe1cd99e9557b61ef51ed88e335ac87df8da
> > > # Parent  31073077267b5e330925d48098dd6e0fd28bdd8e
> > > revlog: use raw revision for rawsize
> > >
> > > diff --git a/mercurial/revlog.py b/mercurial/revlog.py
> > > --- a/mercurial/revlog.py
> > > +++ b/mercurial/revlog.py
> > > @@ -439,5 +439,5 @@ class revlog(object):
> > >               return l
> > >   
> > > -        t = self.revision(self.node(rev))
> > > +        t = self.revision(self.node(rev), raw=True)
> > >           return len(t)
> > >       size = rawsize
> > >
> > 
> > Having looked at the code, this is inside the "rawsize" function so this 
> > is an obviously correct fix. I have marked pre-reviewed in patchwork.
> 
> Yeah sounds good, but I'm not pretty sure if the "raw" of rawsize() is related
> to flagprocessor's. It was added at 56a7721ee3ec to bypass filelog.size().

Thanks for the context.

It seems it's filelog.size who needs to be fixed. It has a "XXX" comment.

With raw involved, things become tricker. I made a table at
https://patchwork.mercurial-scm.org/patch/19851/ exploring raw, non-raw,
renamed , non-raw with rename but no metadata. Maybe that should go to
help/internals.

> 
> This should be queued by someone who has more knowledge.

Patch

diff --git a/mercurial/revlog.py b/mercurial/revlog.py
--- a/mercurial/revlog.py
+++ b/mercurial/revlog.py
@@ -439,5 +439,5 @@  class revlog(object):
             return l
 
-        t = self.revision(self.node(rev))
+        t = self.revision(self.node(rev), raw=True)
         return len(t)
     size = rawsize