Patchwork [4,of,5] changegroup: let callers pass in transaction to apply() (API)

login
register
mail settings
Submitter via Mercurial-devel
Date June 18, 2017, 7:02 a.m.
Message ID <d0254015a38d27271404.1497769347@martinvonz.svl.corp.google.com>
Download mbox | patch
Permalink /patch/21477/
State Accepted
Headers show

Comments

via Mercurial-devel - June 18, 2017, 7:02 a.m.
# HG changeset patch
# User Martin von Zweigbergk <martinvonz@google.com>
# Date 1497591998 25200
#      Thu Jun 15 22:46:38 2017 -0700
# Node ID d0254015a38d2727140432aa26efbe18ea8cab8b
# Parent  1a23893eb7f7ea55df8835bd6208f8427fa36d13
changegroup: let callers pass in transaction to apply() (API)

I think passing in the transaction makes it a little clearer and more
consistent with bundle2.

Patch

diff --git a/hgext/histedit.py b/hgext/histedit.py
--- a/hgext/histedit.py
+++ b/hgext/histedit.py
@@ -1199,7 +1199,7 @@ 
             gen = exchange.readbundle(ui, f, backupfile)
             with repo.transaction('histedit.abort') as tr:
                 if not isinstance(gen, bundle2.unbundle20):
-                    gen.apply(repo, 'histedit', 'bundle:' + backupfile)
+                    gen.apply(repo, tr, 'histedit', 'bundle:' + backupfile)
                 else:
                     bundle2.applybundle(repo, gen, tr,
                                         source='histedit',
diff --git a/hgext/shelve.py b/hgext/shelve.py
--- a/hgext/shelve.py
+++ b/hgext/shelve.py
@@ -127,7 +127,7 @@ 
         try:
             gen = exchange.readbundle(self.repo.ui, fp, self.fname, self.vfs)
             if not isinstance(gen, bundle2.unbundle20):
-                gen.apply(self.repo, 'unshelve',
+                gen.apply(self.repo, self.repo.currenttransaction(), 'unshelve',
                           'bundle:' + self.vfs.join(self.fname),
                           targetphase=phases.secret)
             else:
diff --git a/mercurial/bundle2.py b/mercurial/bundle2.py
--- a/mercurial/bundle2.py
+++ b/mercurial/bundle2.py
@@ -1473,12 +1473,7 @@ 
     This is a very early implementation that will massive rework before being
     inflicted to any end-user.
     """
-    # Make sure we trigger a transaction creation
-    #
-    # The addchangegroup function will get a transaction object by itself, but
-    # we need to make sure we trigger the creation of a transaction object used
-    # for the whole processing scope.
-    op.gettransaction()
+    tr = op.gettransaction()
     unpackerversion = inpart.params.get('version', '01')
     # We should raise an appropriate exception here
     cg = changegroup.getunbundler(unpackerversion, inpart, None)
@@ -1496,7 +1491,8 @@ 
         op.repo.requirements.add('treemanifest')
         op.repo._applyopenerreqs()
         op.repo._writerequirements()
-    ret = cg.apply(op.repo, 'bundle2', 'bundle2', expectedtotal=nbchangesets)
+    ret = cg.apply(op.repo, tr, 'bundle2', 'bundle2',
+                   expectedtotal=nbchangesets)
     op.records.add('changegroup', {'return': ret})
     if op.reply is not None:
         # This is definitely not the final form of this
@@ -1554,18 +1550,13 @@ 
 
     real_part = util.digestchecker(url.open(op.ui, raw_url), size, digests)
 
-    # Make sure we trigger a transaction creation
-    #
-    # The addchangegroup function will get a transaction object by itself, but
-    # we need to make sure we trigger the creation of a transaction object used
-    # for the whole processing scope.
-    op.gettransaction()
+    tr = op.gettransaction()
     from . import exchange
     cg = exchange.readbundle(op.repo.ui, real_part, raw_url)
     if not isinstance(cg, changegroup.cg1unpacker):
         raise error.Abort(_('%s: not a bundle version 1.0') %
             util.hidepassword(raw_url))
-    ret = cg.apply(op.repo, 'bundle2', 'bundle2')
+    ret = cg.apply(op.repo, tr, 'bundle2', 'bundle2')
     op.records.add('changegroup', {'return': ret})
     if op.reply is not None:
         # This is definitely not the final form of this
diff --git a/mercurial/changegroup.py b/mercurial/changegroup.py
--- a/mercurial/changegroup.py
+++ b/mercurial/changegroup.py
@@ -256,7 +256,7 @@ 
         repo.ui.progress(_('manifests'), None)
         self.callback = None
 
-    def apply(self, repo, srctype, url, emptyok=False,
+    def apply(self, repo, tr, srctype, url, emptyok=False,
               targetphase=phases.draft, expectedtotal=None):
         """Add the changegroup returned by source.read() to this repo.
         srctype is a string like 'push', 'pull', or 'unbundle'.  url is
@@ -279,12 +279,11 @@ 
         changesets = files = revisions = 0
 
         try:
-            with repo.transaction("\n".join([srctype,
-                                             util.hidepassword(url)])) as tr:
-                # The transaction could have been created before and already
-                # carries source information. In this case we use the top
-                # level data. We overwrite the argument because we need to use
-                # the top level value (if they exist) in this function.
+            if True:
+                # The transaction may already carry source information. In this
+                # case we use the top level data. We overwrite the argument
+                # because we need to use the top level value (if they exist)
+                # in this function.
                 srctype = tr.hookargs.setdefault('source', srctype)
                 url = tr.hookargs.setdefault('url', url)
                 repo.hook('prechangegroup', throw=True, **tr.hookargs)
diff --git a/mercurial/commands.py b/mercurial/commands.py
--- a/mercurial/commands.py
+++ b/mercurial/commands.py
@@ -5339,8 +5339,8 @@ 
                 modheads = changegroup.combineresults(changes)
             else:
                 txnname = 'unbundle\n%s' % util.hidepassword(url)
-                with repo.transaction(txnname):
-                    modheads = gen.apply(repo, 'unbundle', url)
+                with repo.transaction(txnname) as tr:
+                    modheads = gen.apply(repo, tr, 'unbundle', url)
 
     return postincoming(ui, repo, modheads, opts.get(r'update'), None, None)
 
diff --git a/mercurial/exchange.py b/mercurial/exchange.py
--- a/mercurial/exchange.py
+++ b/mercurial/exchange.py
@@ -1429,7 +1429,7 @@ 
         pullop.repo.ui.status(_("no changes found\n"))
         pullop.cgresult = 0
         return
-    pullop.gettransaction()
+    tr = pullop.gettransaction()
     if pullop.heads is None and list(pullop.common) == [nullid]:
         pullop.repo.ui.status(_("requesting all changes\n"))
     elif pullop.heads is None and pullop.remote.capable('changegroupsubset'):
@@ -1448,7 +1448,7 @@ 
                            "changegroupsubset."))
     else:
         cg = pullop.remote.changegroupsubset(pullop.fetch, pullop.heads, 'pull')
-    pullop.cgresult = cg.apply(pullop.repo, 'pull', pullop.remote.url())
+    pullop.cgresult = cg.apply(pullop.repo, tr, 'pull', pullop.remote.url())
 
 def _pullphase(pullop):
     # Get remote phases data from remote
@@ -1734,8 +1734,8 @@ 
         if not util.safehasattr(cg, 'params'):
             # legacy case: bundle1 (changegroup 01)
             txnname = "\n".join([source, util.hidepassword(url)])
-            with repo.lock(), repo.transaction(txnname):
-                r = cg.apply(repo, source, url)
+            with repo.lock(), repo.transaction(txnname) as tr:
+                r = cg.apply(repo, tr, source, url)
         else:
             r = None
             try:
@@ -2000,7 +2000,7 @@ 
             elif isinstance(cg, streamclone.streamcloneapplier):
                 cg.apply(repo)
             else:
-                cg.apply(repo, 'clonebundles', url)
+                cg.apply(repo, tr, 'clonebundles', url)
             return True
         except urlerr.httperror as e:
             ui.warn(_('HTTP error fetching bundle: %s\n') % str(e))
diff --git a/mercurial/repair.py b/mercurial/repair.py
--- a/mercurial/repair.py
+++ b/mercurial/repair.py
@@ -209,8 +209,8 @@ 
                                         url=tmpbundleurl)
             else:
                 txnname = "strip\n%s" % util.hidepassword(tmpbundleurl)
-                with repo.lock(), repo.transaction(txnname):
-                    gen.apply(repo, 'strip', tmpbundleurl, True)
+                with repo.lock(), repo.transaction(txnname) as tr:
+                    gen.apply(repo, tr, 'strip', tmpbundleurl, True)
             if not repo.ui.verbose:
                 repo.ui.popbuffer()
             f.close()