From ce558a5aebc649e52da9598f248bd15fd08a9ba2 Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Fri, 1 Mar 2024 13:31:19 +0200 Subject: Use original variable name in config report --- doc/manual.cli | 26 ++++++++++++++ libbuild2/file.cxx | 55 +++++++++++++++++++++-------- libbuild2/parser.cxx | 74 ++++++++++++++++++++++++++------------- libbuild2/parser.hxx | 13 +++++-- tests/directive/config.testscript | 4 +++ 5 files changed, 130 insertions(+), 42 deletions(-) diff --git a/doc/manual.cli b/doc/manual.cli index 93053e4..847691d 100644 --- a/doc/manual.cli +++ b/doc/manual.cli @@ -4966,6 +4966,32 @@ if! $defined(config.libhello.database) fail 'config.libhello.database must be specified' \ +\N|A configuration variable without a default value is omitted from +\c{config.build} unless the value is specified by the user. This semantics is +useful for values that are normally derived from other configuration values +but could also be specified by the user. If the value is derived, then we +don't want it saved in \c{config.build} since that would prevent it from +being re-derived if the configuration values it is based on are changed. +For example: + +\ +config [strings] config.hello.database + +assert ($size($config.hello.database) > 0) \ + 'database must be specified with config.hello.database' + +config [bool, config.report.variable=multi] config.hello.multi_database + +multi = ($defined(config.hello.multi_database) \ + ? $config.hello.multi_database \ + : $size(config.hello.database) > 1) + +assert ($multi || $size(config.hello.database) == 1) \ + 'one database can be specified if config.hello.multi_database=false' +\ + +| + If computing the default value is expensive or requires elaborate logic, then the handling of a configuration variable can be broken down into two steps along these lines: diff --git a/libbuild2/file.cxx b/libbuild2/file.cxx index 0122889..ee1b6b1 100644 --- a/libbuild2/file.cxx +++ b/libbuild2/file.cxx @@ -4,6 +4,7 @@ #include #include +#include // strlen() #include // left, setw() #include @@ -1639,8 +1640,10 @@ namespace build2 // Print the project configuration report(s), similar to how we do it in // build system modules. // + using config_report = parser::config_report; + const project_name* proj (nullptr); // Resolve lazily. - for (const parser::config_report& cr: p.config_reports) + for (const config_report& cr: p.config_reports) { if (verb < (cr.new_value ? 2 : 3)) continue; @@ -1673,26 +1676,45 @@ namespace build2 ? '.' + proj->variable () + '.' : string ())); - // Calculate max name length. + // Return the variable name for printing. // - size_t pad (10); - for (const pair& lf: cr.values) + auto name = [&stem] (const config_report::value& cv) -> const char* { - lookup l (lf.first); + lookup l (cv.val); - size_t n; if (l.value == nullptr) { - n = l.var->name.size (); + if (cv.org.empty ()) + return l.var->name.c_str (); + + // This case may or may not have the prefix. + // + size_t p, n ( + !stem.empty () + ? (p = cv.org.find (stem)) != string::npos ? p + stem.size () : 0 + : cv.org.compare (0, 7, "config.") == 0 ? 7 : 0); + + return cv.org.c_str () + n; } else { + assert (cv.org.empty ()); // Sanity check. + size_t p (!stem.empty () ? l.var->name.find (stem) + stem.size () : 7); // "config." - n = l.var->name.size () - p; + + return l.var->name.c_str () + p; } + }; + + // Calculate max name length. + // + size_t pad (10); + for (const config_report::value& cv: cr.values) + { + size_t n (strlen (name (cv))); if (n > pad) pad = n; } @@ -1705,13 +1727,14 @@ namespace build2 << ' ' << *proj << '@' << root; names storage; - for (const pair& lf: cr.values) + for (const config_report::value& cv: cr.values) { - lookup l (lf.first); - const string& f (lf.second); + lookup l (cv.val); + const string& f (cv.fmt); // If the report variable has been overriden, now is the time to - // lookup its value. + // lookup its value. Note: see also the name() lambda above if + // changing anything here. // string n; if (l.value == nullptr) @@ -1727,6 +1750,8 @@ namespace build2 n = string (l.var->name, p); } + const char* pn (name (cv)); // Print name. + dr << "\n "; if (l) @@ -1736,15 +1761,15 @@ namespace build2 if (f == "multiline") { - dr << n; + dr << pn; for (auto& n: ns) dr << "\n " << n; } else - dr << left << setw (static_cast (pad)) << n << ' ' << ns; + dr << left << setw (static_cast (pad)) << pn << ' ' << ns; } else - dr << left << setw (static_cast (pad)) << n << " [null]"; + dr << left << setw (static_cast (pad)) << pn << " [null]"; } } diff --git a/libbuild2/parser.cxx b/libbuild2/parser.cxx index 2623bf3..4cbb30d 100644 --- a/libbuild2/parser.cxx +++ b/libbuild2/parser.cxx @@ -3852,6 +3852,9 @@ namespace build2 try { report_var = convert (move (i->value)); + + if (!report) + report = string ("true"); } catch (const invalid_argument& e) { @@ -3889,9 +3892,13 @@ namespace build2 // As a way to print custom (discovered, computed, etc) configuration // information we allow specifying a non config.* variable provided it is - // explicitly marked with the config.report attribute. + // explicitly marked with the config.report attribute (or another + // attribute that implies it). // bool new_val (false); + string org_var; // Original variable if config.report.variable specified. + + const variable* var (nullptr); // config.* variable. lookup l; if (report && *report != "false" && !config) @@ -3910,7 +3917,14 @@ namespace build2 // philosophical question. In either case it doesn't seem useful for it // to unconditionally force reporting at level 2. // - report_var = move (name); + if (!report_var.empty ()) + { + // For example, config [config.report.variable=multi] multi_database + // + org_var = move (name); + } + else + report_var = move (name); next (t, tt); // We shouldn't have the default value part. } @@ -4040,17 +4054,16 @@ namespace build2 << (proj.empty () ? "" : proj.c_str ()) << ".**' form"; } - const variable& var ( - parse_variable_name (move (name), get_location (t))); - apply_variable_attributes (var); + var = &parse_variable_name (move (name), get_location (t)); + apply_variable_attributes (*var); // Note that even though we are relying on the config.** variable // pattern to set global visibility, let's make sure as a sanity check. // - if (var.visibility != variable_visibility::global) + if (var->visibility != variable_visibility::global) { - fail (t) << "configuration variable " << var << " has " - << var.visibility << " visibility"; + fail (t) << "configuration variable " << *var << " has " + << var->visibility << " visibility"; } // See if we have the default value part. @@ -4072,15 +4085,15 @@ namespace build2 // bool dev; { - size_t p (var.name.rfind ('.')); - dev = p != 6 && var.name.compare (p + 1, string::npos, "develop") == 0; + size_t p (var->name.rfind ('.')); + dev = p != 6 && var->name.compare (p + 1, string::npos, "develop") == 0; } uint64_t sflags (0); if (dev) { - if (var.type != &value_traits::value_type) - fail (loc) << var << " variable must be of type bool"; + if (var->type != &value_traits::value_type) + fail (loc) << *var << " variable must be of type bool"; // This is quite messy: below we don't always parse the value (plus it // may be computed) so here we just peek at the next token. But we @@ -4089,10 +4102,10 @@ namespace build2 if (!def_val || peek (lexer_mode::value, '@') != type::word || peeked ().value != "false") - fail (loc) << var << " variable default value must be literal false"; + fail (loc) << *var << " variable default value must be literal false"; if (nullable) - fail (loc) << var << " variable must not be nullable"; + fail (loc) << *var << " variable must not be nullable"; sflags |= config::save_false_omitted; } @@ -4101,7 +4114,7 @@ namespace build2 // in order to mark it as saved. We also have to do this to get the new // value status. // - l = config::lookup_config (new_val, *root_, var, sflags); + l = config::lookup_config (new_val, *root_, *var, sflags); // Handle the default value. // @@ -4137,12 +4150,12 @@ namespace build2 else { value lhs, rhs (parse_variable_value (t, tt, !dev /* mode */)); - apply_value_attributes (&var, lhs, move (rhs), type::assign); + apply_value_attributes (var, lhs, move (rhs), type::assign); if (!nullable) nullable = lhs.null; - l = config::lookup_config (new_val, *root_, var, move (lhs), sflags); + l = config::lookup_config (new_val, *root_, *var, move (lhs), sflags); } } @@ -4153,7 +4166,7 @@ namespace build2 // then the user is expected to handle the undefined case). // if (!nullable && l.defined () && l->null) - fail (loc) << "null value in non-nullable variable " << var; + fail (loc) << "null value in non-nullable variable " << *var; } // We will be printing the report at either level 2 (-v) or 3 (-V) @@ -4188,6 +4201,9 @@ namespace build2 // if (!report_var.empty ()) { + if (org_var.empty () && var != nullptr) + org_var = var->name; + // In a somewhat hackish way we pass the variable in an undefined // lookup. // @@ -4201,23 +4217,33 @@ namespace build2 if (l.var != nullptr) { - auto r (make_pair (l, move (*report))); - // If we have a duplicate, update it (it could be useful to have // multiple config directives to "probe" the value before calculating // the default; see lookup_config() for details). // + // Since the original variable is what the user will see in the + // report, we prefer that as a key. + // auto i (find_if (report_values.begin (), report_values.end (), - [&l] (const pair& p) + [&org_var, &l] (const config_report::value& v) { - return p.first.var == l.var; + return (v.org.empty () && org_var.empty () + ? v.val.var == l.var + : (v.org.empty () + ? v.val.var->name == org_var + : v.org == l.var->name)); })); if (i == report_values.end ()) - report_values.push_back (move (r)); + report_values.push_back ( + config_report::value {l, move (*report), move (org_var)}); else - *i = move (r); + { + i->val = l; + i->fmt = move (*report); + if (i->org.empty ()) i->org = move (org_var); + } report_new_value = report_new_value || new_val; } diff --git a/libbuild2/parser.hxx b/libbuild2/parser.hxx index 0645b5a..3e1d0a0 100644 --- a/libbuild2/parser.hxx +++ b/libbuild2/parser.hxx @@ -136,9 +136,16 @@ namespace build2 // struct config_report { - project_name module; // Reporting module name. - vector> values; // Config value and format. - bool new_value; // One of values is new. + struct value + { + lookup val; // Value. + string fmt; // Format. + string org; // Original variable if config.report.variable. + }; + + project_name module; // Reporting module name. + vector values; + bool new_value; // One of values is new. }; small_vector config_reports; diff --git a/tests/directive/config.testscript b/tests/directive/config.testscript index fba858f..ebdd6ac 100644 --- a/tests/directive/config.testscript +++ b/tests/directive/config.testscript @@ -212,12 +212,14 @@ test.arguments = config [strings, config.report=multiline] config.test.d ?= 1 2 3 config [string, config.report.variable=e] config.test.e ?= abc config [ config.report] f + config [ config.report.variable=g] gg config [bool] config.test.n ?= [null] config [bool] config.test.p config [bool] config.test.p ?= true e = "'$config.test.e'" f = ($config.test.b || $config.test.c) + g = abc EOI @@ -240,6 +242,7 @@ test.arguments = 3 e 'abc' f true + gg abc n [null] p true EOO @@ -262,6 +265,7 @@ test.arguments = 3 e 'xyz' f true + gg abc n true p false EOO -- cgit v1.1