Patchwork [4,of,8] bundlerepo: make baserevision return raw text

login
register
mail settings
Submitter Jun Wu
Date April 7, 2017, 2:08 a.m.
Message ID <f05275cd80eb1afde5c3.1491530892@x1c>
Download mbox | patch
Permalink /patch/19983/
State Accepted
Headers show

Comments

Jun Wu - April 7, 2017, 2:08 a.m.
# HG changeset patch
# User Jun Wu <quark@fb.com>
# Date 1491525809 25200
#      Thu Apr 06 17:43:29 2017 -0700
# Node ID f05275cd80eb1afde5c36470fadf224a44733c45
# Parent  2a254e0cac392c1e0af8bbf0645ecb02b2352f8c
# Available At https://bitbucket.org/quark-zju/hg-draft
#              hg pull https://bitbucket.org/quark-zju/hg-draft -r f05275cd80eb
bundlerepo: make baserevision return raw text

"baserevision" returns the text that will be used to apply deltas. Since
deltas are against raw texts, "baserevision" should return raw text.

Now test-flagprocessor.t points us to a new error.
Yuya Nishihara - April 7, 2017, 1:56 p.m.
On Thu, 6 Apr 2017 19:08:12 -0700, Jun Wu wrote:
> # HG changeset patch
> # User Jun Wu <quark@fb.com>
> # Date 1491525809 25200
> #      Thu Apr 06 17:43:29 2017 -0700
> # Node ID f05275cd80eb1afde5c36470fadf224a44733c45
> # Parent  2a254e0cac392c1e0af8bbf0645ecb02b2352f8c
> # Available At https://bitbucket.org/quark-zju/hg-draft
> #              hg pull https://bitbucket.org/quark-zju/hg-draft -r f05275cd80eb
> bundlerepo: make baserevision return raw text

It might be better to pass 'raw' argument to baserevision(), but I'm not
sure.

> @@ -211,5 +211,10 @@ class bundlemanifest(bundlerevlog, manif
>              result = '%s' % self.fulltextcache[node]
>          else:
> -            result = manifest.manifestrevlog.revision(self, nodeorrev)
> +            try:
> +                result = manifest.manifestrevlog.revision(self, nodeorrev,
> +                                                          raw=True)
> +            except TypeError:
> +                # some manifestrevlog implementation does not accept "raw"
> +                result = manifest.manifestrevlog.revision(self, nodeorrev)
>          return result

I think the general rule is to change the API and fix non-core extensions.
Jun Wu - April 7, 2017, 2:04 p.m.
Excerpts from Yuya Nishihara's message of 2017-04-07 22:56:54 +0900:
> On Thu, 6 Apr 2017 19:08:12 -0700, Jun Wu wrote:
> > # HG changeset patch
> > # User Jun Wu <quark@fb.com>
> > # Date 1491525809 25200
> > #      Thu Apr 06 17:43:29 2017 -0700
> > # Node ID f05275cd80eb1afde5c36470fadf224a44733c45
> > # Parent  2a254e0cac392c1e0af8bbf0645ecb02b2352f8c
> > # Available At https://bitbucket.org/quark-zju/hg-draft 
> > #              hg pull https://bitbucket.org/quark-zju/hg-draft  -r f05275cd80eb
> > bundlerepo: make baserevision return raw text
> 
> It might be better to pass 'raw' argument to baserevision(), but I'm not
> sure.

Since no deltas are built against external content in all cases, and base
revision is related to delta application. I think it's safer to not allow
raw=False.

> 
> > @@ -211,5 +211,10 @@ class bundlemanifest(bundlerevlog, manif
> >              result = '%s' % self.fulltextcache[node]
> >          else:
> > -            result = manifest.manifestrevlog.revision(self, nodeorrev)
> > +            try:
> > +                result = manifest.manifestrevlog.revision(self, nodeorrev,
> > +                                                          raw=True)
> > +            except TypeError:
> > +                # some manifestrevlog implementation does not accept "raw"
> > +                result = manifest.manifestrevlog.revision(self, nodeorrev)
> >          return result
> 
> I think the general rule is to change the API and fix non-core extensions.

I agree. I was a bit worried that it may make the series too long. But it
turns out the try block is actually not necessary here.

It should be just:

    result = manifest.manifestrevlog.revision(self, nodeorrev, raw=True)
Yuya Nishihara - April 7, 2017, 2:23 p.m.
On Fri, 7 Apr 2017 07:04:40 -0700, Jun Wu wrote:
> Excerpts from Yuya Nishihara's message of 2017-04-07 22:56:54 +0900:
> > On Thu, 6 Apr 2017 19:08:12 -0700, Jun Wu wrote:
> > > # HG changeset patch
> > > # User Jun Wu <quark@fb.com>
> > > # Date 1491525809 25200
> > > #      Thu Apr 06 17:43:29 2017 -0700
> > > # Node ID f05275cd80eb1afde5c36470fadf224a44733c45
> > > # Parent  2a254e0cac392c1e0af8bbf0645ecb02b2352f8c
> > > # Available At https://bitbucket.org/quark-zju/hg-draft 
> > > #              hg pull https://bitbucket.org/quark-zju/hg-draft  -r f05275cd80eb
> > > bundlerepo: make baserevision return raw text
> > 
> > It might be better to pass 'raw' argument to baserevision(), but I'm not
> > sure.
> 
> Since no deltas are built against external content in all cases, and base
> revision is related to delta application. I think it's safer to not allow
> raw=False.
> 
> > 
> > > @@ -211,5 +211,10 @@ class bundlemanifest(bundlerevlog, manif
> > >              result = '%s' % self.fulltextcache[node]
> > >          else:
> > > -            result = manifest.manifestrevlog.revision(self, nodeorrev)
> > > +            try:
> > > +                result = manifest.manifestrevlog.revision(self, nodeorrev,
> > > +                                                          raw=True)
> > > +            except TypeError:
> > > +                # some manifestrevlog implementation does not accept "raw"
> > > +                result = manifest.manifestrevlog.revision(self, nodeorrev)
> > >          return result
> > 
> > I think the general rule is to change the API and fix non-core extensions.
> 
> I agree. I was a bit worried that it may make the series too long. But it
> turns out the try block is actually not necessary here.
> 
> It should be just:
> 
>     result = manifest.manifestrevlog.revision(self, nodeorrev, raw=True)

Dropped try-except and queued, thanks!

Patch

diff --git a/mercurial/bundlerepo.py b/mercurial/bundlerepo.py
--- a/mercurial/bundlerepo.py
+++ b/mercurial/bundlerepo.py
@@ -159,5 +159,5 @@  class bundlerevlog(revlog.revlog):
         # content retrieved from revlog. To use bundlerevlog with such class one
         # needs to override 'baserevision' and make more specific call here.
-        return revlog.revlog.revision(self, nodeorrev)
+        return revlog.revlog.revision(self, nodeorrev, raw=True)
 
     def addrevision(self, text, transaction, link, p1=None, p2=None, d=None):
@@ -187,5 +187,5 @@  class bundlechangelog(bundlerevlog, chan
         try:
             self.filteredrevs = ()
-            return changelog.changelog.revision(self, nodeorrev)
+            return changelog.changelog.revision(self, nodeorrev, raw=True)
         finally:
             self.filteredrevs = oldfilter
@@ -211,5 +211,10 @@  class bundlemanifest(bundlerevlog, manif
             result = '%s' % self.fulltextcache[node]
         else:
-            result = manifest.manifestrevlog.revision(self, nodeorrev)
+            try:
+                result = manifest.manifestrevlog.revision(self, nodeorrev,
+                                                          raw=True)
+            except TypeError:
+                # some manifestrevlog implementation does not accept "raw"
+                result = manifest.manifestrevlog.revision(self, nodeorrev)
         return result
 
@@ -229,5 +234,5 @@  class bundlefilelog(bundlerevlog, filelo
 
     def baserevision(self, nodeorrev):
-        return filelog.filelog.revision(self, nodeorrev)
+        return filelog.filelog.revision(self, nodeorrev, raw=True)
 
 class bundlepeer(localrepo.localpeer):
diff --git a/tests/test-flagprocessor.t b/tests/test-flagprocessor.t
--- a/tests/test-flagprocessor.t
+++ b/tests/test-flagprocessor.t
@@ -189,10 +189,9 @@ 
   $ hg --config extensions.strip= strip -r 2 --no-backup --force -q
   $ hg -R bundle.hg log --stat -T '{rev} {desc}\n' base64 2>&1 | egrep -v '^(\*\*|  )'
-  Traceback (most recent call last):
-  mercurial.mpatch.mpatchError: invalid patch
+  abort: integrity check failed on data/base64.i:2!
 
   $ hg bundle -R bundle.hg --base 1 bundle-again.hg -q 2>&1 | egrep -v '^(\*\*|  )'
   Traceback (most recent call last):
-  TypeError: Incorrect padding
+  mercurial.mpatch.mpatchError: invalid patch
   $ hg -R bundle-again.hg log --stat -T '{rev} {desc}\n' base64 2>&1 | egrep -v '^(\*\*|  )'
   abort: repository bundle-again.hg not found!