# # # patch "NEWS" # from [f5310db9d9d323b7c859000bcb482545e2a6f907] # to [7ea4147ccff843083f444258f81173c0897ed4a8] # # patch "restrictions.cc" # from [87c7a8736ee7c289e9de64923ef04bd8016f2801] # to [69361677eaddd7346cd72dab35896609948f0a5d] # # patch "tests/automate_inventory_restricted/__driver__.lua" # from [621a159a9ac30e2e099c6ee20926c831fac8b6c4] # to [ab30596e5444d0134d518cf7e14f40e99a475ae8] # # patch "tests/manifest_restrictions/__driver__.lua" # from [aa910e255f6d111a094570a41c30e88a4c2a54fd] # to [83cb52d1aa70372b3596e999c2d8e75c58038e95] # # patch "tests/restricted_commands_are_consistent/__driver__.lua" # from [c0e5031b850d122bc5568d053101937e1f201a93] # to [882f829c95a6b3e9aa08ba01828661da2df33651] # ============================================================ --- NEWS f5310db9d9d323b7c859000bcb482545e2a6f907 +++ NEWS 7ea4147ccff843083f444258f81173c0897ed4a8 @@ -1,9 +1,18 @@ ??? ??? ?? ??:??:?? UTC ???? 0.40 release. Changes + - Values used with the --depth option used to control recursion with + node and path restrictions have changed. Using --depth=0 now means + exactly the specified directories and *not* their children. Using + --depth=1 now means the specified directories and their immediate + children. Previously --depth=0 included children and --depth=1 + included grandchildren and it was not possible to exclude children + using --depth. The simple fix for anyone using --depth is to add 1 to + the values they are using. + Bugs fixed New features ============================================================ --- restrictions.cc 87c7a8736ee7c289e9de64923ef04bd8016f2801 +++ restrictions.cc 69361677eaddd7346cd72dab35896609948f0a5d @@ -95,6 +95,7 @@ namespace virtual bool operator()(file_path const &) const = 0; protected: unknown_p() {} + virtual ~unknown_p() {} }; struct unknown_node : public unknown_p @@ -106,7 +107,7 @@ namespace { return known_paths.find(p) == known_paths.end(); } - + private: set const & known_paths; }; @@ -134,7 +135,7 @@ namespace return (known_paths.find(p) == known_paths.end() && !work.ignore_file(p)); } - + private: set const & known_paths; workspace & work; @@ -320,7 +321,7 @@ node_restriction::includes(roster_t cons if (depth != -1) { int path_depth = fp.depth(); - if (path_depth <= depth + 1) + if (path_depth <= depth) { L(FL("depth includes nid %d path '%s'") % nid % fp); return true; @@ -342,13 +343,7 @@ node_restriction::includes(roster_t cons node_id current = nid; int path_depth = 0; - // FIXME: this uses depth+1 because the old semantics of depth=0 were - // something like "the current directory and its immediate children". it - // seems somewhat more reasonable here to use depth=0 to mean "exactly - // this directory" and depth=1 to mean "this directory and its immediate - // children" - - while (!null_node(current) && (depth == -1 || path_depth <= depth + 1)) + while (!null_node(current) && (depth == -1 || path_depth <= depth)) { map::const_iterator r = node_map.find(current); @@ -400,7 +395,7 @@ path_restriction::includes(file_path con if (depth != -1) { int path_depth = pth.depth(); - if (path_depth <= depth + 1) + if (path_depth <= depth) { L(FL("depth includes path '%s'") % pth); return true; @@ -418,15 +413,9 @@ path_restriction::includes(file_path con } } - // FIXME: this uses depth+1 because the old semantics of depth=0 were - // something like "the current directory and its immediate children". it - // seems somewhat more reasonable here to use depth=0 to mean "exactly - // this directory" and depth=1 to mean "this directory and its immediate - // children" - int path_depth = 0; file_path fp = pth; - while (depth == -1 || path_depth <= depth + 1) + while (depth == -1 || path_depth <= depth) { map::const_iterator r = path_map.find(fp); @@ -1007,9 +996,6 @@ UNIT_TEST(restrictions, include_depth_0) includes.push_back(file_path_internal("x")); includes.push_back(file_path_internal("y")); - // FIXME: depth == 0 currently means directory + immediate children - // this should be changed to mean just the named directory but for - // compatibility with old restrictions this behaviour has been preserved long depth = 0; // check restricted nodes @@ -1023,6 +1009,78 @@ UNIT_TEST(restrictions, include_depth_0) UNIT_TEST_CHECK(!nmask.includes(roster, nid_g)); UNIT_TEST_CHECK( nmask.includes(roster, nid_x)); + UNIT_TEST_CHECK(!nmask.includes(roster, nid_xf)); + UNIT_TEST_CHECK(!nmask.includes(roster, nid_xg)); + UNIT_TEST_CHECK(!nmask.includes(roster, nid_xx)); + UNIT_TEST_CHECK(!nmask.includes(roster, nid_xxf)); + UNIT_TEST_CHECK(!nmask.includes(roster, nid_xxg)); + UNIT_TEST_CHECK(!nmask.includes(roster, nid_xy)); + UNIT_TEST_CHECK(!nmask.includes(roster, nid_xyf)); + UNIT_TEST_CHECK(!nmask.includes(roster, nid_xyg)); + + UNIT_TEST_CHECK( nmask.includes(roster, nid_y)); + UNIT_TEST_CHECK(!nmask.includes(roster, nid_yf)); + UNIT_TEST_CHECK(!nmask.includes(roster, nid_yg)); + UNIT_TEST_CHECK(!nmask.includes(roster, nid_yx)); + UNIT_TEST_CHECK(!nmask.includes(roster, nid_yxf)); + UNIT_TEST_CHECK(!nmask.includes(roster, nid_yxg)); + UNIT_TEST_CHECK(!nmask.includes(roster, nid_yy)); + UNIT_TEST_CHECK(!nmask.includes(roster, nid_yyf)); + UNIT_TEST_CHECK(!nmask.includes(roster, nid_yyg)); + + // check restricted paths + + path_restriction pmask(includes, excludes, depth); + + UNIT_TEST_CHECK(!pmask.empty()); + + UNIT_TEST_CHECK(!pmask.includes(fp_root)); + UNIT_TEST_CHECK(!pmask.includes(fp_f)); + UNIT_TEST_CHECK(!pmask.includes(fp_g)); + + UNIT_TEST_CHECK( pmask.includes(fp_x)); + UNIT_TEST_CHECK(!pmask.includes(fp_xf)); + UNIT_TEST_CHECK(!pmask.includes(fp_xg)); + UNIT_TEST_CHECK(!pmask.includes(fp_xx)); + UNIT_TEST_CHECK(!pmask.includes(fp_xxf)); + UNIT_TEST_CHECK(!pmask.includes(fp_xxg)); + UNIT_TEST_CHECK(!pmask.includes(fp_xy)); + UNIT_TEST_CHECK(!pmask.includes(fp_xyf)); + UNIT_TEST_CHECK(!pmask.includes(fp_xyg)); + + UNIT_TEST_CHECK( pmask.includes(fp_y)); + UNIT_TEST_CHECK(!pmask.includes(fp_yf)); + UNIT_TEST_CHECK(!pmask.includes(fp_yg)); + UNIT_TEST_CHECK(!pmask.includes(fp_yx)); + UNIT_TEST_CHECK(!pmask.includes(fp_yxf)); + UNIT_TEST_CHECK(!pmask.includes(fp_yxg)); + UNIT_TEST_CHECK(!pmask.includes(fp_yy)); + UNIT_TEST_CHECK(!pmask.includes(fp_yyf)); + UNIT_TEST_CHECK(!pmask.includes(fp_yyg)); +} + +UNIT_TEST(restrictions, include_depth_1) +{ + roster_t roster; + setup(roster); + + vector includes, excludes; + includes.push_back(file_path_internal("x")); + includes.push_back(file_path_internal("y")); + + long depth = 1; + + // check restricted nodes + + node_restriction nmask(includes, excludes, depth, roster); + + UNIT_TEST_CHECK(!nmask.empty()); + + UNIT_TEST_CHECK(!nmask.includes(roster, nid_root)); + UNIT_TEST_CHECK(!nmask.includes(roster, nid_f)); + UNIT_TEST_CHECK(!nmask.includes(roster, nid_g)); + + UNIT_TEST_CHECK( nmask.includes(roster, nid_x)); UNIT_TEST_CHECK( nmask.includes(roster, nid_xf)); UNIT_TEST_CHECK( nmask.includes(roster, nid_xg)); UNIT_TEST_CHECK( nmask.includes(roster, nid_xx)); @@ -1073,17 +1131,14 @@ UNIT_TEST(restrictions, include_depth_0) UNIT_TEST_CHECK(!pmask.includes(fp_yyg)); } -UNIT_TEST(restrictions, include_depth_0_empty_restriction) +UNIT_TEST(restrictions, include_depth_1_empty_restriction) { roster_t roster; setup(roster); vector includes, excludes; - // FIXME: depth == 0 currently means directory + immediate children - // this should be changed to mean just the named directory but for - // compatibility with old restrictions this behaviour has been preserved - long depth = 0; + long depth = 1; // check restricted nodes @@ -1146,7 +1201,7 @@ UNIT_TEST(restrictions, include_depth_0_ UNIT_TEST_CHECK(!pmask.includes(fp_yyg)); } -UNIT_TEST(restrictions, include_depth_1) +UNIT_TEST(restrictions, include_depth_2) { roster_t roster; setup(roster); @@ -1155,10 +1210,7 @@ UNIT_TEST(restrictions, include_depth_1) includes.push_back(file_path_internal("x")); includes.push_back(file_path_internal("y")); - // FIXME: depth == 1 currently means directory + children + grand children - // this should be changed to mean directory + immediate children but for - // compatibility with old restrictions this behaviour has been preserved - long depth = 1; + long depth = 2; // check restricted nodes ============================================================ --- tests/automate_inventory_restricted/__driver__.lua 621a159a9ac30e2e099c6ee20926c831fac8b6c4 +++ tests/automate_inventory_restricted/__driver__.lua ab30596e5444d0134d518cf7e14f40e99a475ae8 @@ -77,9 +77,9 @@ index = check_inventory (parsed, index, -- skip the test files in root ---------- --- Test that 'automate inventory --depth=0' shows only the files in root +-- Test that 'automate inventory --depth=1' shows only the files in root -check(mtn("automate", "inventory", "--depth=0"), 0, true, false) +check(mtn("automate", "inventory", "--depth=1"), 0, true, false) parsed = parse_basic_io(readfile("stdout")) index = 1 @@ -247,11 +247,11 @@ checkexp ("checked all", #parsed, index- checkexp ("checked all", #parsed, index-1) ---------- --- rename a file from root to dir_a, test --depth=0 +-- rename a file from root to dir_a, test --depth=1 check(mtn("rename", "--bookkeep-only", "file_0", "dir_a/file_0"), 0, true, false) -check(mtn("automate", "inventory", "--depth=0"), 0, true, false) +check(mtn("automate", "inventory", "--depth=1"), 0, true, false) parsed = parse_basic_io(readfile("stdout")) index = 1 @@ -295,7 +295,7 @@ rename("file_0", "dir_a/file_0") rename("file_0", "dir_a/file_0") -check(mtn("automate", "inventory", "--depth=0"), 0, true, false) +check(mtn("automate", "inventory", "--depth=1"), 0, true, false) parsed = parse_basic_io(readfile("stdout")) index = 1 ============================================================ --- tests/manifest_restrictions/__driver__.lua aa910e255f6d111a094570a41c30e88a4c2a54fd +++ tests/manifest_restrictions/__driver__.lua 83cb52d1aa70372b3596e999c2d8e75c58038e95 @@ -28,16 +28,16 @@ commit() --check(mtn("ls", "known", "--depth=0"), 0, true, false) --check(not qgrep("fileX", "stdout")) -check(mtn("ls", "known", "--depth=0", ".") , 0, true, false) +check(mtn("ls", "known", "--depth=1", ".") , 0, true, false) check(not qgrep("fileX", "stdout")) -check(mtn("ls", "known", "--depth=1", ".") , 0, true, false) +check(mtn("ls", "known", "--depth=2", ".") , 0, true, false) check(qgrep("fileX", "stdout")) -check(mtn("ls", "known", "--depth=0", "work/A") , 0, true, false) +check(mtn("ls", "known", "--depth=1", "work/A") , 0, true, false) check(not qgrep("fileAB", "stdout")) -check(mtn("ls", "known", "--depth=1", "work/A") , 0, true, false) +check(mtn("ls", "known", "--depth=2", "work/A") , 0, true, false) check(qgrep("fileAB", "stdout")) -- test restriction of unknown, missing, ignored files @@ -148,16 +148,16 @@ check(included("X", 1, 2, 3, 4)) check(mtn("diff"), 0, true, false) check(included("X", 1, 2, 3, 4)) -check(mtn("diff", "--depth=0", "."), 0, true, false) +check(mtn("diff", "--depth=1", "."), 0, true, false) check(not qgrep("fileAB", "stdout")) -check(mtn("diff", "--depth=2", "."), 0, true, false) +check(mtn("diff", "--depth=3", "."), 0, true, false) check(qgrep("fileA", "stdout")) -check(mtn("diff", "--context", "--depth=0", "."), 0, true, false) +check(mtn("diff", "--context", "--depth=1", "."), 0, true, false) check(not qgrep("fileAB", "stdout")) -check(mtn("diff", "--context", "--depth=2", "."), 0, true, false) +check(mtn("diff", "--context", "--depth=3", "."), 0, true, false) check(qgrep("fileA", "stdout")) -- include both source and target of rename ============================================================ --- tests/restricted_commands_are_consistent/__driver__.lua c0e5031b850d122bc5568d053101937e1f201a93 +++ tests/restricted_commands_are_consistent/__driver__.lua 882f829c95a6b3e9aa08ba01828661da2df33651 @@ -68,7 +68,7 @@ data.depth = {} data.both.excluded = {"file1", "file2", "foo/foo1", "foo/bar/bar1"} data.depth = {} -data.depth.args = {".", "--depth", "1"} +data.depth.args = {".", "--depth", "2"} data.depth.included = {"file1", "file2", "foo/foo1", "foo/foo2"} data.depth.excluded = {"foo/bar/bar1", "foo/bar/bar2"}