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

[code]

revision:            fe35a2d7a47e7e05ca1b06b962a2c40c87086463
date:                2012-07-21T23:14:40
author:              address@hidden
branch:              net.venge.monotone.issue-209
changelog:
resolve_conflicts_dropped_modified: more tests passing

* src/merge_conflict.cc (resolve_dropped_modified_one): fix bug

* src/merge_roster.cc (roster_merge): fix bugs. detach recreated 
dropped_modified node

* src/roster.cc (dump old_locations): new
  (check_sane): MM(old_locations), add comment

* test/func/resolve_conflicts_dropped_modified_1/__driver__.lua: match code 
changes

manifest:
format_version "1"

new_manifest [0210d55d0fd7ab5b1148c6dd61b00e66f1f2391a]

old_revision [23435fa4cb449a54601998a3ac8cb989ec6668c6]

patch "src/merge_conflict.cc"
 from [9eaf4ac857852fa7bd65251c9202aff97b5b91d6]
   to [37b47f167e03c72b9279f362b7fceb435b29150b]

patch "src/merge_roster.cc"
 from [a082529e5a0fced55068a65a67bc6f21074d6579]
   to [d5e74a871f0c4c22281c1e663b9498d73531225f]

patch "src/roster.cc"
 from [82bdb78ac2f38369f5972510e6ca5bcc3d0cdec5]
   to [b0608d6ec3e3f2d23a5c97bb64c4354eeb01773a]

patch "test/func/resolve_conflicts_dropped_modified_1/__driver__.lua"
 from [0bbd4c665fb269d56291f256f3480ff74ea7a3d5]
   to [70accbfc43f720b90015d36f7df006a46d473351]
============================================================
--- src/merge_conflict.cc	9eaf4ac857852fa7bd65251c9202aff97b5b91d6
+++ src/merge_conflict.cc	37b47f167e03c72b9279f362b7fceb435b29150b
@@ -2955,6 +2955,7 @@ resolve_dropped_modified_one(lua_hooks &
         {
           // recreated; replace the contents of the recreated node
           replace_content(side_roster, side_image, nid, result_roster, resolution.content, adaptor);
+          attach_node(lua, result_roster, nid, name);
         }
       else
         {
@@ -2975,7 +2976,7 @@ resolve_dropped_modified_one(lua_hooks &
 
     case resolve_conflicts::drop:
       // The node is either modified, recreated or duplicate name; in
-      // any case, it is present in the result roster, so drop it
+      // any case, it is present but detached in the result roster, so drop it
       P(F("dropping '%s' from %s") % name % side_image);
       result_roster.drop_detached_node(nid);
       break;
============================================================
--- src/merge_roster.cc	a082529e5a0fced55068a65a67bc6f21074d6579
+++ src/merge_roster.cc	d5e74a871f0c4c22281c1e663b9498d73531225f
@@ -818,18 +818,21 @@ roster_merge(roster_t const & left_paren
     I(new_i == result.roster.all_nodes().end());
   }
 
-  // now we can look for dropped_modified conflicts with recreated nodes
+  // now we can look for dropped_modified conflicts with recreated nodes or
+  // duplicate names
   for (size_t i = 0; i < result.dropped_modified_conflicts.size(); ++i)
     {
       dropped_modified_conflict & conflict = result.dropped_modified_conflicts[i];
 
-      // If the file name was recreated it may now subject to a
-      // duplicate_name conflict (see
+      // If the file name was recreated, it is present in the result with
+      // the modified_name but different node id; find that and unattach it.
+      // Or, it may now subject to a duplicate_name conflict (see
       // test/func/resolve_conflicts_dropped_modified_upstream_vs_local_2).
       // In which case modified_name will be present in the other parent,
       // but not in the result.
 
       file_path modified_name;
+      node_id   nid;
       bool duplicate_name = false;
 
       switch (conflict.dropped_side)
@@ -837,31 +840,46 @@ roster_merge(roster_t const & left_paren
         case resolve_conflicts::left_side:
           right_parent.get_name(conflict.right_nid, modified_name);
 
-          if (left_parent.has_node (modified_name))
+          if (result.roster.has_node(modified_name))
             {
-              conflict.left_nid = left_parent.get_node(modified_name)->self;
-              duplicate_name = true;
+              // recreated; we need to detach the node for the conflict
+              // resolution process. We'd like to just do:
+              // result.roster.detach_node(modified_name);
+              // but that doesn't erase result.roster.old_locations properly
+
+              const_node_t const & left_node      = left_parent.get_node(modified_name);
+              node_id              parent         = left_node->parent;
+              path_component       component_name = left_node->name;
+              dir_t                p              = downcast_to_dir_t(result.roster.get_node_for_update(parent));
+              conflict.left_nid                   = left_node->self;
+              p->detach_child(component_name);
             }
-          else if (result.roster.has_node(modified_name))
+          else if (left_parent.has_node (modified_name))
             {
-              // recreated
-              conflict.left_nid = result.roster.get_node(modified_name)->self;
+              conflict.left_nid = left_parent.get_node(modified_name)->self;
+              nid               = conflict.left_nid;
+              duplicate_name    = true;
             }
           break;
 
         case resolve_conflicts::right_side:
           left_parent.get_name(conflict.left_nid, modified_name);
 
-          if (right_parent.has_node (modified_name))
+          if (result.roster.has_node(modified_name))
             {
-              conflict.right_nid = right_parent.get_node(modified_name)->self;
-              duplicate_name = true;
+              // recreated; see comment in left_side above
+              const_node_t const & right_node     = right_parent.get_node(modified_name);
+              node_id              parent         = right_node->parent;
+              path_component       component_name = right_node->name;
+              dir_t                p              = downcast_to_dir_t(result.roster.get_node_for_update(parent));
+              conflict.right_nid                  = right_node->self;
+              p->detach_child(component_name);
             }
-
-          else if (result.roster.has_node(modified_name))
+          else if (right_parent.has_node (modified_name))
             {
-              // recreated
-              conflict.right_nid = result.roster.get_node(modified_name)->self;
+              conflict.right_nid = right_parent.get_node(modified_name)->self;
+              nid                = conflict.right_nid;
+              duplicate_name     = true;
             }
           break;
         }
@@ -869,17 +887,12 @@ roster_merge(roster_t const & left_paren
       if (duplicate_name)
         {
           // delete the duplicate name conflict; it will be handled by dropped_modified.
-          for (std::vector<duplicate_name_conflict>::iterator i = result.duplicate_name_conflicts.begin();
-               i != result.duplicate_name_conflicts.end();
-               ++i)
-            {
-              duplicate_name_conflict & dn_conf = *i;
-              if (dn_conf.left_nid == conflict.left_nid || dn_conf.right_nid == conflict.right_nid)
-                {
-                  result.duplicate_name_conflicts.erase(i);
-                  break;
-                }
-            }
+          std::vector<duplicate_name_conflict>::iterator i =
+            find(result.duplicate_name_conflicts.begin(),
+                 result.duplicate_name_conflicts.end(),
+                 nid);
+
+          result.duplicate_name_conflicts.erase(i);
         }
     } // end dropped_modified loop
 
============================================================
--- src/roster.cc	82bdb78ac2f38369f5972510e6ca5bcc3d0cdec5
+++ src/roster.cc	b0608d6ec3e3f2d23a5c97bb64c4354eeb01773a
@@ -1188,14 +1188,30 @@ dump(roster_t const & val, string & out)
   out = oss.str();
 }
 
+template <> void
+dump(std::map<node_id, std::pair<node_id, path_component> > const & val, string & out)
+{
+  ostringstream oss;
+  for (std::map<node_id, std::pair<node_id, path_component> >::const_iterator i = val.begin();
+       i != val.end();
+       ++i)
+    {
+      oss << "Node " << i->first;
+      oss << " node " << i->second.first;
+      oss << " path " << i->second.second << "\n";
+    }
+  out = oss.str();
+}
+
 void
 roster_t::check_sane(bool temp_nodes_ok) const
 {
   MM(*this);
+  MM(old_locations);
 
   node_id parent_id(the_null_node);
   const_dir_t parent_dir;
-  I(old_locations.empty());
+  I(old_locations.empty()); // if fail, some renamed node is still present and detached
   I(has_root());
   size_t maxdepth = nodes.size();
   bool is_first = true;
@@ -1235,7 +1251,7 @@ roster_t::check_sane(bool temp_nodes_ok)
       I(n == get_node(nid));
       I(maxdepth-- > 0);
     }
-  I(maxdepth == 0); // if fails, some node is not attached
+  I(maxdepth == 0); // if fails, some newly created node is not attached
 }
 
 void
============================================================
--- test/func/resolve_conflicts_dropped_modified_1/__driver__.lua	0bbd4c665fb269d56291f256f3480ff74ea7a3d5
+++ test/func/resolve_conflicts_dropped_modified_1/__driver__.lua	70accbfc43f720b90015d36f7df006a46d473351
@@ -414,11 +414,11 @@ check(samelines("stderr",
  "mtn: history for 'file_12' from left will be lost; see user manual Merge Conflicts section",
  "mtn: dropping 'file_12' from right",
  "mtn: replacing content of 'file_13' from left with '_MTN/resolutions/file_13'",
- "mtn: history for 'file_13' from left will be lost; see user manual Merge Conflicts section",
  "mtn: dropping 'file_13' from right",
- "mtn: [merged] 306eb31064512a8a2f4d316ff7a7ec32a1f64f4c"}))
+ "mtn: [merged] 21f4b2cfb7386246e682c4a862cffeb77a435c1a"}))
 
 check(mtn("update"), 0, nil, true)
 check(samelines("file_12", {"file_12 left"}))
+check(samelines("file_13", {"file_13 user"}))
 
 -- end of file