Patchwork [6,of,6] merge: make in-memory changes visible to external update hooks

login
register
mail settings
Submitter Katsunori FUJIWARA
Date Oct. 14, 2015, 5:35 p.m.
Message ID <c8a1919b918367540ff5.1444844121@feefifofum>
Download mbox | patch
Permalink /patch/11067/
State Superseded
Commit 949e8c626d1988f9eb4eadf99ec778c7ad4aad2b
Headers show

Comments

Katsunori FUJIWARA - Oct. 14, 2015, 5:35 p.m.
# HG changeset patch
# User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
# Date 1444843899 -32400
#      Thu Oct 15 02:31:39 2015 +0900
# Node ID c8a1919b918367540ff519ff1e48c8feb099d90e
# Parent  806dc96ae4a3526b9c20a9e89428bff08631a2cf
merge: make in-memory changes visible to external update hooks

51844b8b5017 (while 3.4 code-freeze) made all 'update' hooks run after
releasing wlock for visibility of in-memory dirstate changes. But this
breaks paired invocation of 'preupdate' and 'update' hooks.

For example, 'hg backout --merge' for TARGET revision, which isn't
parent of CURRENT, consists of steps below:

  1. update from CURRENT to TARGET
  2. commit BACKOUT revision, which backs TARGET out
  3. update from BACKOUT to CURRENT
  4. merge TARGET into CURRENT

Then, we expects hooks to run in the order below:

  - 'preupdate' on CURRENT for (1)
  - 'update'    on TARGET  for (1)
  - 'preupdate' on BACKOUT for (3)
  - 'update'    on CURRENT for (3)
  - 'preupdate' on TARGET  for (4)
  - 'update'    on CURRENT/TARGET for (4)

But hooks actually run in the order below:

  - 'preupdate' on CURRENT for (1)
  - 'preupdate' on BACKOUT for (3)
  - 'preupdate' on TARGET  for (4)
  - 'update'    on TARGET  for (1), but actually on CURRENT/TARGET
  - 'update'    on CURRENT for (3), but actually on CURRENT/TARGET
  - 'update'    on CURRENT for (4), but actually on CURRENT/TARGET

Root cause of the issue focused by 51844b8b5017 is that external
'update' hook process can't view in-memory changes (especially, of
dirstate), because they aren't written out until the end of
transaction (or wlock).

Now, hooks can be invoked just after updating, because previous
patches made in-memory changes visible to external process.

This patch may break backward compatibility from the point of view of
"scheduling hook execution", but should be reasonable because 'update'
hooks had been executed in this order before 3.4.

This patch tests "hg backout" and "hg unshelve", because the former
activates the transaction before 'update' hook invocation, but the
former doesn't.
Augie Fackler - Oct. 15, 2015, 1:36 p.m.
On Thu, Oct 15, 2015 at 02:35:21AM +0900, FUJIWARA Katsunori wrote:
> # HG changeset patch
> # User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
> # Date 1444843899 -32400
> #      Thu Oct 15 02:31:39 2015 +0900
> # Node ID c8a1919b918367540ff519ff1e48c8feb099d90e
> # Parent  806dc96ae4a3526b9c20a9e89428bff08631a2cf
> merge: make in-memory changes visible to external update hooks

Queued these. Impressive work.

>
> 51844b8b5017 (while 3.4 code-freeze) made all 'update' hooks run after
> releasing wlock for visibility of in-memory dirstate changes. But this
> breaks paired invocation of 'preupdate' and 'update' hooks.
>
> For example, 'hg backout --merge' for TARGET revision, which isn't
> parent of CURRENT, consists of steps below:
>
>   1. update from CURRENT to TARGET
>   2. commit BACKOUT revision, which backs TARGET out
>   3. update from BACKOUT to CURRENT
>   4. merge TARGET into CURRENT
>
> Then, we expects hooks to run in the order below:
>
>   - 'preupdate' on CURRENT for (1)
>   - 'update'    on TARGET  for (1)
>   - 'preupdate' on BACKOUT for (3)
>   - 'update'    on CURRENT for (3)
>   - 'preupdate' on TARGET  for (4)
>   - 'update'    on CURRENT/TARGET for (4)
>
> But hooks actually run in the order below:
>
>   - 'preupdate' on CURRENT for (1)
>   - 'preupdate' on BACKOUT for (3)
>   - 'preupdate' on TARGET  for (4)
>   - 'update'    on TARGET  for (1), but actually on CURRENT/TARGET
>   - 'update'    on CURRENT for (3), but actually on CURRENT/TARGET
>   - 'update'    on CURRENT for (4), but actually on CURRENT/TARGET
>
> Root cause of the issue focused by 51844b8b5017 is that external
> 'update' hook process can't view in-memory changes (especially, of
> dirstate), because they aren't written out until the end of
> transaction (or wlock).
>
> Now, hooks can be invoked just after updating, because previous
> patches made in-memory changes visible to external process.
>
> This patch may break backward compatibility from the point of view of
> "scheduling hook execution", but should be reasonable because 'update'
> hooks had been executed in this order before 3.4.
>
> This patch tests "hg backout" and "hg unshelve", because the former
> activates the transaction before 'update' hook invocation, but the
> former doesn't.
>
> diff --git a/mercurial/merge.py b/mercurial/merge.py
> --- a/mercurial/merge.py
> +++ b/mercurial/merge.py
> @@ -1180,9 +1180,7 @@
>          wlock.release()
>
>      if not partial:
> -        def updatehook(parent1=xp1, parent2=xp2, error=stats[3]):
> -            repo.hook('update', parent1=parent1, parent2=parent2, error=error)
> -        repo._afterlock(updatehook)
> +        repo.hook('update', parent1=xp1, parent2=xp2, error=stats[3])
>      return stats
>
>  def graft(repo, ctx, pctx, labels):
> diff --git a/tests/test-backout.t b/tests/test-backout.t
> --- a/tests/test-backout.t
> +++ b/tests/test-backout.t
> @@ -313,6 +313,41 @@
>    > preupdate.visibility =
>    > EOF
>
> +== test visibility to external update hook
> +
> +  $ hg update -q -C 2
> +  $ hg --config extensions.strip= strip 3
> +  saved backup bundle to * (glob)
> +
> +  $ cat >> .hg/hgrc <<EOF
> +  > [hooks]
> +  > update.visibility = sh $TESTTMP/checkvisibility.sh update
> +  > EOF
> +
> +  $ hg backout --merge -d '3 0' 1 --tool=true -m 'fixed comment'
> +  ==== update:
> +  1:5a50a024c182
> +  ====
> +  reverting a
> +  created new head
> +  changeset 3:d92a3f57f067 backs out changeset 1:5a50a024c182
> +  ==== update:
> +  2:6ea3f2a197a2
> +  ====
> +  merging with changeset 3:d92a3f57f067
> +  merging a
> +  ==== update:
> +  2:6ea3f2a197a2
> +  3:d92a3f57f067
> +  ====
> +  0 files updated, 1 files merged, 0 files removed, 0 files unresolved
> +  (branch merge, don't forget to commit)
> +
> +  $ cat >> .hg/hgrc <<EOF
> +  > [hooks]
> +  > update.visibility =
> +  > EOF
> +
>    $ cd ..
>
>  backout should not back out subsequent changesets
> diff --git a/tests/test-blackbox.t b/tests/test-blackbox.t
> --- a/tests/test-blackbox.t
> +++ b/tests/test-blackbox.t
> @@ -119,8 +119,8 @@
>    $ echo '[hooks]' >> .hg/hgrc
>    $ echo 'update = echo hooked' >> .hg/hgrc
>    $ hg update
> +  hooked
>    1 files updated, 0 files merged, 0 files removed, 0 files unresolved
> -  hooked
>    $ hg blackbox -l 5
>    1970/01/01 00:00:00 bob (*)> update (glob)
>    1970/01/01 00:00:00 bob (*)> writing .hg/cache/tags2-visible with 0 tags (glob)
> diff --git a/tests/test-hook.t b/tests/test-hook.t
> --- a/tests/test-hook.t
> +++ b/tests/test-hook.t
> @@ -223,8 +223,8 @@
>    $ echo "update = printenv.py update" >> .hg/hgrc
>    $ hg update
>    preupdate hook: HG_PARENT1=539e4b31b6dc
> +  update hook: HG_ERROR=0 HG_PARENT1=539e4b31b6dc
>    2 files updated, 0 files merged, 0 files removed, 0 files unresolved
> -  update hook: HG_ERROR=0 HG_PARENT1=539e4b31b6dc
>
>  pushkey hook
>
> @@ -644,8 +644,8 @@
>    $ hg ci -ma
>    223eafe2750c tip
>    $ hg up 0 --config extensions.largefiles=
> +  cb9a9f314b8b
>    1 files updated, 0 files merged, 0 files removed, 0 files unresolved
> -  cb9a9f314b8b
>
>  make sure --verbose (and --quiet/--debug etc.) are propagated to the local ui
>  that is passed to pre/post hooks
> diff --git a/tests/test-shelve.t b/tests/test-shelve.t
> --- a/tests/test-shelve.t
> +++ b/tests/test-shelve.t
> @@ -1087,4 +1087,50 @@
>    ACTUAL  5:703117a2acfb
>    ====
>
> +== test visibility to external update hook
> +
> +  $ hg update -q -C 5
> +
> +  $ cat >> .hg/hgrc <<EOF
> +  > [hooks]
> +  > update.visibility = sh $TESTTMP/checkvisibility.sh update
> +  > EOF
> +
> +  $ echo nnnn >> n
> +
> +  $ sh $TESTTMP/checkvisibility.sh before-unshelving
> +  ==== before-unshelving:
> +  VISIBLE 5:703117a2acfb
> +  ACTUAL  5:703117a2acfb
> +  ====
> +
> +  $ hg unshelve --keep default
> +  temporarily committing pending changes (restore with 'hg unshelve --abort')
> +  rebasing shelved changes
> +  rebasing 7:fcbb97608399 "changes to 'create conflict'" (tip)
> +  ==== update:
> +  VISIBLE 6:66b86db80ee4
> +  VISIBLE 7:fcbb97608399
> +  ACTUAL  5:703117a2acfb
> +  ====
> +  ==== update:
> +  VISIBLE 6:66b86db80ee4
> +  ACTUAL  5:703117a2acfb
> +  ====
> +  ==== update:
> +  VISIBLE 5:703117a2acfb
> +  ACTUAL  5:703117a2acfb
> +  ====
> +
> +  $ cat >> .hg/hgrc <<EOF
> +  > [hooks]
> +  > update.visibility =
> +  > EOF
> +
> +  $ sh $TESTTMP/checkvisibility.sh after-unshelving
> +  ==== after-unshelving:
> +  VISIBLE 5:703117a2acfb
> +  ACTUAL  5:703117a2acfb
> +  ====
> +
>    $ cd ..
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> https://selenic.com/mailman/listinfo/mercurial-devel

Patch

diff --git a/mercurial/merge.py b/mercurial/merge.py
--- a/mercurial/merge.py
+++ b/mercurial/merge.py
@@ -1180,9 +1180,7 @@ 
         wlock.release()
 
     if not partial:
-        def updatehook(parent1=xp1, parent2=xp2, error=stats[3]):
-            repo.hook('update', parent1=parent1, parent2=parent2, error=error)
-        repo._afterlock(updatehook)
+        repo.hook('update', parent1=xp1, parent2=xp2, error=stats[3])
     return stats
 
 def graft(repo, ctx, pctx, labels):
diff --git a/tests/test-backout.t b/tests/test-backout.t
--- a/tests/test-backout.t
+++ b/tests/test-backout.t
@@ -313,6 +313,41 @@ 
   > preupdate.visibility =
   > EOF
 
+== test visibility to external update hook
+
+  $ hg update -q -C 2
+  $ hg --config extensions.strip= strip 3
+  saved backup bundle to * (glob)
+
+  $ cat >> .hg/hgrc <<EOF
+  > [hooks]
+  > update.visibility = sh $TESTTMP/checkvisibility.sh update
+  > EOF
+
+  $ hg backout --merge -d '3 0' 1 --tool=true -m 'fixed comment'
+  ==== update:
+  1:5a50a024c182
+  ====
+  reverting a
+  created new head
+  changeset 3:d92a3f57f067 backs out changeset 1:5a50a024c182
+  ==== update:
+  2:6ea3f2a197a2
+  ====
+  merging with changeset 3:d92a3f57f067
+  merging a
+  ==== update:
+  2:6ea3f2a197a2
+  3:d92a3f57f067
+  ====
+  0 files updated, 1 files merged, 0 files removed, 0 files unresolved
+  (branch merge, don't forget to commit)
+
+  $ cat >> .hg/hgrc <<EOF
+  > [hooks]
+  > update.visibility =
+  > EOF
+
   $ cd ..
 
 backout should not back out subsequent changesets
diff --git a/tests/test-blackbox.t b/tests/test-blackbox.t
--- a/tests/test-blackbox.t
+++ b/tests/test-blackbox.t
@@ -119,8 +119,8 @@ 
   $ echo '[hooks]' >> .hg/hgrc
   $ echo 'update = echo hooked' >> .hg/hgrc
   $ hg update
+  hooked
   1 files updated, 0 files merged, 0 files removed, 0 files unresolved
-  hooked
   $ hg blackbox -l 5
   1970/01/01 00:00:00 bob (*)> update (glob)
   1970/01/01 00:00:00 bob (*)> writing .hg/cache/tags2-visible with 0 tags (glob)
diff --git a/tests/test-hook.t b/tests/test-hook.t
--- a/tests/test-hook.t
+++ b/tests/test-hook.t
@@ -223,8 +223,8 @@ 
   $ echo "update = printenv.py update" >> .hg/hgrc
   $ hg update
   preupdate hook: HG_PARENT1=539e4b31b6dc
+  update hook: HG_ERROR=0 HG_PARENT1=539e4b31b6dc
   2 files updated, 0 files merged, 0 files removed, 0 files unresolved
-  update hook: HG_ERROR=0 HG_PARENT1=539e4b31b6dc
 
 pushkey hook
 
@@ -644,8 +644,8 @@ 
   $ hg ci -ma
   223eafe2750c tip
   $ hg up 0 --config extensions.largefiles=
+  cb9a9f314b8b
   1 files updated, 0 files merged, 0 files removed, 0 files unresolved
-  cb9a9f314b8b
 
 make sure --verbose (and --quiet/--debug etc.) are propagated to the local ui
 that is passed to pre/post hooks
diff --git a/tests/test-shelve.t b/tests/test-shelve.t
--- a/tests/test-shelve.t
+++ b/tests/test-shelve.t
@@ -1087,4 +1087,50 @@ 
   ACTUAL  5:703117a2acfb
   ====
 
+== test visibility to external update hook
+
+  $ hg update -q -C 5
+
+  $ cat >> .hg/hgrc <<EOF
+  > [hooks]
+  > update.visibility = sh $TESTTMP/checkvisibility.sh update
+  > EOF
+
+  $ echo nnnn >> n
+
+  $ sh $TESTTMP/checkvisibility.sh before-unshelving
+  ==== before-unshelving:
+  VISIBLE 5:703117a2acfb
+  ACTUAL  5:703117a2acfb
+  ====
+
+  $ hg unshelve --keep default
+  temporarily committing pending changes (restore with 'hg unshelve --abort')
+  rebasing shelved changes
+  rebasing 7:fcbb97608399 "changes to 'create conflict'" (tip)
+  ==== update:
+  VISIBLE 6:66b86db80ee4
+  VISIBLE 7:fcbb97608399
+  ACTUAL  5:703117a2acfb
+  ====
+  ==== update:
+  VISIBLE 6:66b86db80ee4
+  ACTUAL  5:703117a2acfb
+  ====
+  ==== update:
+  VISIBLE 5:703117a2acfb
+  ACTUAL  5:703117a2acfb
+  ====
+
+  $ cat >> .hg/hgrc <<EOF
+  > [hooks]
+  > update.visibility =
+  > EOF
+
+  $ sh $TESTTMP/checkvisibility.sh after-unshelving
+  ==== after-unshelving:
+  VISIBLE 5:703117a2acfb
+  ACTUAL  5:703117a2acfb
+  ====
+
   $ cd ..