#
#
# patch "ChangeLog"
# from [0e56d5cc8c66c8de9e9a294a4c2f13eed17723e7]
# to [c3ec3850928b5d2399a88867eb9502f7ab8922dc]
#
# patch "restrictions.cc"
# from [9000266d8470672a6d623df3c5d0c4ad866678de]
# to [77252f64fc5d5cbd4380de33ee362336379b45ec]
#
# patch "restrictions.hh"
# from [997d9f85ddfb423b9a535acdef8a4c44bc384395]
# to [8d68ee9d4981f22f01ecf499602fcbc7b6d4b6fe]
#
============================================================
--- ChangeLog 0e56d5cc8c66c8de9e9a294a4c2f13eed17723e7
+++ ChangeLog c3ec3850928b5d2399a88867eb9502f7ab8922dc
@@ -1,5 +1,9 @@
2006-02-20 Derek Scherger
+ * restrictions.{cc,hh}: attempt to simplify and remove duplication
+
+2006-02-20 Derek Scherger
+
* commands.cc (status, ls_known, find_unknown_and_ignored,
find_missing, commit, diff, revert, log): add excludes to restrictions
* restrictions.{cc,hh}: preliminary support for excludes and unit
============================================================
--- restrictions.cc 9000266d8470672a6d623df3c5d0c4ad866678de
+++ restrictions.cc 77252f64fc5d5cbd4380de33ee362336379b45ec
@@ -13,8 +13,12 @@
#include "transforms.hh"
using std::make_pair;
+using std::set;
// TODO: add support for --depth (replace recursive boolean with depth value)
+// depth really only makes sense when one directory is specified however. it would
+// be much better to support foo/... style recursive paths
+
// TODO: add check for relevant rosters to be used by log
//
// i.e. as log goes back through older and older rosters it may hit one that
@@ -33,6 +37,54 @@
}
}
+// FIXME: should we really be doing implicit includes of parents?
+static void
+get_parent_paths(path_set const & paths, path_set & parents)
+{
+ for (path_set::const_iterator i = paths.begin(); i != paths.end(); ++i)
+ {
+ split_path sp(*i);
+ sp.pop_back();
+ while (!sp.empty() && parents.find(sp) == parents.end())
+ {
+ parents.insert(sp);
+ sp.pop_back();
+ }
+ }
+}
+
+static void
+get_nodes(path_set const & paths, roster_t const & roster,
+ set & nodes,
+ path_set & known_paths)
+{
+ for (path_set::const_iterator i = paths.begin(); i != paths.end(); ++i)
+ {
+ if (roster.has_node(*i))
+ {
+ known_paths.insert(*i);
+ node_id nid = roster.get_node(*i)->self;
+ nodes.insert(nid);
+ }
+ }
+}
+
+// FIXME: should we really be doing implicit includes of parents?
+static void
+get_parent_nodes(set const & nodes, roster_t const & roster, set & parents)
+{
+ for (set::const_iterator i = nodes.begin(); i != nodes.end(); ++i)
+ {
+ I(roster.has_node(*i));
+ node_id nid = roster.get_node(*i)->parent;
+ while (!null_node(nid))
+ {
+ parents.insert(nid);
+ nid = roster.get_node(nid)->parent;
+ }
+ }
+}
+
static void
merge_states(path_state const & old_state,
path_state const & new_state,
@@ -51,6 +103,8 @@
else if (old_state == explicit_exclude && new_state == implicit_include ||
old_state == implicit_include && new_state == explicit_exclude)
{
+ // FIXME: should we really be doing implicit includes of parents?
+
// allowing a mix of explicit excludes and implicit includes on a path is
// rather questionable but is required by things like exclude x include
// x/y. (i.e. includes within excludes) these are also somewhat
@@ -69,201 +123,124 @@
}
static void
-update_path_map(map & path_map, split_path const & sp, path_state const state)
+add_paths(map & path_map, path_set const & paths, path_state const state)
{
- map::iterator p = path_map.find(sp);
- if (p != path_map.end())
+ for (path_set::const_iterator i = paths.begin(); i != paths.end(); ++i)
{
- path_state merged;
- merge_states(p->second.state, state, merged, sp);
- p->second.state = merged;
- }
- else
- {
- path_map.insert(make_pair(sp, path_entry(state)));
- }
-}
-
-static void
-add_included_paths(map & path_map, path_set const & includes)
-{
- for (path_set::const_iterator i = includes.begin(); i != includes.end(); ++i)
- {
- split_path sp(*i);
- path_state state = explicit_include;
-
- while (!sp.empty())
+ map::iterator p = path_map.find(*i);
+ if (p != path_map.end())
{
- update_path_map(path_map, sp, state);
- sp.pop_back();
- state = implicit_include;
+ path_state merged;
+ merge_states(p->second, state, merged, *i);
+ p->second = merged;
}
-
+ else
+ {
+ path_map.insert(make_pair(*i, state));
+ }
}
}
static void
-add_excluded_paths(map & path_map, path_set const & excludes)
+add_nodes(map & node_map,
+ roster_t const & roster,
+ set const & nodes,
+ path_state const state)
{
- for (path_set::const_iterator i = excludes.begin(); i != excludes.end(); ++i)
+ for (set::const_iterator i = nodes.begin(); i != nodes.end(); ++i)
{
- update_path_map(path_map, *i, explicit_exclude);
- }
-}
+ I(roster.has_node(*i));
-static void
-update_node_map(map & node_map, node_id const nid, split_path const & sp, path_state const state)
-{
- map::iterator n = node_map.find(nid);
- if (n != node_map.end())
- {
- path_state merged;
- merge_states(n->second, state, merged, sp);
- n->second = merged;
+ map::iterator n = node_map.find(*i);
+ if (n != node_map.end())
+ {
+ path_state merged;
+ split_path sp;
+ roster.get_name(*i, sp);
+ merge_states(n->second, state, merged, sp);
+ n->second = merged;
+ }
+ else
+ {
+ node_map.insert(make_pair(*i, state));
+ }
}
- else
- {
- node_map.insert(make_pair(nid, state));
- }
}
static void
-add_included_nodes(map & node_map,
- map & path_map,
- path_set const & includes,
- roster_t const & roster)
+add_paths(map & path_map,
+ path_set const & includes,
+ path_set const & excludes)
{
- for (path_set::const_iterator i = includes.begin(); i != includes.end(); ++i)
- {
+ path_set parents;
+ get_parent_paths(includes, parents);
- // TODO: (future) handle some sort of peg revision path syntax here.
- // note that the idea of a --peg option doesn't work because each
- // path may be relative to a different revision.
-
- if (roster.has_node(*i))
- {
-
- // FIXME: this may be better as a list of invalid paths
- map::iterator p = path_map.find(*i);
- I(p != path_map.end());
-
- p->second.roster_count++;
-
- // TODO: proper recursive wildcard paths like foo/...
- // for now explicit paths are recursive
- // and implicit parents are non-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. it does
- // match the current (old restrictions) semantics though.
-
- node_id nid = roster.get_node(*i)->self;
-
- path_state state = explicit_include;
-
- while (!null_node(nid))
- {
- split_path sp;
- roster.get_name(nid, sp);
-
- update_node_map(node_map, nid, sp, state);
-
- nid = roster.get_node(nid)->parent;
- state = implicit_include;
- }
- }
- }
+ add_paths(path_map, includes, explicit_include);
+ add_paths(path_map, excludes, explicit_exclude);
+ add_paths(path_map, parents, implicit_include);
}
static void
-add_excluded_nodes(map & node_map,
- map & path_map,
- path_set const & excludes,
- roster_t const & roster)
+add_nodes(map & node_map,
+ roster_t const & roster,
+ path_set const & includes,
+ path_set const & excludes,
+ path_set & known_paths)
{
- for (path_set::const_iterator i = excludes.begin(); i != excludes.end(); ++i)
- {
+ set included_nodes, excluded_nodes, parent_nodes;
+ get_nodes(includes, roster, included_nodes, known_paths);
+ get_nodes(excludes, roster, excluded_nodes, known_paths);
+ get_parent_nodes(included_nodes, roster, parent_nodes);
- // TODO: (future) handle some sort of peg revision path syntax here.
- // note that the idea of a --peg option doesn't work because each
- // path may be relative to a different revision.
-
- if (roster.has_node(*i))
- {
- // FIXME: this may be better as a list of invalid paths
- map::iterator p = path_map.find(*i);
- I(p != path_map.end());
-
- p->second.roster_count++;
-
- // TODO: proper recursive wildcard paths like foo/...
- // for now explicit paths are recursive
- // and implicit parents are non-recursive
-
- node_id nid = roster.get_node(*i)->self;
-
- split_path sp;
- roster.get_name(nid, sp);
- update_node_map(node_map, nid, sp, explicit_exclude);
- }
- }
-
+ add_nodes(node_map, roster, included_nodes, explicit_include);
+ add_nodes(node_map, roster, excluded_nodes, explicit_exclude);
+ add_nodes(node_map, roster, parent_nodes, implicit_include);
}
static void
-check_paths(map const & path_map)
+check_paths(path_set const & includes, path_set const & excludes, path_set const & known_paths)
{
int bad = 0;
- for (map::const_iterator i = path_map.begin();
- i != path_map.end(); ++i)
+ for (path_set::const_iterator i = includes.begin(); i != includes.end(); ++i)
{
- L(F("%d %d %s") % i->second.state % i->second.roster_count % i->first);
-
- if (i->second.state == explicit_include && i->second.roster_count == 0)
+ if (known_paths.find(*i) == known_paths.end())
{
bad++;
- W(F("unknown path included %s") % i->first);
+ W(F("unknown path included %s") % *i);
}
- else if (i->second.state == explicit_exclude && i->second.roster_count == 0)
+ }
+
+ for (path_set::const_iterator i = excludes.begin(); i != excludes.end(); ++i)
+ {
+ if (known_paths.find(*i) == known_paths.end())
{
bad++;
- W(F("unknown path excluded %s") % i->first);
+ W(F("unknown path excluded %s") % *i);
}
}
N(bad == 0, F("%d unknown paths") % bad);
}
-
////////////////////////////////////////////////////////////////////////////////
// public constructors
////////////////////////////////////////////////////////////////////////////////
-// FIXME: add_paths and add_nodes should take the nodes to add and their state
-// includes/explicit_include, excludes/explicit_exclude, parents/implicit_include
-// then we need things to generate lists of parent paths and nodes
-
restriction::restriction(vector const & include_args,
vector const & exclude_args,
roster_t const & roster)
{
- path_set includes, excludes;
+ path_set includes, excludes, known_paths;
make_path_set(include_args, includes);
make_path_set(exclude_args, excludes);
- add_included_paths(path_map, includes);
- add_excluded_paths(path_map, excludes);
+ add_paths(path_map, includes, excludes);
+ add_nodes(node_map, roster, includes, excludes, known_paths);
- add_included_nodes(node_map, path_map, includes, roster);
- add_excluded_nodes(node_map, path_map, excludes, roster);
-
default_result = includes.empty();
- check_paths(path_map);
+ check_paths(includes, excludes, known_paths);
}
restriction::restriction(vector const & include_args,
@@ -271,25 +248,19 @@
roster_t const & roster1,
roster_t const & roster2)
{
- path_set includes, excludes;
+ path_set includes, excludes, known_paths;
make_path_set(include_args, includes);
make_path_set(exclude_args, excludes);
- add_included_paths(path_map, includes);
- add_excluded_paths(path_map, excludes);
+ add_paths(path_map, includes, excludes);
+ add_nodes(node_map, roster1, includes, excludes, known_paths);
+ add_nodes(node_map, roster2, includes, excludes, known_paths);
- add_included_nodes(node_map, path_map, includes, roster1);
- add_excluded_nodes(node_map, path_map, excludes, roster1);
-
- add_included_nodes(node_map, path_map, includes, roster2);
- add_excluded_nodes(node_map, path_map, excludes, roster2);
-
default_result = includes.empty();
- check_paths(path_map);
+ check_paths(includes, excludes, known_paths);
}
-
////////////////////////////////////////////////////////////////////////////////
// public api
////////////////////////////////////////////////////////////////////////////////
@@ -367,11 +338,11 @@
while (!current.empty())
{
- map::const_iterator r = path_map.find(current);
+ map::const_iterator r = path_map.find(current);
if (r != path_map.end())
{
- switch (r->second.state)
+ switch (r->second)
{
case explicit_include:
L(F("explicit include of path '%s'") % file_path(sp));
@@ -901,6 +872,24 @@
BOOST_CHECK_THROW(restriction(includes, excludes, roster), informative_failure);
}
+static void
+test_get_parent_paths()
+{
+ path_set paths, parents;
+ split_path a, ab, abc;
+
+ file_path_internal("a").split(a);
+ file_path_internal("a/b").split(ab);
+ file_path_internal("a/b/c").split(abc);
+
+ paths.insert(abc);
+ get_parent_paths(paths, parents);
+
+ BOOST_CHECK(parents.find(a) != parents.end());
+ BOOST_CHECK(parents.find(ab) != parents.end());
+ BOOST_CHECK(parents.find(abc) == parents.end());
+}
+
void
add_restrictions_tests(test_suite * suite)
{
@@ -911,5 +900,7 @@
suite->add(BOOST_TEST_CASE(&test_include_exclude));
suite->add(BOOST_TEST_CASE(&test_exclude_include));
suite->add(BOOST_TEST_CASE(&test_invalid_paths));
+ suite->add(BOOST_TEST_CASE(&test_get_parent_paths));
+
}
#endif // BUILD_UNIT_TESTS
============================================================
--- restrictions.hh 997d9f85ddfb423b9a535acdef8a4c44bc384395
+++ restrictions.hh 8d68ee9d4981f22f01ecf499602fcbc7b6d4b6fe
@@ -50,16 +50,13 @@
//
// revision A ... included ... revision X ... excluded ... revision B
+// explicit in the sense that the path was explicitly given on the command line
+// implicit in the sense that parent directories are included for explicit paths
+// FIXME: should we really be doing implicit includes of parents?
+
// TODO: move these into the class below?!?
enum path_state { explicit_include, explicit_exclude, implicit_include };
-struct path_entry {
- path_state state;
- int roster_count;
- path_entry(path_state const s) : state(s), roster_count(0) {}
-};
-
-
class restriction
{
public:
@@ -80,18 +77,15 @@
bool empty() { return node_map.empty(); }
- // explicit in the sense that the path was explicitly given on the command line
- // implicit in the sense that parent directories are included for explicit paths
-
private:
bool default_result;
- // we maintain maps by node_id and also by split_path which is not
- // particularly nice, but is required for checking unknown and ignored
- // files and used for tracking the paths that are known in the rosters
+ // we maintain maps by node_id and also by split_path, which is not
+ // particularly nice, but paths are required for checking unknown and ignored
+ // files
map node_map;
- map path_map;
+ map path_map;
};