Patchwork D1658: memfilectx: make changectx argument mandatory in constructor

login
register
mail settings
Submitter phabricator
Date Dec. 11, 2017, 5:48 p.m.
Message ID <differential-rev-PHID-DREV-bee7g7jf5kiymdrrhwyq-req@phab.mercurial-scm.org>
Download mbox | patch
Permalink /patch/26220/
State Superseded
Headers show

Comments

phabricator - Dec. 11, 2017, 5:48 p.m.
martinvonz created this revision.
Herald added a reviewer: durin42.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  committablefilectx has three subclasses: workingfilectx, memfilectx,
  and overlayfilectx. committablefilectx takes an optional (change) ctx
  instance to its constructor. If it's provided, it's set on the
  instance as self._changectx. If not, that property is supposed to be
  defined by the class. However, only workingfilectx does that. The
  other two will have the property undefined if it's not passed in the
  constructor. That seems bad to me. This patch makes the changectx
  argument to the memfilectx constructor mandatory because that fixes
  the failure I ran into. It seems like we should also fix the
  overlayfilectx case.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1658

AFFECTED FILES
  contrib/synthrepo.py
  hgext/convert/hg.py
  hgext/histedit.py
  hgext/largefiles/lfcommands.py
  hgext/uncommit.py
  mercurial/cmdutil.py
  mercurial/context.py
  mercurial/debugcommands.py
  tests/test-commit.t
  tests/test-context.py

CHANGE DETAILS




To: martinvonz, durin42, #hg-reviewers
Cc: mercurial-devel
phabricator - Dec. 11, 2017, 6:35 p.m.
durin42 requested changes to this revision.
durin42 added a comment.
This revision now requires changes to proceed.


  This seems to break test-rebase-inmemory.t - could you take a look?

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1658

To: martinvonz, durin42, #hg-reviewers
Cc: mercurial-devel
phabricator - Dec. 11, 2017, 8:35 p.m.
martinvonz added a comment.


  In https://phab.mercurial-scm.org/D1658#28406, @durin42 wrote:
  
  > This seems to break test-rebase-inmemory.t - could you take a look?
  
  
  Sorry, I was lazy and didn't run the tests after the last rebase. Phil's new test-rebase-inmemory.t caught a bug (thanks!). I also found another bug in synthrepo that I have now fixed.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1658

To: martinvonz, durin42, #hg-reviewers
Cc: mercurial-devel
phabricator - Dec. 12, 2017, 1:36 p.m.
yuja added a comment.


  Looks good, but can you flag this as (API) change?

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1658

To: martinvonz, durin42, #hg-reviewers
Cc: yuja, mercurial-devel
phabricator - Dec. 12, 2017, 7:39 p.m.
martinvonz added a comment.


  In https://phab.mercurial-scm.org/D1658#28514, @yuja wrote:
  
  > Looks good, but can you flag this as (API) change?
  
  
  Good point. Done.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1658

To: martinvonz, durin42, #hg-reviewers
Cc: yuja, mercurial-devel

Patch

diff --git a/tests/test-context.py b/tests/test-context.py
--- a/tests/test-context.py
+++ b/tests/test-context.py
@@ -32,7 +32,7 @@ 
 # test memctx with non-ASCII commit message
 
 def filectxfn(repo, memctx, path):
-    return context.memfilectx(repo, "foo", "")
+    return context.memfilectx(repo, memctx, "foo", "")
 
 ctx = context.memctx(repo, ['tip', None],
                      encoding.tolocal("Gr\xc3\xbcezi!"),
@@ -49,7 +49,7 @@ 
     data, flags = fctx.data(), fctx.flags()
     if f == 'foo':
         data += 'bar\n'
-    return context.memfilectx(repo, f, data, 'l' in flags, 'x' in flags)
+    return context.memfilectx(repo, memctx, f, data, 'l' in flags, 'x' in flags)
 
 ctxa = repo.changectx(0)
 ctxb = context.memctx(repo, [ctxa.node(), None], "test diff", ["foo"],
diff --git a/tests/test-commit.t b/tests/test-commit.t
--- a/tests/test-commit.t
+++ b/tests/test-commit.t
@@ -648,7 +648,8 @@ 
   > u = uimod.ui.load()
   > r = hg.repository(u, '.')
   > def filectxfn(repo, memctx, path):
-  >     return context.memfilectx(repo, path, '[hooks]\nupdate = echo owned')
+  >     return context.memfilectx(repo, memctx, path,
+  >         '[hooks]\nupdate = echo owned')
   > c = context.memctx(r, [r['tip'].node(), node.nullid],
   >                    'evil', [notrc], filectxfn, 0)
   > r.commitctx(c)
@@ -673,7 +674,8 @@ 
   > u = uimod.ui.load()
   > r = hg.repository(u, '.')
   > def filectxfn(repo, memctx, path):
-  >     return context.memfilectx(repo, path, '[hooks]\nupdate = echo owned')
+  >     return context.memfilectx(repo, memctx, path,
+  >         '[hooks]\nupdate = echo owned')
   > c = context.memctx(r, [r['tip'].node(), node.nullid],
   >                    'evil', [notrc], filectxfn, 0)
   > r.commitctx(c)
@@ -692,7 +694,8 @@ 
   > u = uimod.ui.load()
   > r = hg.repository(u, '.')
   > def filectxfn(repo, memctx, path):
-  >     return context.memfilectx(repo, path, '[hooks]\nupdate = echo owned')
+  >     return context.memfilectx(repo, memctx, path,
+  >         '[hooks]\nupdate = echo owned')
   > c = context.memctx(r, [r['tip'].node(), node.nullid],
   >                    'evil', [notrc], filectxfn, 0)
   > r.commitctx(c)
diff --git a/mercurial/debugcommands.py b/mercurial/debugcommands.py
--- a/mercurial/debugcommands.py
+++ b/mercurial/debugcommands.py
@@ -225,7 +225,8 @@ 
 
                 def fctxfn(repo, cx, path):
                     if path in filecontent:
-                        return context.memfilectx(repo, path, filecontent[path])
+                        return context.memfilectx(repo, cx, path,
+                                                  filecontent[path])
                     return None
 
                 if len(ps) == 0 or ps[0] < 0:
diff --git a/mercurial/context.py b/mercurial/context.py
--- a/mercurial/context.py
+++ b/mercurial/context.py
@@ -2239,9 +2239,9 @@ 
         copied = fctx.renamed()
         if copied:
             copied = copied[0]
-        return memfilectx(repo, path, fctx.data(),
+        return memfilectx(repo, memctx, path, fctx.data(),
                           islink=fctx.islink(), isexec=fctx.isexec(),
-                          copied=copied, memctx=memctx)
+                          copied=copied)
 
     return getfilectx
 
@@ -2255,9 +2255,8 @@ 
         if data is None:
             return None
         islink, isexec = mode
-        return memfilectx(repo, path, data, islink=islink,
-                          isexec=isexec, copied=copied,
-                          memctx=memctx)
+        return memfilectx(repo, memctx, path, data, islink=islink,
+                          isexec=isexec, copied=copied)
 
     return getfilectx
 
@@ -2389,16 +2388,16 @@ 
 
     See memctx and committablefilectx for more details.
     """
-    def __init__(self, repo, path, data, islink=False,
-                 isexec=False, copied=None, memctx=None):
+    def __init__(self, repo, changectx, path, data, islink=False,
+                 isexec=False, copied=None):
         """
         path is the normalized file path relative to repository root.
         data is the file content as a string.
         islink is True if the file is a symbolic link.
         isexec is True if the file is executable.
         copied is the source file path if current file was copied in the
         revision being committed, or None."""
-        super(memfilectx, self).__init__(repo, path, None, memctx)
+        super(memfilectx, self).__init__(repo, path, None, changectx)
         self._data = data
         self._flags = (islink and 'l' or '') + (isexec and 'x' or '')
         self._copied = None
diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
--- a/mercurial/cmdutil.py
+++ b/mercurial/cmdutil.py
@@ -3192,7 +3192,7 @@ 
 
                     fctx = wctx[path]
                     flags = fctx.flags()
-                    mctx = context.memfilectx(repo,
+                    mctx = context.memfilectx(repo, ctx_,
                                               fctx.path(), fctx.data(),
                                               islink='l' in flags,
                                               isexec='x' in flags,
diff --git a/hgext/uncommit.py b/hgext/uncommit.py
--- a/hgext/uncommit.py
+++ b/hgext/uncommit.py
@@ -77,7 +77,7 @@ 
         if path not in contentctx:
             return None
         fctx = contentctx[path]
-        mctx = context.memfilectx(repo, fctx.path(), fctx.data(),
+        mctx = context.memfilectx(repo, memctx, fctx.path(), fctx.data(),
                                   fctx.islink(),
                                   fctx.isexec(),
                                   copied=copied.get(path))
diff --git a/hgext/largefiles/lfcommands.py b/hgext/largefiles/lfcommands.py
--- a/hgext/largefiles/lfcommands.py
+++ b/hgext/largefiles/lfcommands.py
@@ -259,7 +259,8 @@ 
                 # doesn't change after rename or copy
                 renamed = lfutil.standin(renamed[0])
 
-            return context.memfilectx(repo, f, lfiletohash[srcfname] + '\n',
+            return context.memfilectx(repo, memctx, f,
+                                      lfiletohash[srcfname] + '\n',
                                       'l' in fctx.flags(), 'x' in fctx.flags(),
                                       renamed)
         else:
@@ -311,7 +312,7 @@ 
     data = fctx.data()
     if f == '.hgtags':
         data = _converttags (repo.ui, revmap, data)
-    return context.memfilectx(repo, f, data, 'l' in fctx.flags(),
+    return context.memfilectx(repo, ctx, f, data, 'l' in fctx.flags(),
                               'x' in fctx.flags(), renamed)
 
 # Remap tag data using a revision map
diff --git a/hgext/histedit.py b/hgext/histedit.py
--- a/hgext/histedit.py
+++ b/hgext/histedit.py
@@ -602,7 +602,7 @@ 
         if path in headmf:
             fctx = last[path]
             flags = fctx.flags()
-            mctx = context.memfilectx(repo,
+            mctx = context.memfilectx(repo, ctx,
                                       fctx.path(), fctx.data(),
                                       islink='l' in flags,
                                       isexec='x' in flags,
diff --git a/hgext/convert/hg.py b/hgext/convert/hg.py
--- a/hgext/convert/hg.py
+++ b/hgext/convert/hg.py
@@ -253,7 +253,7 @@ 
                 data = self._rewritetags(source, revmap, data)
             if f == '.hgsubstate':
                 data = self._rewritesubstate(source, data)
-            return context.memfilectx(self.repo, f, data, 'l' in mode,
+            return context.memfilectx(self.repo, memctx, f, data, 'l' in mode,
                                       'x' in mode, copies.get(f))
 
         pl = []
@@ -401,7 +401,7 @@ 
 
         data = "".join(newlines)
         def getfilectx(repo, memctx, f):
-            return context.memfilectx(repo, f, data, False, False, None)
+            return context.memfilectx(repo, memctx, f, data, False, False, None)
 
         self.ui.status(_("updating tags\n"))
         date = "%s 0" % int(time.mktime(time.gmtime()))
diff --git a/contrib/synthrepo.py b/contrib/synthrepo.py
--- a/contrib/synthrepo.py
+++ b/contrib/synthrepo.py
@@ -376,7 +376,7 @@ 
                 dir = os.path.dirname(dir)
 
         def filectxfn(repo, memctx, path):
-            return context.memfilectx(repo, path, files[path])
+            return context.memfilectx(repo, memctx, path, files[path])
 
         ui.progress(_synthesizing, None)
         message = 'synthesized wide repo with %d files' % (len(files),)