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

[code]

revision:            435e3033125bd3f8ec04ac12da2f966354ae7434
date:                2012-07-23T01:46:11
author:              address@hidden
branch:              net.venge.monotone.issue-209
changelog:
resolve_conflicts_dropped_modified: all tests complete and passing.

* src/cmd_conflicts.cc (show_conflicts): improve formatting, check for invalid 
resolutions
  (set_resolution): better error message

* src/merge_roster.cc (resolve_conflicts): better image

* test/func/resolve_conflicts_dropped_modified_1/__driver__.lua: test error for 
orphaned file invalid resolutions

* test/func/resolve_conflicts_dropped_modified_3: New directory.

manifest:
format_version "1"

new_manifest [5bc9d094e58c5ba44e8c6cb1fe648a32a8083298]

old_revision [b9bca9e1b62bd1c9e8355b74981c1c39a8435fd6]

add_dir "test/func/resolve_conflicts_dropped_modified_3"

add_file "test/func/resolve_conflicts_dropped_modified_3/__driver__.lua"
 content [fd2a4dd0c38dff10bd3ff2f20e983f3f548b3d18]

patch "src/cmd_conflicts.cc"
 from [16d359c7c1defe51ec7c930266fee21326aa7017]
   to [28995825acadb97eaa4b5cda378c66e548e9ad16]

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

patch "test/func/resolve_conflicts_dropped_modified_1/__driver__.lua"
 from [d90af7e9fcf95394fa2c4f5fdd62781552483a8b]
   to [2c87a2a5810a34019c4ca4148eb2b9bede764267]
============================================================
--- src/cmd_conflicts.cc	16d359c7c1defe51ec7c930266fee21326aa7017
+++ src/cmd_conflicts.cc	28995825acadb97eaa4b5cda378c66e548e9ad16
@@ -100,8 +100,10 @@ show_conflicts(database & db, conflicts_
     {
       dropped_modified_conflict & conflict = *i;
 
-      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))
+      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;
 
@@ -192,7 +194,9 @@ show_conflicts(database & db, conflicts_
                   P(F("resolve_first_left rename"));
                   P(F("resolve_first_left user_rename \"new_content_name\" \"new_file_name\""));
 
-                  if (!conflict.orphaned)
+                  if (!conflict.orphaned &&
+                      conflict.right_resolution.resolution != resolve_conflicts::keep &&
+                      conflict.right_resolution.resolution != resolve_conflicts::content_user)
                     {
                       P(F("resolve_first_left keep"));
                       P(F("resolve_first_left user \"name\""));
@@ -205,7 +209,9 @@ show_conflicts(database & db, conflicts_
                   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)
+                  if (!conflict.orphaned &&
+                      conflict.left_resolution.resolution != resolve_conflicts::keep &&
+                      conflict.left_resolution.resolution != resolve_conflicts::content_user)
                     {
                       P(F("resolve_first_right keep"));
                       P(F("resolve_first_right user \"name\""));
@@ -392,7 +398,8 @@ set_resolution(resolve_conflicts::file_r
         other_resolution.resolution == resolve_conflicts::rename ||
         other_resolution.resolution == resolve_conflicts::content_user_rename,
         origin::user,
-        F("other resolution must be 'drop', 'rename', or 'user_rename'"));
+        F("other resolution is %s; specify 'drop', 'rename', or 'user_rename'") %
+        image(other_resolution.resolution));
       resolution.resolution = resolve_conflicts::keep;
     }
   else if ("rename" == idx(args, 0)())
@@ -409,7 +416,8 @@ set_resolution(resolve_conflicts::file_r
         other_resolution.resolution == resolve_conflicts::rename ||
         other_resolution.resolution == resolve_conflicts::content_user_rename,
         origin::user,
-        F("other resolution must be 'drop', 'rename', or 'user_rename'"));
+        F("other resolution is %s; specify 'drop', 'rename', or 'user_rename'") %
+        image(other_resolution.resolution));
 
       resolution.resolution  = resolve_conflicts::content_user;
       resolution.content = new_optimal_path(idx(args,1)(), false);
============================================================
--- src/merge_roster.cc	d5e74a871f0c4c22281c1e663b9498d73531225f
+++ src/merge_roster.cc	c3eb70578a3b8b88c4d1b7af43b7685d6f636c23
@@ -74,9 +74,9 @@ namespace resolve_conflicts
         ostringstream oss;
         oss << "resolution: " << image(res.resolution);
         if (res.content != 0)
-          oss << " content: " << res.content;
+          oss << ", content: '" << res.content->as_external() << "'";
         if (res.rename.as_internal().length()>0)
-          oss << "rename: " << res.rename;
+          oss << ", rename: '" << res.rename.as_external() << "'";
         oss << "\n";
         return oss.str();
       }
============================================================
--- test/func/resolve_conflicts_dropped_modified_1/__driver__.lua	d90af7e9fcf95394fa2c4f5fdd62781552483a8b
+++ test/func/resolve_conflicts_dropped_modified_1/__driver__.lua	2c87a2a5810a34019c4ca4148eb2b9bede764267
@@ -308,6 +308,15 @@ check(mtn("conflicts", "resolve_first", 
 writefile("_MTN/resolutions/file_10", "file_10 user")
 check(mtn("conflicts", "resolve_first", "user_rename", "_MTN/resolutions/file_10", "file_10"), 0, nil, true)
 
+-- Test error message from invalid resolution
+check(mtn("conflicts", "resolve_first", "keep"), 1, nil, true)
+check(samelines("stderr",
+{"mtn: misuse: orphaned files must be renamed"}))
+
+check(mtn("conflicts", "resolve_first", "user", "foo"), 1, nil, true)
+check(samelines("stderr",
+{"mtn: misuse: orphaned files must be renamed"}))
+
 check(mtn("conflicts", "resolve_first", "drop"), 0, nil, nil)
 
 check(mtn("conflicts", "resolve_first", "rename", "file_9"), 0, nil, nil)
============================================================
--- /dev/null	
+++ test/func/resolve_conflicts_dropped_modified_3/__driver__.lua	fd2a4dd0c38dff10bd3ff2f20e983f3f548b3d18
@@ -0,0 +1,276 @@
+-- Test enforcement of consistent resolutions for drop/modified conflicts.
+--
+-- Only invalid combinations are tested here (we verify a good error
+-- message); valid combinations are tested in
+-- resolve_conflicts_dropped_modified_1.
+--
+-- The left and right conflicts chosen by the user must be consistent;
+-- they must give different names for the two sides. 
+--
+-- When one file is in the dropped state, only one resolution can be
+-- specified; that of the modified file.
+--
+-- Rename on both sides is valid, unless the user specifies the same
+-- new name for both; that is tested only once here.
+--
+-- The only inconsistent cases are between modified and recreated
+-- files. A recreated file is detected by having the same name as the
+-- modified file; if the modified file has also been renamed, the
+-- recreated file must have the same name as the renamed file. Thus we
+-- do not need to consider a renamed modified file as a separate case.
+--
+-- Orphaned file resolution cannot be keep or user; those error
+-- messages are tested in resolve_conflicts_dropped_modified_1.
+--
+-- We need to test all invalid combinations of left/right resolution:
+--
+--      left                    right
+-- state    resolution      state       resolution  file    case
+-----------------------------------------------------------
+-- dropped  -               dropped     -           (not a conflict)
+--          -               modified    keep        (valid)
+--          -               modified    rename      (valid)
+--          -               modified    user        (valid)
+--          -               modified    user_rename (valid)
+--          -               recreated   -           (not a conflict)
+--
+-- modified drop            dropped     -           (valid)
+--          keep            dropped     -           (valid)
+--          rename          dropped     -           (valid)
+--          user            dropped     -           (valid)
+--          user_rename     dropped     -           (valid)
+--
+-- modified -               modified    -           (file content conflict)
+--          drop            recreated   drop        (valid)
+--          drop            recreated   keep        (valid)
+--          drop            recreated   rename      (valid)
+--          drop            recreated   user        (valid)
+--          drop            recreated   user_rename (valid)
+--          keep            recreated   drop        (valid)
+--          keep            recreated   keep        file_2  1
+--          keep            recreated   rename      (valid)
+--          keep            recreated   user        file_2  2
+--          keep            recreated   user_rename (valid)
+--          rename          recreated   drop        (valid)
+--          rename          recreated   keep        (valid)
+--          rename          recreated   rename      (valid)
+--          rename          recreated   user        (valid)
+--          rename          recreated   user_rename (valid)
+--          user            recreated   drop        (valid)
+--          user            recreated   keep        file_4  3
+--          user            recreated   rename      (valid) 
+--          user            recreated   user        file_4  4
+--          user            recreated   user_rename (valid)
+--          user_rename     recreated   drop        (valid)
+--          user_rename     recreated   keep        (valid)
+--          user_rename     recreated   rename      (valid)
+--          user_rename     recreated   user        (valid)
+--          user_rename     recreated   user_rename (valid)
+-- 
+-- recreated drop           dropped     -           (valid)
+--           keep           dropped     -           (valid)
+--           rename         dropped     -           (valid)
+--           user           dropped     -           (valid)
+--           user_rename    dropped     -           (valid)
+--           drop           modified    drop        (valid)
+--           drop           modified    keep        (valid)
+--           drop           modified    rename      (valid)
+--           drop           modified    user        (valid)
+--           drop           modified    user_rename (valid)
+--           keep           modified    drop        (valid)
+--           keep           modified    keep        file_3  5
+--           keep           modified    rename      (valid)
+--           keep           modified    user        file_3  6
+--           keep           modified    user_rename (valid)
+--           rename         modified    drop        (valid)
+--           rename         modified    keep        (valid)
+--           rename         modified    rename      (valid)
+--           rename         modified    user        (valid)
+--           rename         modified    user_rename (valid)
+--           user           modified    drop        (valid)
+--           user           modified    keep        file_3  7
+--           user           modified    rename      (valid)
+--           user           modified    user        file_3  8 
+--           user           modified    user_rename (valid)
+--           user_rename    modified    drop        (valid)
+--           user_rename    modified    keep        (valid)
+--           user_rename    modified    rename      (valid)
+--           user_rename    modified    user        (valid)
+--           user_rename    modified    user_rename (valid)
+
+mtn_setup()
+
+-- Create the test files
+
+addfile("file_2", "file_2 base") -- modified left, recreated right
+addfile("file_3", "file_3 base") -- recreated left, modified right
+commit("testbranch", "base")
+base = base_revision()
+
+writefile("file_2", "file_2 left")
+check(mtn("drop", "file_3"), 0, false, false)
+commit("testbranch", "left 1a")
+
+addfile("file_3", "file_3 left recreated")
+commit("testbranch", "left 1b")
+left_1 = base_revision()
+
+revert_to(base)
+
+check(mtn("drop", "file_2"), 0, false, false)
+writefile("file_3", "file_3 right")
+commit("testbranch", "right 1a")
+
+addfile("file_2", "file_2 right recreated")
+commit("testbranch", "right 1b")
+right_1 = base_revision()
+
+-- Store and show inconsistency error messages
+check(mtn("conflicts", "store", left_1, right_1), 0, nil, true)
+check(samelines("stderr",
+{"mtn: 2 conflicts with supported resolutions.",
+ "mtn: stored in '_MTN/conflicts'"}))
+
+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: 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\""}))
+
+-- case 1, 2; keep *
+check(mtn("conflicts", "resolve_first_left", "keep"), 0, nil, false)
+
+-- check that inconsistent resolutions for right are not displayed
+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_right drop",
+ "mtn: resolve_first_right rename",
+ "mtn: resolve_first_right user_rename \"new_content_name\" \"new_file_name\""}))
+
+-- check for errors from inconsistent resolutions
+
+-- case 1: keep, keep
+check(mtn("conflicts", "resolve_first_right", "keep"), 1, nil, true)
+check(samelines("stderr",
+{"mtn: misuse: other resolution is keep; specify 'drop', 'rename', or 'user_rename'"}))
+
+-- case 2: keep, user
+check(mtn("conflicts", "resolve_first_right", "user", "_MTN/resolutions/file_2"), 1, nil, true)
+check(samelines("stderr",
+{"mtn: misuse: other resolution is keep; specify 'drop', 'rename', or 'user_rename'"}))
+
+-- case 1, 2, but specify right resolution first
+check(mtn("conflicts", "store", left_1, right_1), 0, nil, true)
+check(mtn("conflicts", "resolve_first_right", "keep"), 0, nil, false)
+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: right_resolution: keep",
+ "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\""}))
+check(mtn("conflicts", "resolve_first_left", "keep"), 1, nil, true)
+check(samelines("stderr",
+{"mtn: misuse: other resolution is keep; specify 'drop', 'rename', or 'user_rename'"}))
+
+-- No error if specify right again, but it actually sets file_3 right resolution. so we have to reset
+check(mtn("conflicts", "store", left_1, right_1), 0, nil, true)
+check(mtn("conflicts", "resolve_first_right", "user", "_MTN/resolutions/file_2"), 0, nil, false)
+check(mtn("conflicts", "show_first"), 0, nil, true)
+check(qgrep("right_resolution: content_user, content: '_MTN/resolutions/file_2'", "stderr"))
+
+check(mtn("conflicts", "resolve_first_left", "keep"), 1, nil, true)
+check(samelines("stderr",
+{"mtn: misuse: other resolution is content_user; specify 'drop', 'rename', or 'user_rename'"}))
+
+-- provide a valid resolution for file_2 so file_3 is first
+check(mtn("conflicts", "resolve_first_left", "drop"), 0, nil, nil)
+
+-- case 3, 4; user *
+check(mtn("conflicts", "resolve_first_left", "user", "_MTN/resolutions/file_3"), 0, nil, false)
+
+check(mtn("conflicts", "show_first"), 0, nil, true)
+check(samelines("stderr",
+{"mtn: conflict: file 'file_3'",
+ "mtn: dropped and recreated on the left",
+ "mtn: modified on the right",
+ "mtn: left_resolution: content_user, content: '_MTN/resolutions/file_3'",
+ "mtn: possible resolutions:",
+ "mtn: resolve_first_right drop",
+ "mtn: resolve_first_right rename",
+ "mtn: resolve_first_right user_rename \"new_content_name\" \"new_file_name\""}))
+
+-- case 3: user, keep
+check(mtn("conflicts", "resolve_first_right", "keep"), 1, nil, true)
+check(samelines("stderr",
+{"mtn: misuse: other resolution is content_user; specify 'drop', 'rename', or 'user_rename'"}))
+
+-- case 4: user, user
+check(mtn("conflicts", "resolve_first_right", "user", "_MTN/resolutions/file_3"), 1, nil, true)
+check(samelines("stderr",
+{"mtn: misuse: other resolution is content_user; specify 'drop', 'rename', or 'user_rename'"}))
+
+-- specify right first
+check(mtn("conflicts", "store", left_1, right_1), 0, nil, true)
+-- resolve file_2
+check(mtn("conflicts", "resolve_first_left", "keep"), 0, nil, nil) 
+check(mtn("conflicts", "resolve_first_right", "drop"), 0, nil, nil)
+
+-- file_3
+check(mtn("conflicts", "resolve_first_right", "keep"), 0, nil, true)
+
+check(mtn("conflicts", "show_first"), 0, nil, true)
+check(samelines("stderr",
+{"mtn: conflict: file 'file_3'",
+ "mtn: dropped and recreated on the left",
+ "mtn: modified on the right",
+ "mtn: right_resolution: keep",
+ "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\""}))
+check(mtn("conflicts", "resolve_first_left", "user", "_MTN/resolutions/file_3"), 1, nil, true)
+check(samelines("stderr",
+{"mtn: misuse: other resolution is keep; specify 'drop', 'rename', or 'user_rename'"}))
+
+-- reset for case 4 reversed
+check(mtn("conflicts", "store", left_1, right_1), 0, nil, true)
+-- resolve file_2
+check(mtn("conflicts", "resolve_first_left", "keep"), 0, nil, nil) 
+check(mtn("conflicts", "resolve_first_right", "drop"), 0, nil, nil)
+
+check(mtn("conflicts", "resolve_first_right", "user", "_MTN/resolutions/file_3"), 0, nil, nil)
+check(mtn("conflicts", "resolve_first_left", "user", "_MTN/resolutions/file_3"), 1, nil, true)
+check(samelines("stderr",
+{"mtn: misuse: other resolution is content_user; specify 'drop', 'rename', or 'user_rename'"}))
+
+-- Test error from user rename both sides to same new name. The error is at merge time.
+check(mtn("conflicts", "store", left_1, right_1), 0, nil, true)
+check(mtn("conflicts", "resolve_first_left", "rename", "file_2_renamed"), 0, nil, nil) 
+check(mtn("conflicts", "resolve_first_right", "rename", "file_2_renamed"), 0, nil, nil) 
+-- file_3
+check(mtn("conflicts", "resolve_first_left", "drop"), 0, nil, nil)
+check(mtn("conflicts", "resolve_first_right", "drop"), 0, nil, nil)
+check(mtn("explicit_merge", "--resolve-conflicts", left_1, right_1, "testbranch"), 1, nil, true)
+check(qgrep("'file_2_renamed' already exists", "stderr"))
+
+-- end of file