Patchwork changelog: make sure datafile is 00changelog.d

login
register
mail settings
Submitter Jun Wu
Date May 17, 2017, 10:30 p.m.
Message ID <ea13a8402d0c78e843e8.1495060218@x1c>
Download mbox | patch
Permalink /patch/20667/
State Accepted
Headers show

Comments

Jun Wu - May 17, 2017, 10:30 p.m.
# HG changeset patch
# User Jun Wu <quark@fb.com>
# Date 1495059840 25200
#      Wed May 17 15:24:00 2017 -0700
# Node ID ea13a8402d0c78e843e811617c41e3d4c23222e6
# Parent  2d19664e257da7ad5cb97150d81838c25872fac7
# Available At https://bitbucket.org/quark-zju/hg-draft
#              hg pull https://bitbucket.org/quark-zju/hg-draft -r ea13a8402d0c
changelog: make sure datafile is 00changelog.d

0ad0d26ff7 makes it possible for changelog datafile to be "00changelog.i.d",
which is wrong. This patch adds an explicit datafile parameter to fix it.
Gregory Szorc - May 17, 2017, 11:45 p.m.
On Wed, May 17, 2017 at 3:30 PM, Jun Wu <quark@fb.com> wrote:

> # HG changeset patch
> # User Jun Wu <quark@fb.com>
> # Date 1495059840 25200
> #      Wed May 17 15:24:00 2017 -0700
> # Node ID ea13a8402d0c78e843e811617c41e3d4c23222e6
> # Parent  2d19664e257da7ad5cb97150d81838c25872fac7
> # Available At https://bitbucket.org/quark-zju/hg-draft
> #              hg pull https://bitbucket.org/quark-zju/hg-draft -r
> ea13a8402d0c
> changelog: make sure datafile is 00changelog.d
>
> 0ad0d26ff7 makes it possible for changelog datafile to be
> "00changelog.i.d",
> which is wrong. This patch adds an explicit datafile parameter to fix it.
>

The bad path to datafile existed before. But it was less exposed since the
changelog/revlog instance having that value was very shortly lived.

Anyway, this shouldn't matter since the changelog/revlog instance
associated with the 00changelog.i.a file is never written to via the revlog
APIs. But, I agree we shouldn't take the chance. I also agree we should
pass the datafile as an argument rather than build ".i.a" parsing into
revlog (since it is changelog specific).


>
> diff --git a/mercurial/changelog.py b/mercurial/changelog.py
> --- a/mercurial/changelog.py
> +++ b/mercurial/changelog.py
> @@ -274,5 +274,7 @@ class changelog(revlog.revlog):
>              indexfile = '00changelog.i'
>
> -        revlog.revlog.__init__(self, opener, indexfile, checkambig=True)
> +        datafile = '00changelog.d'
> +        revlog.revlog.__init__(self, opener, indexfile, datafile=datafile,
> +                               checkambig=True)
>
>          if self._initempty:
> diff --git a/mercurial/revlog.py b/mercurial/revlog.py
> --- a/mercurial/revlog.py
> +++ b/mercurial/revlog.py
> @@ -253,5 +253,5 @@ class revlog(object):
>      writing, to avoid file stat ambiguity.
>      """
> -    def __init__(self, opener, indexfile, checkambig=False):
> +    def __init__(self, opener, indexfile, datafile=None,
> checkambig=False):
>

Being paranoid, I think we should preserve the argument order in case any
caller (in extensions) isn't used named arguments. That, or this should be
marked (API).


>          """
>          create a revlog object
> @@ -261,5 +261,5 @@ class revlog(object):
>          """
>          self.indexfile = indexfile
> -        self.datafile = indexfile[:-2] + ".d"
> +        self.datafile = datafile or (indexfile[:-2] + ".d")
>          self.opener = opener
>          #  When True, indexfile is opened with checkambig=True at
> writing, to
>

Patch

diff --git a/mercurial/changelog.py b/mercurial/changelog.py
--- a/mercurial/changelog.py
+++ b/mercurial/changelog.py
@@ -274,5 +274,7 @@  class changelog(revlog.revlog):
             indexfile = '00changelog.i'
 
-        revlog.revlog.__init__(self, opener, indexfile, checkambig=True)
+        datafile = '00changelog.d'
+        revlog.revlog.__init__(self, opener, indexfile, datafile=datafile,
+                               checkambig=True)
 
         if self._initempty:
diff --git a/mercurial/revlog.py b/mercurial/revlog.py
--- a/mercurial/revlog.py
+++ b/mercurial/revlog.py
@@ -253,5 +253,5 @@  class revlog(object):
     writing, to avoid file stat ambiguity.
     """
-    def __init__(self, opener, indexfile, checkambig=False):
+    def __init__(self, opener, indexfile, datafile=None, checkambig=False):
         """
         create a revlog object
@@ -261,5 +261,5 @@  class revlog(object):
         """
         self.indexfile = indexfile
-        self.datafile = indexfile[:-2] + ".d"
+        self.datafile = datafile or (indexfile[:-2] + ".d")
         self.opener = opener
         #  When True, indexfile is opened with checkambig=True at writing, to