[Monotone-commits-diffs] net.venge.monotone.issue-209: 898bc317e5a1db50c15daa6038a4cecc6d4e8230

[code]

revision:            898bc317e5a1db50c15daa6038a4cecc6d4e8230
date:                2012-05-25T09:22:45
author:              address@hidden
branch:              net.venge.monotone.issue-209
changelog:
dropped_modified conflicts; more tests, better implementation, almost working

* src/merge_conflict.cc (read_dropped_modified_conflict): finish
  (resolve_dropped_modified_conflicts): drop the detached node

* src/merge_roster.cc (insert_if_unborn): create nodes with dropped_modified, 
so resolution can keep it.
  (roster_merge): don't attach nodes with dropped_modified conflicts

* src/merge_roster.hh (dropped_modified_conflict): add operator==

* test/func/resolve_conflicts_dropped_modified/__driver__.lua: test all three 
resolutions; almost working

* test/func/resolve_conflicts_dropped_modified/conflicts: renamed

* test/func/resolve_conflicts_dropped_modified/conflicts-resolved: renamed

manifest:
format_version "1"

new_manifest [1414821dd81aab8d41f43f4054917f1a17689dfb]

old_revision [bf033095aaea2a9278e5f869efb4dc3116ff5a12]

rename "test/func/resolve_conflicts_dropped_modified/conflicts-1"
    to "test/func/resolve_conflicts_dropped_modified/conflicts"

rename "test/func/resolve_conflicts_dropped_modified/conflicts-1-resolved"
    to "test/func/resolve_conflicts_dropped_modified/conflicts-resolved"

patch "src/merge_conflict.cc"
 from [9921b266cef59c6e2b3d3fd5fda16cf2c48fced1]
   to [15f8399fb683061a7f301d74f34d93af9d75dc98]

patch "src/merge_roster.cc"
 from [8f846a5a40e36dba86f99f8ed48b3579c3bbe494]
   to [2eadb8046723bafb4a41f36d439abccc68f3f2fd]

patch "src/merge_roster.hh"
 from [605b6f3f2a3e674e8542864523660ca86e797693]
   to [0756147ceb36ee1af0d2eb02371587b3c8a8ffe6]

patch "test/func/resolve_conflicts_dropped_modified/__driver__.lua"
 from [5cc8d92ebd6d5590401e902cfaccf8d8252c996a]
   to [bef29a13e2c5cd3b053e3b6905506a8e3341f68b]

patch "test/func/resolve_conflicts_dropped_modified/conflicts"
 from [f67001a294d492e4e3c0a2196e37c2a23077e27a]
   to [b791de2fc74adc9d489666b7ab097bb229dc4b5f]

patch "test/func/resolve_conflicts_dropped_modified/conflicts-resolved"
 from [ecb345496807dd3a509bc7f4ef751abf55c8233a]
   to [3e6671f4e719c9171bdd0cb9bbe9f4b5db57b2eb]
============================================================
--- src/merge_conflict.cc	9921b266cef59c6e2b3d3fd5fda16cf2c48fced1
+++ src/merge_conflict.cc	15f8399fb683061a7f301d74f34d93af9d75dc98
@@ -1896,11 +1896,16 @@ read_dropped_modified_conflict(basic_io:
           conflict.resolution.first = resolve_conflicts::drop;
           pars.sym();
         }
-      else if (pars.symp (syms::resolved_rename_left))
+      else if (pars.symp (syms::resolved_keep_left))
         {
-          conflict.resolution.first = resolve_conflicts::rename;
+          conflict.resolution.first = resolve_conflicts::keep;
           pars.sym();
-          conflict.resolution.second = new_optimal_path(pars.token, true);
+        }
+      else if (pars.symp (syms::resolved_user_left))
+        {
+          conflict.resolution.first = resolve_conflicts::content_user;
+          pars.sym();
+          conflict.resolution.second = new_optimal_path(pars.token, false);
           pars.str();
         }
       else
@@ -2649,7 +2654,7 @@ roster_merge_result::resolve_dropped_mod
         case resolve_conflicts::drop:
           P(F("dropping '%s'") % modified_name);
 
-          // already dropped; nothing to do
+          roster.drop_detached_node(nid);
           break;
 
         case resolve_conflicts::keep:
============================================================
--- src/merge_roster.cc	8f846a5a40e36dba86f99f8ed48b3579c3bbe494
+++ src/merge_roster.cc	2eadb8046723bafb4a41f36d439abccc68f3f2fd
@@ -344,33 +344,31 @@ namespace
       create_node_for(n, result.roster);
     else
       {
-        // In this branch we are NOT inserting the node into the new roster as it
-        // has been deleted from the other side of the merge.
-        // In this case, create a conflict if there are changes to the file on the
-        // side of the merge where it still exists.
+        // The node has been deleted from the other side of the merge. If
+        // there are changes to the file on this side of the merge, insert
+        // it into the new roster, but leave it detached, so the conflict
+        // resolutions can deal with it easily. Note that attaching is done
+        // later; see roster_merge below.
         set<revision_id> const & content_marks = m->file_content;
-        bool found_one_ignored_content = false;
         for (set<revision_id>::const_iterator it = content_marks.begin();
-             (!found_one_ignored_content) && (it != content_marks.end());
+             it != content_marks.end();
              it++)
           {
             if (uncommon_ancestors.find(*it) != uncommon_ancestors.end())
               {
-                if (!found_one_ignored_content)
+                create_node_for(n, result.roster);
+                switch (present_in)
                   {
-                    switch (present_in)
-                    {
-                    case left_side:
-                      result.dropped_modified_conflicts.push_back
-                        (dropped_modified_conflict(n->self, the_null_node));
-                      break;
-                    case right_side:
-                      result.dropped_modified_conflicts.push_back
-                        (dropped_modified_conflict(the_null_node, n->self));
-                      break;
-                    }
+                  case left_side:
+                    result.dropped_modified_conflicts.push_back
+                      (dropped_modified_conflict(n->self, the_null_node));
+                    break;
+                  case right_side:
+                    result.dropped_modified_conflicts.push_back
+                      (dropped_modified_conflict(the_null_node, n->self));
+                    break;
                   }
-                found_one_ignored_content = true;
+                return;
               }
           }
       }
@@ -579,13 +577,20 @@ roster_merge(roster_t const & left_paren
             {
               node_t const & left_n = i.left_data();
               // we skip nodes that aren't in the result roster (were
-              // deleted in the lifecycles step above)
+              // deleted in the lifecycles step above), or that have
+              // dropped_modified conflicts.
               if (result.roster.has_node(left_n->self))
                 {
-                  // attach this node from the left roster. this may cause
-                  // a name collision with the previously attached node from
-                  // the other side of the merge.
-                  copy_node_forward(result, new_i->second, left_n, left_side);
+                  if (result.dropped_modified_conflicts.end() ==
+                      find(result.dropped_modified_conflicts.begin(),
+                           result.dropped_modified_conflicts.end(),
+                           left_n->self))
+                    {
+                      // attach this node from the left roster. this may cause
+                      // a name collision with the previously attached node from
+                      // the other side of the merge.
+                      copy_node_forward(result, new_i->second, left_n, left_side);
+                    }
                   ++new_i;
                 }
               ++left_mi;
@@ -595,13 +600,20 @@ roster_merge(roster_t const & left_paren
           case parallel::in_right:
             {
               node_t const & right_n = i.right_data();
-              // we skip nodes that aren't in the result roster
+              // we skip nodes that aren't in the result roster, or that have
+              // dropped_modified conflicts.
               if (result.roster.has_node(right_n->self))
                 {
-                  // attach this node from the right roster. this may cause
-                  // a name collision with the previously attached node from
-                  // the other side of the merge.
-                  copy_node_forward(result, new_i->second, right_n, right_side);
+                  if (result.dropped_modified_conflicts.end() ==
+                      find(result.dropped_modified_conflicts.begin(),
+                           result.dropped_modified_conflicts.end(),
+                           right_n->self))
+                    {
+                      // attach this node from the right roster. this may cause
+                      // a name collision with the previously attached node from
+                      // the other side of the merge.
+                      copy_node_forward(result, new_i->second, right_n, right_side);
+                    }
                   ++new_i;
                 }
               ++right_mi;
============================================================
--- src/merge_roster.hh	605b6f3f2a3e674e8542864523660ca86e797693
+++ src/merge_roster.hh	0756147ceb36ee1af0d2eb02371587b3c8a8ffe6
@@ -99,6 +99,8 @@ struct dropped_modified_conflict
 
   dropped_modified_conflict(node_id left_nid, node_id right_nid) : left_nid(left_nid), right_nid(right_nid)
   {resolution.first = resolve_conflicts::none;}
+
+  bool operator==(node_id n) {return left_nid == n || right_nid == n;}
 };
 
 // this is when two distinct nodes want to have the same name.  these nodes
============================================================
--- test/func/resolve_conflicts_dropped_modified/__driver__.lua	5cc8d92ebd6d5590401e902cfaccf8d8252c996a
+++ test/func/resolve_conflicts_dropped_modified/__driver__.lua	bef29a13e2c5cd3b053e3b6905506a8e3341f68b
@@ -1,62 +1,104 @@
 -- Test reporting and resolving drop/modified conflicts
---
--- this is currently not supported; we are documenting a test case
--- that must be considered when implementing it.
 
 mtn_setup()
 
--- Create conflict; modify and rename file in one head, drop in other
+-- Create conflicts; modify and rename file in one head, drop in
+-- other.
+-- 
+-- Three conflicts to test the three possible resolutions, and
+-- left/right swap.
+
 addfile("foo", "foo base")
+addfile("file_2", "file_2 base")
+addfile("file_3", "file_3 base")
 commit("testbranch", "base")
 base = base_revision()
 
 writefile("foo", "foo left")
 check(mtn("mv", "foo", "bar"), 0, false, false)
 
+writefile("file_2", "file_2 left")
+writefile("file_3", "file_3 left")
+
 commit("testbranch", "left 1")
 left_1 = base_revision()
 
+check(mtn("drop", "file_3"), 0, false, false)
+commit("testbranch", "left 2")
+left_2 = base_revision()
+
 revert_to(base)
 
 writefile("foo", "foo right")
+writefile("file_2", "file_2 right")
+writefile("file_3", "file_3 right")
+
 commit("testbranch", "right 1")
 right_1 = base_revision()
 
 check(mtn("drop", "foo"), 0, false, false)
+check(mtn("drop", "file_2"), 0, false, false)
 commit("testbranch", "right 2")
 right_2 = base_revision()
 
-check(mtn("conflicts", "store"), 0, nil, true)
+
+check(mtn("conflicts", "store", left_2, right_2), 0, nil, true)
 check(samelines("stderr",
-{"mtn: 1 conflict with supported resolutions.",
+{"mtn: 3 conflicts with supported resolutions.",
  "mtn: stored in '_MTN/conflicts'"}))
 
 canonicalize("_MTN/conflicts")
-check(samefilestd("conflicts-1", "_MTN/conflicts"))
+check(samefilestd("conflicts", "_MTN/conflicts"))
 
 check(mtn("conflicts", "show_first"), 0, nil, true)
 check(samelines("stderr",
-{"mtn: conflict: file 'bar'",
- "mtn: dropped on the left",
- "mtn: modified on the right",
+{"mtn: conflict: file 'file_2'",
+ "mtn: modified on the left",
+ "mtn: dropped on the right",
  "mtn: possible resolutions:",
  "mtn: resolve_first drop",
  "mtn: resolve_first keep",
  "mtn: resolve_first user \"name\""}))
 
+check(mtn("conflicts", "resolve_first", "keep"), 0, nil, true)
+
+-- check for nice error message if not all conflicts are resolved
 check(mtn("merge", "--resolve-conflicts"), 1, nil, true)
 check(qgrep("no resolution provided for", "stderr"))
              
+check(mtn("conflicts", "show_first"), 0, nil, true)
+check(samelines("stderr",
+{"mtn: conflict: file 'file_3'",
+ "mtn: dropped on the left",
+ "mtn: modified on the right",
+ "mtn: possible resolutions:",
+ "mtn: resolve_first drop",
+ "mtn: resolve_first keep",
+ "mtn: resolve_first user \"name\""}))
+
+mkdir("_MTN/resolutions")
+writefile("_MTN/resolutions/file_3_resolved", "file_3 resolved")
+check(mtn("conflicts", "resolve_first", "user", "file_3_resolved"), 0, nil, true)
+
+check(mtn("conflicts", "show_first"), 0, nil, true)
+check(samelines("stderr",
+{"mtn: conflict: file 'bar'",
+ "mtn: modified on the left",
+ "mtn: dropped on the right",
+ "mtn: possible resolutions:",
+ "mtn: resolve_first drop",
+ "mtn: resolve_first keep",
+ "mtn: resolve_first user \"name\""}))
+
 check(mtn("conflicts", "resolve_first", "drop"), 0, nil, true)
 
 canonicalize("_MTN/conflicts")
-check(samefilestd("conflicts-1-resolved", "_MTN/conflicts"))
-
+check(samefilestd("conflicts-resolved", "_MTN/conflicts"))
 check(mtn("merge", "--resolve-conflicts"), 0, nil, true)
 check(qgrep("dropping 'bar'", "stderr"))
+check(qgrep("keeping 'file_2'", "stderr"))
+check(qgrep("replacing 'file_3' with '_MTN/resolutions/file_3_resolved", "stderr"))
 
--- FIXME: do three files at once: swap left/right, resolve keep; resolve user
-
 -- FIXME: add dropped_modified to 'show_conflicts' test (etc?)
                            
 -- end of file
============================================================
--- test/func/resolve_conflicts_dropped_modified/conflicts-1	f67001a294d492e4e3c0a2196e37c2a23077e27a
+++ test/func/resolve_conflicts_dropped_modified/conflicts	b791de2fc74adc9d489666b7ab097bb229dc4b5f
@@ -1,11 +1,27 @@
-    left [92ea0d12182ab4761f8fe56a103aed9d65aff68d]
-   right [f5b4b65976746d1a2aa203ceaa67701353cc4172]
-ancestor [871b706565c414aa20f58f5f6db56778d64ece88]
+    left [2e6a6ed9d74c90369818dd6e8f0c99244243b05d]
+   right [c24409a7b849994b403befa5ba9bccb0b0208aa4]
+ancestor [5737187332315f436eab91e5d3fafd966c57c330]
 
         conflict dropped_modified
+   ancestor_name "file_2"
+ancestor_file_id [4fd0fa24812427ee6c13a839d2a90bc0c6fc0091]
+       left_type "modified file"
+       left_name "file_2"
+    left_file_id [afbd804a8c606a93f9d8bc0fdacc1db9f34b4548]
+      right_type "dropped file"
+
+        conflict dropped_modified
+   ancestor_name "file_3"
+ancestor_file_id [311aac8e6f1fb6fca84da5153aa6d5a1c6faff79]
+       left_type "dropped file"
+      right_type "modified file"
+      right_name "file_3"
+   right_file_id [da7ea65160c9c92f4ed120568229342fe7daa924]
+
+        conflict dropped_modified
    ancestor_name "foo"
 ancestor_file_id [b02b6245b4b750465e12f65148d5369536b1b780]
-       left_type "dropped file"
-      right_type "modified file"
-      right_name "bar"
-   right_file_id [abd7d750f84aafa7c0b1f26540e2da28095bc082]
+       left_type "modified file"
+       left_name "bar"
+    left_file_id [abd7d750f84aafa7c0b1f26540e2da28095bc082]
+      right_type "dropped file"
============================================================
--- test/func/resolve_conflicts_dropped_modified/conflicts-1-resolved	ecb345496807dd3a509bc7f4ef751abf55c8233a
+++ test/func/resolve_conflicts_dropped_modified/conflicts-resolved	3e6671f4e719c9171bdd0cb9bbe9f4b5db57b2eb
@@ -1,12 +1,30 @@
-    left [92ea0d12182ab4761f8fe56a103aed9d65aff68d]
-   right [f5b4b65976746d1a2aa203ceaa67701353cc4172]
-ancestor [871b706565c414aa20f58f5f6db56778d64ece88]
+    left [2e6a6ed9d74c90369818dd6e8f0c99244243b05d]
+   right [c24409a7b849994b403befa5ba9bccb0b0208aa4]
+ancestor [5737187332315f436eab91e5d3fafd966c57c330]
 
           conflict dropped_modified
-     ancestor_name "foo"
-  ancestor_file_id [b02b6245b4b750465e12f65148d5369536b1b780]
+     ancestor_name "file_2"
+  ancestor_file_id [4fd0fa24812427ee6c13a839d2a90bc0c6fc0091]
+         left_type "modified file"
+         left_name "file_2"
+      left_file_id [afbd804a8c606a93f9d8bc0fdacc1db9f34b4548]
+        right_type "dropped file"
+resolved_keep_left
+
+          conflict dropped_modified
+     ancestor_name "file_3"
+  ancestor_file_id [311aac8e6f1fb6fca84da5153aa6d5a1c6faff79]
          left_type "dropped file"
         right_type "modified file"
-        right_name "bar"
-     right_file_id [abd7d750f84aafa7c0b1f26540e2da28095bc082]
-resolved_drop_left 
+        right_name "file_3"
+     right_file_id [da7ea65160c9c92f4ed120568229342fe7daa924]
+resolved_user_left "_MTN/resolutions/file_3_resolved"
+
+          conflict dropped_modified
+     ancestor_name "foo"
+  ancestor_file_id [b02b6245b4b750465e12f65148d5369536b1b780]
+         left_type "modified file"
+         left_name "bar"
+      left_file_id [abd7d750f84aafa7c0b1f26540e2da28095bc082]
+        right_type "dropped file"
+resolved_drop_left