From 45ae0f38febb82a99844df84b65ec0fec31d2918 Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Mon, 11 Nov 2019 15:50:44 +0200 Subject: Change default for unused config.import.* variables from drop to keep --- libbuild2/config/init.cxx | 14 +++---- libbuild2/config/module.cxx | 18 +++++---- libbuild2/config/module.hxx | 10 +++-- libbuild2/config/operation.cxx | 86 ++++++++++++++++++++++++++++++++++-------- 4 files changed, 94 insertions(+), 34 deletions(-) diff --git a/libbuild2/config/init.cxx b/libbuild2/config/init.cxx index bcaea83..e1c1dd0 100644 --- a/libbuild2/config/init.cxx +++ b/libbuild2/config/init.cxx @@ -40,14 +40,15 @@ namespace build2 auto& vp (rs.ctx.var_pool.rw (rs)); - // While config.import (see below) could theoretically be specified in a - // buildfile, config.export is expected to always be specified as a - // command line override. + // While config.import could theoretically be specified in a buildfile, + // config.export is expected to always be specified as a command line + // override. // - // Note: must be entered during bootstrap since we need it in + // Note: must be entered during bootstrap since we need them in // configure_execute(). // - vp.insert ("config.export", true /* ovr */); + vp.insert ("config.export", true /* ovr */); + vp.insert ("config.import", true /* ovr */); // Only create the module if we are configuring or creating or if it was // forced with config.module (useful if we need to call $config.export() @@ -124,7 +125,6 @@ namespace build2 auto& vp (rs.ctx.var_pool.rw (rs)); auto& c_v (vp.insert ("config.version", false /*ovr*/)); - auto& c_i (vp.insert ("config.import", true /* ovr */)); // Load config.build if one exists followed by extra files specified in // config.import (we don't need to worry about disfigure since we will @@ -181,7 +181,7 @@ namespace build2 load_config_file (f, l); } - if (lookup l = rs[c_i]) + if (lookup l = rs["config.import"]) { // Only load files that were specified on our root scope as well as // global overrides. This way we can use our override "positioning" diff --git a/libbuild2/config/module.cxx b/libbuild2/config/module.cxx index b43f1d9..10ec833 100644 --- a/libbuild2/config/module.cxx +++ b/libbuild2/config/module.cxx @@ -10,7 +10,7 @@ namespace build2 { namespace config { - void module:: + bool module:: save_variable (const variable& var, uint64_t flags) { const string& n (var.name); @@ -27,7 +27,7 @@ namespace build2 { // @@ For now with 'config.' prefix. // - i = sm.insert (string (n, 0, n.find ('.', 7))); + i = sm.insert (string (n, 0, n.find ('.', 7))).first; } // Don't insert duplicates. The config.import.* vars are particularly @@ -36,16 +36,20 @@ namespace build2 saved_variables& sv (i->second); auto j (sv.find (var)); - if (j == sv.end ()) - sv.push_back (saved_variable {var, flags}); - else + if (j != sv.end ()) + { assert (j->flags == flags); + return false; + } + + sv.push_back (saved_variable {var, flags}); + return true; } - void module:: + bool module:: save_module (const char* name, int prio) { - saved_modules.insert (string ("config.") += name, prio); + return saved_modules.insert (string ("config.") += name, prio).second; } const string module::name ("config"); diff --git a/libbuild2/config/module.hxx b/libbuild2/config/module.hxx index 4b9d4f2..296dec5 100644 --- a/libbuild2/config/module.hxx +++ b/libbuild2/config/module.hxx @@ -62,7 +62,7 @@ namespace build2 // std::multimap order; - iterator + pair insert (string name, int prio = 0) { auto p (emplace (move (name), saved_variables ())); @@ -70,7 +70,7 @@ namespace build2 if (p.second) order.emplace (prio, p.first); - return p.first; + return p; } }; @@ -78,10 +78,12 @@ namespace build2 { config::saved_modules saved_modules; - void + // Return true if variable/module were newly inserted. + // + bool save_variable (const variable&, uint64_t flags = 0); - void + bool save_module (const char* name, int prio = 0); static const string name; diff --git a/libbuild2/config/operation.cxx b/libbuild2/config/operation.cxx index 14576a0..ea6ff15 100644 --- a/libbuild2/config/operation.cxx +++ b/libbuild2/config/operation.cxx @@ -85,6 +85,36 @@ namespace build2 using project_set = set; // Use pointers to get comparison. + // Mark all the config.import.* variables defined in this variable map as + // saved optionally issuing a warning if the variable would otherwise be + // unsaved. + // + static void + save_config_import (context& ctx, + module& m, + const variable_map& vars, + bool warning = false, + const location& loc = location ()) + { + auto& vp (ctx.var_pool); + + for (auto p (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 (m.save_variable (*var) && warning) + warn (loc) << "keeping potentially no longer used variable " << *var; + } + } + // If inherit is false, then don't rely on inheritance from outer scopes // (used for config.export). // @@ -96,7 +126,11 @@ namespace build2 { context& ctx (rs.ctx); - const module* mod (rs.lookup_module (module::name)); + // @@ We are modifying the module (marking additional variables as + // saved) and this function can be called from a buildfile (probably + // only during serial execution but still). + // + module* mod (rs.lookup_module (module::name)); if (mod == nullptr) fail (on) << "no configuration information available during this " @@ -122,6 +156,21 @@ namespace build2 } } + // 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. + // + save_config_import (ctx, *mod, rs.vars, true /* warn */, location (on)); + // Save config variables. // names storage; @@ -165,7 +214,9 @@ namespace build2 // definitely want to move them to our config.build since they // will be dropped from the amalgamation's config.build on the // next reconfigure. Let's also warn the user just in case, - // unless there is no module and thus we couldn't really check. + // unless there is no module and thus we couldn't really check + // (the latter could happen when calling $config.export() during + // other meta-operations). // // There is also another case that falls under this now that // overrides are by default amalgamation-wide rather than just @@ -194,6 +245,21 @@ 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). + // + // 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 it will be + // quite obscure and error-prone. + // + if (!found) + found = var.name.compare (0, 14, "config.import.") == 0; + // Handle that other case: if this is an override but // the outer project itself is not being configured, // then we need to save this override. @@ -949,22 +1015,10 @@ namespace build2 // Save all the global config.import.* variables. // - variable_pool& vp (ctx.var_pool.rw (rs)); - for (auto p (gs.vars.find_namespace (vp.insert ("config.import"))); - p.first != p.second; - ++p.first) - { - const variable& var (p.first->first); - - // Annoyingly, this can be (always is?) one of the overrides - // (__override, __prefix, etc). - // - size_t n (var.override ()); - m.save_variable (n != 0 ? *vp.find (string (var.name, 0, n)) : var); - } + save_config_import (ctx, m, gs.vars); // Now project-specific. For now we just save all of them and let - // save_config() above weed out the ones that don't apply. + // save_config() above weed out the ones that do not apply. // for (const variable_override& vo: ctx.var_overrides) { -- cgit v1.1