Patchwork import: move tryone closure in cmdutil

login
register
mail settings
Submitter Pierre-Yves David
Date Feb. 12, 2014, 12:59 a.m.
Message ID <9b59241d45fcdc69a134.1392166742@marginatus.alto.octopoid.net>
Download mbox | patch
Permalink /patch/3599/
State Superseded
Commit ce3f3082ec45f26fc1a4d25c20f7d59a5700b5f7
Headers show

Comments

Pierre-Yves David - Feb. 12, 2014, 12:59 a.m.
# HG changeset patch
# User Pierre-Yves David <pierre-yves.david@logilab.fr>
# Date 1392166356 28800
#      Tue Feb 11 16:52:36 2014 -0800
# Node ID 9b59241d45fcdc69a13459c0a90942b808faf69e
# Parent  1478a9ce679097c03234201d179e48c58d0b5c1d
import: move tryone closure in cmdutil

We extract the `tryone` function into the `cmdutil` module. A lot fo the command
context have to be passed to the utility function, but having and explicit
declaration will allow extension to wrap it. This will allows use to make
changeset evolution related experiment in dedicated extension.
Matt Mackall - Feb. 12, 2014, 3:57 a.m.
On Tue, 2014-02-11 at 16:59 -0800, pierre-yves.david@ens-lyon.org wrote:
> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david@logilab.fr>
> # Date 1392166356 28800
> #      Tue Feb 11 16:52:36 2014 -0800
> # Node ID 9b59241d45fcdc69a13459c0a90942b808faf69e
> # Parent  1478a9ce679097c03234201d179e48c58d0b5c1d
> import: move tryone closure in cmdutil
> 
> We extract the `tryone` function into the `cmdutil` module. A lot fo the command
> context have to be passed to the utility function, but having and explicit
> declaration will allow extension to wrap it. This will allows use to make
> changeset evolution related experiment in dedicated extension.
> 
> diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
> --- a/mercurial/cmdutil.py
> +++ b/mercurial/cmdutil.py
> @@ -540,10 +540,124 @@ def service(opts, parentfn=None, initfn=
>              os.close(logfilefd)
>  
>      if runfn:
>          return runfn()
>  
> +def tryimportone(ui, repo, hunk, parents, opts, msgs, updatefunc):
> +    """Utility function used by commands.import to import a single patch

This is perhaps a bit too ugly as an API. Can we try to extract the core
"import a patch" function from this? At the very least, we should
document the horrendous args this accepts.
Pierre-Yves David - Feb. 12, 2014, 7:31 p.m.
On 02/11/2014 07:57 PM, Matt Mackall wrote:
> On Tue, 2014-02-11 at 16:59 -0800, pierre-yves.david@ens-lyon.org wrote:
>> # HG changeset patch
>> # User Pierre-Yves David <pierre-yves.david@logilab.fr>
>> # Date 1392166356 28800
>> #      Tue Feb 11 16:52:36 2014 -0800
>> # Node ID 9b59241d45fcdc69a13459c0a90942b808faf69e
>> # Parent  1478a9ce679097c03234201d179e48c58d0b5c1d
>> import: move tryone closure in cmdutil
>>
>> We extract the `tryone` function into the `cmdutil` module. A lot fo the command
>> context have to be passed to the utility function, but having and explicit
>> declaration will allow extension to wrap it. This will allows use to make
>> changeset evolution related experiment in dedicated extension.
>>
>> diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
>> --- a/mercurial/cmdutil.py
>> +++ b/mercurial/cmdutil.py
>> @@ -540,10 +540,124 @@ def service(opts, parentfn=None, initfn=
>>               os.close(logfilefd)
>>
>>       if runfn:
>>           return runfn()
>>
>> +def tryimportone(ui, repo, hunk, parents, opts, msgs, updatefunc):
>> +    """Utility function used by commands.import to import a single patch
>
> This is perhaps a bit too ugly as an API.

Yes, but this patch focus on moving the code around with minimal change 
to it.

> Can we try to extract the core
> "import a patch" function from this?

We can probably build something nicer. Such change is bit out of my 
original scope "patchbombing one year old draft changeset out my working 
copy"

> At the very least, we should document the horrendous args this accepts.

I'll update the patch with such information and a note to future reader 
about how we would like it to be nicer.

Patch

diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
--- a/mercurial/cmdutil.py
+++ b/mercurial/cmdutil.py
@@ -540,10 +540,124 @@  def service(opts, parentfn=None, initfn=
             os.close(logfilefd)
 
     if runfn:
         return runfn()
 
+def tryimportone(ui, repo, hunk, parents, opts, msgs, updatefunc):
+    """Utility function used by commands.import to import a single patch
+
+    This function is explicitly defined here to help the evolve extension to
+    wrap this part of the import logic.
+    """
+    tmpname, message, user, date, branch, nodeid, p1, p2 = \
+        patch.extract(ui, hunk)
+
+    editor = commiteditor
+    if opts.get('edit'):
+        editor = commitforceeditor
+    update = not opts.get('bypass')
+    strip = opts["strip"]
+    sim = float(opts.get('similarity') or 0)
+    if not tmpname:
+        return (None, None)
+    msg = _('applied to working directory')
+
+    try:
+        cmdline_message = logmessage(ui, opts)
+        if cmdline_message:
+            # pickup the cmdline msg
+            message = cmdline_message
+        elif message:
+            # pickup the patch msg
+            message = message.strip()
+        else:
+            # launch the editor
+            message = None
+        ui.debug('message:\n%s\n' % message)
+
+        if len(parents) == 1:
+            parents.append(repo[nullid])
+        if opts.get('exact'):
+            if not nodeid or not p1:
+                raise util.Abort(_('not a Mercurial patch'))
+            p1 = repo[p1]
+            p2 = repo[p2 or nullid]
+        elif p2:
+            try:
+                p1 = repo[p1]
+                p2 = repo[p2]
+                # Without any options, consider p2 only if the
+                # patch is being applied on top of the recorded
+                # first parent.
+                if p1 != parents[0]:
+                    p1 = parents[0]
+                    p2 = repo[nullid]
+            except error.RepoError:
+                p1, p2 = parents
+        else:
+            p1, p2 = parents
+
+        n = None
+        if update:
+            if p1 != parents[0]:
+                updatefunc(repo, p1.node())
+            if p2 != parents[1]:
+                repo.setparents(p1.node(), p2.node())
+
+            if opts.get('exact') or opts.get('import_branch'):
+                repo.dirstate.setbranch(branch or 'default')
+
+            files = set()
+            patch.patch(ui, repo, tmpname, strip=strip, files=files,
+                        eolmode=None, similarity=sim / 100.0)
+            files = list(files)
+            if opts.get('no_commit'):
+                if message:
+                    msgs.append(message)
+            else:
+                if opts.get('exact') or p2:
+                    # If you got here, you either use --force and know what
+                    # you are doing or used --exact or a merge patch while
+                    # being updated to its first parent.
+                    m = None
+                else:
+                    m = scmutil.matchfiles(repo, files or [])
+                n = repo.commit(message, opts.get('user') or user,
+                                opts.get('date') or date, match=m,
+                                editor=editor)
+        else:
+            if opts.get('exact') or opts.get('import_branch'):
+                branch = branch or 'default'
+            else:
+                branch = p1.branch()
+            store = patch.filestore()
+            try:
+                files = set()
+                try:
+                    patch.patchrepo(ui, repo, p1, store, tmpname, strip,
+                                    files, eolmode=None)
+                except patch.PatchError, e:
+                    raise util.Abort(str(e))
+                memctx = context.makememctx(repo, (p1.node(), p2.node()),
+                                            message,
+                                            opts.get('user') or user,
+                                            opts.get('date') or date,
+                                            branch, files, store,
+                                            editor=commiteditor)
+                repo.savecommitmessage(memctx.description())
+                n = memctx.commit()
+            finally:
+                store.close()
+        if opts.get('exact') and hex(n) != nodeid:
+            raise util.Abort(_('patch is damaged or loses information'))
+        if n:
+            # i18n: refers to a short changeset id
+            msg = _('created %s') % short(n)
+        return (msg, n)
+    finally:
+        os.unlink(tmpname)
+
 def export(repo, revs, template='hg-%h.patch', fp=None, switch_parent=False,
            opts=None):
     '''export changesets as hg patches.'''
 
     total = len(revs)
diff --git a/mercurial/commands.py b/mercurial/commands.py
--- a/mercurial/commands.py
+++ b/mercurial/commands.py
@@ -3678,14 +3678,10 @@  def import_(ui, repo, patch1=None, *patc
 
     date = opts.get('date')
     if date:
         opts['date'] = util.parsedate(date)
 
-    editor = cmdutil.commiteditor
-    if opts.get('edit'):
-        editor = cmdutil.commitforceeditor
-
     update = not opts.get('bypass')
     if not update and opts.get('no_commit'):
         raise util.Abort(_('cannot use --no-commit with --bypass'))
     try:
         sim = float(opts.get('similarity') or 0)
@@ -3700,116 +3696,13 @@  def import_(ui, repo, patch1=None, *patc
         cmdutil.checkunfinished(repo)
     if (opts.get('exact') or not opts.get('force')) and update:
         cmdutil.bailifchanged(repo)
 
     base = opts["base"]
-    strip = opts["strip"]
     wlock = lock = tr = None
     msgs = []
 
-    def tryone(ui, hunk, parents):
-        tmpname, message, user, date, branch, nodeid, p1, p2 = \
-            patch.extract(ui, hunk)
-
-        if not tmpname:
-            return (None, None)
-        msg = _('applied to working directory')
-
-        try:
-            cmdline_message = cmdutil.logmessage(ui, opts)
-            if cmdline_message:
-                # pickup the cmdline msg
-                message = cmdline_message
-            elif message:
-                # pickup the patch msg
-                message = message.strip()
-            else:
-                # launch the editor
-                message = None
-            ui.debug('message:\n%s\n' % message)
-
-            if len(parents) == 1:
-                parents.append(repo[nullid])
-            if opts.get('exact'):
-                if not nodeid or not p1:
-                    raise util.Abort(_('not a Mercurial patch'))
-                p1 = repo[p1]
-                p2 = repo[p2 or nullid]
-            elif p2:
-                try:
-                    p1 = repo[p1]
-                    p2 = repo[p2]
-                    # Without any options, consider p2 only if the
-                    # patch is being applied on top of the recorded
-                    # first parent.
-                    if p1 != parents[0]:
-                        p1 = parents[0]
-                        p2 = repo[nullid]
-                except error.RepoError:
-                    p1, p2 = parents
-            else:
-                p1, p2 = parents
-
-            n = None
-            if update:
-                if p1 != parents[0]:
-                    hg.clean(repo, p1.node())
-                if p2 != parents[1]:
-                    repo.setparents(p1.node(), p2.node())
-
-                if opts.get('exact') or opts.get('import_branch'):
-                    repo.dirstate.setbranch(branch or 'default')
-
-                files = set()
-                patch.patch(ui, repo, tmpname, strip=strip, files=files,
-                            eolmode=None, similarity=sim / 100.0)
-                files = list(files)
-                if opts.get('no_commit'):
-                    if message:
-                        msgs.append(message)
-                else:
-                    if opts.get('exact') or p2:
-                        # If you got here, you either use --force and know what
-                        # you are doing or used --exact or a merge patch while
-                        # being updated to its first parent.
-                        m = None
-                    else:
-                        m = scmutil.matchfiles(repo, files or [])
-                    n = repo.commit(message, opts.get('user') or user,
-                                    opts.get('date') or date, match=m,
-                                    editor=editor)
-            else:
-                if opts.get('exact') or opts.get('import_branch'):
-                    branch = branch or 'default'
-                else:
-                    branch = p1.branch()
-                store = patch.filestore()
-                try:
-                    files = set()
-                    try:
-                        patch.patchrepo(ui, repo, p1, store, tmpname, strip,
-                                        files, eolmode=None)
-                    except patch.PatchError, e:
-                        raise util.Abort(str(e))
-                    memctx = context.makememctx(repo, (p1.node(), p2.node()),
-                                                message,
-                                                opts.get('user') or user,
-                                                opts.get('date') or date,
-                                                branch, files, store,
-                                                editor=cmdutil.commiteditor)
-                    repo.savecommitmessage(memctx.description())
-                    n = memctx.commit()
-                finally:
-                    store.close()
-            if opts.get('exact') and hex(n) != nodeid:
-                raise util.Abort(_('patch is damaged or loses information'))
-            if n:
-                # i18n: refers to a short changeset id
-                msg = _('created %s') % short(n)
-            return (msg, n)
-        finally:
-            os.unlink(tmpname)
 
     try:
         try:
             wlock = repo.wlock()
             if not opts.get('no_commit'):
@@ -3826,11 +3719,12 @@  def import_(ui, repo, patch1=None, *patc
                     ui.status(_('applying %s\n') % patchurl)
                     patchfile = hg.openpath(ui, patchurl)
 
                 haspatch = False
                 for hunk in patch.split(patchfile):
-                    (msg, node) = tryone(ui, hunk, parents)
+                    (msg, node) = cmdutil.tryimportone(ui, repo, hunk, parents,
+                                                       opts, msgs, hg.clean)
                     if msg:
                         haspatch = True
                         ui.note(msg + '\n')
                     if update or opts.get('exact'):
                         parents = repo.parents()