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

[code]

revision:            48b14e6f132ad73fc3ed421bbf318c4ce6ec5b4e
date:                2012-07-17T23:49:02
author:              address@hidden
branch:              net.venge.monotone.issue-209
changelog:
resolve_conflicts_dropped_modified_upstream_vs_local_2 : more passing

* src/cmd_conflicts.cc (show_resolution): new
  (show_conflicts): improve dropped_modified
  (set_first_conflict): more error detection in dropped_modified

* src/merge_conflict.cc (report_dropped_modified_conflicts): fix

* src/merge_roster.cc (resolve_conflicts): move image functions into namespace
  (roster_merge): fix bug

* src/merge_roster.hh (dropped_modified_conflict): fix bug

* 
test/func/resolve_conflicts_dropped_modified_upstream_vs_local_2/__driver__.lua 
: improve; more passing

* 
test/func/resolve_conflicts_dropped_modified_upstream_vs_local_2/conflicts_3_2_resolved:
 add right resolution

manifest:
format_version "1"

new_manifest [2022be19738bb8445b0f4ab97a7d47d9569ccabb]

old_revision [d4a22d4d54c5ab65e491cd43f0f12a3b96b45c78]

patch "src/cmd_conflicts.cc"
 from [c4c2c75feb32cce46f146e05aba424c038152ded]
   to [21bfaa34a4c999f8214b8a2619cbbf82986f9158]

patch "src/merge_conflict.cc"
 from [d653ef4d1d748fa833b2500eed77ecc0107d48fb]
   to [ed311d6cdf1fa47f00a602e0db5b57f84be6a690]

patch "src/merge_roster.cc"
 from [b4de8ec7db2c60b3ebb777bfa58d9e0d8986558d]
   to [5d36f441612f9de68ce237b13226bdffd1e2c92a]

patch "src/merge_roster.hh"
 from [63eab7b216f8839626f9d9d04dc8e58c3eed90bf]
   to [b013f590a5c9e643afa06d41d3ed362f6c27d0e6]

patch 
"test/func/resolve_conflicts_dropped_modified_upstream_vs_local_2/__driver__.lua"
 from [46e918155fe50737229d3c2cb31899cf204ef9fd]
   to [897b4a52ad2ebdb38453f98db1b4284da562a03c]

patch 
"test/func/resolve_conflicts_dropped_modified_upstream_vs_local_2/conflicts_3_2_resolved"
 from [954b8dfc7ee161c1ca809b61d03ad8fd12bfb539]
   to [4482ad2e755308ce8a546191348cc648ad13d782]
============================================================
--- src/cmd_conflicts.cc	c4c2c75feb32cce46f146e05aba424c038152ded
+++ src/cmd_conflicts.cc	21bfaa34a4c999f8214b8a2619cbbf82986f9158
@@ -50,6 +50,16 @@ static void
 typedef enum {first, remaining} show_conflicts_case_t;
 
 static void
+show_resolution(resolve_conflicts::file_resolution_t resolution, char const * const prefix)
+{
+
+  if (resolution.resolution != resolve_conflicts::none)
+    {
+      P(F(string(prefix).append(image(resolution)).c_str()));
+    }
+}
+
+static void
 show_conflicts(database & db, conflicts_t conflicts, show_conflicts_case_t show_case)
 {
   // Go thru the conflicts we know how to resolve in the same order
@@ -90,8 +100,8 @@ show_conflicts(database & db, conflicts_
     {
       dropped_modified_conflict & conflict = *i;
 
-      if (conflict.left_resolution.resolution == resolve_conflicts::none ||
-          conflict.right_resolution.resolution == resolve_conflicts::none)
+      if ((conflict.left_nid != the_null_node && conflict.left_resolution.resolution == resolve_conflicts::none) ||
+          (conflict.right_nid != the_null_node && conflict.right_resolution.resolution == resolve_conflicts::none))
         {
           file_path modified_name;
 
@@ -149,13 +159,15 @@ show_conflicts(database & db, conflicts_
                 }
             }
 
-          if (show_case == remaining) return;
+          show_resolution(conflict.left_resolution, "left_");
+          show_resolution(conflict.right_resolution, "right_");
 
-          P(F("possible resolutions:"));
+          if (show_case == remaining) return;
 
           if (conflict.left_nid == the_null_node || conflict.right_nid == the_null_node)
             {
               // only one file involved; only need one resolution
+              P(F("possible resolutions:"));
               P(F("resolve_first drop"));
               P(F("resolve_first rename"));
               P(F("resolve_first user_rename \"new_content_name\" \"new_file_name\""));
@@ -170,21 +182,35 @@ show_conflicts(database & db, conflicts_
           else
             {
               // recreated or repeated duplicate name; need two resolutions
-              P(F("resolve_first_left drop"));
-              P(F("resolve_first_right drop"));
 
-              P(F("resolve_first_left rename"));
-              P(F("resolve_first_right rename"));
-              P(F("resolve_first_left user_rename \"new_content_name\" \"new_file_name\""));
-              P(F("resolve_first_right user_rename \"new_content_name\" \"new_file_name\""));
+              P(F("possible resolutions:"));
 
-              if (!conflict.orphaned)
+              if (conflict.left_nid != the_null_node &&
+                  conflict.left_resolution.resolution == resolve_conflicts::none)
                 {
-                  P(F("resolve_first_left keep"));
-                  P(F("resolve_first_right keep"));
-                  P(F("resolve_first_left user \"name\""));
-                  P(F("resolve_first_right user \"name\""));
+                  P(F("resolve_first_left drop"));
+                  P(F("resolve_first_left rename"));
+                  P(F("resolve_first_left user_rename \"new_content_name\" \"new_file_name\""));
+
+                  if (!conflict.orphaned)
+                    {
+                      P(F("resolve_first_left keep"));
+                      P(F("resolve_first_left user \"name\""));
+                    }
                 }
+
+              if (conflict.right_nid != the_null_node &&
+                  conflict.right_resolution.resolution == resolve_conflicts::none)
+                {
+                  P(F("resolve_first_right drop"));
+                  P(F("resolve_first_right rename"));
+                  P(F("resolve_first_right user_rename \"new_content_name\" \"new_file_name\""));
+                  if (!conflict.orphaned)
+                    {
+                      P(F("resolve_first_right keep"));
+                      P(F("resolve_first_right user \"name\""));
+                    }
+                }
               return;
             }
         }
@@ -528,6 +554,9 @@ set_first_conflict(database & db,
           switch (conflict.dropped_side)
             {
             case resolve_conflicts::left_side:
+              E(conflict.left_nid == the_null_node, origin::user,
+                F("must specify 'resolve_first_left' or 'resolve_first_right' (not just 'resolve_first')"));
+
               if (conflict.left_resolution.resolution == resolve_conflicts::none)
                 {
                   if ("drop" == idx(args,0)())
@@ -574,7 +603,11 @@ set_first_conflict(database & db,
                   return;
                 }
               break;
+
             case resolve_conflicts::right_side:
+              E(conflict.right_nid == the_null_node, origin::user,
+                F("must specify 'resolve_first_left' or 'resolve_first_right' (not just 'resolve_first')"));
+
               if (conflict.right_resolution.resolution == resolve_conflicts::none)
                 {
                   if ("drop" == idx(args,0)())
@@ -585,10 +618,10 @@ set_first_conflict(database & db,
                     }
                   else if ("keep" == idx(args,0)())
                     {
-                  E(args.size() == 1, origin::user, F("wrong number of arguments"));
-                  E(!conflict.orphaned, origin::user, F("orphaned files must be renamed"));
+                      E(args.size() == 1, origin::user, F("wrong number of arguments"));
+                      E(!conflict.orphaned, origin::user, F("orphaned files must be renamed"));
 
-                  conflict.right_resolution.resolution  = resolve_conflicts::keep;
+                      conflict.right_resolution.resolution  = resolve_conflicts::keep;
                     }
                   else if ("user" == idx(args,0)())
                     {
============================================================
--- src/merge_conflict.cc	d653ef4d1d748fa833b2500eed77ecc0107d48fb
+++ src/merge_conflict.cc	ed311d6cdf1fa47f00a602e0db5b57f84be6a690
@@ -1160,13 +1160,18 @@ roster_merge_result::report_dropped_modi
                     }
                 }
 
+              st.push_str_pair(syms::right_type, "modified file");
+              st.push_str_pair(syms::right_name, modified_name.as_external());
+              db_adaptor.db.get_file_content (db_adaptor.right_rid, nid, fid);
+              st.push_binary_pair(syms::right_file_id, fid.inner());
+              break;
+
+            case resolve_conflicts::right_side:
               st.push_str_pair(syms::left_type, "modified file");
               st.push_str_pair(syms::left_name, modified_name.as_external());
               db_adaptor.db.get_file_content (db_adaptor.left_rid, nid, fid);
               st.push_binary_pair(syms::left_file_id, fid.inner());
-              break;
 
-            case resolve_conflicts::right_side:
               if (conflict.orphaned)
                 {
                   st.push_str_pair(syms::right_type, "orphaned file");
@@ -1189,11 +1194,6 @@ roster_merge_result::report_dropped_modi
                       st.push_binary_pair(syms::right_file_id, fid.inner());
                     }
                 }
-
-              st.push_str_pair(syms::right_type, "modified file");
-              st.push_str_pair(syms::right_name, modified_name.as_external());
-              db_adaptor.db.get_file_content (db_adaptor.right_rid, nid, fid);
-              st.push_binary_pair(syms::right_file_id, fid.inner());
               break;
             }
 
@@ -1243,6 +1243,8 @@ roster_merge_result::report_dropped_modi
           switch (conflict.left_resolution.resolution)
             {
             case resolve_conflicts::none:
+              break;
+
             case resolve_conflicts::content_user:
             case resolve_conflicts::content_user_rename:
             case resolve_conflicts::rename:
@@ -1258,6 +1260,8 @@ roster_merge_result::report_dropped_modi
           switch (conflict.right_resolution.resolution)
             {
             case resolve_conflicts::none:
+              break;
+
             case resolve_conflicts::content_user:
             case resolve_conflicts::content_user_rename:
             case resolve_conflicts::rename:
============================================================
--- src/merge_roster.cc	b4de8ec7db2c60b3ebb777bfa58d9e0d8986558d
+++ src/merge_roster.cc	5d36f441612f9de68ce237b13226bdffd1e2c92a
@@ -26,42 +26,48 @@ enum side_t {left_side, right_side};
 
 enum side_t {left_side, right_side};
 
-static char const *
-image(resolve_conflicts::resolution_t resolution)
+namespace resolve_conflicts
 {
-  switch (resolution)
-    {
-    case resolve_conflicts::none:
-      return "none";
-    case resolve_conflicts::content_user:
-      return "content_user";
-    case resolve_conflicts::content_internal:
-      return "content_internal";
-    case resolve_conflicts::drop:
-      return "drop";
-    case resolve_conflicts::keep:
-      return "keep";
-    case resolve_conflicts::rename:
-      return "rename";
-    case resolve_conflicts::content_user_rename:
-      return "content_user_rename";
-    }
-  I(false); // keep compiler happy
-}
+  char const *
+  image(resolution_t resolution)
+  {
+    switch (resolution)
+      {
+      case resolve_conflicts::none:
+        return "none";
+      case resolve_conflicts::content_user:
+        return "content_user";
+      case resolve_conflicts::content_internal:
+        return "content_internal";
+      case resolve_conflicts::drop:
+        return "drop";
+      case resolve_conflicts::keep:
+        return "keep";
+      case resolve_conflicts::rename:
+        return "rename";
+      case resolve_conflicts::content_user_rename:
+        return "content_user_rename";
+      }
+    I(false); // keep compiler happy
+  }
 
-static string
-image(resolve_conflicts::file_resolution_t res)
-{
-  if (res.resolution == resolve_conflicts::none)
-    return string("");
-  else
-    {
-      ostringstream oss;
-      oss << "resolution: " << image(res.resolution) << " "
-          << "content: " << res.content << " "
-          << "rename: " << res.rename << "\n";
-      return oss.str();
-    }
+  string
+  image(file_resolution_t res)
+  {
+    if (res.resolution == resolve_conflicts::none)
+      return string("");
+    else
+      {
+        ostringstream oss;
+        oss << "resolution: " << image(res.resolution);
+        if (res.content != 0)
+          oss << " content: " << res.content;
+        if (res.rename.as_internal().length()>0)
+          oss << "rename: " << res.rename;
+        oss << "\n";
+        return oss.str();
+      }
+  }
 }
 
 template <> void
@@ -833,7 +839,7 @@ roster_merge(roster_t const & left_paren
           if (right_parent.has_node (modified_name))
             {
               conflict.right_nid = right_parent.get_node(modified_name)->self;
-              duplicate_name = false;
+              duplicate_name = true;
             }
 
           else if (result.roster.has_node(modified_name))
============================================================
--- src/merge_roster.hh	63eab7b216f8839626f9d9d04dc8e58c3eed90bf
+++ src/merge_roster.hh	b013f590a5c9e643afa06d41d3ed362f6c27d0e6
@@ -31,6 +31,8 @@ namespace resolve_conflicts
 {
   enum resolution_t {none, content_user, content_internal, drop, keep, rename, content_user_rename};
 
+  char const * image(resolution_t item);
+
   enum side_t {left_side, right_side};
 
   struct file_resolution_t
@@ -46,6 +48,8 @@ namespace resolve_conflicts
       {}
   };
 
+  std::string image(file_resolution_t res);
+
   // For filename read from conflicts file; converts path to utf8. basic_io
   // parser should return utf8 in the first place.
   file_path file_path_external(std::string path);
@@ -145,7 +149,7 @@ struct dropped_modified_conflict
     right_rid(),
     left_resolution(),
     right_resolution()
-  {dropped_side = (left_nid == the_null_node ? resolve_conflicts::right_side : resolve_conflicts::left_side);}
+  {dropped_side = (left_nid == the_null_node ? resolve_conflicts::left_side : resolve_conflicts::right_side);}
 
   dropped_modified_conflict() :
     left_nid(the_null_node),
============================================================
--- test/func/resolve_conflicts_dropped_modified_upstream_vs_local_2/__driver__.lua	46e918155fe50737229d3c2cb31899cf204ef9fd
+++ test/func/resolve_conflicts_dropped_modified_upstream_vs_local_2/__driver__.lua	897b4a52ad2ebdb38453f98db1b4284da562a03c
@@ -90,24 +90,45 @@ check(samefilestd("conflicts_3_2", "_MTN
 check(mtn("conflicts", "store", upstream_3, local_2), 0, nil, true)
 check(samefilestd("conflicts_3_2", "_MTN/conflicts"))
 
--- 'keep' is ambiguous here, so it gives an error
+-- since we have a duplicate name conflict, we need to specify both
+-- right and left resolutions, so 'resolve_first' is wrong here
 check(mtn("conflicts", "resolve_first", "keep"), 1, nil, true)
-check(qgrep("ambiguous; use 'keep_left' or 'keep_right'", "stderr"))
+check(qgrep("must specify 'resolve_first_left' or 'resolve_first_right'", "stderr"))
 
 check(mtn("conflicts", "show_first"), 0, nil, true)
 check(samelines("stderr",
- {"mtn: conflict: file 'file_2' from revision 27d41ae9f2b3cb73b130d9845d77574a11021b17",
-  "mtn: modified on the left, named file_2",
+ {"mtn: conflict: file 'file_2'",
+  "mtn: modified on the left",
   "mtn: dropped and recreated on the right",
+  "mtn: possible resolutions:",
+  "mtn: resolve_first_left drop",
+  "mtn: resolve_first_left rename",
+  "mtn: resolve_first_left user_rename \"new_content_name\" \"new_file_name\"",
+  "mtn: resolve_first_left keep",
+  "mtn: resolve_first_left user \"name\"",
+  "mtn: resolve_first_right drop",
+  "mtn: resolve_first_right rename",
+  "mtn: resolve_first_right user_rename \"new_content_name\" \"new_file_name\"",
+  "mtn: resolve_first_right keep",
+  "mtn: resolve_first_right user \"name\""}))   
+
+-- We want to keep the upstream node to avoid future conflicts
+check(mtn("conflicts", "resolve_first_left", "keep"), 0, nil, true)
+
+check(mtn("conflicts", "show_first"), 0, nil, true)
+check(samelines("stderr",
+ {"mtn: conflict: file 'file_2'",
   "mtn: modified on the left",
+  "mtn: dropped and recreated on the right",
+  "mtn: left_resolution: keep",
   "mtn: possible resolutions:",
-  "mtn: resolve_first keep_left",
-  "mtn: resolve_first keep_right",
-  "mtn: resolve_first user_left \"name\"",
-  "mtn: resolve_first user_right \"name\""}))
+  "mtn: resolve_first_right drop",
+  "mtn: resolve_first_right rename",
+  "mtn: resolve_first_right user_rename \"new_content_name\" \"new_file_name\"",
+  "mtn: resolve_first_right keep",
+  "mtn: resolve_first_right user \"name\""}))   
 
--- We want to keep the upstream node to avoid future conflicts
-check(mtn("conflicts", "resolve_first", "keep_left"), 0, nil, true)
+check(mtn("conflicts", "resolve_first_right", "drop"), 0, nil, true)
 check(samefilestd("conflicts_3_2_resolved", "_MTN/conflicts"))
 
 check(mtn("explicit_merge", "--resolve-conflicts", upstream_3, local_2, "testbranch"), 0, nil, true)
============================================================
--- test/func/resolve_conflicts_dropped_modified_upstream_vs_local_2/conflicts_3_2_resolved	954b8dfc7ee161c1ca809b61d03ad8fd12bfb539
+++ test/func/resolve_conflicts_dropped_modified_upstream_vs_local_2/conflicts_3_2_resolved	4482ad2e755308ce8a546191348cc648ad13d782
@@ -2,13 +2,14 @@ ancestor [27d41ae9f2b3cb73b130d9845d7757
    right [650057e8a81bd41991dc5ff10b2d60343f1032ae]
 ancestor [27d41ae9f2b3cb73b130d9845d77574a11021b17]
 
-          conflict dropped_modified
-     ancestor_name "file_2"
-  ancestor_file_id [7fc990de4797bd6534a5c1deb344e11964f6b353]
-         left_type "modified file"
-         left_name "file_2"
-      left_file_id [b7e3240a78dc6afce4507f5a18ab516963e72022]
-        right_type "recreated file"
-        right_name "file_2"
-     right_file_id [6e49d17f382dc2f03d495181490e7653f1a14ad9]
-resolved_keep_left
+           conflict dropped_modified
+      ancestor_name "file_2"
+   ancestor_file_id [7fc990de4797bd6534a5c1deb344e11964f6b353]
+          left_type "modified file"
+          left_name "file_2"
+       left_file_id [b7e3240a78dc6afce4507f5a18ab516963e72022]
+         right_type "recreated file"
+         right_name "file_2"
+      right_file_id [6e49d17f382dc2f03d495181490e7653f1a14ad9]
+ resolved_keep_left 
+resolved_drop_right