Patchwork [2,of,6,py3,progress,v2] amend: move cmdutil.amend to a new module to break an import cycle

login
register
mail settings
Submitter Augie Fackler
Date Feb. 4, 2014, 10:24 p.m.
Message ID <46037c7c4644ae92e9b9.1391552676@augie-macbookair>
Download mbox | patch
Permalink /patch/3472/
State Deferred
Headers show

Comments

Augie Fackler - Feb. 4, 2014, 10:24 p.m.
# HG changeset patch
# User Augie Fackler <raf@durin42.com>
# Date 1391471430 18000
#      Mon Feb 03 18:50:30 2014 -0500
# Node ID 46037c7c4644ae92e9b9225a0cd91ebfa92dec85
# Parent  6732f7bdd931471a66693387b22220cbee1b91b5
amend: move cmdutil.amend to a new module to break an import cycle
Matt Mackall - March 21, 2014, 10:36 p.m.
On Tue, 2014-02-04 at 17:24 -0500, Augie Fackler wrote:
> # HG changeset patch
> # User Augie Fackler <raf@durin42.com>
> # Date 1391471430 18000
> #      Mon Feb 03 18:50:30 2014 -0500
> # Node ID 46037c7c4644ae92e9b9225a0cd91ebfa92dec85
> # Parent  6732f7bdd931471a66693387b22220cbee1b91b5
> amend: move cmdutil.amend to a new module to break an import cycle

It's not clear what cycle this is breaking, but it appears to be
fundamentally caused by subrepo. And the recursion in subrepo is..
awful.

It seems like it would be better to dynamically import cmdutil in
subrepo using function-level imports. Then the pain is isolated to the
module which caused it and not inflicted on everything that touches
subrepos.

Patch

diff --git a/hgext/keyword.py b/hgext/keyword.py
--- a/hgext/keyword.py
+++ b/hgext/keyword.py
@@ -82,6 +82,7 @@ 
 {desc}" expands to the first line of the changeset description.
 '''
 
+from mercurial import amend as amendmod
 from mercurial import commands, context, cmdutil, dispatch, filelog, extensions
 from mercurial import localrepo, match, patch, templatefilters, templater, util
 from mercurial import scmutil, pathutil
@@ -722,7 +723,7 @@ 
     extensions.wrapfunction(context.filectx, 'cmp', kwfilectx_cmp)
     extensions.wrapfunction(patch.patchfile, '__init__', kwpatchfile_init)
     extensions.wrapfunction(patch, 'diff', kw_diff)
-    extensions.wrapfunction(cmdutil, 'amend', kw_amend)
+    extensions.wrapfunction(amendmod, 'amend', kw_amend)
     extensions.wrapfunction(cmdutil, 'copy', kw_copy)
     for c in 'annotate changeset rev filediff diff'.split():
         extensions.wrapfunction(webcommands, c, kwweb_skip)
diff --git a/mercurial/amend.py b/mercurial/amend.py
new file mode 100644
--- /dev/null
+++ b/mercurial/amend.py
@@ -0,0 +1,189 @@ 
+from node import nullid
+from i18n import _
+import copies
+import context, repair, phases, obsolete
+import changelog
+import lock as lockmod
+import cmdutil
+
+def amend(ui, repo, commitfunc, old, extra, pats, opts):
+    ui.note(_('amending changeset %s\n') % old)
+    base = old.p1()
+
+    wlock = lock = newid = None
+    try:
+        wlock = repo.wlock()
+        lock = repo.lock()
+        tr = repo.transaction('amend')
+        try:
+            # See if we got a message from -m or -l, if not, open the editor
+            # with the message of the changeset to amend
+            message = cmdutil.logmessage(ui, opts)
+            # ensure logfile does not conflict with later enforcement of the
+            # message. potential logfile content has been processed by
+            # `logmessage` anyway.
+            opts.pop('logfile')
+            # First, do a regular commit to record all changes in the working
+            # directory (if there are any)
+            ui.callhooks = False
+            currentbookmark = repo._bookmarkcurrent
+            try:
+                repo._bookmarkcurrent = None
+                opts['message'] = 'temporary amend commit for %s' % old
+                node = cmdutil.commit(ui, repo, commitfunc, pats, opts)
+            finally:
+                repo._bookmarkcurrent = currentbookmark
+                ui.callhooks = True
+            ctx = repo[node]
+
+            # Participating changesets:
+            #
+            # node/ctx o - new (intermediate) commit that contains changes
+            #          |   from working dir to go into amending commit
+            #          |   (or a workingctx if there were no changes)
+            #          |
+            # old      o - changeset to amend
+            #          |
+            # base     o - parent of amending changeset
+
+            # Update extra dict from amended commit (e.g. to preserve graft
+            # source)
+            extra.update(old.extra())
+
+            # Also update it from the intermediate commit or from the wctx
+            extra.update(ctx.extra())
+
+            if len(old.parents()) > 1:
+                # ctx.files() isn't reliable for merges, so fall back to the
+                # slower repo.status() method
+                files = set([fn for st in repo.status(base, old)[:3]
+                             for fn in st])
+            else:
+                files = set(old.files())
+
+            # Second, we use either the commit we just did, or if there were no
+            # changes the parent of the working directory as the version of the
+            # files in the final amend commit
+            if node:
+                ui.note(_('copying changeset %s to %s\n') % (ctx, base))
+
+                user = ctx.user()
+                date = ctx.date()
+                # Recompute copies (avoid recording a -> b -> a)
+                copied = copies.pathcopies(base, ctx)
+
+                # Prune files which were reverted by the updates: if old
+                # introduced file X and our intermediate commit, node,
+                # renamed that file, then those two files are the same and
+                # we can discard X from our list of files. Likewise if X
+                # was deleted, it's no longer relevant
+                files.update(ctx.files())
+
+                def samefile(f):
+                    if f in ctx.manifest():
+                        a = ctx.filectx(f)
+                        if f in base.manifest():
+                            b = base.filectx(f)
+                            return (not a.cmp(b)
+                                    and a.flags() == b.flags())
+                        else:
+                            return False
+                    else:
+                        return f not in base.manifest()
+                files = [f for f in files if not samefile(f)]
+
+                def filectxfn(repo, ctx_, path):
+                    try:
+                        fctx = ctx[path]
+                        flags = fctx.flags()
+                        mctx = context.memfilectx(fctx.path(), fctx.data(),
+                                                  islink='l' in flags,
+                                                  isexec='x' in flags,
+                                                  copied=copied.get(path))
+                        return mctx
+                    except KeyError:
+                        raise IOError
+            else:
+                ui.note(_('copying changeset %s to %s\n') % (old, base))
+
+                # Use version of files as in the old cset
+                def filectxfn(repo, ctx_, path):
+                    try:
+                        return old.filectx(path)
+                    except KeyError:
+                        raise IOError
+
+                user = opts.get('user') or old.user()
+                date = opts.get('date') or old.date()
+            editmsg = False
+            if not message:
+                editmsg = True
+                message = old.description()
+
+            pureextra = extra.copy()
+            extra['amend_source'] = old.hex()
+
+            new = context.memctx(repo,
+                                 parents=[base.node(), old.p2().node()],
+                                 text=message,
+                                 files=files,
+                                 filectxfn=filectxfn,
+                                 user=user,
+                                 date=date,
+                                 extra=extra)
+            if editmsg:
+                new._text = cmdutil.commitforceeditor(repo, new, [])
+
+            newdesc =  changelog.stripdesc(new.description())
+            if ((not node)
+                and newdesc == old.description()
+                and user == old.user()
+                and date == old.date()
+                and pureextra == old.extra()):
+                # nothing changed. continuing here would create a new node
+                # anyway because of the amend_source noise.
+                #
+                # This not what we expect from amend.
+                return old.node()
+
+            ph = repo.ui.config('phases', 'new-commit', phases.draft)
+            try:
+                repo.ui.setconfig('phases', 'new-commit', old.phase())
+                newid = repo.commitctx(new)
+            finally:
+                repo.ui.setconfig('phases', 'new-commit', ph)
+            if newid != old.node():
+                # Reroute the working copy parent to the new changeset
+                repo.setparents(newid, nullid)
+
+                # Move bookmarks from old parent to amend commit
+                bms = repo.nodebookmarks(old.node())
+                if bms:
+                    marks = repo._bookmarks
+                    for bm in bms:
+                        marks[bm] = newid
+                    marks.write()
+            #commit the whole amend process
+            if obsolete._enabled and newid != old.node():
+                # mark the new changeset as successor of the rewritten one
+                new = repo[newid]
+                obs = [(old, (new,))]
+                if node:
+                    obs.append((ctx, ()))
+
+                obsolete.createmarkers(repo, obs)
+            tr.close()
+        finally:
+            tr.release()
+        if (not obsolete._enabled) and newid != old.node():
+            # Strip the intermediate commit (if there was one) and the amended
+            # commit
+            if node:
+                ui.note(_('stripping intermediate changeset %s\n') % ctx)
+            ui.note(_('stripping amended changeset %s\n') % old)
+            repair.strip(ui, repo, old.node(), topic='amend-backup')
+    finally:
+        if newid is None:
+            repo.dirstate.invalidate()
+        lockmod.release(lock, wlock)
+    return newid
diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
--- a/mercurial/cmdutil.py
+++ b/mercurial/cmdutil.py
@@ -10,10 +10,8 @@ 
 import os, sys, errno, re, tempfile
 import util, scmutil, templater, patch, error, templatekw, revlog, copies
 import match as matchmod
-import context, repair, graphmod, revset, phases, obsolete, pathutil
-import changelog
+import graphmod, revset, pathutil
 import bookmarks
-import lock as lockmod
 
 def parsealiases(cmd):
     return cmd.lstrip("^").split("|")
@@ -1655,188 +1653,6 @@ 
     return commitfunc(ui, repo, message,
                       scmutil.match(repo[None], pats, opts), opts)
 
-def amend(ui, repo, commitfunc, old, extra, pats, opts):
-    ui.note(_('amending changeset %s\n') % old)
-    base = old.p1()
-
-    wlock = lock = newid = None
-    try:
-        wlock = repo.wlock()
-        lock = repo.lock()
-        tr = repo.transaction('amend')
-        try:
-            # See if we got a message from -m or -l, if not, open the editor
-            # with the message of the changeset to amend
-            message = logmessage(ui, opts)
-            # ensure logfile does not conflict with later enforcement of the
-            # message. potential logfile content has been processed by
-            # `logmessage` anyway.
-            opts.pop('logfile')
-            # First, do a regular commit to record all changes in the working
-            # directory (if there are any)
-            ui.callhooks = False
-            currentbookmark = repo._bookmarkcurrent
-            try:
-                repo._bookmarkcurrent = None
-                opts['message'] = 'temporary amend commit for %s' % old
-                node = commit(ui, repo, commitfunc, pats, opts)
-            finally:
-                repo._bookmarkcurrent = currentbookmark
-                ui.callhooks = True
-            ctx = repo[node]
-
-            # Participating changesets:
-            #
-            # node/ctx o - new (intermediate) commit that contains changes
-            #          |   from working dir to go into amending commit
-            #          |   (or a workingctx if there were no changes)
-            #          |
-            # old      o - changeset to amend
-            #          |
-            # base     o - parent of amending changeset
-
-            # Update extra dict from amended commit (e.g. to preserve graft
-            # source)
-            extra.update(old.extra())
-
-            # Also update it from the intermediate commit or from the wctx
-            extra.update(ctx.extra())
-
-            if len(old.parents()) > 1:
-                # ctx.files() isn't reliable for merges, so fall back to the
-                # slower repo.status() method
-                files = set([fn for st in repo.status(base, old)[:3]
-                             for fn in st])
-            else:
-                files = set(old.files())
-
-            # Second, we use either the commit we just did, or if there were no
-            # changes the parent of the working directory as the version of the
-            # files in the final amend commit
-            if node:
-                ui.note(_('copying changeset %s to %s\n') % (ctx, base))
-
-                user = ctx.user()
-                date = ctx.date()
-                # Recompute copies (avoid recording a -> b -> a)
-                copied = copies.pathcopies(base, ctx)
-
-                # Prune files which were reverted by the updates: if old
-                # introduced file X and our intermediate commit, node,
-                # renamed that file, then those two files are the same and
-                # we can discard X from our list of files. Likewise if X
-                # was deleted, it's no longer relevant
-                files.update(ctx.files())
-
-                def samefile(f):
-                    if f in ctx.manifest():
-                        a = ctx.filectx(f)
-                        if f in base.manifest():
-                            b = base.filectx(f)
-                            return (not a.cmp(b)
-                                    and a.flags() == b.flags())
-                        else:
-                            return False
-                    else:
-                        return f not in base.manifest()
-                files = [f for f in files if not samefile(f)]
-
-                def filectxfn(repo, ctx_, path):
-                    try:
-                        fctx = ctx[path]
-                        flags = fctx.flags()
-                        mctx = context.memfilectx(fctx.path(), fctx.data(),
-                                                  islink='l' in flags,
-                                                  isexec='x' in flags,
-                                                  copied=copied.get(path))
-                        return mctx
-                    except KeyError:
-                        raise IOError
-            else:
-                ui.note(_('copying changeset %s to %s\n') % (old, base))
-
-                # Use version of files as in the old cset
-                def filectxfn(repo, ctx_, path):
-                    try:
-                        return old.filectx(path)
-                    except KeyError:
-                        raise IOError
-
-                user = opts.get('user') or old.user()
-                date = opts.get('date') or old.date()
-            editmsg = False
-            if not message:
-                editmsg = True
-                message = old.description()
-
-            pureextra = extra.copy()
-            extra['amend_source'] = old.hex()
-
-            new = context.memctx(repo,
-                                 parents=[base.node(), old.p2().node()],
-                                 text=message,
-                                 files=files,
-                                 filectxfn=filectxfn,
-                                 user=user,
-                                 date=date,
-                                 extra=extra)
-            if editmsg:
-                new._text = commitforceeditor(repo, new, [])
-
-            newdesc =  changelog.stripdesc(new.description())
-            if ((not node)
-                and newdesc == old.description()
-                and user == old.user()
-                and date == old.date()
-                and pureextra == old.extra()):
-                # nothing changed. continuing here would create a new node
-                # anyway because of the amend_source noise.
-                #
-                # This not what we expect from amend.
-                return old.node()
-
-            ph = repo.ui.config('phases', 'new-commit', phases.draft)
-            try:
-                repo.ui.setconfig('phases', 'new-commit', old.phase())
-                newid = repo.commitctx(new)
-            finally:
-                repo.ui.setconfig('phases', 'new-commit', ph)
-            if newid != old.node():
-                # Reroute the working copy parent to the new changeset
-                repo.setparents(newid, nullid)
-
-                # Move bookmarks from old parent to amend commit
-                bms = repo.nodebookmarks(old.node())
-                if bms:
-                    marks = repo._bookmarks
-                    for bm in bms:
-                        marks[bm] = newid
-                    marks.write()
-            #commit the whole amend process
-            if obsolete._enabled and newid != old.node():
-                # mark the new changeset as successor of the rewritten one
-                new = repo[newid]
-                obs = [(old, (new,))]
-                if node:
-                    obs.append((ctx, ()))
-
-                obsolete.createmarkers(repo, obs)
-            tr.close()
-        finally:
-            tr.release()
-        if (not obsolete._enabled) and newid != old.node():
-            # Strip the intermediate commit (if there was one) and the amended
-            # commit
-            if node:
-                ui.note(_('stripping intermediate changeset %s\n') % ctx)
-            ui.note(_('stripping amended changeset %s\n') % old)
-            repair.strip(ui, repo, old.node(), topic='amend-backup')
-    finally:
-        if newid is None:
-            repo.dirstate.invalidate()
-        lockmod.release(lock, wlock)
-    return newid
-
 def commiteditor(repo, ctx, subs):
     if ctx.description():
         return ctx.description()
diff --git a/mercurial/commands.py b/mercurial/commands.py
--- a/mercurial/commands.py
+++ b/mercurial/commands.py
@@ -20,6 +20,7 @@ 
 import random
 import setdiscovery, treediscovery, dagutil, pvec, localrepo
 import phases, obsolete
+import amend as amendmod
 
 table = {}
 
@@ -1419,7 +1420,7 @@ 
 
         current = repo._bookmarkcurrent
         marks = old.bookmarks()
-        node = cmdutil.amend(ui, repo, commitfunc, old, extra, pats, opts)
+        node = amendmod.amend(ui, repo, commitfunc, old, extra, pats, opts)
         if node == old.node():
             ui.status(_("nothing changed\n"))
             return 1
diff --git a/tests/test-module-imports.t b/tests/test-module-imports.t
--- a/tests/test-module-imports.t
+++ b/tests/test-module-imports.t
@@ -35,5 +35,5 @@ 
      config, error, formatter, scmutil, util
   Import cycle: mercurial.repoview -> mercurial.revset -> mercurial.repoview
   Import cycle: mercurial.fileset -> mercurial.merge -> mercurial.subrepo -> mercurial.match -> mercurial.fileset
-  Import cycle: mercurial.cmdutil -> mercurial.context -> mercurial.subrepo -> mercurial.cmdutil -> mercurial.cmdutil
   Import cycle: mercurial.filemerge -> mercurial.match -> mercurial.fileset -> mercurial.merge -> mercurial.filemerge
+  Import cycle: mercurial.cmdutil -> mercurial.match -> mercurial.fileset -> mercurial.merge -> mercurial.subrepo -> mercurial.cmdutil -> mercurial.cmdutil