# # # patch "hybrid_map.hh" # from [c0b815591a24a6442550a32afdd54bebc8524784] # to [db32b019cc674026856f0fa6c41b972fd1ab56db] # # patch "merge.cc" # from [0aa7054c267c833ca753a85ea1bad3ca9997d667] # to [c70546fe818693647be0483beda94da09f8656a8] # # patch "parallel_iter.hh" # from [2ee45ae23e10ab433da7101315ad4ea52435f706] # to [6e558a9e12888f151c08fb47a78f694345d43fac] # # patch "roster.cc" # from [9eb37f9fb2be303fc606c698427c3450b77c22d6] # to [478a80fbf93015f064209e4222be626726dcbb47] # # patch "roster_merge.cc" # from [7213f45b1eaf58a223235a3569e07ba8d724e4bd] # to [cd5f29d931fffca5e7e5f6874f9832c4edb1e0be] # # patch "roster_merge.hh" # from [95c9ce256cafc7df3ced08337e104b85fccfbe37] # to [ac60af8e6a41966a00430a85604e9d1c08acc67d] # # patch "tests/resolve_duplicate_name_conflict/__driver__.lua" # from [8bde1a08cec21f13e69bbb1ccae604dc3d5146d7] # to [baf6a71e1f54cdf070340d504d6c9c330db0aa8e] # # patch "tests/resolve_duplicate_name_conflict/expected-merge-messages-abe_1-beth_1" # from [8cc5c13905078a96dd1e308964537c69ed78f4e8] # to [b40e0b77db581192a3751a3c267177e1451dfe0e] # ============================================================ --- hybrid_map.hh c0b815591a24a6442550a32afdd54bebc8524784 +++ hybrid_map.hh db32b019cc674026856f0fa6c41b972fd1ab56db @@ -1,6 +1,7 @@ #ifndef __HYBRID_MAP_HH__ #define __HYBRID_MAP_HH__ +// Copyright 2008 Stephen Leake // Copyright 2007 Timothy Brownawell // // This program is made available under the GNU GPL version 2.0 or @@ -171,8 +172,55 @@ public: return !(*this == other); } }; + class const_reverse_iterator + { + friend class hybrid_map; + bool valid; + typename omap::const_reverse_iterator o; + hybrid_map const * n; + public: + const_reverse_iterator(hybrid_map const * n, + typename omap::const_reverse_iterator const & i) + : valid(i != n->ordered.rend()), o(i), n(n) + {} + const_reverse_iterator() + : valid(false) + {} + const_reverse_iterator & operator++() + { + I(valid); + ++o; + valid = (o != n->ordered.rend()); + return *this; + } + const_reverse_iterator operator++(int) + { + const_reverse_iterator out = *this; + ++*this; + return out; + } + value_type const & operator*() const + { + I(valid); + return *o; + } + value_type const * operator->() const + { + I(valid); + return o.operator->(); + } + bool operator==(const_reverse_iterator const & other) const + { + return (valid == other.valid) && (!valid || (*this)->first == other->first); + } + bool operator!=(const_reverse_iterator const & other) const + { + return !(*this == other); + } + }; friend class iterator; friend class const_iterator; + friend class const_reverse_iterator; iterator begin() { return iterator(this, ordered.begin()); @@ -193,6 +241,14 @@ public: { return const_iterator(this, ordered.end()); } + const_reverse_iterator rbegin() const + { + return const_reverse_iterator(this, ordered.rbegin()); + } + const_reverse_iterator rend() const + { + return const_reverse_iterator(this, ordered.rend()); + } const_iterator find(key_type const & k) const { return const_iterator(this, unordered.find(k)); ============================================================ --- merge.cc 0aa7054c267c833ca753a85ea1bad3ca9997d667 +++ merge.cc c70546fe818693647be0483beda94da09f8656a8 @@ -62,25 +62,23 @@ namespace MM(conflict); - revision_id rid; - shared_ptr roster_for_file_lca; - adaptor.get_ancestral_roster(conflict.nid, rid, roster_for_file_lca); + node_id ancestor_nid; + revision_id ancestor_rid; + shared_ptr ancestor_roster; + conflict.get_ancestor_roster(adaptor, ancestor_nid, ancestor_rid, ancestor_roster); - // 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)); + I(ancestor_roster); + I(ancestor_roster->has_node(ancestor_nid)); // this fails if there is no least common ancestor 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); + get_file_details(*ancestor_roster, ancestor_nid, anc_id, anc_path); + get_file_details(left_roster, conflict.left_nid, left_id, left_path); + get_file_details(right_roster, conflict.right_nid, right_id, right_path); file_id merged_id; - content_merger cm(lua, *roster_for_file_lca, - left_roster, right_roster, - adaptor); + content_merger cm(lua, *ancestor_roster, left_roster, right_roster, adaptor); bool merged = false; @@ -111,7 +109,7 @@ namespace { 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)); + file_t f = downcast_to_file_t(result.roster.get_node(conflict.result_nid)); f->content = merged_id; it = result.file_content_conflicts.erase(it); ============================================================ --- parallel_iter.hh 2ee45ae23e10ab433da7101315ad4ea52435f706 +++ parallel_iter.hh 6e558a9e12888f151c08fb47a78f694345d43fac @@ -1,6 +1,7 @@ #ifndef __PARALLEL_ITER_HH__ #define __PARALLEL_ITER_HH__ +// Copyright (C) 2008 Stephen Leake // Copyright (C) 2005 Nathaniel Smith // // This program is made available under the GNU GPL version 2.0 or @@ -167,6 +168,129 @@ namespace parallel out += "\n"; } + template + class reverse_iter + { + public: + M const & left_map; + M const & right_map; + + reverse_iter(M const & left_map, M const & right_map) + : left_map(left_map), right_map(right_map), + state_(invalid), started_(false), finished_(false) + { + } + + bool next() + { + I(!finished_); + // update iterators past the last item returned + if (!started_) + { + left_ = left_map.rbegin(); + right_ = right_map.rbegin(); + started_ = true; + } + else + { + I(state_ != invalid); + if (state_ == in_left || state_ == in_both) + ++left_; + if (state_ == in_right || state_ == in_both) + ++right_; + } + // determine new state + I(started_); + if (left_ == left_map.rend() && right_ == right_map.rend()) + { + finished_ = true; + state_ = invalid; + } + else if (left_ == left_map.rend() && right_ != right_map.rend()) + state_ = in_right; + else if (left_ != left_map.rend() && right_ == right_map.rend()) + state_ = in_left; + else + { + // Both iterators valid. + // ">" true is nearer end for reverse order + if (left_->first > right_->first) + state_ = in_left; + else if (right_->first > left_->first) + state_ = in_right; + else + { + I(left_->first == right_->first); + state_ = in_both; + } + } + return !finished_; + } + + state_t state() const + { + return state_; + } + + typename M::value_type const & + left_value() + { + I(state_ == in_left || state_ == in_both); + return *left_; + } + + typename M::key_type const & + left_key() + { + return left_value().first; + } + + typename M::value_type::second_type const & + left_data() + { + return left_value().second; + } + + typename M::value_type const & + right_value() + { + I(state_ == in_right || state_ == in_both); + return *right_; + } + + typename M::key_type const & + right_key() + { + return right_value().first; + } + + typename M::value_type::second_type const & + right_data() + { + return right_value().second; + } + + private: + state_t state_; + bool started_, finished_; + typename M::const_reverse_iterator left_; + typename M::const_reverse_iterator right_; + + }; + + template void + dump(reverse_iter const & i, std::string & out) + { + out = boost::lexical_cast(i.state()); + switch (i.state()) + { + case in_left: out += " in_left"; break; + case in_right: out += " in_right"; break; + case in_both: out += " in_both"; break; + case invalid: out += " invalid"; break; + } + out += "\n"; + } } // Local Variables: ============================================================ --- roster.cc 9eb37f9fb2be303fc606c698427c3450b77c22d6 +++ roster.cc 478a80fbf93015f064209e4222be626726dcbb47 @@ -94,24 +94,24 @@ dump(std::pairattrs != b->attrs) return false; + if (a->ancestors != b->ancestors) + return false; + if (! same_type(a,b)) return false; @@ -574,9 +577,8 @@ shallow_equal(node_t a, node_t b, } -// FIXME_ROSTERS: why does this do two loops? why does it pass 'true' to -// shallow_equal? -// -- njs +// FIXME_ROSTERS: why does this do two loops? why does it pass 'true' for +// shallow_compare_dir_children to shallow_equal? -- njs bool roster_t::operator==(roster_t const & other) const { @@ -867,14 +869,10 @@ roster_t::drop_detached_node(node_id nid I(downcast_to_dir_t(n)->children.empty()); // all right, kill it safe_erase(nodes, nid); - // can use safe_erase here, because while not every detached node appears in - // old_locations, all those that used to be in the tree do. and you should - // only ever be dropping nodes that were detached, not nodes that you just - // created and that have never been attached. - // Update; resolving a duplicate name conflict via suture requires - // dropping nodes that were never attached. So we erase the key without - // checking whether it was present. FIXME: clean up these comments. + // Resolving a duplicate name conflict via suture requires dropping nodes + // that were never attached. So we erase the key without checking whether + // it was present. old_locations.erase(nid); } @@ -1361,7 +1359,7 @@ namespace node_t new_bn = b.get_node(new_nid); new_bn->ancestors.first = an->ancestors.first; - new_bn->ancestors.second = bn->ancestors.first; + new_bn->ancestors.second = an->ancestors.second; }; b_new.erase(bid); @@ -1587,7 +1585,12 @@ namespace mark_new_node(revision_id const & new_rid, node_t n, marking_t & new_marking) { new_marking.birth_revision = new_rid; - new_marking.birth_cause = make_pair(marking_t::add, null_ancestors); + + if (n->ancestors.first == n->self || n->ancestors.first == the_null_node) + new_marking.birth_cause = make_pair(marking_t::add, null_ancestors); + else + new_marking.birth_cause = make_pair(marking_t::suture, n->ancestors); + I(new_marking.parent_name.empty()); new_marking.parent_name.insert(new_rid); I(new_marking.file_content.empty()); @@ -1822,19 +1825,59 @@ mark_merge_roster(roster_t const & left_ } else if (exists_in_left && !exists_in_right) { - // FIXME_SUTURE: handle case where n is an ancestor of a sutured node on - // the right; abe_3 in test. node_t const & left_node = lni->second; marking_t const & left_marking = safe_get(left_markings, n->self); - // Must be unborn on the right (as opposed to dead); otherwise it - // would not be in the merge. Therefore the birth revision for - // this node must be in the uncommon ancestors on the left: - I(left_uncommon_ancestors.find(left_marking.birth_revision) - != left_uncommon_ancestors.end()); + switch (left_marking.birth_cause.first) + { + case marking_t::invalid: + I(false); - mark_unmerged_node(left_marking, left_node, - new_rid, n, new_marking); + case marking_t::add: + // Must be unborn on the right (as opposed to dead); + // otherwise it would not be in the merge. Therefore the + // birth revision for this node must be in the uncommon + // ancestors on the left: + I(left_uncommon_ancestors.find(left_marking.birth_revision) + != left_uncommon_ancestors.end()); + + mark_unmerged_node(left_marking, left_node, + new_rid, n, new_marking); + + break; + + case marking_t::suture: + { + // If one of the ancestor nodes is in right, merge marks with it. + node_id right_nid = left_marking.birth_cause.second.first; + node_map::const_iterator rni = right_roster.all_nodes().find(right_nid); + if (rni == right_roster.all_nodes().end()) + { + right_nid = left_marking.birth_cause.second.second; + rni = right_roster.all_nodes().find(right_nid); + } + + if (rni == right_roster.all_nodes().end()) + { + // Neither ancestor is in right. + I(left_uncommon_ancestors.find(left_marking.birth_revision) + != left_uncommon_ancestors.end()); + + mark_unmerged_node(left_marking, left_node, + new_rid, n, new_marking); + } + else + { + mark_merged_node(safe_get(left_markings, n->self), left_uncommon_ancestors, left_node, + safe_get(right_markings, right_nid), right_uncommon_ancestors, rni->second, + new_rid, n, new_marking); + } + } + break; + + case marking_t::split: + I(false); // not implemented yet + } } else { @@ -2773,15 +2816,15 @@ parse_marking(basic_io::parser & pa, pa.sym(); pa.str(tmp_1); pa.str(tmp_2); - marking.birth_cause = make_pair (marking_t::add, + marking.birth_cause = make_pair (marking_t::suture, make_pair(lexical_cast(tmp_1), lexical_cast(tmp_2))); } - else if (pa.symp (syms::birth_suture)) + else if (pa.symp (syms::birth_split)) { pa.sym(); pa.str(tmp_1); - marking.birth_cause = make_pair (marking_t::add, + marking.birth_cause = make_pair (marking_t::split, make_pair(lexical_cast(tmp_1), the_null_node)); } ============================================================ --- roster_merge.cc 7213f45b1eaf58a223235a3569e07ba8d724e4bd +++ roster_merge.cc cd5f29d931fffca5e7e5f6874f9832c4edb1e0be @@ -11,8 +11,6 @@ #include "base.hh" #include -#include - #include "basic_io.hh" #include "lua_hooks.hh" #include "vocab.hh" @@ -98,9 +96,12 @@ dump(file_content_conflict const & confl dump(file_content_conflict const & conflict, string & out) { ostringstream oss; - oss << "file_content_conflict on node: " << conflict.nid << " " - << "left: " << conflict.left << " " - << "right: " << conflict.right << "\n"; + oss << "file_content_conflict: " + << "left_node: "<< conflict.left_nid << " " + << "left_content: " << conflict.left << " " + << "right_node: " << conflict.right_nid << " " + << "right_content: " << conflict.right << " " + << "result_node: " << conflict.result_nid << "\n"; out = oss.str(); } @@ -427,46 +428,40 @@ put_content_conflict (basic_io::stanza & static void put_content_conflict (basic_io::stanza & st, + roster_t const & left_roster, + roster_t const & right_roster, content_merge_adaptor & adaptor, file_content_conflict const & conflict) { // Always report ancestor, left, and right information, for completeness - content_merge_database_adaptor & db_adaptor (dynamic_cast(adaptor)); - - // This ensures that the ancestor roster is computed + node_id ancestor_nid; boost::shared_ptr ancestor_roster; revision_id ancestor_rid; - db_adaptor.get_ancestral_roster (conflict.nid, ancestor_rid, ancestor_roster); - boost::shared_ptr left_roster(db_adaptor.rosters[db_adaptor.left_rid]); - I(0 != left_roster); - boost::shared_ptr right_roster(db_adaptor.rosters[db_adaptor.right_rid]); - I(0 != right_roster); + conflict.get_ancestor_roster(adaptor, ancestor_nid, ancestor_rid, ancestor_roster); + content_merge_database_adaptor & db_adaptor (dynamic_cast(adaptor)); + file_path ancestor_name; file_path left_name; file_path right_name; - ancestor_roster->get_name (conflict.nid, ancestor_name); - left_roster->get_name (conflict.nid, left_name); - right_roster->get_name (conflict.nid, right_name); + ancestor_roster->get_name (ancestor_nid, ancestor_name); + left_roster.get_name (conflict.left_nid, left_name); + right_roster.get_name (conflict.right_nid, right_name); - if (file_type == get_type (*ancestor_roster, conflict.nid)) + if (file_type == get_type (*ancestor_roster, ancestor_nid)) { st.push_str_pair(syms::node_type, "file"); file_id ancestor_fid; - db_adaptor.db.get_file_content (db_adaptor.lca, conflict.nid, ancestor_fid); + db_adaptor.db.get_file_content (ancestor_rid, ancestor_nid, ancestor_fid); st.push_str_pair(syms::ancestor_name, ancestor_name.as_external()); st.push_binary_pair(syms::ancestor_file_id, ancestor_fid.inner()); - file_id left_fid; - db_adaptor.db.get_file_content (db_adaptor.left_rid, conflict.nid, left_fid); st.push_file_pair(syms::left_name, left_name); - st.push_binary_pair(syms::left_file_id, left_fid.inner()); - file_id right_fid; - db_adaptor.db.get_file_content (db_adaptor.right_rid, conflict.nid, right_fid); + st.push_binary_pair(syms::left_file_id, conflict.left.inner()); st.push_file_pair(syms::right_name, right_name); - st.push_binary_pair(syms::right_file_id, right_fid.inner()); + st.push_binary_pair(syms::right_file_id, conflict.right.inner()); } else { @@ -1277,6 +1272,36 @@ void } void +file_content_conflict::get_ancestor_roster(content_merge_adaptor & adaptor, + node_id & ancestor_nid, + revision_id & ancestor_rid, + shared_ptr & ancestor_roster) const +{ + if (left_nid == right_nid) + { + // Either there is a least common ancestor, or we use the birth + // revision for the node as the ancestor. + ancestor_nid = left_nid; + } + else + { + // One side is a suture or split; it will have a larger node id. Use + // the smaller nid to retrieve the least common ancestor. + // FIXME_SUTURE: what if both sides are sutured? need to find + // ancestor_nid via birth records; database_adaptor has those in the + // marking maps. get_ancestral_roster needs to take left_nid, + // right_nid. + if (left_nid < right_nid) + ancestor_nid = left_nid; + else + ancestor_nid = right_nid; + }; + + // This also sets adaptor.lca. + adaptor.get_ancestral_roster (ancestor_nid, ancestor_rid, ancestor_roster); +} + +void roster_merge_result::report_file_content_conflicts(roster_t const & left_roster, roster_t const & right_roster, content_merge_adaptor & adaptor, @@ -1291,21 +1316,20 @@ roster_merge_result::report_file_content file_content_conflict const & conflict = file_content_conflicts[i]; MM(conflict); - if (basic_io) { basic_io::stanza st; st.push_str_pair(syms::conflict, syms::content); - put_content_conflict (st, adaptor, conflict); + put_content_conflict (st, left_roster, right_roster, adaptor, conflict); put_stanza (st, output); } else { - if (roster.is_attached(conflict.nid)) + if (roster.is_attached(conflict.result_nid)) { file_path name; - roster.get_name(conflict.nid, name); + roster.get_name(conflict.result_nid, name); P(F("conflict: content conflict on file '%s'") % name); @@ -1318,19 +1342,19 @@ roster_merge_result::report_file_content // isn't really a good name for it so report both the left // and right names using a slightly different format - file_path left_name, right_name; - left_roster.get_name(conflict.nid, left_name); - right_roster.get_name(conflict.nid, right_name); + node_id ancestor_nid; + boost::shared_ptr ancestor_roster; + revision_id ancestor_rid; - shared_ptr lca_roster; - revision_id lca_rid; - file_path lca_name; + conflict.get_ancestor_roster(adaptor, ancestor_nid, ancestor_rid, ancestor_roster); - adaptor.get_ancestral_roster(conflict.nid, lca_rid, lca_roster); - lca_roster->get_name(conflict.nid, lca_name); + file_path left_name, right_name, ancestor_name; + left_roster.get_name(conflict.left_nid, left_name); + right_roster.get_name(conflict.right_nid, right_name); + ancestor_roster->get_name(ancestor_nid, ancestor_name); P(F("conflict: content conflict on file '%s' from revision %s") - % lca_name % lca_rid); + % ancestor_name % ancestor_rid); P(F("content hash is %s on the left in file '%s'") % conflict.left % left_name); P(F("content hash is %s on the right in file '%s'") @@ -1523,7 +1547,7 @@ parse_resolve_conflicts_opts (options co basic_io::tokenizer tok(src); basic_io::parser pars(tok); - // Skip left, right, ancestor. FIXME: should check these! But don't + // Skip left, right, ancestor. FIXME_SUTURE: should check these! But don't // see how to access them right now. for (int i = 1; i <= 3; i++) { @@ -1813,21 +1837,29 @@ namespace marking_map const & markings, set const & uncommon_ancestors, roster_t const & parent_roster, - roster_t & new_roster) + roster_t const & other_parent_roster, + roster_t & new_roster, + set & already_handled) { MM(markings); MM(uncommon_ancestors); - // See comment in roster_merge below for cases. n is either the left or - // right parent node. We are in case iii, iv, or v. We determine which - // by searching for the birth revision in uncommon_ancestors. + // See ss-existence-merge.text for cases. n is either the left or + // right parent node. + // First we see if we've already handled this node, due to suturing. + if (already_handled.find(n->self) != already_handled.end()) + return; + + // We are in case ii, iii, iv, or v. We determine which by searching for + // the birth revision in uncommon_ancestors. + revision_id const & birth = safe_get(markings, n->self).birth_revision; set::const_iterator const uncommon_birth_i = uncommon_ancestors.find(birth); if (uncommon_birth_i != uncommon_ancestors.end()) { - // case iii or iv + // case ii, iii or iv std::pair > const & birth_cause = safe_get(markings, n->self).birth_cause; @@ -1837,24 +1869,42 @@ namespace I(false); case marking_t::add: - // case iv + // case ii, iv; if ii, conflict will be discovered in next phase create_node_for(n, new_roster); break; + case marking_t::split: + I(false); // not supported yet + break; + case marking_t::suture: - case marking_t::split: - // case iii + // case iii, sutured node { std::pair birth_ancestors = birth_cause.second; - if (n->self == birth_ancestors.first) + bool left_anc_in_other = other_parent_roster.has_node(birth_ancestors.first); + bool right_anc_in_other = other_parent_roster.has_node(birth_ancestors.second); + + if (left_anc_in_other) { + I(!right_anc_in_other); + + // Set ancestors so mark-merge step will know how to check for + // conflicts. We can't tell whether n is in left or right, so + // we always put it in ancestors.first. + create_node_for(n, make_pair (n->self, birth_ancestors.first), new_roster); + already_handled.insert(birth_ancestors.first); + } + else if (right_anc_in_other) + { + I(!left_anc_in_other); create_node_for(n, make_pair (n->self, birth_ancestors.second), new_roster); + already_handled.insert(birth_ancestors.second); } else { - I(n->self == birth_ancestors.second); - create_node_for(n, make_pair (birth_ancestors.second, n->self), new_roster); + // both ancestors deleted in other parent + create_node_for(n, new_roster); } } break; @@ -2065,7 +2115,7 @@ namespace // if a file, merge content if (is_file_t(new_n)) { - file_content_conflict conflict(new_n->self); + file_content_conflict conflict(left_n->self, right_n->self, new_n->self); if (merge_scalar(downcast_to_file_t(left_n)->content, left_marking.file_content, left_uncommon_ancestors, @@ -2145,6 +2195,8 @@ roster_merge(roster_t const & left_paren set const & right_uncommon_ancestors, roster_merge_result & result) { + set already_handled; + L(FL("Performing a roster_merge")); result.clear(); @@ -2154,71 +2206,14 @@ roster_merge(roster_t const & left_paren MM(right_markings); MM(result); - // First handle lifecycles. Several cases: - // - // A1 B1 - // i) \ / - // C1 - // - // The node is merged in the child - // - // - // A1 B2 - // ii) \ / - // C3 - // - // The node is sutured in the child. This is only possible in a - // conflict resolution, which will occur later in the merge process. - // FIXME_SUTURE: support user sutures. - // - // - // A1 B3 - // iiia) \ / - // C3 - // - // The node was born in a suture or split in an uncommon ancestor of B - // - // A3 B1 - // iiib) \ / - // C3 - // - // The node was born in a suture or split in an uncommon ancestor of A - // - // - // A1 B - // iva) \ / - // C1 - // - // The node was born new in A's uncommon ancestors - // - // A B1 - // ivb) \ / - // C1 - // - // The node was born new in B's uncommon ancestors - // - // - // A1 B - // v) \ / - // C - // - // The node was deleted in B's uncommon ancestors - // - // A B1 - // \ / - // C - // - // The node was deleted in A's uncommon ancestors - // - // - // In monotone 0.4 and earlier, cases ii and iii did not exist. - // - // In case v, deletion wins over any other change; it might be better to - // have deletion conflict with any other change. But that's left for - // another time. - + // First handle existence merge (lifecycles). See ss-existence-merge.text. + // FIXME_SUTURE: We don't support user suture yet, so case ii can + // only happen in a conflict resolution, which happens later in the + // process. { - parallel::iter i(left_parent.all_nodes(), right_parent.all_nodes()); + // Iterate in reverse order so we see sutured nodes before the + // corresponding non-sutured node; see ss-existence-merge.text. + parallel::reverse_iter i(left_parent.all_nodes(), right_parent.all_nodes()); while (i.next()) { switch (i.state()) @@ -2227,17 +2222,17 @@ roster_merge(roster_t const & left_paren I(false); case parallel::in_left: - // case iii, iva, or va + // case ii, iii, iva, or va insert_if_unborn_or_sutured(i.left_data(), left_markings, left_uncommon_ancestors, left_parent, - result.roster); + right_parent, result.roster, already_handled); break; case parallel::in_right: - // case iii, ivb, or vb + // case ii, iii, ivb, or vb insert_if_unborn_or_sutured(i.right_data(), right_markings, right_uncommon_ancestors, right_parent, - result.roster); + left_parent, result.roster, already_handled); break; case parallel::in_both: @@ -2266,39 +2261,36 @@ 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), unless they are - // parents of a suture. + // deleted in the lifecycles step above). if (result.roster.has_node(left_n->self)) { - if (left_n->ancestors.first != the_null_node && - left_n->ancestors.first != left_n->self) + node_t result_n = result.roster.get_node(left_n->self); + + if (result_n->ancestors.second != the_null_node) { - // This node is the child of a suture; if right parent - // exists in right, merge with it. - node_map::const_iterator right_i = right_parent.all_nodes().find (left_n->ancestors.second); - if (right_i != right_parent.all_nodes().end()) - { - marking_map::const_iterator right_mi = right_markings.find(right_i->second->ancestors.first); + // This node was sutured in left_uncommon, and its right parent + // exists in right; merge with it. + node_t right_n = right_parent.get_node(result_n->ancestors.second); + marking_map::const_iterator right_mi = right_markings.find(right_n->self); - // check that iterators are in sync. - I(new_i->first == i.left_key()); - I(left_mi->first == i.left_key()); + // check that iterators are in sync. + I(new_i->first == i.left_key()); + I(left_mi->first == i.left_key()); - merge_nodes(i.left_data(), // left_n - left_mi->second, // left_marking - left_uncommon_ancestors, - right_i->second, // right_n - right_mi->second, - right_uncommon_ancestors, - new_i->second, // new_n - result); - ++new_i; - } + merge_nodes(left_n, + left_mi->second, // left_marking + left_uncommon_ancestors, + right_n, + right_mi->second, + right_uncommon_ancestors, + new_i->second, // new_n + result); + ++new_i; } else { - // Not child of a suture. + // Not sutured. // // Attach this node from the left roster. This may cause // a name collision with the previously attached node from @@ -2319,34 +2311,32 @@ roster_merge(roster_t const & left_paren if (result.roster.has_node(right_n->self)) { - if (right_n->ancestors.first != the_null_node && - right_n->ancestors.first != right_n->self) + node_t result_n = result.roster.get_node(right_n->self); + + if (result_n->ancestors.second != the_null_node) { - // This node is the child of a suture; if right parent - // exists in right, merge with it. - node_map::const_iterator left_i = left_parent.all_nodes().find (right_n->ancestors.first); - if (left_i != left_parent.all_nodes().end()) - { - marking_map::const_iterator left_mi = left_markings.find(right_n->ancestors.first); + // This node was sutured in right_uncommon, and its left parent + // exists in left; merge with it. + node_t left_n = left_parent.get_node(result_n->ancestors.second); + marking_map::const_iterator left_mi = left_markings.find(left_n->self); - // check that iterators are in sync. - I(new_i->first == i.right_key()); - I(right_mi->first == i.right_key()); + // check that iterators are in sync. + I(new_i->first == i.right_key()); + I(right_mi->first == i.right_key()); - merge_nodes(left_i->second, // left_n - left_mi->second, // left_marking - left_uncommon_ancestors, - i.right_data(), // right_n - right_mi->second, // right_marking - right_uncommon_ancestors, - new_i->second, // new_n - result); - ++new_i; - } + merge_nodes(left_i->second, // left_n + left_mi->second, // left_marking + left_uncommon_ancestors, + i.right_data(), // right_n + right_mi->second, // right_marking + right_uncommon_ancestors, + new_i->second, // new_n + result); + ++new_i; } else { - // Not child of a suture. + // Not sutured. // // Attach this node from the right roster. This may // cause a name collision with the previously attached ============================================================ --- roster_merge.hh 95c9ce256cafc7df3ced08337e104b85fccfbe37 +++ roster_merge.hh ac60af8e6a41966a00430a85604e9d1c08acc67d @@ -11,6 +11,8 @@ // implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR // PURPOSE. +#include + #include "rev_types.hh" #include "database.hh" #include "diff_patch.hh" @@ -121,9 +123,15 @@ struct file_content_conflict // detached for some other reason), but with a null content hash. struct file_content_conflict { - node_id nid; - file_content_conflict(node_id nid) : nid(nid) {} + node_id left_nid, right_nid, result_nid; + file_content_conflict(node_id left_nid, node_id right_nid, node_id result_nid) : + left_nid(left_nid), right_nid(right_nid), result_nid(result_nid) {} file_id left, right; + + void get_ancestor_roster(content_merge_adaptor & adaptor, + node_id & ancestor_nid, + revision_id & rid, + boost::shared_ptr & ancestor_roster) const; }; ============================================================ --- tests/resolve_duplicate_name_conflict/__driver__.lua 8bde1a08cec21f13e69bbb1ccae604dc3d5146d7 +++ tests/resolve_duplicate_name_conflict/__driver__.lua baf6a71e1f54cdf070340d504d6c9c330db0aa8e @@ -165,8 +165,18 @@ abe_2 = base_revision() commit("testbranch", "abe_2") abe_2 = base_revision() -check(mtn("merge"), 0, nil, true) +-- This fails with content conflicts +check(mtn("merge"), 1, nil, true) canonicalize("stderr") +get ("expected-merge-messages-abe_2-jim_1-conflicts") +check(samefile("expected-merge-messages-abe_2-jim_1-conflicts", "stderr")) + +check (mtn("automate", "show_conflicts"), 0, true, nil) + +-- This succeeds +get ("merge-abe_2-jim_1-resolve_conflicts", "_MTN/conflicts") +check(mtn("merge", "--resolve-conflicts-file=_MTN/conflicts"), 0, nil, true) +canonicalize("stderr") get ("expected-merge-messages-abe_2-jim_1") check(samefile("expected-merge-messages-abe_2-jim_1", "stderr")) ============================================================ --- tests/resolve_duplicate_name_conflict/expected-merge-messages-abe_1-beth_1 8cc5c13905078a96dd1e308964537c69ed78f4e8 +++ tests/resolve_duplicate_name_conflict/expected-merge-messages-abe_1-beth_1 b40e0b77db581192a3751a3c267177e1451dfe0e @@ -1,8 +1,8 @@ mtn: 2 heads on branch 'testbranch' mtn: 2 heads on branch 'testbranch' -mtn: [left] 5285636b9d9f988e79b3dcd9a40e64d15fb7fc9f -mtn: [right] e9ad84a3fc40ef1109251c308428439c21ad1de9 +mtn: [left] c898aad0501a43b1cc6d5138afdbc7844e4efc91 +mtn: [right] ff5f7c356494ecf4c447701cc0c31b59b14981eb mtn: suturing checkout.sh, checkout.sh into checkout.sh mtn: renaming thermostat.c to thermostat-westinghouse.c mtn: renaming thermostat.c to thermostat-honeywell.c -mtn: [merged] 257729bebdb32819cd1fc059806e0fb4144f7ec7 +mtn: [merged] 80f7e6f31b93c075aa76eed810925f1c45a593c8 mtn: note: your workspaces have not been updated