Patchwork [2,of,2] manifestmerge: handle workdir removed, remote removed with flags

login
register
mail settings
Submitter Siddharth Agarwal
Date April 10, 2013, 6:21 p.m.
Message ID <f3cd655a54d1f8532835.1365618119@sid0x220>
Download mbox | patch
Permalink /patch/1268/
State Superseded, archived
Headers show

Comments

Siddharth Agarwal - April 10, 2013, 6:21 p.m.
# HG changeset patch
# User Siddharth Agarwal <sid0@fb.com>
# Date 1365618000 25200
#      Wed Apr 10 11:20:00 2013 -0700
# Node ID f3cd655a54d1f8532835bfe8ea3074b528b6542f
# Parent  645bee3e5241dd659dae1c9bcc68f208484d6eb0
manifestmerge: handle workdir removed, remote removed with flags

This can happen when a file with flags is removed or deleted in the working
directory and also not present in m2. The obvious solution is to add a
__delitem__ override to manifestdict that removes the file from flags if
necessary, but that has a significant performance cost in some cases, e.g.
hg status --rev rev1 --rev rev2 <file>.
Bryan O'Sullivan - April 10, 2013, 6:32 p.m.
On Wed, Apr 10, 2013 at 11:21 AM, Siddharth Agarwal <sid0@fb.com> wrote:

> +            if not n1:
> +                # Since n1 == n2, the file isn't present in m2 either.
> This
> +                # means that the file was removed or deleted locally and
> +                # removed remotely, but that residual entries remain in
> flags.
> +                # This can happen in manifests generated by workingctx.
>

In your previous patch, you mentioned "we can legitimately have
manifestdict's _flags set to ''", but here you're doing one of those Python
truthy-false tests that will pass if n1 is either '' or None. That's too
clever for me - I have no idea what expectations I should have of this
code's behaviour.
Siddharth Agarwal - April 10, 2013, 6:34 p.m.
On 04/10/2013 11:32 AM, Bryan O'Sullivan wrote:
> In your previous patch, you mentioned "we can legitimately have 
> manifestdict's _flags set to ''", but here you're doing one of those 
> Python truthy-false tests that will pass if n1 is either '' or None. 
> That's too clever for me - I have no idea what expectations I should 
> have of this code's behaviour.

n1 is a node, not a flag. I see where the confusion is coming from 
though, and I agree that it probably makes sense to use "if n1 is not 
None" here.
Bryan O'Sullivan - April 10, 2013, 6:49 p.m.
On Wed, Apr 10, 2013 at 11:34 AM, Siddharth Agarwal <sid0@fb.com> wrote:

> n1 is a node, not a flag. I see where the confusion is coming from though,
> and I agree that it probably makes sense to use "if n1 is not None" here.
>
>
OK, will await a resend, then.

Patch

diff -r 645bee3e5241 -r f3cd655a54d1 mercurial/merge.py
--- a/mercurial/merge.py	Wed Apr 10 10:49:20 2013 -0700
+++ b/mercurial/merge.py	Wed Apr 10 11:20:00 2013 -0700
@@ -246,7 +246,13 @@  def manifestmerge(repo, wctx, p2, pa, br
         if n12:
             n1, n2 = n12
         else: # file contents didn't change, but flags did
-            n1 = n2 = m1[f]
+            n1 = n2 = m1.get(f, None)
+            if not n1:
+                # Since n1 == n2, the file isn't present in m2 either. This
+                # means that the file was removed or deleted locally and
+                # removed remotely, but that residual entries remain in flags.
+                # This can happen in manifests generated by workingctx.
+                continue
         if fl12:
             fl1, fl2 = fl12
         else: # flags didn't change, file contents did
diff -r 645bee3e5241 -r f3cd655a54d1 tests/test-update-issue1456.t
--- a/tests/test-update-issue1456.t	Wed Apr 10 10:49:20 2013 -0700
+++ b/tests/test-update-issue1456.t	Wed Apr 10 11:20:00 2013 -0700
@@ -6,9 +6,16 @@ 
 
   $ echo foo > foo
   $ hg ci -qAm0
-  $ chmod +x foo
-  $ hg ci -m1
+  $ echo toremove > toremove
+  $ echo todelete > todelete
+  $ chmod +x foo toremove todelete
+  $ hg ci -qAm1
+
+Test that local removed/deleted, remote removed works with flags
+  $ hg rm toremove
+  $ rm todelete
   $ hg co -q 0
+
   $ echo dirty > foo
   $ hg up -c
   abort: uncommitted local changes
@@ -18,11 +25,13 @@ 
   dirty
   $ hg st -A
   M foo
+  C todelete
+  C toremove
 
 Validate update of standalone execute bit change:
 
   $ hg up -C 0
-  1 files updated, 0 files merged, 0 files removed, 0 files unresolved
+  1 files updated, 0 files merged, 2 files removed, 0 files unresolved
   $ chmod -x foo
   $ hg ci -m removeexec
   nothing changed
@@ -30,7 +39,7 @@  Validate update of standalone execute bi
   $ hg up -C 0
   0 files updated, 0 files merged, 0 files removed, 0 files unresolved
   $ hg up
-  1 files updated, 0 files merged, 0 files removed, 0 files unresolved
+  3 files updated, 0 files merged, 0 files removed, 0 files unresolved
   $ hg st
 
   $ cd ..