# # # patch "diff_patch.cc" # from [cdb4b1b3e7008e0e7b0f6e3f338331fe20525ece] # to [9683e035dc2e1ab8d9dd17a002ab808e5b40d792] # # patch "diff_patch.hh" # from [c617b890148ef718647145fdd9d106d79c0e2ed9] # to [ae5f59a69f0e07c5360b424c3a967b7031232a74] # # patch "merge.cc" # from [ad37299ca3a569d95a5591f0ecbf4cbca22d8d96] # to [54cca8e44707cd0c1ca8d0805efb2521185076b1] # # patch "tests/conflict_messages/__driver__.lua" # from [c2eecfd2d5bb9786bf10e0710a3e87d9c94f898e] # to [1af78d5744833cc37c601e2f9c24638cb31f45ea] # ============================================================ --- diff_patch.cc cdb4b1b3e7008e0e7b0f6e3f338331fe20525ece +++ diff_patch.cc 9683e035dc2e1ab8d9dd17a002ab808e5b40d792 @@ -764,19 +764,15 @@ content_merger::attribute_manual_merge(f return false; // default: enable auto merge } -// TODO: split this into auto_merge_files and manual_merge_files -// then make an automatic merge pass over all conflicting content hashes -// and list all remaining conflicts before asking the user to merge files - bool -content_merger::try_to_merge_files(file_path const & anc_path, - file_path const & left_path, - file_path const & right_path, - file_path const & merged_path, - file_id const & ancestor_id, - file_id const & left_id, - file_id const & right_id, - file_id & merged_id) +content_merger::try_auto_merge(file_path const & anc_path, + file_path const & left_path, + file_path const & right_path, + file_path const & merged_path, + file_id const & ancestor_id, + file_id const & left_id, + file_id const & right_id, + file_id & merged_id) { // This version of try_to_merge_files should only be called when there is a // real merge3 to perform. @@ -784,8 +780,8 @@ content_merger::try_to_merge_files(file_ I(!null_id(left_id)); I(!null_id(right_id)); - L(FL("trying to merge %s <-> %s (ancestor: %s)") - % left_id % right_id % ancestor_id); + L(FL("trying auto merge '%s' %s <-> %s (ancestor: %s)") + % merged_path % left_id % right_id % ancestor_id); if (left_id == right_id) { @@ -822,10 +818,7 @@ content_merger::try_to_merge_files(file_ split_into_lines(ancestor_unpacked(), anc_encoding, ancestor_lines); split_into_lines(right_unpacked(), right_encoding, right_lines); - if (merge3(ancestor_lines, - left_lines, - right_lines, - merged_lines)) + if (merge3(ancestor_lines, left_lines, right_lines, merged_lines)) { hexenc tmp_id; file_data merge_data; @@ -845,6 +838,46 @@ content_merger::try_to_merge_files(file_ } } + return false; +} + +bool +content_merger::try_user_merge(file_path const & anc_path, + file_path const & left_path, + file_path const & right_path, + file_path const & merged_path, + file_id const & ancestor_id, + file_id const & left_id, + file_id const & right_id, + file_id & merged_id) +{ + // This version of try_to_merge_files should only be called when there is a + // real merge3 to perform. + I(!null_id(ancestor_id)); + I(!null_id(left_id)); + I(!null_id(right_id)); + + L(FL("trying user merge '%s' %s <-> %s (ancestor: %s)") + % merged_path % left_id % right_id % ancestor_id); + + if (left_id == right_id) + { + L(FL("files are identical")); + merged_id = left_id; + return true; + } + + file_data left_data, right_data, ancestor_data; + data left_unpacked, ancestor_unpacked, right_unpacked, merged_unpacked; + + adaptor.get_version(left_id, left_data); + adaptor.get_version(ancestor_id, ancestor_data); + adaptor.get_version(right_id, right_data); + + left_unpacked = left_data.inner(); + ancestor_unpacked = ancestor_data.inner(); + right_unpacked = right_data.inner(); + P(F("help required for 3-way merge\n" "[ancestor] %s\n" "[ left] %s\n" ============================================================ --- diff_patch.hh c617b890148ef718647145fdd9d106d79c0e2ed9 +++ diff_patch.hh ae5f59a69f0e07c5360b424c3a967b7031232a74 @@ -171,15 +171,24 @@ struct content_merger content_merge_adaptor & adaptor); // merge3 on a file (line by line) - bool try_to_merge_files(file_path const & anc_path, - file_path const & left_path, - file_path const & right_path, - file_path const & merged_path, - file_id const & ancestor_id, - file_id const & left_id, - file_id const & right, - file_id & merged_id); + bool try_auto_merge(file_path const & anc_path, + file_path const & left_path, + file_path const & right_path, + file_path const & merged_path, + file_id const & ancestor_id, + file_id const & left_id, + file_id const & right, + file_id & merged_id); + bool try_user_merge(file_path const & anc_path, + file_path const & left_path, + file_path const & right_path, + file_path const & merged_path, + file_id const & ancestor_id, + file_id const & left_id, + file_id const & right, + file_id & merged_id); + std::string get_file_encoding(file_path const & path, roster_t const & ros); ============================================================ --- merge.cc ad37299ca3a569d95a5591f0ecbf4cbca22d8d96 +++ merge.cc 54cca8e44707cd0c1ca8d0805efb2521185076b1 @@ -27,15 +27,100 @@ using boost::shared_ptr; using boost::shared_ptr; -static void -get_file_details(roster_t const & ros, node_id nid, - file_id & fid, - file_path & pth) +namespace { - I(ros.has_node(nid)); - file_t f = downcast_to_file_t(ros.get_node(nid)); - fid = f->content; - ros.get_name(nid, pth); + enum merge_method { auto_merge, user_merge }; + + void + get_file_details(roster_t const & ros, node_id nid, + file_id & fid, + file_path & pth) + { + I(ros.has_node(nid)); + file_t f = downcast_to_file_t(ros.get_node(nid)); + fid = f->content; + ros.get_name(nid, pth); + } + + void + try_to_merge_files(app_state & app, + roster_t const & left_roster, roster_t const & right_roster, + roster_merge_result & result, content_merge_adaptor & adaptor, + merge_method const method) + { + size_t cnt; + size_t total_conflicts = result.file_content_conflicts.size(); + std::vector::iterator it; + + for (cnt = 1, it = result.file_content_conflicts.begin(); + it != result.file_content_conflicts.end(); ++cnt) + { + file_content_conflict const & conflict = *it; + + MM(conflict); + + revision_id rid; + shared_ptr roster_for_file_lca; + adaptor.get_ancestral_roster(conflict.nid, rid, roster_for_file_lca); + + // Now we should certainly have a roster, which has the node. + I(roster_for_file_lca); + I(roster_for_file_lca->has_node(conflict.nid)); + + file_id anc_id, left_id, right_id; + file_path anc_path, left_path, right_path; + get_file_details(*roster_for_file_lca, conflict.nid, anc_id, anc_path); + get_file_details(left_roster, conflict.nid, left_id, left_path); + get_file_details(right_roster, conflict.nid, right_id, right_path); + + file_id merged_id; + + content_merger cm(app, *roster_for_file_lca, + left_roster, right_roster, + adaptor); + + bool merged; + + switch (method) + { + case auto_merge: + merged = cm.try_auto_merge(anc_path, left_path, right_path, + right_path, anc_id, left_id, right_id, + merged_id); + break; + + case user_merge: + merged = cm.try_user_merge(anc_path, left_path, right_path, + right_path, anc_id, left_id, right_id, + merged_id); + + // If the user merge has failed, there's no point + // trying to continue -- we'll only frustrate users by + // encouraging them to continue working with their merge + // tool on a merge that is now destined to fail. + if (!merged) + return; + + break; + } + + if (merged) + { + L(FL("resolved content conflict %d / %d on file '%s'") + % cnt % total_conflicts % right_path); + file_t f = downcast_to_file_t(result.roster.get_node(conflict.nid)); + f->content = merged_id; + + it = result.file_content_conflicts.erase(it); + } + else + { + ++it; + } + } + } + + } void @@ -74,68 +159,18 @@ resolve_merge_conflicts(roster_t const & L(FL("examining content conflicts")); - // FIXME: need this for the tests to pass but it doesn't really make sense - // because the auto merger may resolve all of the content conflicts + try_to_merge_files(app, left_roster, right_roster, + result, adaptor, auto_merge); - result.report_file_content_conflicts(left_roster, right_roster, adaptor); - - // TODO: split this into two merge passes - // first make an automatic merge pass over all content conflicts - // then list all remaining conflicts before making a second - // manual merge pass over the remaining conflicts - - size_t cnt; - size_t total_conflicts = result.file_content_conflicts.size(); - std::vector::iterator it; - - for (cnt = 1, it = result.file_content_conflicts.begin(); - it != result.file_content_conflicts.end(); ++cnt) + size_t remaining = result.file_content_conflicts.size(); + if (remaining > 0) { - file_content_conflict const & conflict = *it; + P(F("%d content conflicts require user intervention") % remaining); + result.report_file_content_conflicts(left_roster, right_roster, adaptor); + } - MM(conflict); - - revision_id rid; - shared_ptr roster_for_file_lca; - adaptor.get_ancestral_roster(conflict.nid, rid, roster_for_file_lca); - - // Now we should certainly have a roster, which has the node. - I(roster_for_file_lca); - I(roster_for_file_lca->has_node(conflict.nid)); - - file_id anc_id, left_id, right_id; - file_path anc_path, left_path, right_path; - get_file_details(*roster_for_file_lca, conflict.nid, anc_id, anc_path); - get_file_details(left_roster, conflict.nid, left_id, left_path); - get_file_details(right_roster, conflict.nid, right_id, right_path); - - file_id merged_id; - - content_merger cm(app, *roster_for_file_lca, - left_roster, right_roster, - adaptor); - - if (cm.try_to_merge_files(anc_path, left_path, right_path, right_path, - anc_id, left_id, right_id, merged_id)) - { - L(FL("resolved content conflict %d / %d") - % cnt % total_conflicts); - file_t f = downcast_to_file_t(result.roster.get_node(conflict.nid)); - f->content = merged_id; - - it = result.file_content_conflicts.erase(it); - } - else - { - ++it; - - // If the content_merger has failed, there's no point - // trying to continue--we'll only frustrate users by - // encouraging them to continue working with their merge - // tool on a merge that is now destined to fail. - break; - } - } + try_to_merge_files(app, left_roster, right_roster, + result, adaptor, user_merge); } E(result.is_clean(), ============================================================ --- tests/conflict_messages/__driver__.lua c2eecfd2d5bb9786bf10e0710a3e87d9c94f898e +++ tests/conflict_messages/__driver__.lua 1af78d5744833cc37c601e2f9c24638cb31f45ea @@ -495,6 +495,7 @@ check(qgrep("conflict: multiple values f check(qgrep("conflict: multiple values for attribute", "stderr")) + -- content conflict on attached node remove("_MTN") @@ -502,19 +503,27 @@ addfile("foo", "content foo attached") remove("foo") addfile("foo", "content foo attached") +addfile("bar", "content bar attached\none\ntwo\nthree") +addfile("baz", "content baz attached\naaa\nbbb\nccc") commit("content-attached") base = base_revision() writefile("foo", "foo first revision") +writefile("bar", "content bar attached\nzero\none\ntwo\nthree") +writefile("baz", "content baz attached\nAAA\nbbb\nccc") commit("content-attached") first = base_revision() revert_to(base) writefile("foo", "foo second revision") +writefile("bar", "content bar attached\none\ntwo\nthree\nfour") +writefile("baz", "content baz attached\naaa\nbbb\nCCC") check(mtn("update", "--debug"), 1, false, true) -check(qgrep("conflict: content conflict on file", "stderr")) +check(qgrep("conflict: content conflict on file 'foo'", "stderr")) +check(not qgrep("conflict: content conflict on file 'bar'", "stderr")) +check(not qgrep("conflict: content conflict on file 'baz'", "stderr")) commit("content-attached") second = base_revision()