# # # patch "automate.cc" # from [a36f5623c1c25c9261022318c069e1ba7521fa99] # to [98ab6c49869faf9653eb7db5d57c7607344495cd] # # patch "commands.cc" # from [aed0ffa2eb35ca26abc0c13343572a2ebe3a28d2] # to [842e3085a1ea0d50bfbc56e6354800da6e602066] # # patch "restrictions.cc" # from [6ceb1e8aa82b2b2a6273d23d9a68574a4e5ae072] # to [bfd98531c798e0ecfc31a3a6b5eb93a99b7b6494] # # patch "restrictions.hh" # from [dba2b28ba5f422a2721c13da278e276ef40b8959] # to [c7aaa32530885b63933e12922aa807da3f5d8cd0] # # patch "unit_tests.cc" # from [cd6c9924afd3ab76ba8bfb91ff23990f3a1cc565] # to [c5ded895649b2381cc1954ae2c7c12820543ca5f] # # patch "unit_tests.hh" # from [b2611fdb586f062e99b396dd5985db3b1f884360] # to [bb80f6ddbc9fb575361740494e979b03c1b5b8af] # # patch "work.cc" # from [91344ff1c00cef1c3ab300b2956bccb27b0d323f] # to [2b3295ee7a54196b03e271328286daa306866e72] # # patch "work.hh" # from [487a9bda1fdf89f4eab461d5c71a10e7bf7fce0f] # to [e7068dfe9d8cf640341e751cad279f76344c4d5b] # ============================================================ --- automate.cc a36f5623c1c25c9261022318c069e1ba7521fa99 +++ automate.cc 98ab6c49869faf9653eb7db5d57c7607344495cd @@ -707,7 +707,8 @@ classify_roster_paths(curr, unchanged, changed, missing, app); curr.extract_path_set(known); - file_itemizer u(app, known, unknown, ignored); + restriction mask; + file_itemizer u(app, known, unknown, ignored, mask); walk_tree(file_path(), u); inventory_node_state(inventory, unchanged, inventory_item::UNCHANGED_NODE); ============================================================ --- commands.cc aed0ffa2eb35ca26abc0c13343572a2ebe3a28d2 +++ commands.cc 842e3085a1ea0d50bfbc56e6354800da6e602066 @@ -1688,8 +1688,7 @@ new_roster.extract_path_set(known); - // FIXME_RESTRICTIONS: use mask to restrict paths instead of nids - file_itemizer u(app, known, unknown, ignored); + file_itemizer u(app, known, unknown, ignored, mask); walk_tree(file_path(), u); } ============================================================ --- restrictions.cc 6ceb1e8aa82b2b2a6273d23d9a68574a4e5ae072 +++ restrictions.cc bfd98531c798e0ecfc31a3a6b5eb93a99b7b6494 @@ -41,15 +41,12 @@ split_path sp; file_path_external(*i).split(sp); paths.insert(sp); - L(F("added path '%s'") % *i); } } void restriction::add_nodes(roster_t const & roster) { - L(F("adding nodes\n")); - for (path_set::const_iterator i = paths.begin(); i != paths.end(); ++i) { // TODO: (future) handle some sort of peg revision path syntax here. @@ -58,41 +55,38 @@ if (roster.has_node(*i)) { - node_t node = roster.get_node(*i); - bool recursive = is_dir_t(node); - node_id nid = node->self; + // TODO: proper recursive wildcard paths like foo/... + // for now explicit paths are recursive + // and implicit parents are non-recursive - valid_paths.insert(*i); - - // TODO: proper wildcard paths like foo/... - // for now we always add directories recursively and files exactly - // TODO: possibly fail with nice error if path is already explicitly // in the map? - L(F("adding nid %d '%s'\n") % nid % file_path(*i)); - insert(nid, recursive); - // currently we need to insert the parents of included nodes so that // the included nodes are not orphaned in a restricted roster. this // happens in cases like add "a" + add "a/b" when only "a/b" is // included. i.e. "a" must be included for "a/b" to be valid. this - // isn't entirely sensible and should probably be revisited. + // isn't entirely sensible and should probably be revisited. it does + // match the current (old restrictions) semantics though. - node_id parent = node->parent; - while (!null_node(parent)) + valid_paths.insert(*i); + + bool recursive = true; + node_id nid = roster.get_node(*i)->self; + + while (!null_node(nid)) { split_path sp; - roster.get_name(parent, sp); - L(F("adding parent %d '%s'\n") % parent % file_path(sp)); - insert(parent, false); - node = roster.get_node(parent); - parent = node->parent; - } + roster.get_name(nid, sp); + L(F("adding nid %d path '%s' recursive %s") % nid % file_path(sp) % recursive); - // TODO: consider keeping a list of valid paths here that we can use - // for doing a set-difference against with the full list of paths to - // find those that are not legal in any roster of this restriction + restricted_node_map[nid] |= recursive; + restricted_path_map[sp] |= recursive; + + recursive = false; + + nid = roster.get_node(nid)->parent; + } } else { @@ -105,30 +99,33 @@ bool restriction::includes(roster_t const & roster, node_id nid) const { + MM(roster); + I(roster.has_node(nid)); + // empty restriction includes everything if (restricted_node_map.empty()) - return true; + { + split_path sp; + roster.get_name(nid, sp); + L(F("included nid %d path '%s'") % nid % file_path(sp)); + return true; + } node_id current = nid; - MM(roster); - - I(roster.has_node(nid)); - while (!null_node(current)) { split_path sp; roster.get_name(current, sp); - L(F("checking nid %d '%s'\n") % current % file_path(sp)); - restriction_map::const_iterator r = restricted_node_map.find(current); + map::const_iterator r = restricted_node_map.find(current); if (r != restricted_node_map.end()) { - // found exact node or a recursive parent + // found exact node or a explicit/recusrive parent if (r->second || current == nid) { - L(F("included nid %d '%s'\n") % current % file_path(sp)); + L(F("included nid %d path '%s'") % current % file_path(sp)); return true; } } @@ -139,21 +136,42 @@ split_path sp; roster.get_name(nid, sp); - L(F("excluded nid %d '%s'\n") % nid % file_path(sp)); + L(F("excluded nid %d path '%s'\n") % nid % file_path(sp)); return false; } -void -restriction::insert(node_id nid, bool recursive) +bool +restriction::includes(split_path const & sp) const { - // we (mistakenly) allow multiple settings of the recursive include flag on a - // nid. this needs to be fixed but we need to track more than a simple boolean - // to do it. nids can be added non-recursively as parents of included nids, or - // explicitly. we should probably prevent explicit inclusion of foo/bar and - // foo/bar/... though. + if (restricted_path_map.empty()) + { + L(F("included path '%s'") % file_path(sp)); + return true; + } - restricted_node_map[nid] |= recursive; + split_path current(sp); + + while (!current.empty()) + { + L(F("checking path '%s'\n") % current); + map::const_iterator r = restricted_path_map.find(current); + + if (r != restricted_path_map.end()) + { + if (r->second || current == sp) + { + L(F("included path '%s'") % file_path(sp)); + return true; + } + } + + current.pop_back(); + } + + L(F("excluded path '%s'") % file_path(sp)); + return false; + } void @@ -171,3 +189,162 @@ E(bad == 0, F("%d unknown paths") % bad); } + +//////////////////////////////////////////////////////////////////// +// testing +//////////////////////////////////////////////////////////////////// + +#ifdef BUILD_UNIT_TESTS +#include "unit_tests.hh" +#include "roster.hh" +#include "sanity.hh" + +using std::string; + +file_id file1_id(string("1000000000000000000000000000000000000000")); +file_id file2_id(string("2000000000000000000000000000000000000000")); +file_id file3_id(string("3000000000000000000000000000000000000000")); + +static void +test_basic_restrictions() +{ + temp_node_id_source nis; + roster_t roster; + + node_id root_nid = roster.create_dir_node(nis); + node_id file1_nid = roster.create_file_node(file1_id, nis); + node_id file2_nid = roster.create_file_node(file2_id, nis); + node_id file3_nid = roster.create_file_node(file3_id, nis); + + split_path root_path, file1_path, file2_path, file3_path; + file_path().split(root_path); + file_path_internal("file1").split(file1_path); + file_path_internal("file2").split(file2_path); + file_path_internal("file3").split(file3_path); + + roster.attach_node(root_nid, root_path); + roster.attach_node(file1_nid, file1_path); + roster.attach_node(file2_nid, file2_path); + roster.attach_node(file3_nid, file3_path); + + { + // empty restriction + restriction mask; + + BOOST_CHECK(mask.empty()); + + // check restricted nodes + BOOST_CHECK(mask.includes(roster, root_nid)); + BOOST_CHECK(mask.includes(roster, file1_nid)); + BOOST_CHECK(mask.includes(roster, file2_nid)); + BOOST_CHECK(mask.includes(roster, file3_nid)); + + // check restricted paths + BOOST_CHECK(mask.includes(root_path)); + BOOST_CHECK(mask.includes(file1_path)); + BOOST_CHECK(mask.includes(file2_path)); + BOOST_CHECK(mask.includes(file3_path)); + } + + { + // non-empty restriction + vector args; + args.push_back(utf8(string("file1"))); + + restriction mask(args, roster); + + BOOST_CHECK(!mask.empty()); + + // check restricted nodes + BOOST_CHECK(mask.includes(roster, root_nid)); + BOOST_CHECK(mask.includes(roster, file1_nid)); + BOOST_CHECK(!mask.includes(roster, file2_nid)); + BOOST_CHECK(!mask.includes(roster, file3_nid)); + + // check restricted paths + BOOST_CHECK(mask.includes(root_path)); + BOOST_CHECK(mask.includes(file1_path)); + BOOST_CHECK(!mask.includes(file2_path)); + BOOST_CHECK(!mask.includes(file3_path)); + } + + { + // invalid paths + // non-empty restriction + vector args; + args.push_back(utf8(string("file4"))); + + BOOST_CHECK_THROW(restriction(args, roster), informative_failure); + } + +} + +static void +test_recursive_nonrecursive() +{ + temp_node_id_source nis; + roster_t roster; + + node_id root_nid = roster.create_dir_node(nis); + + node_id dir1_nid = roster.create_dir_node(nis); + node_id dir2_nid = roster.create_dir_node(nis); + + node_id file1_nid = roster.create_file_node(file1_id, nis); + node_id file2_nid = roster.create_file_node(file2_id, nis); + node_id file3_nid = roster.create_file_node(file3_id, nis); + + // root/file1 + // root/dir1 + // root/dir1/file2 + // root/dir1/dir2 + // root/dir1/dir2/file3 + + split_path root_path, file1_path, dir1_path, file2_path, dir2_path, file3_path; + + file_path().split(root_path); + file_path_internal("file1").split(file1_path); + file_path_internal("dir1").split(dir1_path); + file_path_internal("dir1/file2").split(file2_path); + file_path_internal("dir1/dir2").split(dir2_path); + file_path_internal("dir1/dir2/file3").split(file3_path); + + roster.attach_node(root_nid, root_path); + roster.attach_node(file1_nid, file1_path); + roster.attach_node(dir1_nid, dir1_path); + roster.attach_node(file2_nid, file2_path); + roster.attach_node(dir2_nid, dir2_path); + roster.attach_node(file3_nid, file3_path); + + vector args; + args.push_back(utf8(string("dir1/dir2"))); + + restriction mask(args, roster); + + BOOST_CHECK(!mask.empty()); + + // check restricted nodes + BOOST_CHECK(mask.includes(roster, root_nid)); + BOOST_CHECK(!mask.includes(roster, file1_nid)); + BOOST_CHECK(mask.includes(roster, dir1_nid)); + BOOST_CHECK(!mask.includes(roster, file2_nid)); + BOOST_CHECK(mask.includes(roster, dir2_nid)); + BOOST_CHECK(mask.includes(roster, file3_nid)); + + // check restricted paths + BOOST_CHECK(mask.includes(root_path)); + BOOST_CHECK(!mask.includes(file1_path)); + BOOST_CHECK(mask.includes(dir1_path)); + BOOST_CHECK(!mask.includes(file2_path)); + BOOST_CHECK(mask.includes(dir2_path)); + BOOST_CHECK(mask.includes(file3_path)); +} + +void +add_restrictions_tests(test_suite * suite) +{ + I(suite); + suite->add(BOOST_TEST_CASE(&test_basic_restrictions)); + suite->add(BOOST_TEST_CASE(&test_recursive_nonrecursive)); +} +#endif // BUILD_UNIT_TESTS ============================================================ --- restrictions.hh dba2b28ba5f422a2721c13da278e276ef40b8959 +++ restrictions.hh c7aaa32530885b63933e12922aa807da3f5d8cd0 @@ -46,18 +46,25 @@ roster_t const & roster2); bool includes(roster_t const & roster, node_id nid) const; + bool includes(split_path const & sp) const; bool empty() const { return restricted_node_map.empty(); } private: - typedef map restriction_map; - restriction_map restricted_node_map; + // true in these two maps indicates an explicitly included node or path and + // explicitly included directories are recursive. alternatively, false in + // these maps indicated an implicitly included parent directory which is + // specifically required for the restriction but does not include any of its + // children + + map restricted_node_map; + map restricted_path_map; path_set paths; + path_set valid_paths; void set_paths(vector const & args); void add_nodes(roster_t const & roster); - void insert(node_id nid, bool recursive); void check_paths(); }; ============================================================ --- unit_tests.cc cd6c9924afd3ab76ba8bfb91ff23990f3a1cc565 +++ unit_tests.cc c5ded895649b2381cc1954ae2c7c12820543ca5f @@ -89,6 +89,9 @@ if (t.empty() || t.find("roster") != t.end()) add_roster_tests(suite); + if (t.empty() || t.find("restrictions") != t.end()) + add_restrictions_tests(suite); + // all done, add our clean-shutdown-indicator suite->add(BOOST_TEST_CASE(&clean_shutdown_dummy_test)); ============================================================ --- unit_tests.hh b2611fdb586f062e99b396dd5985db3b1f884360 +++ unit_tests.hh bb80f6ddbc9fb575361740494e979b03c1b5b8af @@ -36,5 +36,6 @@ void add_string_queue_tests(test_suite * suite); void add_paths_tests(test_suite * suite); void add_roster_tests(test_suite * suite); +void add_restrictions_tests(test_suite * suite); #endif ============================================================ --- work.cc 91344ff1c00cef1c3ab300b2956bccb27b0d323f +++ work.cc 2b3295ee7a54196b03e271328286daa306866e72 @@ -14,6 +14,7 @@ #include "cset.hh" #include "file_io.hh" #include "platform.hh" +#include "restrictions.hh" #include "sanity.hh" #include "safe_map.hh" #include "transforms.hh" @@ -40,12 +41,7 @@ split_path sp; path.split(sp); - // FIXME_RESTRICTIONS: this doesn't look so good for roster restrictions - - // unknown and ignored will never have nodes in the rosters so some other form - // of restriction is going to be needed here - - if (app.restriction_includes(sp) && known.find(sp) == known.end()) + if (mask.includes(sp) && known.find(sp) == known.end()) { if (app.lua.hook_ignore_file(path)) ignored.insert(sp); ============================================================ --- work.hh 487a9bda1fdf89f4eab461d5c71a10e7bf7fce0f +++ work.hh e7068dfe9d8cf640341e751cad279f76344c4d5b @@ -54,8 +54,9 @@ path_set & known; path_set & unknown; path_set & ignored; - file_itemizer(app_state & a, path_set & k, path_set & u, path_set & i) - : app(a), known(k), unknown(u), ignored(i) {} + restriction const & mask; + file_itemizer(app_state & a, path_set & k, path_set & u, path_set & i, restriction const & r) + : app(a), known(k), unknown(u), ignored(i), mask(r) {} virtual void visit_dir(file_path const & path); virtual void visit_file(file_path const & path); };