Patchwork convert: fix syncing deletes from p2 merge commit

login
register
mail settings
Submitter Durham Goode
Date Aug. 25, 2015, 11:51 p.m.
Message ID <1dbd7cfe255e4b26c463.1440546679@dev2000.prn2.facebook.com>
Download mbox | patch
Permalink /patch/10279/
State Accepted
Headers show

Comments

Durham Goode - Aug. 25, 2015, 11:51 p.m.
# HG changeset patch
# User Durham Goode <durham@fb.com>
# Date 1440543273 25200
#      Tue Aug 25 15:54:33 2015 -0700
# Node ID 1dbd7cfe255e4b26c4633685e28d0a13ab1d04b5
# Parent  05e7f57c74ac5b556b49870af86f61aa0c54babb
convert: fix syncing deletes from p2 merge commit

Recently we fixed converting merges to correctly sync changes from p2. We missed
the case of deletes though (so p2 deleted a file that p1 had not yet deleted,
and the file does not belong to the source).

The fix is to detect when p2 doesn't have the file, so we just sync it as a
delete to p1 in the merge.

Updated the test, and verified it failed before the fix.
Augie Fackler - Aug. 27, 2015, 3:19 p.m.
On Tue, Aug 25, 2015 at 04:51:19PM -0700, Durham Goode wrote:
> # HG changeset patch
> # User Durham Goode <durham@fb.com>
> # Date 1440543273 25200
> #      Tue Aug 25 15:54:33 2015 -0700
> # Node ID 1dbd7cfe255e4b26c4633685e28d0a13ab1d04b5
> # Parent  05e7f57c74ac5b556b49870af86f61aa0c54babb
> convert: fix syncing deletes from p2 merge commit

Queued. I only mostly understand it, but the fix seems clearly-right
enough I'll just take it. Thanks!
>
> Recently we fixed converting merges to correctly sync changes from p2. We missed
> the case of deletes though (so p2 deleted a file that p1 had not yet deleted,
> and the file does not belong to the source).
>
> The fix is to detect when p2 doesn't have the file, so we just sync it as a
> delete to p1 in the merge.
>
> Updated the test, and verified it failed before the fix.
>
> diff --git a/hgext/convert/hg.py b/hgext/convert/hg.py
> --- a/hgext/convert/hg.py
> +++ b/hgext/convert/hg.py
> @@ -223,7 +223,12 @@ class mercurial_sink(converter_sink):
>          def getfilectx(repo, memctx, f):
>              if p2ctx and f in p2files and f not in copies:
>                  self.ui.debug('reusing %s from p2\n' % f)
> -                return p2ctx[f]
> +                try:
> +                    return p2ctx[f]
> +                except error.ManifestLookupError:
> +                    # If the file doesn't exist in p2, then we're syncing a
> +                    # delete, so just return None.
> +                    return None
>              try:
>                  v = files[f]
>              except KeyError:
> diff --git a/tests/test-convert-filemap.t b/tests/test-convert-filemap.t
> --- a/tests/test-convert-filemap.t
> +++ b/tests/test-convert-filemap.t
> @@ -677,34 +677,39 @@ test converting merges into a repo that
>
>    $ hg init merge-test1
>    $ cd merge-test1
> -  $ touch a && hg commit -Aqm a
> -  $ hg up -q null
> -  $ touch b && hg commit -Aqm b
> -  $ hg merge -q 0 && hg commit -qm merge
> +  $ touch a && hg commit -Aqm 'add a'
> +  $ echo a > a && hg commit -Aqm 'edit a'
> +  $ hg up -q 0
> +  $ touch b && hg commit -Aqm 'add b'
> +  $ hg merge -q 1 && hg commit -qm 'merge a & b'
> +
>    $ cd ..
>    $ hg init merge-test2
>    $ cd merge-test2
>    $ mkdir converted
> -  $ touch converted/a && hg commit -Aqm 'a'
> -  $ touch x && hg commit -Aqm 'x'
> +  $ touch converted/a toberemoved && hg commit -Aqm 'add converted/a & toberemoved'
> +  $ touch x && rm toberemoved && hg commit -Aqm 'add x & remove tobremoved'
>    $ cd ..
> -  $ hg log -G -T '{node}' -R merge-test1
> -  @    ea7c1a7ae9588677a715ce4f204cd89c28d5471f
> +  $ hg log -G -T '{shortest(node)} {desc}' -R merge-test1
> +  @    1191 merge a & b
>    |\
> -  | o  d7486e00c6f1b633dcadc0582f78006d805c7a0f
> +  | o  9077 add b
> +  | |
> +  o |  d19f edit a
> +  |/
> +  o  ac82 add a
> +
> +  $ hg log -G -T '{shortest(node)} {desc}' -R merge-test2
> +  @  150e add x & remove tobremoved
>    |
> -  o  3903775176ed42b1458a6281db4a0ccf4d9f287a
> -
> -  $ hg log -G -T '{node}' -R merge-test2
> -  @  34f1aa7da42559bae87920880b522d47b3ddbc0d
> -  |
> -  o  e01a12b07b4fdfd61ff90a2a1b4560a7a776f323
> +  o  bbac add converted/a & toberemoved
>
>  - Build a shamap where the target converted/a is in on top of an unrelated
>  - change to 'x'. This simulates using convert to merge several repositories
>  - together.
>    $ cat >> merge-test2/.hg/shamap <<EOF
> -  > 3903775176ed42b1458a6281db4a0ccf4d9f287a 34f1aa7da42559bae87920880b522d47b3ddbc0d
> +  > $(hg -R merge-test1 log -r 0 -T '{node}') $(hg -R merge-test2 log -r 0 -T '{node}')
> +  > $(hg -R merge-test1 log -r 1 -T '{node}') $(hg -R merge-test2 log -r 1 -T '{node}')
>    > EOF
>    $ cat >> merge-test-filemap <<EOF
>    > rename . converted/
> @@ -713,19 +718,26 @@ test converting merges into a repo that
>    scanning source...
>    sorting...
>    converting...
> -  1 b
> -  0 merge
> +  1 add b
> +  0 merge a & b
>    $ hg -R merge-test2 manifest -r tip
>    converted/a
>    converted/b
>    x
> -  $ hg -R merge-test2 log -G -T '{node}\n{files % "{file}\n"}'
> -  o    4b5e2f0218d3442a0c14892b18685bf9c8059c4a
> -  |\
> -  | o  214325dd2e4cff981dcf00cb120cd39e1ea36dcc
> -  |    converted/b
> -  @  34f1aa7da42559bae87920880b522d47b3ddbc0d
> -  |  x
> -  o  e01a12b07b4fdfd61ff90a2a1b4560a7a776f323
> -     converted/a
> +  $ hg -R merge-test2 log -G -T '{shortest(node)} {desc}\n{files % "- {file}\n"}\n'
> +  o    6eaa merge a & b
> +  |\   - converted/a
> +  | |  - toberemoved
> +  | |
> +  | o  2995 add b
> +  | |  - converted/b
> +  | |
> +  @ |  150e add x & remove tobremoved
> +  |/   - toberemoved
> +  |    - x
> +  |
> +  o  bbac add converted/a & toberemoved
> +     - converted/a
> +     - toberemoved
> +
>
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> https://selenic.com/mailman/listinfo/mercurial-devel

Patch

diff --git a/hgext/convert/hg.py b/hgext/convert/hg.py
--- a/hgext/convert/hg.py
+++ b/hgext/convert/hg.py
@@ -223,7 +223,12 @@  class mercurial_sink(converter_sink):
         def getfilectx(repo, memctx, f):
             if p2ctx and f in p2files and f not in copies:
                 self.ui.debug('reusing %s from p2\n' % f)
-                return p2ctx[f]
+                try:
+                    return p2ctx[f]
+                except error.ManifestLookupError:
+                    # If the file doesn't exist in p2, then we're syncing a
+                    # delete, so just return None.
+                    return None
             try:
                 v = files[f]
             except KeyError:
diff --git a/tests/test-convert-filemap.t b/tests/test-convert-filemap.t
--- a/tests/test-convert-filemap.t
+++ b/tests/test-convert-filemap.t
@@ -677,34 +677,39 @@  test converting merges into a repo that 
 
   $ hg init merge-test1
   $ cd merge-test1
-  $ touch a && hg commit -Aqm a
-  $ hg up -q null
-  $ touch b && hg commit -Aqm b
-  $ hg merge -q 0 && hg commit -qm merge
+  $ touch a && hg commit -Aqm 'add a'
+  $ echo a > a && hg commit -Aqm 'edit a'
+  $ hg up -q 0
+  $ touch b && hg commit -Aqm 'add b'
+  $ hg merge -q 1 && hg commit -qm 'merge a & b'
+
   $ cd ..
   $ hg init merge-test2
   $ cd merge-test2
   $ mkdir converted
-  $ touch converted/a && hg commit -Aqm 'a'
-  $ touch x && hg commit -Aqm 'x'
+  $ touch converted/a toberemoved && hg commit -Aqm 'add converted/a & toberemoved'
+  $ touch x && rm toberemoved && hg commit -Aqm 'add x & remove tobremoved'
   $ cd ..
-  $ hg log -G -T '{node}' -R merge-test1
-  @    ea7c1a7ae9588677a715ce4f204cd89c28d5471f
+  $ hg log -G -T '{shortest(node)} {desc}' -R merge-test1
+  @    1191 merge a & b
   |\
-  | o  d7486e00c6f1b633dcadc0582f78006d805c7a0f
+  | o  9077 add b
+  | |
+  o |  d19f edit a
+  |/
+  o  ac82 add a
+  
+  $ hg log -G -T '{shortest(node)} {desc}' -R merge-test2
+  @  150e add x & remove tobremoved
   |
-  o  3903775176ed42b1458a6281db4a0ccf4d9f287a
-  
-  $ hg log -G -T '{node}' -R merge-test2
-  @  34f1aa7da42559bae87920880b522d47b3ddbc0d
-  |
-  o  e01a12b07b4fdfd61ff90a2a1b4560a7a776f323
+  o  bbac add converted/a & toberemoved
   
 - Build a shamap where the target converted/a is in on top of an unrelated
 - change to 'x'. This simulates using convert to merge several repositories
 - together.
   $ cat >> merge-test2/.hg/shamap <<EOF
-  > 3903775176ed42b1458a6281db4a0ccf4d9f287a 34f1aa7da42559bae87920880b522d47b3ddbc0d
+  > $(hg -R merge-test1 log -r 0 -T '{node}') $(hg -R merge-test2 log -r 0 -T '{node}')
+  > $(hg -R merge-test1 log -r 1 -T '{node}') $(hg -R merge-test2 log -r 1 -T '{node}')
   > EOF
   $ cat >> merge-test-filemap <<EOF
   > rename . converted/
@@ -713,19 +718,26 @@  test converting merges into a repo that 
   scanning source...
   sorting...
   converting...
-  1 b
-  0 merge
+  1 add b
+  0 merge a & b
   $ hg -R merge-test2 manifest -r tip
   converted/a
   converted/b
   x
-  $ hg -R merge-test2 log -G -T '{node}\n{files % "{file}\n"}'
-  o    4b5e2f0218d3442a0c14892b18685bf9c8059c4a
-  |\
-  | o  214325dd2e4cff981dcf00cb120cd39e1ea36dcc
-  |    converted/b
-  @  34f1aa7da42559bae87920880b522d47b3ddbc0d
-  |  x
-  o  e01a12b07b4fdfd61ff90a2a1b4560a7a776f323
-     converted/a
+  $ hg -R merge-test2 log -G -T '{shortest(node)} {desc}\n{files % "- {file}\n"}\n'
+  o    6eaa merge a & b
+  |\   - converted/a
+  | |  - toberemoved
+  | |
+  | o  2995 add b
+  | |  - converted/b
+  | |
+  @ |  150e add x & remove tobremoved
+  |/   - toberemoved
+  |    - x
+  |
+  o  bbac add converted/a & toberemoved
+     - converted/a
+     - toberemoved
+