diff options
Diffstat (limited to 'libbuild2/config')
| -rw-r--r-- | libbuild2/config/init.cxx | 30 | ||||
| -rw-r--r-- | libbuild2/config/module.cxx | 2 | ||||
| -rw-r--r-- | libbuild2/config/module.hxx | 10 | ||||
| -rw-r--r-- | libbuild2/config/operation.cxx | 229 |
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"}}; } } |