Patchwork [4,of,4] changegroup: store source and url in the `hookargs` dict

login
register
mail settings
Submitter Pierre-Yves David
Date Oct. 14, 2014, 10:58 p.m.
Message ID <d870ea046efb83f55200.1413327483@marginatus.alto.octopoid.net>
Download mbox | patch
Permalink /patch/6264/
State Accepted
Headers show

Comments

Pierre-Yves David - Oct. 14, 2014, 10:58 p.m.
# HG changeset patch
# User Pierre-Yves David <pierre-yves.david@fb.com>
# Date 1413270406 25200
#      Tue Oct 14 00:06:46 2014 -0700
# Node ID d870ea046efb83f55200213e373e3f0a57789b82
# Parent  56653edfa2e80c7e57617b9ee16a412e89bcb7c4
changegroup: store source and url in the `hookargs` dict

We store the source and url of the current data into `transaction.hookargs` this
let us inherit it from upper layers that may have created a much wider
transaction. We have to modify bundle2 at the same time to register the source
and url in the transaction. We have to do it in the same patch otherwise, the
`addchangegroup` call would fill these values and the hook calling will crash
because of the duplicated 'source' and 'url' arguments passed to the hook call.
Augie Fackler - Oct. 15, 2014, 7:39 p.m.
On Tue, Oct 14, 2014 at 03:58:03PM -0700, Pierre-Yves David wrote:
> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david@fb.com>
> # Date 1413270406 25200
> #      Tue Oct 14 00:06:46 2014 -0700
> # Node ID d870ea046efb83f55200213e373e3f0a57789b82
> # Parent  56653edfa2e80c7e57617b9ee16a412e89bcb7c4
> changegroup: store source and url in the `hookargs` dict

queued these, thanks

>
> We store the source and url of the current data into `transaction.hookargs` this
> let us inherit it from upper layers that may have created a much wider
> transaction. We have to modify bundle2 at the same time to register the source
> and url in the transaction. We have to do it in the same patch otherwise, the
> `addchangegroup` call would fill these values and the hook calling will crash
> because of the duplicated 'source' and 'url' arguments passed to the hook call.
>
> diff --git a/mercurial/changegroup.py b/mercurial/changegroup.py
> --- a/mercurial/changegroup.py
> +++ b/mercurial/changegroup.py
> @@ -600,13 +600,18 @@ def addchangegroup(repo, source, srctype
>      cl = repo.changelog
>      cl.delayupdate()
>      oldheads = cl.heads()
>
>      tr = repo.transaction("\n".join([srctype, util.hidepassword(url)]))
> +    # The transaction could have been created before and 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)
>      try:
> -        repo.hook('prechangegroup', throw=True, source=srctype, url=url,
> -                  **tr.hookargs)
> +        repo.hook('prechangegroup', throw=True, **tr.hookargs)
>
>          trp = weakref.proxy(tr)
>          # pull off the changeset group
>          repo.ui.status(_("adding changesets\n"))
>          clstart = len(cl)
> @@ -686,12 +691,12 @@ def addchangegroup(repo, source, srctype
>
>          if changesets > 0:
>              p = lambda: cl.writepending() and repo.root or ""
>              if 'node' not in tr.hookargs:
>                  tr.hookargs['node'] = hex(cl.node(clstart))
> -            repo.hook('pretxnchangegroup', throw=True, source=srctype,
> -                      url=url, pending=p, **tr.hookargs)
> +            repo.hook('pretxnchangegroup', throw=True, pending=p,
> +                      **tr.hookargs)
>
>          added = [cl.node(r) for r in xrange(clstart, clend)]
>          publishing = repo.ui.configbool('phases', 'publish', True)
>          if srctype in ('push', 'serve'):
>              # Old servers can not push the boundary themselves.
> @@ -732,17 +737,16 @@ def addchangegroup(repo, source, srctype
>                  if clstart >= len(repo):
>                      return
>
>                  # forcefully update the on-disk branch cache
>                  repo.ui.debug("updating the branch cache\n")
> -                repo.hook("changegroup", source=srctype, url=url,
> -                          **tr.hookargs)
> +                repo.hook("changegroup", **tr.hookargs)
>
>                  for n in added:
>                      hookargs = tr.hookargs.copy()
>                      hookargs['node'] = hex(n)
> -                    repo.hook("incoming", source=srctype, url=url, **hookargs)
> +                    repo.hook("incoming", **hookargs)
>
>                  newheads = [h for h in repo.heads() if h not in oldheads]
>                  repo.ui.log("incoming",
>                              "%s incoming changes - new heads: %s\n",
>                              len(added),
> diff --git a/mercurial/exchange.py b/mercurial/exchange.py
> --- a/mercurial/exchange.py
> +++ b/mercurial/exchange.py
> @@ -1219,19 +1219,20 @@ def unbundle(repo, cg, heads, source, ur
>          check_heads(repo, heads, 'uploading changes')
>          # push can proceed
>          if util.safehasattr(cg, 'params'):
>              try:
>                  tr = repo.transaction('unbundle')
> +                tr.hookargs['source'] = source
> +                tr.hookargs['url'] = url
>                  tr.hookargs['bundle2-exp'] = '1'
>                  r = bundle2.processbundle(repo, cg, lambda: tr).reply
>                  cl = repo.unfiltered().changelog
>                  p = cl.writepending() and repo.root or ""
> -                repo.hook('b2x-pretransactionclose', throw=True, source=source,
> -                          url=url, pending=p, **tr.hookargs)
> +                repo.hook('b2x-pretransactionclose', throw=True, pending=p,
> +                          **tr.hookargs)
>                  tr.close()
> -                repo.hook('b2x-transactionclose', source=source, url=url,
> -                          **tr.hookargs)
> +                repo.hook('b2x-transactionclose', **tr.hookargs)
>              except Exception, exc:
>                  exc.duringunbundle2 = True
>                  raise
>          else:
>              r = changegroup.addchangegroup(repo, cg, source, url)
> diff --git a/tests/test-bundle2-exchange.t b/tests/test-bundle2-exchange.t
> --- a/tests/test-bundle2-exchange.t
> +++ b/tests/test-bundle2-exchange.t
> @@ -157,11 +157,11 @@ add extra data to test their exchange du
>  push
>    $ hg -R main push other --rev eea13746799a --bookmark book_eea1
>    pushing to other
>    searching for changes
>    b2x-transactionclose hook: HG_BOOKMARK_MOVED=1 HG_BUNDLE2-EXP=1 HG_NEW_OBSMARKERS=1 HG_PHASES_MOVED=1 HG_SOURCE=push HG_URL=push
> -  changegroup hook: HG_BOOKMARK_MOVED=1 HG_BUNDLE2-EXP=1 HG_NEW_OBSMARKERS=1 HG_PHASES_MOVED=1 HG_SOURCE=bundle2 HG_URL=bundle2
> +  changegroup hook: HG_BOOKMARK_MOVED=1 HG_BUNDLE2-EXP=1 HG_NEW_OBSMARKERS=1 HG_PHASES_MOVED=1 HG_SOURCE=push HG_URL=push
>    remote: adding changesets
>    remote: adding manifests
>    remote: adding file changes
>    remote: added 1 changesets with 0 changes to 0 files (-1 heads)
>    remote: 1 new obsolescence markers
> @@ -233,11 +233,11 @@ push over ssh
>    remote: adding file changes
>    remote: added 1 changesets with 1 changes to 1 files
>    remote: 1 new obsolescence markers
>    updating bookmark book_5fdd
>    remote: b2x-transactionclose hook: HG_BOOKMARK_MOVED=1 HG_BUNDLE2-EXP=1 HG_NEW_OBSMARKERS=1 HG_SOURCE=serve HG_URL=remote:ssh:127.0.0.1
> -  remote: changegroup hook: HG_BOOKMARK_MOVED=1 HG_BUNDLE2-EXP=1 HG_NEW_OBSMARKERS=1 HG_SOURCE=bundle2 HG_URL=bundle2
> +  remote: changegroup hook: HG_BOOKMARK_MOVED=1 HG_BUNDLE2-EXP=1 HG_NEW_OBSMARKERS=1 HG_SOURCE=serve HG_URL=remote:ssh:127.0.0.1
>    $ hg -R other log -G
>    o  6:5fddd98957c8 draft Nicolas Dumazet <nicdumz.commits@gmail.com> book_5fdd C
>    |
>    o  5:42ccdea3bb16 draft Nicolas Dumazet <nicdumz.commits@gmail.com> book_42cc B
>    |
> @@ -460,21 +460,21 @@ Doing the actual push: hook abort
>    $ hg -R main push other -r e7ec4e813ba6
>    pushing to other
>    searching for changes
>    transaction abort!
>    rollback completed
> -  changegroup hook: HG_BUNDLE2-EXP=1 HG_NEW_OBSMARKERS=0 HG_SOURCE=bundle2 HG_URL=bundle2
> +  changegroup hook: HG_BUNDLE2-EXP=1 HG_NEW_OBSMARKERS=0 HG_SOURCE=push HG_URL=push
>    abort: b2x-pretransactionclose.failpush hook exited with status 1
>    [255]
>
>    $ hg -R main push ssh://user@dummy/other -r e7ec4e813ba6
>    pushing to ssh://user@dummy/other
>    searching for changes
>    abort: b2x-pretransactionclose.failpush hook exited with status 1
>    remote: transaction abort!
>    remote: rollback completed
> -  remote: changegroup hook: HG_BUNDLE2-EXP=1 HG_NEW_OBSMARKERS=0 HG_SOURCE=bundle2 HG_URL=bundle2
> +  remote: changegroup hook: HG_BUNDLE2-EXP=1 HG_NEW_OBSMARKERS=0 HG_SOURCE=serve HG_URL=remote:ssh:127.0.0.1
>    [255]
>
>    $ hg -R main push http://localhost:$HGPORT2/ -r e7ec4e813ba6
>    pushing to http://localhost:$HGPORT2/
>    searching for changes
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel

Patch

diff --git a/mercurial/changegroup.py b/mercurial/changegroup.py
--- a/mercurial/changegroup.py
+++ b/mercurial/changegroup.py
@@ -600,13 +600,18 @@  def addchangegroup(repo, source, srctype
     cl = repo.changelog
     cl.delayupdate()
     oldheads = cl.heads()
 
     tr = repo.transaction("\n".join([srctype, util.hidepassword(url)]))
+    # The transaction could have been created before and 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)
     try:
-        repo.hook('prechangegroup', throw=True, source=srctype, url=url,
-                  **tr.hookargs)
+        repo.hook('prechangegroup', throw=True, **tr.hookargs)
 
         trp = weakref.proxy(tr)
         # pull off the changeset group
         repo.ui.status(_("adding changesets\n"))
         clstart = len(cl)
@@ -686,12 +691,12 @@  def addchangegroup(repo, source, srctype
 
         if changesets > 0:
             p = lambda: cl.writepending() and repo.root or ""
             if 'node' not in tr.hookargs:
                 tr.hookargs['node'] = hex(cl.node(clstart))
-            repo.hook('pretxnchangegroup', throw=True, source=srctype,
-                      url=url, pending=p, **tr.hookargs)
+            repo.hook('pretxnchangegroup', throw=True, pending=p,
+                      **tr.hookargs)
 
         added = [cl.node(r) for r in xrange(clstart, clend)]
         publishing = repo.ui.configbool('phases', 'publish', True)
         if srctype in ('push', 'serve'):
             # Old servers can not push the boundary themselves.
@@ -732,17 +737,16 @@  def addchangegroup(repo, source, srctype
                 if clstart >= len(repo):
                     return
 
                 # forcefully update the on-disk branch cache
                 repo.ui.debug("updating the branch cache\n")
-                repo.hook("changegroup", source=srctype, url=url,
-                          **tr.hookargs)
+                repo.hook("changegroup", **tr.hookargs)
 
                 for n in added:
                     hookargs = tr.hookargs.copy()
                     hookargs['node'] = hex(n)
-                    repo.hook("incoming", source=srctype, url=url, **hookargs)
+                    repo.hook("incoming", **hookargs)
 
                 newheads = [h for h in repo.heads() if h not in oldheads]
                 repo.ui.log("incoming",
                             "%s incoming changes - new heads: %s\n",
                             len(added),
diff --git a/mercurial/exchange.py b/mercurial/exchange.py
--- a/mercurial/exchange.py
+++ b/mercurial/exchange.py
@@ -1219,19 +1219,20 @@  def unbundle(repo, cg, heads, source, ur
         check_heads(repo, heads, 'uploading changes')
         # push can proceed
         if util.safehasattr(cg, 'params'):
             try:
                 tr = repo.transaction('unbundle')
+                tr.hookargs['source'] = source
+                tr.hookargs['url'] = url
                 tr.hookargs['bundle2-exp'] = '1'
                 r = bundle2.processbundle(repo, cg, lambda: tr).reply
                 cl = repo.unfiltered().changelog
                 p = cl.writepending() and repo.root or ""
-                repo.hook('b2x-pretransactionclose', throw=True, source=source,
-                          url=url, pending=p, **tr.hookargs)
+                repo.hook('b2x-pretransactionclose', throw=True, pending=p,
+                          **tr.hookargs)
                 tr.close()
-                repo.hook('b2x-transactionclose', source=source, url=url,
-                          **tr.hookargs)
+                repo.hook('b2x-transactionclose', **tr.hookargs)
             except Exception, exc:
                 exc.duringunbundle2 = True
                 raise
         else:
             r = changegroup.addchangegroup(repo, cg, source, url)
diff --git a/tests/test-bundle2-exchange.t b/tests/test-bundle2-exchange.t
--- a/tests/test-bundle2-exchange.t
+++ b/tests/test-bundle2-exchange.t
@@ -157,11 +157,11 @@  add extra data to test their exchange du
 push
   $ hg -R main push other --rev eea13746799a --bookmark book_eea1
   pushing to other
   searching for changes
   b2x-transactionclose hook: HG_BOOKMARK_MOVED=1 HG_BUNDLE2-EXP=1 HG_NEW_OBSMARKERS=1 HG_PHASES_MOVED=1 HG_SOURCE=push HG_URL=push
-  changegroup hook: HG_BOOKMARK_MOVED=1 HG_BUNDLE2-EXP=1 HG_NEW_OBSMARKERS=1 HG_PHASES_MOVED=1 HG_SOURCE=bundle2 HG_URL=bundle2
+  changegroup hook: HG_BOOKMARK_MOVED=1 HG_BUNDLE2-EXP=1 HG_NEW_OBSMARKERS=1 HG_PHASES_MOVED=1 HG_SOURCE=push HG_URL=push
   remote: adding changesets
   remote: adding manifests
   remote: adding file changes
   remote: added 1 changesets with 0 changes to 0 files (-1 heads)
   remote: 1 new obsolescence markers
@@ -233,11 +233,11 @@  push over ssh
   remote: adding file changes
   remote: added 1 changesets with 1 changes to 1 files
   remote: 1 new obsolescence markers
   updating bookmark book_5fdd
   remote: b2x-transactionclose hook: HG_BOOKMARK_MOVED=1 HG_BUNDLE2-EXP=1 HG_NEW_OBSMARKERS=1 HG_SOURCE=serve HG_URL=remote:ssh:127.0.0.1
-  remote: changegroup hook: HG_BOOKMARK_MOVED=1 HG_BUNDLE2-EXP=1 HG_NEW_OBSMARKERS=1 HG_SOURCE=bundle2 HG_URL=bundle2
+  remote: changegroup hook: HG_BOOKMARK_MOVED=1 HG_BUNDLE2-EXP=1 HG_NEW_OBSMARKERS=1 HG_SOURCE=serve HG_URL=remote:ssh:127.0.0.1
   $ hg -R other log -G
   o  6:5fddd98957c8 draft Nicolas Dumazet <nicdumz.commits@gmail.com> book_5fdd C
   |
   o  5:42ccdea3bb16 draft Nicolas Dumazet <nicdumz.commits@gmail.com> book_42cc B
   |
@@ -460,21 +460,21 @@  Doing the actual push: hook abort
   $ hg -R main push other -r e7ec4e813ba6
   pushing to other
   searching for changes
   transaction abort!
   rollback completed
-  changegroup hook: HG_BUNDLE2-EXP=1 HG_NEW_OBSMARKERS=0 HG_SOURCE=bundle2 HG_URL=bundle2
+  changegroup hook: HG_BUNDLE2-EXP=1 HG_NEW_OBSMARKERS=0 HG_SOURCE=push HG_URL=push
   abort: b2x-pretransactionclose.failpush hook exited with status 1
   [255]
 
   $ hg -R main push ssh://user@dummy/other -r e7ec4e813ba6
   pushing to ssh://user@dummy/other
   searching for changes
   abort: b2x-pretransactionclose.failpush hook exited with status 1
   remote: transaction abort!
   remote: rollback completed
-  remote: changegroup hook: HG_BUNDLE2-EXP=1 HG_NEW_OBSMARKERS=0 HG_SOURCE=bundle2 HG_URL=bundle2
+  remote: changegroup hook: HG_BUNDLE2-EXP=1 HG_NEW_OBSMARKERS=0 HG_SOURCE=serve HG_URL=remote:ssh:127.0.0.1
   [255]
 
   $ hg -R main push http://localhost:$HGPORT2/ -r e7ec4e813ba6
   pushing to http://localhost:$HGPORT2/
   searching for changes