[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Monotone-commits-diffs] net.venge.monotone.issue-209: d6fdf316792e79267
|
From: |
code |
|
Subject: |
[Monotone-commits-diffs] net.venge.monotone.issue-209: d6fdf316792e79267f9594e6761852b4ef6a9a6e |
|
Date: |
Mon, 23 Jul 2012 11:01:13 +0200 (CEST) |
revision: d6fdf316792e79267f9594e6761852b4ef6a9a6e
date: 2012-07-21T13:23:31
author: address@hidden
branch: net.venge.monotone.issue-209
changelog:
resolve_conflicts_dropped_modified_upstream_vs_local_2: Now passing. Still more
tests to add
* src/merge_conflict.cc (resolve_dropped_modified_one): factor out, improve
(resolve_dropped_modified_conflicts): use it for left and right resolution
* src/merge_content.cc (resolve_merge_conflicts): resolve_dropped_modified now
requires a db adaptor
* src/merge_roster.cc (resolve_conflicts::image(side_t)): new
(dump dropped_modified): improve
(dump duplicate_name): improve
*
test/func/resolve_conflicts_dropped_modified_upstream_vs_local_2/__driver__.lua:
add comment. Now passing.
manifest:
format_version "1"
new_manifest [bfcf68fecad2b6c9e9b5739468b99350bb37a754]
old_revision [48b14e6f132ad73fc3ed421bbf318c4ce6ec5b4e]
patch "src/merge_conflict.cc"
from [ed311d6cdf1fa47f00a602e0db5b57f84be6a690]
to [9b89912a0ccd71b9d77d5978ddf1b8bb89368b28]
patch "src/merge_content.cc"
from [6a1d7ad7184f88674a585f5065e965f15294f7dc]
to [095c2d20ac8d8d495806f6463c311d253cb6a686]
patch "src/merge_roster.cc"
from [5d36f441612f9de68ce237b13226bdffd1e2c92a]
to [a082529e5a0fced55068a65a67bc6f21074d6579]
patch "src/merge_roster.hh"
from [b013f590a5c9e643afa06d41d3ed362f6c27d0e6]
to [d214f006b7f4f1e3161313c39f3aac706372ccc5]
patch
"test/func/resolve_conflicts_dropped_modified_upstream_vs_local_2/__driver__.lua"
from [897b4a52ad2ebdb38453f98db1b4284da562a03c]
to [b3dfb58300c6e9e6762b9132c33e910e05901c29]
============================================================
--- src/merge_conflict.cc ed311d6cdf1fa47f00a602e0db5b57f84be6a690
+++ src/merge_conflict.cc 9b89912a0ccd71b9d77d5978ddf1b8bb89368b28
@@ -2915,36 +2915,147 @@ static void
}
static void
-error_extra_left(node_id left_nid, node_id right_nid, roster_t const & right_roster)
+resolve_dropped_modified_one(lua_hooks & lua,
+ string side_image,
+ bool handling_dropped_side,
+ resolve_conflicts::file_resolution_t const & resolution,
+ resolve_conflicts::file_resolution_t const & other_resolution,
+ roster_t const & side_roster,
+ file_path const & name,
+ file_id const & fid,
+ node_id const nid,
+ content_merge_database_adaptor & adaptor,
+ temp_node_id_source & nis,
+ roster_t & result_roster)
{
- if (left_nid == the_null_node)
+ if (nid == the_null_node)
{
- file_path right_name;
- file_id right_fid;
- right_roster.get_file_details(right_nid, right_fid, right_name);
- E(false, origin::user,
- F("extra left_resolution provided for dropped_modifed '%s'") % right_name);
+ E(resolution.resolution == resolve_conflicts::none, origin::user,
+ F("extra %s_resolution provided for dropped_modified '%s'") % side_image % name);
+ return;
}
-}
+ else
+ {
+ E(resolution.resolution != resolve_conflicts::none, origin::user,
+ F("no %s_resolution provided for dropped_modified '%s'") % side_image % name);
+ }
-static void
-error_extra_right(node_id left_nid, node_id right_nid, roster_t const & left_roster)
-{
- if (right_nid == the_null_node)
+ switch (resolution.resolution)
{
- file_path left_name;
- file_id left_fid;
- left_roster.get_file_details(left_nid, left_fid, left_name);
- E(false, origin::user,
- F("extra right_resolution provided for dropped_modifed '%s'") % left_name);
+ case resolve_conflicts::none:
+ // handled above; can't get here
+ break;
+
+ case resolve_conflicts::content_user:
+ // FIXME: check other_resolution for consistency
+ if (handling_dropped_side)
+ {
+ // recreated; replace the contents of the recreated node
+ replace_content(side_roster, nid, result_roster, resolution.content, adaptor);
+ }
+ else
+ {
+ // modified; drop and create a new node
+ // See comments in keep below on why we drop first
+ result_roster.drop_detached_node(nid);
+
+ node_id new_nid = create_new_node (side_roster, nid, result_roster, resolution.content, adaptor, nis);
+
+ attach_node(lua, result_roster, new_nid, name);
+ }
+ break;
+
+ case resolve_conflicts::content_internal:
+ // not valid for dropped_modified
+ I(false);
+
+ 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
+ P(F("dropping '%s'") % name);
+ result_roster.drop_detached_node(nid);
+ break;
+
+ case resolve_conflicts::keep:
+ if (handling_dropped_side)
+ {
+ // recreated; keep the recreated contents
+ P(F("keeping '%s'") % name);
+ attach_node(lua, result_roster, nid, name);
+ }
+ else
+ {
+ // modified; keep the modified contents
+
+ P(F("keeping '%s'") % name);
+ P(F("history for '%s' will be lost; see user manual Merge Conflicts section") %
+ name);
+
+ // We'd like to just attach_node here, but that violates a
+ // fundamental design principle of mtn; nodes are born once,
+ // and die once. If we attach here, the node is born, died,
+ // and then born again.
+ //
+ // So we have to drop the old node, and create a new node with
+ // the same contents. That loses history; 'mtn log <path>'
+ // will end here, not showing the history of the original
+ // node.
+ result_roster.drop_detached_node(nid);
+ node_id nid = result_roster.create_file_node(fid, nis);
+ attach_node (lua, result_roster, nid, name);
+ }
+ break;
+
+ case resolve_conflicts::rename:
+ if (handling_dropped_side)
+ {
+ // recreated; rename the recreated contents
+ P(F("renaming '%s' to '%s'") % name % resolution.rename.as_external());
+ attach_node(lua, result_roster, nid, resolution.rename);
+ }
+ else
+ {
+ // modified; drop, create new node with the modified contents, rename
+ // See comment in keep above on why we drop first.
+ result_roster.drop_detached_node(nid);
+
+ P(F("renaming '%s' to '%s'") % name % resolution.rename.as_external());
+ P(F("history for '%s' will be lost; see user manual Merge Conflicts section") % name);
+
+ node_id new_nid = result_roster.create_file_node(fid, nis);
+ attach_node (lua, result_roster, new_nid, resolution.rename);
+ }
+ break;
+
+ case resolve_conflicts::content_user_rename:
+ if (handling_dropped_side)
+ {
+ // recreated; rename and replace the recreated contents
+ replace_content(side_roster, nid, result_roster, resolution.content, adaptor);
+
+ P(F("renaming '%s' to '%s'") % name % resolution.rename.as_external());
+
+ attach_node (lua, result_roster, nid, resolution.rename);
+ }
+ else
+ {
+ // modified; drop, rename and replace the modified contents
+ node_id nid = create_new_node
+ (side_roster, nid, result_roster, resolution.content, adaptor, nis);
+
+ P(F("renaming '%s' to '%s'") % name % resolution.rename.as_external());
+
+ attach_node(lua, result_roster, nid, resolution.rename);
+ }
+ break;
}
-}
+} // resolve_dropped_modified_one
void
roster_merge_result::resolve_dropped_modified_conflicts(lua_hooks & lua,
roster_t const & left_roster,
roster_t const & right_roster,
- content_merge_adaptor & adaptor,
+ content_merge_database_adaptor & adaptor,
temp_node_id_source & nis)
{
MM(left_roster);
@@ -2963,164 +3074,56 @@ roster_merge_result::resolve_dropped_mod
file_path right_name;
file_id right_fid;
- switch (conflict.left_resolution.resolution)
+ if (null_id(conflict.left_rid))
{
- case resolve_conflicts::none:
if (conflict.left_nid != the_null_node)
- {
- // FIXME: use left_name here if it's not too hard
- right_roster.get_file_details(conflict.right_nid, right_fid, right_name);
- E(false, origin::user,
- F("no left_resolution provided for dropped_modifed '%s'") % right_name);
- }
- break;
+ left_roster.get_file_details(conflict.left_nid, left_fid, left_name);
+ }
+ else
+ {
+ roster_t tmp;
+ adaptor.db.get_roster(conflict.left_rid, tmp);
+ tmp.get_file_details(conflict.left_nid, left_fid, left_name);
+ }
- case resolve_conflicts::content_user:
- switch (conflict.dropped_side)
- {
- case resolve_conflicts::left_side:
- error_extra_left(conflict.left_nid, conflict.right_nid, right_roster);
+ if (null_id (conflict.right_rid))
+ {
+ if (conflict.right_nid != the_null_node)
+ right_roster.get_file_details(conflict.right_nid, right_fid, right_name);
+ }
+ else
+ {
+ roster_t tmp;
+ adaptor.db.get_roster(conflict.right_rid, tmp);
+ tmp.get_file_details(conflict.right_nid, right_fid, right_name);
+ }
- // recreated; replace the contents of the recreated node
- replace_content(left_roster, conflict.left_nid, roster, conflict.left_resolution.content, adaptor);
- break;
+ resolve_dropped_modified_one (lua,
+ string("left"),
+ conflict.dropped_side == resolve_conflicts::left_side,
+ conflict.left_resolution,
+ conflict.right_resolution,
+ left_roster,
+ left_name,
+ left_fid,
+ conflict.left_nid,
+ adaptor,
+ nis,
+ roster);
- case resolve_conflicts::right_side:
- error_extra_right(conflict.left_nid, conflict.right_nid, left_roster);
+ resolve_dropped_modified_one (lua,
+ string("right"),
+ conflict.dropped_side == resolve_conflicts::right_side,
+ conflict.right_resolution,
+ conflict.left_resolution,
+ right_roster,
+ right_name,
+ right_fid,
+ conflict.right_nid,
+ adaptor,
+ nis,
+ roster);
- // modified; drop and create a new node
- // See comments in keep below on why we drop first
- roster.drop_detached_node(conflict.left_nid);
-
- node_id new_nid = create_new_node
- (left_roster, conflict.left_nid, roster, conflict.left_resolution.content, adaptor, nis);
-
- attach_node(lua, roster, new_nid, left_name);
-
- break;
- }
- break;
-
- case resolve_conflicts::content_internal:
- I(false);
-
- case resolve_conflicts::drop:
- switch (conflict.dropped_side)
- {
- case resolve_conflicts::left_side:
- error_extra_left(conflict.left_nid, conflict.right_nid, right_roster);
-
- left_roster.get_file_details(conflict.left_nid, left_fid, left_name);
- P(F("dropping '%s'") % left_name);
-
- roster.drop_detached_node(conflict.left_nid);
- break;
-
- case resolve_conflicts::right_side:
- error_extra_right(conflict.left_nid, conflict.right_nid, left_roster);
-
- right_roster.get_file_details(conflict.right_nid, right_fid, right_name);
- P(F("dropping '%s'") % right_name);
-
- roster.drop_detached_node(conflict.right_nid);
- break;
- }
-
- case resolve_conflicts::keep:
- switch (conflict.dropped_side)
- {
- case resolve_conflicts::left_side:
- error_extra_left(conflict.left_nid, conflict.right_nid, right_roster);
-
- // recreated; keep the recreated contents
- left_roster.get_file_details(conflict.left_nid, left_fid, left_name);
- P(F("keeping '%s'") % left_name);
- attach_node(lua, roster, conflict.left_nid, left_name);
- break;
-
- case resolve_conflicts::right_side:
- // modified; keep the modified contents
-
- left_roster.get_file_details(conflict.left_nid, left_fid, left_name);
- P(F("keeping '%s'") % left_name);
- P(F("history for '%s' will be lost; see user manual Merge Conflicts section") %
- left_name);
-
- // We'd like to just attach_node here, but that violates a
- // fundamental design principle of mtn; nodes are born once,
- // and die once. If we attach here, the node is born, died,
- // and then born again.
- //
- // So we have to drop the old node, and create a new node with
- // the same contents. That loses history; 'mtn log <path>'
- // will end here, not showing the history of the original
- // node.
- roster.drop_detached_node(conflict.left_nid);
- node_id nid = roster.create_file_node(left_fid, nis);
- attach_node (lua, roster, nid, left_name);
- break;
- }
-
- case resolve_conflicts::rename:
- switch (conflict.dropped_side)
- {
- case resolve_conflicts::left_side:
- error_extra_left(conflict.left_nid, conflict.right_nid, right_roster);
-
- // recreated; rename the recreated contents
- left_roster.get_file_details(conflict.left_nid, left_fid, left_name);
- P(F("renaming '%s' to '%s'") % left_name % conflict.left_resolution.rename.as_external());
- attach_node(lua, roster, conflict.left_nid, conflict.left_resolution.rename);
- break;
-
- case resolve_conflicts::right_side:
- error_extra_right(conflict.left_nid, conflict.right_nid, left_roster);
-
- // modified; drop, create new node with the modified contents, rename
- // See comment in keep above on why we drop first.
- roster.drop_detached_node(conflict.left_nid);
-
- left_roster.get_file_details(conflict.left_nid, left_fid, left_name);
- P(F("renaming '%s' to '%s'") % left_name % conflict.left_resolution.rename.as_external());
- P(F("history for '%s' will be lost; see user manual Merge Conflicts section") %
- left_name);
-
- node_id new_nid = roster.create_file_node(left_fid, nis);
- attach_node (lua, roster, new_nid, conflict.left_resolution.rename);
- break;
- }
-
- case resolve_conflicts::content_user_rename:
- switch (conflict.dropped_side)
- {
- case resolve_conflicts::left_side:
- error_extra_left(conflict.left_nid, conflict.right_nid, right_roster);
-
- // recreated; rename and replace the recreated contents
- replace_content(left_roster, conflict.left_nid, roster, conflict.left_resolution.content, adaptor);
-
- left_roster.get_file_details(conflict.left_nid, left_fid, left_name);
- P(F("renaming '%s' to '%s'") % left_name % conflict.left_resolution.rename.as_external());
-
- attach_node (lua, roster, conflict.left_nid, conflict.left_resolution.rename);
- break;
-
- case resolve_conflicts::right_side:
- // modified; drop, rename and replace the modified contents
- node_id nid = create_new_node
- (left_roster, conflict.left_nid, roster, conflict.left_resolution.content, adaptor, nis);
-
- left_roster.get_file_details(conflict.left_nid, left_fid, left_name);
- P(F("renaming '%s' to '%s'") % left_name % conflict.left_resolution.rename.as_external());
-
- attach_node(lua, roster, conflict.left_nid, conflict.left_resolution.rename);
- break;
- }
- }
-
- // FIXME: handle right_resolution; factor out resolve_dropped_modified_one_side
- // add inconsistent left/right resolution checks
-
} // end for
dropped_modified_conflicts.clear();
============================================================
--- src/merge_content.cc 6a1d7ad7184f88674a585f5065e965f15294f7dc
+++ src/merge_content.cc 095c2d20ac8d8d495806f6463c311d253cb6a686
@@ -747,7 +747,8 @@ resolve_merge_conflicts(lua_hooks & lua,
// Resolve the ones we can, if they have resolutions specified. Each
// conflict list is deleted once all are resolved.
result.resolve_orphaned_node_conflicts(lua, left_roster, right_roster, adaptor);
- result.resolve_dropped_modified_conflicts(lua, left_roster, right_roster, adaptor, nis);
+ result.resolve_dropped_modified_conflicts(lua, left_roster, right_roster,
+ dynamic_cast <content_merge_database_adaptor&>(adaptor), nis);
result.resolve_duplicate_name_conflicts(lua, left_roster, right_roster, adaptor);
result.resolve_file_content_conflicts (lua, left_roster, right_roster, adaptor);
============================================================
--- src/merge_roster.cc 5d36f441612f9de68ce237b13226bdffd1e2c92a
+++ src/merge_roster.cc a082529e5a0fced55068a65a67bc6f21074d6579
@@ -29,6 +29,19 @@ namespace resolve_conflicts
namespace resolve_conflicts
{
char const *
+ image(side_t item)
+ {
+ switch (item)
+ {
+ case resolve_conflicts::left_side:
+ return "left_side";
+ case resolve_conflicts::right_side:
+ return "right_side";
+ }
+ I(false); // keep compiler happy
+ }
+
+ char const *
image(resolution_t resolution)
{
switch (resolution)
@@ -55,7 +68,7 @@ namespace resolve_conflicts
image(file_resolution_t res)
{
if (res.resolution == resolve_conflicts::none)
- return string("");
+ return string("\n");
else
{
ostringstream oss;
@@ -116,12 +129,15 @@ dump(dropped_modified_conflict const & c
dump(dropped_modified_conflict const & conflict, string & out)
{
ostringstream oss;
- oss << "dropped_modified_conflict on node: " <<
- conflict.left_nid == the_null_node ? conflict.right_nid : conflict.left_nid;
- oss << " orphaned: " << conflict.orphaned;
- oss << " left_resolution: " << image(conflict.left_resolution);
- oss << " right_resolution: " << image(conflict.left_resolution);
- oss << "\n";
+ oss << "dropped_modified_conflict -\n";
+ oss << " dropped_side : " << image(conflict.dropped_side) << "\n";
+ oss << " left_nid : " << conflict.left_nid << "\n";
+ oss << " right_nid : " << conflict.right_nid << "\n";
+ oss << " orphaned : " << conflict.orphaned << "\n";
+ oss << " left_rid : " << conflict.left_rid << "\n";
+ oss << " right_rid : " << conflict.right_rid << "\n";
+ oss << " left_resolution : " << image(conflict.left_resolution);
+ oss << " right_resolution: " << image(conflict.right_resolution);
out = oss.str();
}
@@ -133,8 +149,8 @@ dump(duplicate_name_conflict const & con
<< "and right node: " << conflict.right_nid << " "
<< "parent: " << conflict.parent_name.first << " "
<< "basename: " << conflict.parent_name.second << " "
- << "left_resolution: " << " "
- << "right_resolution: "
+ << "left_resolution: " << image(conflict.left_resolution)
+ << "right_resolution: " << image(conflict.right_resolution)
<< "\n";
out = oss.str();
}
============================================================
--- src/merge_roster.hh b013f590a5c9e643afa06d41d3ed362f6c27d0e6
+++ src/merge_roster.hh d214f006b7f4f1e3161313c39f3aac706372ccc5
@@ -35,6 +35,8 @@ namespace resolve_conflicts
enum side_t {left_side, right_side};
+ char const * image(side_t item);
+
struct file_resolution_t
{
resolution_t resolution;
@@ -137,7 +139,8 @@ struct dropped_modified_conflict
bool orphaned; // if true, the dropped side is due to a dropped parent directory
- // read_dropped_modified sets rid for dropped, non-recreated file FIXME: can we use this?
+ // read_dropped_modified sets rid when corresponding nid is non-null and
+ // not in the corresponding parent roster
revision_id left_rid, right_rid;
resolve_conflicts::file_resolution_t left_resolution, right_resolution;
@@ -313,7 +316,7 @@ struct roster_merge_result
void resolve_dropped_modified_conflicts(lua_hooks & lua,
roster_t const & left_roster,
roster_t const & right_roster,
- content_merge_adaptor & adaptor,
+ content_merge_database_adaptor & adaptor,
temp_node_id_source & nis);
void report_duplicate_name_conflicts(roster_t const & left,
============================================================
--- test/func/resolve_conflicts_dropped_modified_upstream_vs_local_2/__driver__.lua 897b4a52ad2ebdb38453f98db1b4284da562a03c
+++ test/func/resolve_conflicts_dropped_modified_upstream_vs_local_2/__driver__.lua b3dfb58300c6e9e6762b9132c33e910e05901c29
@@ -87,6 +87,12 @@ check(samelines("stderr",
"mtn: dropped and recreated on the right",
"mtn: 1 conflict with supported resolutions."}))
+-- There are two nodes with filename 'file_2'; node 4 in upstream,
+-- node 3 in local. At this point, node 4 is modified in upstream and
+-- dropped in local; node 3 is unborn in upstream and modified in
+-- local. Therefore this is a combination of dropped_modified and
+-- duplicate_name conflicts, which we handle as a dropped_modified
+-- conflict.
check(mtn("conflicts", "store", upstream_3, local_2), 0, nil, true)
check(samefilestd("conflicts_3_2", "_MTN/conflicts"))
| [Prev in Thread] |
Current Thread |
[Next in Thread] |
- [Monotone-commits-diffs] net.venge.monotone.issue-209: d6fdf316792e79267f9594e6761852b4ef6a9a6e,
code <=