aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorBoris Kolpackov <boris@codesynthesis.com>2019-11-13 09:46:04 +0200
committerBoris Kolpackov <boris@codesynthesis.com>2019-11-13 09:46:04 +0200
commit1dc4b29bb57b14bfd8f700be80224b6d865f0184 (patch)
treec18e6c62de1f71510876285a259e5782eae2c3af
parent8ac1daf33da807635eddd881f2b178af8e22863a (diff)
Implement config.config.persist logic
Note that the inherited value part is documented but is not yet fully implemented.
-rw-r--r--libbuild2/config/init.cxx30
-rw-r--r--libbuild2/config/module.cxx2
-rw-r--r--libbuild2/config/module.hxx10
-rw-r--r--libbuild2/config/operation.cxx229
4 files changed, 193 insertions, 78 deletions
diff --git a/libbuild2/config/init.cxx b/libbuild2/config/init.cxx
index 1f053e9..c9c6130 100644
--- a/libbuild2/config/init.cxx
+++ b/libbuild2/config/init.cxx
@@ -56,7 +56,35 @@ namespace build2
//
vp.insert<path> ("config.config.save", true /* ovr */);
- // @@ TODO
+ // Configuration variables persistence mode.
+ //
+ // By default a config.* variable is saved in the config.build file if
+ // (1) it is explicitly marked as persistent with save_variable() and
+ // (2) it is not inherited from an amalgamation that also saves this
+ // variable (however, there are some exception; see save_config() for
+ // details). If the first condition is not met, then the variable is
+ // presumed to be no longer used.
+ //
+ // The config.config.persist can be used to adjust this default logic.
+ // It contains a list of key-value pairs with the key being a variable
+ // name pattern and the value specifying the condition/action:
+ //
+ // <pair> = <pattern>@<condition>=<action>
+ // <condition> = unused|inherited|inherited-used|inherited-unused
+ // <action> = (save|drop)[+warn]
+ //
+ // The last pattern and condition that matches is used (we may want to
+ // change this to more specific pattern later).
+ //
+ // Note that support for inherited conditions is still a @@ TODO.
+ //
+ // The create meta-operation by default (i.e., unless a custom value is
+ // specified) saves unused config.import.* variables without a warning
+ // (since there is no way to "use" such variables in a configuration).
+ //
+ // Note that variable patterns must be quoted, for example:
+ //
+ // b "config.config.persist='config.*'@unused=save+warn"
//
// Use the NULL value to clear.
//
diff --git a/libbuild2/config/module.cxx b/libbuild2/config/module.cxx
index 10ec833..f2fcb79 100644
--- a/libbuild2/config/module.cxx
+++ b/libbuild2/config/module.cxx
@@ -25,7 +25,7 @@ namespace build2
//
if (i == sm.end ())
{
- // @@ For now with 'config.' prefix.
+ // Note: with 'config.' prefix.
//
i = sm.insert (string (n, 0, n.find ('.', 7))).first;
}
diff --git a/libbuild2/config/module.hxx b/libbuild2/config/module.hxx
index eccd61b..0a35369 100644
--- a/libbuild2/config/module.hxx
+++ b/libbuild2/config/module.hxx
@@ -86,6 +86,16 @@ namespace build2
bool
save_module (const char* name, int prio = 0);
+ // Return true if the variable is already saved.
+ //
+ bool
+ saved (const variable& var)
+ {
+ auto i (saved_modules.find_sup (var.name));
+ return i != saved_modules.end () &&
+ i->second.find (var) != i->second.end ();
+ }
+
// Cached (during init) config.config.persist value, if defined.
//
const vector<pair<string, string>>* persist = nullptr;
diff --git a/libbuild2/config/operation.cxx b/libbuild2/config/operation.cxx
index 5f1c8cd..944070b 100644
--- a/libbuild2/config/operation.cxx
+++ b/libbuild2/config/operation.cxx
@@ -85,47 +85,77 @@ namespace build2
using project_set = set<const scope*>; // Use pointers to get comparison.
- // Mark all the config.import.* variables defined on this scope as saved
- // optionally warning if the variable would otherwise be dropped.
+ // Return (first) whether an unused/inherited variable should be saved
+ // according to the config.config.persist value and (second) whether the
+ // user should be warned about it.
//
- static void
- save_config_import (module& m,
- const scope& rs,
- const location& loc,
- bool warning)
+ static pair<bool, bool>
+ save_config_variable (const variable& var,
+ const vector<pair<string, string>>* persist,
+ bool inherited,
+ bool unused)
{
- auto& vp (rs.ctx.var_pool);
+ assert (inherited || unused);
- for (auto p (rs.vars.find_namespace (*vp.find ("config.import")));
- p.first != p.second;
- ++p.first)
+ if (persist != nullptr)
{
- const variable* var (&p.first->first.get ());
+ for (const pair<string, string>& pc: reverse_iterate (*persist))
+ {
+ if (!path_match (var.name, pc.first))
+ continue;
- // Annoyingly, this can be one of the overrides (__override, __prefix,
- // etc).
- //
- if (size_t n = var->override ())
- var = vp.find (string (var->name, 0, n));
+ const string& c (pc.second);
- if (m.save_variable (*var) && warning)
- {
- // Consistently with save_config() below we don't warn about an
- // overriden variable.
- //
- if (var->overrides != nullptr)
+ size_t p;
+ if (c.compare (0, (p = 7), "unused=") == 0)
{
- lookup l {p.first->second, *var, rs.vars};
- pair<lookup, size_t> org {l, 1 /* depth */};
- pair<lookup, size_t> ovr (rs.find_override (*var, org));
-
- if (org.first != ovr.first)
+ if (!unused || inherited)
+ continue;
+ }
+ else if (c.compare (0, (p = 10), "inherited=") == 0)
+ {
+ // Applies to both used an unused.
+ //
+ if (!inherited)
+ continue;
+ }
+ else if (c.compare (0, (p = 15), "inherited-used=") == 0)
+ {
+ if (!inherited || unused)
+ continue;
+ }
+ else if (c.compare (0, (p = 17), "inherited-unused=") == 0)
+ {
+ if (!inherited || !unused)
continue;
}
+ else
+ fail << "invalid config.config.persist condition '" << c << "'";
+
+ bool r;
+ if (c.compare (p, 4 , "save") == 0) r = true;
+ else if (c.compare (p, 4 , "drop") == 0) r = false;
+ else fail << "invalid config.config.persist action '" << c << "'";
+
+ bool w (false);
+ if ((p += 4) != c.size ())
+ {
+ if (c.compare (p, string::npos, "+warn") == 0) w = true;
+ else fail << "invalid config.config.persist action '" << c << "'";
+ }
- warn (loc) << "keeping potentially no longer used variable " << *var;
+ return make_pair (r, w);
}
}
+
+ // Defaults.
+ //
+ if (!inherited)
+ return make_pair (false, true); // unused: drop warn
+ else if (unused)
+ return make_pair (true, true); // inherited-unused: save warn
+ else
+ return make_pair (false, false); // inherited-used: drop !warn
}
// If inherit is false, then don't rely on inheritance from outer scopes.
@@ -148,6 +178,21 @@ namespace build2
fail (on) << "no configuration information available during this "
<< "meta-operation";
+ names storage;
+
+ auto info_value = [&storage] (diag_record& dr, const value& v) mutable
+ {
+ dr << info << "variable value: ";
+
+ if (v)
+ {
+ storage.clear ();
+ dr << "'" << reverse (v, storage) << "'";
+ }
+ else
+ dr << "[null]";
+ };
+
try
{
os << "# Created automatically by the config module, but feel " <<
@@ -168,26 +213,71 @@ namespace build2
}
}
- // Pre-mark all the config.import.* variables defined on our root
- // scope as saved.
- //
- // This is in contrast to module-specific config variables (and later
- // probably to project-specific) where by default we drop no longer
- // used values (see below). Note that this also used to be the case
- // for the config.import.* variables but that turned out to be a poor
- // default since there is no way to "use" such variables except to
- // import something. So now the default for config.import.* is to keep
- // with a warning.
- //
- // Note that in the future we may make these defaults configurable.
+ // Mark the unused config.* variables defined on our root scope as
+ // saved according to config.config.persist potentially warning if the
+ // variable would otherwise be dropped.
//
- if (mod->persist) //@@ TMP
- save_config_import (*mod, rs, location (on), false /* warn */);
+ auto& vp (ctx.var_pool);
+
+ for (auto p (rs.vars.find_namespace (*vp.find ("config.import")));
+ p.first != p.second;
+ ++p.first)
+ {
+ const variable* var (&p.first->first.get ());
+
+ // Annoyingly, this can be one of the overrides (__override,
+ // __prefix, etc).
+ //
+ if (size_t n = var->override ())
+ var = vp.find (string (var->name, 0, n));
+
+ if (mod->saved (*var))
+ continue;
+
+ const value& v (p.first->second);
+
+ pair<bool, bool> r (save_config_variable (*var,
+ mod->persist,
+ false /* inherited */,
+ true /* unused */));
+ if (r.first) // save
+ {
+ mod->save_variable (*var);
+
+ if (r.second) // warn
+ {
+ // Consistently with save_config() below we don't warn about an
+ // overriden variable.
+ //
+ if (var->overrides != nullptr)
+ {
+ lookup l {v, *var, rs.vars};
+ pair<lookup, size_t> org {l, 1 /* depth */};
+ pair<lookup, size_t> ovr (rs.find_override (*var, org));
+
+ if (org.first != ovr.first)
+ continue;
+ }
+
+ diag_record dr;
+ dr << warn (on) << "saving no longer used variable " << *var;
+ if (verb >= 2)
+ info_value (dr, v);
+ }
+ }
+ else // drop
+ {
+ if (r.second) // warn
+ {
+ diag_record dr;
+ dr << warn (on) << "dropping no longer used variable " << *var;
+ info_value (dr, v);
+ }
+ }
+ }
// Save config variables.
//
- names storage;
-
for (auto p: mod->saved_modules.order)
{
const string& sname (p.second->first);
@@ -214,10 +304,11 @@ namespace build2
// Handle inherited from outer scope values.
//
- // Note that we keep this logic (with warnings and all) even if
- // inherit is false to make things easier to reason about.
+ // Note that we skip this entire logic if inherit is false since
+ // we save the inherited values regardless of whether they are
+ // used or not.
//
- if (!(l.belongs (rs) || l.belongs (ctx.global_scope)))
+ if (inherit && !(l.belongs (rs) || l.belongs (ctx.global_scope)))
{
// This is presumably an inherited value. But it could also be
// some left-over garbage. For example, an amalgamation could
@@ -229,7 +320,7 @@ namespace build2
// next reconfigure. Let's also warn the user just in case,
// unless there is no module and thus we couldn't really check
// (the latter could happen when calling $config.save() during
- // other meta-operations).
+ // other meta-operations, though it passes false for inherit).
//
// There is also another case that falls under this now that
// overrides are by default amalgamation-wide rather than just
@@ -258,20 +349,17 @@ namespace build2
const saved_variables& sv (i->second);
found = sv.find (var) != sv.end ();
- // By default treat config.import.* values as "found"
- // (see above for details).
+ // If not marked as saved, check whether overriden via
+ // config.config.persist.
//
- // Note that if/when we support overriding this default,
- // we would need to get the "effective" value for an
- // outer project (and the user would need to specify
- // such an override even though that project is not
- // being configured -- i.e., they would need to
- // communicate to us what will have happened once the
- // outer project is reconfigured). Feels like this will
- // be quite obscure and error-prone.
- //
- if (!found && m->persist) //@@ TMP
- found = var.name.compare (0, 14, "config.import.") == 0;
+ if (!found && m->persist != nullptr)
+ {
+ found = save_config_variable (
+ var,
+ m->persist,
+ false /* inherited */,
+ true /* unused */).first;
+ }
// Handle that other case: if this is an override but
// the outer project itself is not being configured,
@@ -300,8 +388,7 @@ namespace build2
{
// Inherited.
//
- if (inherit)
- continue;
+ continue;
}
else
{
@@ -327,17 +414,7 @@ namespace build2
<< "it in its configuration";
if (verb >= 2)
- {
- dr << info << "variable value: ";
-
- if (*l)
- {
- storage.clear ();
- dr << "'" << reverse (*l, storage) << "'";
- }
- else
- dr << "[null]";
- }
+ info_value (dr, *l);
}
}
}
@@ -1032,7 +1109,7 @@ namespace build2
if (!rs[var].defined ())
{
rs.assign (var) = vector<pair<string, string>> {
- pair<string, string> {"config.import.*", "unused=keep"}};
+ pair<string, string> {"config.import.*", "unused=save"}};
}
}