Patchwork [3,of,7,V3] changegroup: use a copy of hookargs when invoking the changegroup hook

login
register
mail settings
Submitter Mike Hommey
Date Oct. 16, 2014, 8:22 a.m.
Message ID <87bf4b74115b4d36a127.1413447748@zenigata.glandium.org>
Download mbox | patch
Permalink /patch/6319/
State Accepted
Headers show

Comments

Mike Hommey - Oct. 16, 2014, 8:22 a.m.
# HG changeset patch
# User Mike Hommey <mh@glandium.org>
# Date 1413442493 -32400
#      Thu Oct 16 15:54:53 2014 +0900
# Node ID 87bf4b74115b4d36a127647a569d4d9e67bb2fed
# Parent  e13a2965567b090b5dfc2c1d42547956840a2581
changegroup: use a copy of hookargs when invoking the changegroup hook

addchangegroup creates a runhook function that is used to invoke the
changegroup and incoming hooks, but at the time the function is called,
the contents of hookargs associated with the transaction may have been
modified externally. For instance, bundle2 code affects it with
obsolescence markers and bookmarks info.

It also creates problems when a single transaction is used with multiple
changegroups added (as per an upcoming change), whereby the contents
of hookargs are that of after adding a latter changegroup when invoking
the hook for the first changegroup.

Patch

diff --git a/mercurial/changegroup.py b/mercurial/changegroup.py
--- a/mercurial/changegroup.py
+++ b/mercurial/changegroup.py
@@ -719,27 +719,29 @@  def addchangegroup(repo, source, srctype
         tr.close()
 
         if changesets > 0:
             if srctype != 'strip':
                 # During strip, branchcache is invalid but coming call to
                 # `destroyed` will repair it.
                 # In other case we can safely update cache on disk.
                 branchmap.updatecache(repo.filtered('served'))
+
+            hookargs = dict(tr.hookargs)
             def runhooks():
                 # These hooks run when the lock releases, not when the
                 # transaction closes. So it's possible for the changelog
                 # to have changed since we last saw it.
                 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)
+                          **hookargs)
 
                 for n in added:
                     repo.hook("incoming", node=hex(n), source=srctype,
                               url=url)
 
                 newheads = [h for h in repo.heads() if h not in oldheads]
                 repo.ui.log("incoming",
                             "%s incoming changes - new heads: %s\n",
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
@@ -60,17 +60,17 @@  clone --pull
 
   $ hg -R main phase --public cd010b8cd998
   $ hg clone main other --pull --rev 9520eea781bc
   adding changesets
   adding manifests
   adding file changes
   added 2 changesets with 2 changes to 2 files
   1 new obsolescence markers
-  changegroup hook: HG_NEW_OBSMARKERS=1 HG_PHASES_MOVED=1 HG_SOURCE=bundle2 HG_URL=bundle2
+  changegroup hook: HG_PHASES_MOVED=1 HG_SOURCE=bundle2 HG_URL=bundle2
   updating to branch default
   2 files updated, 0 files merged, 0 files removed, 0 files unresolved
   $ hg -R other log -G
   @  1:9520eea781bc draft Nicolas Dumazet <nicdumz.commits@gmail.com>  E
   |
   o  0:cd010b8cd998 public Nicolas Dumazet <nicdumz.commits@gmail.com>  A
   
   $ hg -R other debugobsolete
@@ -82,17 +82,17 @@  pull
   $ hg -R other pull -r 24b6387c8c8c
   pulling from $TESTTMP/main (glob)
   searching for changes
   adding changesets
   adding manifests
   adding file changes
   added 1 changesets with 1 changes to 1 files (+1 heads)
   1 new obsolescence markers
-  changegroup hook: HG_NEW_OBSMARKERS=1 HG_PHASES_MOVED=1 HG_SOURCE=bundle2 HG_URL=bundle2
+  changegroup hook: HG_PHASES_MOVED=1 HG_SOURCE=bundle2 HG_URL=bundle2
   (run 'hg heads' to see heads, 'hg merge' to merge)
   $ hg -R other log -G
   o  2:24b6387c8c8c draft Nicolas Dumazet <nicdumz.commits@gmail.com>  F
   |
   | @  1:9520eea781bc draft Nicolas Dumazet <nicdumz.commits@gmail.com>  E
   |/
   o  0:cd010b8cd998 public Nicolas Dumazet <nicdumz.commits@gmail.com>  A
   
@@ -154,17 +154,17 @@  add extra data to test their exchange du
 
   $ hg -R main phase --public eea13746799a
 
 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_BUNDLE2-EXP=1 HG_SOURCE=bundle2 HG_URL=bundle2
   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
   updating bookmark book_eea1
   $ hg -R other log -G
   o    3:eea13746799a public Nicolas Dumazet <nicdumz.commits@gmail.com> book_eea1 G
@@ -186,17 +186,17 @@  pull over ssh
   pulling from ssh://user@dummy/main
   searching for changes
   adding changesets
   adding manifests
   adding file changes
   added 1 changesets with 1 changes to 1 files (+1 heads)
   1 new obsolescence markers
   updating bookmark book_02de
-  changegroup hook: HG_BOOKMARK_MOVED=1 HG_NEW_OBSMARKERS=1 HG_PHASES_MOVED=1 HG_SOURCE=bundle2 HG_URL=bundle2
+  changegroup hook: HG_PHASES_MOVED=1 HG_SOURCE=bundle2 HG_URL=bundle2
   (run 'hg heads' to see heads, 'hg merge' to merge)
   $ hg -R other debugobsolete
   1111111111111111111111111111111111111111 9520eea781bcca16c1e15acc0ba14335a0e8e5ba 0 (Thu Jan 01 00:00:00 1970 +0000) {'user': 'test'}
   2222222222222222222222222222222222222222 24b6387c8c8cae37178880f3fa95ded3cb1cf785 0 (Thu Jan 01 00:00:00 1970 +0000) {'user': 'test'}
   3333333333333333333333333333333333333333 eea13746799a9e0bfd88f29d3c2e9dc9389f524f 0 (Thu Jan 01 00:00:00 1970 +0000) {'user': 'test'}
   4444444444444444444444444444444444444444 02de42196ebee42ef284b6780a87cdc96e8eaab6 0 (Thu Jan 01 00:00:00 1970 +0000) {'user': 'test'}
 
 pull over http
@@ -208,17 +208,17 @@  pull over http
   pulling from http://localhost:$HGPORT/
   searching for changes
   adding changesets
   adding manifests
   adding file changes
   added 1 changesets with 1 changes to 1 files (+1 heads)
   1 new obsolescence markers
   updating bookmark book_42cc
-  changegroup hook: HG_BOOKMARK_MOVED=1 HG_NEW_OBSMARKERS=1 HG_PHASES_MOVED=1 HG_SOURCE=bundle2 HG_URL=bundle2
+  changegroup hook: HG_PHASES_MOVED=1 HG_SOURCE=bundle2 HG_URL=bundle2
   (run 'hg heads .' to see heads, 'hg merge' to merge)
   $ cat main-error.log
   $ hg -R other debugobsolete
   1111111111111111111111111111111111111111 9520eea781bcca16c1e15acc0ba14335a0e8e5ba 0 (Thu Jan 01 00:00:00 1970 +0000) {'user': 'test'}
   2222222222222222222222222222222222222222 24b6387c8c8cae37178880f3fa95ded3cb1cf785 0 (Thu Jan 01 00:00:00 1970 +0000) {'user': 'test'}
   3333333333333333333333333333333333333333 eea13746799a9e0bfd88f29d3c2e9dc9389f524f 0 (Thu Jan 01 00:00:00 1970 +0000) {'user': 'test'}
   4444444444444444444444444444444444444444 02de42196ebee42ef284b6780a87cdc96e8eaab6 0 (Thu Jan 01 00:00:00 1970 +0000) {'user': 'test'}
   5555555555555555555555555555555555555555 42ccdea3bb16d28e1848c95fe2e44c000f3f21b1 0 (Thu Jan 01 00:00:00 1970 +0000) {'user': 'test'}
@@ -230,17 +230,17 @@  push over ssh
   searching for changes
   remote: adding changesets
   remote: adding manifests
   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_BUNDLE2-EXP=1 HG_SOURCE=bundle2 HG_URL=bundle2
   $ 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
   |
   | o  4:02de42196ebe draft Nicolas Dumazet <nicdumz.commits@gmail.com> book_02de H
   | |
   | | o  3:eea13746799a public Nicolas Dumazet <nicdumz.commits@gmail.com> book_eea1 G
@@ -457,27 +457,27 @@  Doing the actual push: hook abort
   $ hg -R other serve -p $HGPORT2 -d --pid-file=other.pid -E other-error.log
   $ cat other.pid >> $DAEMON_PIDS
 
   $ 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_SOURCE=bundle2 HG_URL=bundle2
   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_SOURCE=bundle2 HG_URL=bundle2
   [255]
 
   $ hg -R main push http://localhost:$HGPORT2/ -r e7ec4e813ba6
   pushing to http://localhost:$HGPORT2/
   searching for changes
   abort: b2x-pretransactionclose.failpush hook exited with status 1
   [255]