# # # rename "tests/restricted_revert_bug" # to "tests/revert_contents_not_parent_rename" # # patch "NEWS" # from [bbd7c2a4e825e004d34f1189b4b9d6a7e11b6f1a] # to [2397d083c518f0845599bf6a0b87b19ae64d55d1] # # patch "cmd_ws_commit.cc" # from [f5b0f2b855eb5ebc84c07c9d2a252aad20e57155] # to [ca4c6f38c8f27580b0a4ba2f142f76ba5c957cc9] # # patch "tests/revert_contents_not_parent_rename/__driver__.lua" # from [55cc8ce4d2b0051d240f2e8d02d32ed3500cf408] # to [3eaedd2c570c4dbb4983c06d04f23ad7bedb9316] # # patch "tests/revert_drop_not_rename/__driver__.lua" # from [ee408a43814864389f2325d62997362596675dad] # to [c040fb915fee3b25630513aa0204013501726e8b] # ============================================================ --- NEWS bbd7c2a4e825e004d34f1189b4b9d6a7e11b6f1a +++ NEWS 2397d083c518f0845599bf6a0b87b19ae64d55d1 @@ -24,6 +24,14 @@ - for changes near the beginning of a file, mtn's unified diff output sometimes contained too many leading context lines. + - the path handling of 'mtn revert' was improved and fixed two bugs: + now a restricted revert on a node "dir1/file1" reverts only the + content changes in "file1", but leaves renames of any of its + ancestor nodes untouched; furthermore, if "dir0/" was renamed to + "dir1" and "dir1/file1" was dropped, mtn now re-creates file1 at the + proper place ("dir1/") and leaves no missing files around because + of the non-existing "dir0/". + New features - 'automate drop_db_variables' which drops one database variable ============================================================ --- cmd_ws_commit.cc f5b0f2b855eb5ebc84c07c9d2a252aad20e57155 +++ cmd_ws_commit.cc ca4c6f38c8f27580b0a4ba2f142f76ba5c957cc9 @@ -217,7 +217,19 @@ CMD(revert, "revert", "", CMD_REF(worksp // to get a cset that gets us back to the restricted roster // from the current workspace roster + // the intermediate paths record the paths of all directory nodes + // paths we reverted on the fly for descendant nodes below them. + // if a children of such a directory node should be recreated, we use + // this recorded path here instead of just + // a) the node's old name, which could eventually be wrong if the parent + // path is a rename_target (i.e. a new path), see the + // "revert_drop_not_rename" test + // b) the parent node's new name + the basename of the old name, + // which may be wrong as well in case of a more complex pivot_rename + + std::map intermediate_paths; node_map const & nodes = old_roster.all_nodes(); + for (node_map::const_iterator i = nodes.begin(); i != nodes.end(); ++i) { @@ -230,41 +242,74 @@ CMD(revert, "revert", "", CMD_REF(worksp if (!mask.includes(old_roster, nid)) continue; - file_path fp; - old_roster.get_name(nid, fp); + file_path old_path, new_path, old_parent, new_parent;; + path_component base; + old_roster.get_name(nid, old_path); + old_path.dirname_basename(old_parent, base); + + // if we recorded the parent node in this rename already + // use the intermediate path (i.e. the new new_path after this + // action) as target path for the reverted item) + const std::map::iterator it = + intermediate_paths.find(node->parent); + if (it != intermediate_paths.end()) + { + new_path = it->second / base; + } + else + { + if (old_roster.is_root(node->parent)) + { + new_path = file_path() / base; + } + else + { + new_roster.get_name(node->parent, new_parent); + new_path = new_parent / base; + } + } + if (is_file_t(node)) { file_t f = downcast_to_file_t(node); - if (file_exists(fp)) + if (file_exists(new_path)) { hexenc ident; - calculate_ident(fp, ident); + calculate_ident(new_path, ident); // don't touch unchanged files if (ident == f->content.inner()) continue; + else + L(FL("skipping unchanged %s") % new_path); } - P(F("reverting %s") % fp); - L(FL("reverting %s to [%s]") % fp % f->content); + P(F("reverting %s") % new_path); + L(FL("reverting %s to [%s]") % new_path % f->content); N(app.db.file_version_exists(f->content), F("no file version %s found in database for %s") - % f->content % fp); + % f->content % new_path); file_data dat; L(FL("writing file %s to %s") - % f->content % fp); + % f->content % new_path); app.db.get_file_version(f->content, dat); - write_data(fp, dat.inner()); + write_data(new_path, dat.inner()); } else { - if (!directory_exists(fp)) + intermediate_paths.insert(std::pair(nid, new_path)); + + if (!directory_exists(new_path)) { - P(F("recreating %s/") % fp); - mkdir_p(fp); + P(F("recreating %s/") % new_path); + mkdir_p(new_path); } + else + { + L(FL("skipping existing %s/") % new_path); + } } } @@ -467,8 +512,8 @@ CMD(rename, "rename", "mv", CMD_REF(work //cases for more than one source item. if (src_paths.size() == 1 && dstr()[dstr().size() -1] == '/') if (get_path_status(*src_paths.begin()) != path::directory) - N(get_path_status(dst_path) == path::directory, - F(_("The specified target directory %s/ doesn't exist.")) % dst_path); + N(get_path_status(dst_path) == path::directory, + F(_("The specified target directory %s/ doesn't exist.")) % dst_path); app.work.perform_rename(src_paths, dst_path, app.opts.bookkeep_only); } ============================================================ --- tests/revert_contents_not_parent_rename/__driver__.lua 55cc8ce4d2b0051d240f2e8d02d32ed3500cf408 +++ tests/revert_contents_not_parent_rename/__driver__.lua 3eaedd2c570c4dbb4983c06d04f23ad7bedb9316 @@ -1,6 +1,8 @@ mtn_setup() mtn_setup() --- this bug appears to be related to tests/restricted_diff_bug +-- this was a bug until 0.38 and appeared to be related to +-- tests/restricted_diff_bug, however was resolved by some clever +-- path handling tricks in revert mkdir("dir1") addfile("dir1/test.txt", "booya") @@ -13,11 +15,10 @@ writefile("dir2/test.txt", "boohoo") -- presumably this should only revert the content of test.txt -- because it only includes that node --- note that it is the parents name that has changed check(mtn("revert", "dir2/test.txt"), 0, false, false) reverted=sha1("dir2/test.txt") -xfail(original == reverted) +check(original == reverted) ============================================================ --- tests/revert_drop_not_rename/__driver__.lua ee408a43814864389f2325d62997362596675dad +++ tests/revert_drop_not_rename/__driver__.lua c040fb915fee3b25630513aa0204013501726e8b @@ -1,11 +1,8 @@ -- --- this is a bug report for a weird behaviour with revert --- when a dropped (previously renamed) node should be reverted --- and the rename has happened on one of the parents of the --- node --- see http://colabti.de/irclogger/irclogger_log/monotone?date=2008-01-18,Fri&sel=92#l174 --- for more details +-- Check if a dropped node is properly reverted to its new path +-- which is determined by a not reverted rename of an ancestor +-- (old_dir -> new_dir) -- mtn_setup() @@ -18,9 +15,10 @@ check(mtn("drop", "new_dir/a"), 0, false check(mtn("mv", "old_dir", "new_dir"), 0, false, false) check(mtn("drop", "new_dir/a"), 0, false, false) --- this is the actual command that misbehaves, in the way --- that it does not re-create a under new_dir/a, but --- old_dir/a, which fails, because old_dir is an old node --- and renamed +-- +-- this should re-create a under new_dir/, but not under the +-- old path old_dir/a, i.e. mtn status should not return nonzero +-- because it found missing files +-- check(mtn("revert", "old_dir/a"), 0, false, false) +check(mtn("status"), 0, false, false) -xfail(mtn("status"), 0, false, false)