Patchwork [1,of,1,STABLE] merge: execute all removing before updating to avoid case-folding collision

login
register
mail settings
Submitter Katsunori FUJIWARA
Date April 28, 2013, 3:34 p.m.
Message ID <5468fa345cc465c8ff2f.1367163294@feefifofum>
Download mbox | patch
Permalink /patch/1493/
State Superseded
Headers show

Comments

Katsunori FUJIWARA - April 28, 2013, 3:34 p.m.
# HG changeset patch
# User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
# Date 1367161582 -32400
#      Mon Apr 29 00:06:22 2013 +0900
# Branch stable
# Node ID 5468fa345cc465c8ff2fa35fbde61dca21495e1d
# Parent  f01a351db79106ba96ac6d527cf69944fd98e665
merge: execute all removing before updating to avoid case-folding collision

Before this patch, merge process may update the file in target context
before removing the existing file which causes case-folding collision
against the former.

Case-folding collision check ensures only that there is no collision
between files in the merged manifest, not that merging/updating
process is collision free.

Sorting "actions" in removal first order is not safe enough, because
parallel execution by "worker" may execute updating before removing on
posix environment.

This patch divides "workeractions" into removing and updating, and
executes the former first.

This patch still scans "actions"/"workeractions" some times for ease
in patch review, even though large list may cost much in this way.
(total cost should be as same as before)

Dividing "workeractions" affects progress indication, so this also
changes some tests.
Katsunori FUJIWARA - April 29, 2013, 5:53 a.m.
At Mon, 29 Apr 2013 00:34:54 +0900,
FUJIWARA Katsunori wrote:
> 
> # HG changeset patch
> # User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
> # Date 1367161582 -32400
> #      Mon Apr 29 00:06:22 2013 +0900
> # Branch stable
> # Node ID 5468fa345cc465c8ff2fa35fbde61dca21495e1d
> # Parent  f01a351db79106ba96ac6d527cf69944fd98e665
> merge: execute all removing before updating to avoid case-folding collision
> 
> Before this patch, merge process may update the file in target context
> before removing the existing file which causes case-folding collision
> against the former.
>
> Case-folding collision check ensures only that there is no collision
> between files in the merged manifest, not that merging/updating
> process is collision free.

Oops, I overlooked "actions.sort(key=actionkey)" sorting, sorry.

Please discard this post. I'll resend with description rewriting.

 
> Sorting "actions" in removal first order is not safe enough, because
> parallel execution by "worker" may execute updating before removing on
> posix environment.
>
> This patch divides "workeractions" into removing and updating, and
> executes the former first.
>
> This patch still scans "actions"/"workeractions" some times for ease
> in patch review, even though large list may cost much in this way.
> (total cost should be as same as before)
> 
> Dividing "workeractions" affects progress indication, so this also
> changes some tests.
> 
> diff --git a/mercurial/merge.py b/mercurial/merge.py
> --- a/mercurial/merge.py
> +++ b/mercurial/merge.py
> @@ -455,8 +455,10 @@
>  
>      numupdates = len(actions)
>      workeractions = [a for a in actions if a[1] in 'gr']
> -    updated = len([a for a in workeractions if a[1] == 'g'])
> -    removed = len([a for a in workeractions if a[1] == 'r'])
> +    updateactions = [a for a in workeractions if a[1] == 'g']
> +    updated = len(updateactions)
> +    removeactions = [a for a in workeractions if a[1] == 'r']
> +    removed = len(removeactions)
>      actions = [a for a in actions if a[1] not in 'gr']
>  
>      hgsub = [a[1] for a in workeractions if a[0] == '.hgsubstate']
> @@ -465,7 +467,13 @@
>  
>      z = 0
>      prog = worker.worker(repo.ui, 0.001, getremove, (repo, mctx, overwrite),
> -                         workeractions)
> +                         removeactions)
> +    for i, item in prog:
> +        z += i
> +        repo.ui.progress(_('updating'), z, item=item, total=numupdates,
> +                         unit=_('files'))
> +    prog = worker.worker(repo.ui, 0.001, getremove, (repo, mctx, overwrite),
> +                         updateactions)
>      for i, item in prog:
>          z += i
>          repo.ui.progress(_('updating'), z, item=item, total=numupdates,
> diff --git a/tests/test-issue672.t b/tests/test-issue672.t
> --- a/tests/test-issue672.t
> +++ b/tests/test-issue672.t
> @@ -37,6 +37,7 @@
>     1: other deleted -> r
>     1a: remote created -> g
>    removing 1
> +  updating: 1 1/2 files (50.00%)
>    getting 1a
>    updating: 1a 2/2 files (100.00%)
>    1 files updated, 0 files merged, 1 files removed, 0 files unresolved
> diff --git a/tests/test-rename-dir-merge.t b/tests/test-rename-dir-merge.t
> --- a/tests/test-rename-dir-merge.t
> +++ b/tests/test-rename-dir-merge.t
> @@ -46,6 +46,7 @@
>     b/b: remote created -> g
>    removing a/a
>    removing a/b
> +  updating: a/b 2/5 files (40.00%)
>    getting b/a
>    getting b/b
>    updating: b/b 4/5 files (80.00%)
> diff --git a/tests/test-rename-merge2.t b/tests/test-rename-merge2.t
> --- a/tests/test-rename-merge2.t
> +++ b/tests/test-rename-merge2.t
> @@ -290,6 +290,7 @@
>     rev: versions differ -> m
>      preserving rev for resolve of rev
>    removing a
> +  updating: a 1/3 files (33.33%)
>    getting b
>    updating: b 2/3 files (66.67%)
>    updating: rev 3/3 files (100.00%)
> diff --git a/tests/test-update-reverse.t b/tests/test-update-reverse.t
> --- a/tests/test-update-reverse.t
> +++ b/tests/test-update-reverse.t
> @@ -73,6 +73,7 @@
>     main: remote created -> g
>    removing side1
>    removing side2
> +  updating: side2 2/3 files (66.67%)
>    getting main
>    updating: main 3/3 files (100.00%)
>    1 files updated, 0 files merged, 2 files removed, 0 files unresolved
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
> 

----------------------------------------------------------------------
[FUJIWARA Katsunori]                             foozy@lares.dti.ne.jp

Patch

diff --git a/mercurial/merge.py b/mercurial/merge.py
--- a/mercurial/merge.py
+++ b/mercurial/merge.py
@@ -455,8 +455,10 @@ 
 
     numupdates = len(actions)
     workeractions = [a for a in actions if a[1] in 'gr']
-    updated = len([a for a in workeractions if a[1] == 'g'])
-    removed = len([a for a in workeractions if a[1] == 'r'])
+    updateactions = [a for a in workeractions if a[1] == 'g']
+    updated = len(updateactions)
+    removeactions = [a for a in workeractions if a[1] == 'r']
+    removed = len(removeactions)
     actions = [a for a in actions if a[1] not in 'gr']
 
     hgsub = [a[1] for a in workeractions if a[0] == '.hgsubstate']
@@ -465,7 +467,13 @@ 
 
     z = 0
     prog = worker.worker(repo.ui, 0.001, getremove, (repo, mctx, overwrite),
-                         workeractions)
+                         removeactions)
+    for i, item in prog:
+        z += i
+        repo.ui.progress(_('updating'), z, item=item, total=numupdates,
+                         unit=_('files'))
+    prog = worker.worker(repo.ui, 0.001, getremove, (repo, mctx, overwrite),
+                         updateactions)
     for i, item in prog:
         z += i
         repo.ui.progress(_('updating'), z, item=item, total=numupdates,
diff --git a/tests/test-issue672.t b/tests/test-issue672.t
--- a/tests/test-issue672.t
+++ b/tests/test-issue672.t
@@ -37,6 +37,7 @@ 
    1: other deleted -> r
    1a: remote created -> g
   removing 1
+  updating: 1 1/2 files (50.00%)
   getting 1a
   updating: 1a 2/2 files (100.00%)
   1 files updated, 0 files merged, 1 files removed, 0 files unresolved
diff --git a/tests/test-rename-dir-merge.t b/tests/test-rename-dir-merge.t
--- a/tests/test-rename-dir-merge.t
+++ b/tests/test-rename-dir-merge.t
@@ -46,6 +46,7 @@ 
    b/b: remote created -> g
   removing a/a
   removing a/b
+  updating: a/b 2/5 files (40.00%)
   getting b/a
   getting b/b
   updating: b/b 4/5 files (80.00%)
diff --git a/tests/test-rename-merge2.t b/tests/test-rename-merge2.t
--- a/tests/test-rename-merge2.t
+++ b/tests/test-rename-merge2.t
@@ -290,6 +290,7 @@ 
    rev: versions differ -> m
     preserving rev for resolve of rev
   removing a
+  updating: a 1/3 files (33.33%)
   getting b
   updating: b 2/3 files (66.67%)
   updating: rev 3/3 files (100.00%)
diff --git a/tests/test-update-reverse.t b/tests/test-update-reverse.t
--- a/tests/test-update-reverse.t
+++ b/tests/test-update-reverse.t
@@ -73,6 +73,7 @@ 
    main: remote created -> g
   removing side1
   removing side2
+  updating: side2 2/3 files (66.67%)
   getting main
   updating: main 3/3 files (100.00%)
   1 files updated, 0 files merged, 2 files removed, 0 files unresolved