From 2cc2772263d17a9b2755990d53e992a94d37e29d Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Fri, 27 Mar 2020 10:39:52 +0200 Subject: Implement project configuration reporting, similar to build system modules --- libbuild2/config/utility.hxx | 4 +- libbuild2/file.cxx | 68 ++++++++++ libbuild2/parser.cxx | 271 ++++++++++++++++++++++++++------------ libbuild2/parser.hxx | 37 ++++-- libbuild2/scope.hxx | 6 + libbuild2/scope.ixx | 18 +++ tests/directive/config.testscript | 64 +++++++++ 7 files changed, 368 insertions(+), 100 deletions(-) diff --git a/libbuild2/config/utility.hxx b/libbuild2/config/utility.hxx index cbfe588..f2bddc2 100644 --- a/libbuild2/config/utility.hxx +++ b/libbuild2/config/utility.hxx @@ -84,8 +84,8 @@ namespace build2 // the value is "new" (but not to false; so it can be used to accumulate // the result from multiple calls). A value is considered new if it was // set to the default value (inherited or not, including overrides). We - // also treat command line overrides (inherited or not) as new. In this - // case new means either the default value was inherited or it was + // also treat command line overrides (inherited or not) as new. For this + // version new means either the default value was inherited or it was // overridden. This flag is usually used to test that the new value is // valid, print the configuration report, etc. // diff --git a/libbuild2/file.cxx b/libbuild2/file.cxx index 613f095..405e1f0 100644 --- a/libbuild2/file.cxx +++ b/libbuild2/file.cxx @@ -3,6 +3,8 @@ #include +#include // left, setw() + #include #include #include @@ -1297,6 +1299,72 @@ namespace build2 if (he) {source_hooks (p, root, hd, true /* pre */); p.reset ();} if (fe) {source_once (p, root, root, f, root);} if (he) {p.reset (); source_hooks (p, root, hd, false /* pre */);} + + // Print the project configuration report, similar to how we do it in + // build system modules. + // + if (!p.config_report.empty () && verb >= (p.config_report_new ? 2 : 3)) + { + const project_name& proj (named_project (root)); // Must be there. + + // @@ TODO/MAYBE: + // + // - Should we be printing NULL values? + // + + // Use the special `config` module name (which doesn't have its own + // report) for project configuration. + // + diag_record dr (text); + dr << "config " << proj << '@' << root; + + // Printing the whole variable name would add too much noise with all + // the repetitive config.. So we are only going to print the + // part after (see parser::parse_config() for details). + // + string stem ('.' + proj.variable () + '.'); + + names storage; + for (const pair& lf: p.config_report) + { + lookup l (lf.first); + const string& f (lf.second); + + // If the report variable has been overriden, now is the time to + // lookup its value. + // + string n; + if (l.value == nullptr) + { + n = l.var->name; // Use the name as is. + l = root[*l.var]; + } + else + { + size_t p (l.var->name.find (stem)); // Must be there. + n = string (l.var->name, p + stem.size ()); + } + + dr << "\n "; + + if (const value& v = *l) + { + storage.clear (); + auto ns (reverse (v, storage)); + + if (f == "multiline") + { + dr << n; + for (auto& n: ns) + dr << "\n " << n; + } + else + dr << left << setw (10) << n << ' ' << ns; + } + else + dr << left << setw (10) << n << " [null]"; + } + } } scope& diff --git a/libbuild2/parser.cxx b/libbuild2/parser.cxx index 2cb51c6..a4fb17b 100644 --- a/libbuild2/parser.cxx +++ b/libbuild2/parser.cxx @@ -1691,23 +1691,14 @@ namespace build2 // named project. // string proj; - for (auto r (root_), a (root_->strong_scope ()); - ; - r = r->parent_scope ()->root_scope ()) { - const project_name& n (project (*r)); - if (!n.empty ()) - { - proj = n.variable (); - break; - } + const project_name& n (named_project (*root_)); - if (r == a) - break; - } + if (n.empty ()) + fail (t) << "configuration variable in unnamed project"; - if (proj.empty ()) - fail (t) << "configuration variable in unnamed project"; + proj = n.variable (); + } // We are now in the normal lexing mode. Since we always have we // don't have to resort to manual parsing (as in import) and can just let @@ -1715,77 +1706,187 @@ namespace build2 // next_with_attributes (t, tt); - // Get variable attributes, if any. + // Get variable attributes, if any, and deal with the special config.* + // attributes. Since currently they can only appear in the config + // directive, we handle them in an ad hoc manner. // attributes_push (t, tt); + attributes& as (attributes_top ()); + + optional report; + string report_var; + + for (auto i (as.begin ()); i != as.end (); ) + { + if (i->name == "config.report") + { + try + { + string v (i->value ? convert (move (i->value)) : "true"); + + if (v == "true" || + v == "false" || + v == "multiline") + report = move (v); + else + throw invalid_argument ( + "expected 'false' or format name instead of '" + v + "'"); + } + catch (const invalid_argument& e) + { + fail << "invalid " << i->name << " attribute value: " << e; + } + } + else if (i->name == "config.report.variable") + { + try + { + report_var = convert (move (i->value)); + } + catch (const invalid_argument& e) + { + fail << "invalid " << i->name << " attribute value: " << e; + } + } + else + { + ++i; + continue; + } + + i = as.erase (i); + } if (tt != type::word) fail (t) << "expected configuration variable name instead of " << t; string name (move (t.value)); - // Enforce the variable name pattern. The simplest is to check for the - // config prefix and the project substring. + // 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. // + bool new_val (false); + lookup l; + + if (report && + report != "false" && + name.compare (0, 7, "config.") != 0) { - diag_record dr; + if (!as.empty ()) + fail (t) << "unexpected attributes for report-only variable"; - if (name.compare (0, 7, "config.") != 0) - dr << fail (t) << "configuration variable '" << name - << "' does not start with 'config.'"; + attributes_pop (); - if (name.find ('.' + proj + '.') == string::npos) - dr << fail (t) << "configuration variable '" << name - << "' does not include project name"; + // Reduce to the config.report.variable-like situation. + // + // What should new_val be? If it's based on a config.* variable (for + // example, some paths extracted from a tool), then it's natural for + // that variable to control newness. And if it's not based on any + // config.* variable, then whether it's always new or never new is a + // philosophical question. In either case it doesn't seem useful for it + // to unconditionally force reporting at level 2. + // + report_var = move (name); - if (!dr.empty ()) - dr << info << "expected variable name in the 'config[.**]." << proj - << ".**' form"; + next (t, tt); // We shouldn't have the default value part. } + else + { + if (!report) + report = "true"; // Default is to report. - const variable& var ( - scope_->var_pool ().insert (move (name), true /* overridable */)); + // Enforce the variable name pattern. The simplest is to check for the + // config prefix and the project substring. + // + { + diag_record dr; - apply_variable_attributes (var); + if (name.compare (0, 7, "config.") != 0) + dr << fail (t) << "configuration variable '" << name + << "' does not start with 'config.'"; - // 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::normal) - { - fail (t) << "configuration variable " << var << " has " << var.visibility - << " visibility"; - } + if (name.find ('.' + proj + '.') == string::npos) + dr << fail (t) << "configuration variable '" << name + << "' does not include project name"; - // We have to lookup the value whether we have the default part or not in - // order to mark it as saved (we would also have to do this to get the new - // value status if we need it). - // - using config::lookup_config; + if (!dr.empty ()) + dr << info << "expected variable name in the 'config[.**]." << proj + << ".**' form"; + } - auto l (lookup_config (*root_, var)); + const variable& var ( + scope_->var_pool ().insert (move (name), true /* overridable */)); - // See if we have the default value part. - // - next (t, tt); + apply_variable_attributes (var); - if (tt != type::newline && tt != type::eos) - { - if (tt != type::default_assign) - fail (t) << "expected '?=' instead of " << t << " after configuration " - << "variable name"; + // 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::normal) + { + fail (t) << "configuration variable " << var << " has " + << var.visibility << " visibility"; + } - // The rest is the default value which we should parse in the value - // mode. But before switching check whether we need to evaluate it at - // all. + // We have to lookup the value whether we have the default part or not + // in order to mark it as saved. We also have to do this to get the new + // value status. // - if (l.defined ()) - skip_line (t, tt); - else + using config::lookup_config; + + l = lookup_config (new_val, *root_, var); + + // See if we have the default value part. + // + next (t, tt); + + if (tt != type::newline && tt != type::eos) { - value lhs, rhs (parse_variable_value (t, tt)); - apply_value_attributes (&var, lhs, move (rhs), type::assign); - lookup_config (*root_, var, move (lhs)); + if (tt != type::default_assign) + fail (t) << "expected '?=' instead of " << t << " after " + << "configuration variable name"; + + // The rest is the default value which we should parse in the value + // mode. But before switching check whether we need to evaluate it at + // all. + // + if (l.defined ()) + skip_line (t, tt); + else + { + value lhs, rhs (parse_variable_value (t, tt)); + apply_value_attributes (&var, lhs, move (rhs), type::assign); + l = lookup_config (new_val, *root_, var, move (lhs)); + } + } + } + + // We will be printing the report at either level 2 (-v) or 3 (-V) + // depending on the final value of config_report_new. + // + // Note that for the config_report_new calculation we only incorporate + // variables that we are actually reporting. + // + if (*report != "false" && verb >= 2) + { + // We don't want to lookup the report variable value here since it's + // most likely not set yet. + // + if (!report_var.empty ()) + { + // In a somewhat hackish way we pass the variable in an undefined + // lookup. + // + l = lookup (); + l.var = &root_->var_pool ().insert ( + move (report_var), true /* overridable */); + } + + if (l.var != nullptr) + { + config_report.push_back (make_pair (l, move (*report))); + config_report_new = config_report_new || new_val; } } @@ -3064,12 +3165,12 @@ namespace build2 void parser:: apply_variable_attributes (const variable& var) { - attributes a (attributes_pop ()); + attributes as (attributes_pop ()); - if (!a) + if (as.empty ()) return; - const location& l (a.loc); + const location& l (as.loc); const value_type* type (nullptr); optional vis; @@ -3081,15 +3182,15 @@ namespace build2 to_stream (dr.os, reverse (v, storage), true /* quote */, '@'); }; - for (auto& p: a.ats) + for (auto& p: as) { - string& k (p.first); - value& v (p.second); + string& n (p.name); + value& v (p.value); - if (const value_type* t = map_type (k)) + if (const value_type* t = map_type (n)) { if (type != nullptr && t != type) - fail (l) << "multiple variable types: " << k << ", " << type->name; + fail (l) << "multiple variable types: " << n << ", " << type->name; type = t; // Fall through. @@ -3097,7 +3198,7 @@ namespace build2 else { diag_record dr (fail (l)); - dr << "unknown variable attribute " << k; + dr << "unknown variable attribute " << n; if (!v.null) { @@ -3109,7 +3210,7 @@ namespace build2 if (!v.null) { diag_record dr (fail (l)); - dr << "unexpected value for attribute " << k << ": "; + dr << "unexpected value for attribute " << n << ": "; print (dr, v); } } @@ -3139,8 +3240,8 @@ namespace build2 value&& rhs, type kind) { - attributes a (attributes_pop ()); - const location& l (a.loc); + attributes as (attributes_pop ()); + const location& l (as.loc); // Essentially this is an attribute-augmented assign/append/prepend. // @@ -3153,12 +3254,12 @@ namespace build2 to_stream (dr.os, reverse (v, storage), true /* quote */, '@'); }; - for (auto& p: a.ats) + for (auto& p: as) { - string& k (p.first); - value& v (p.second); + string& n (p.name); + value& v (p.value); - if (k == "null") + if (n == "null") { if (rhs && !rhs.empty ()) // Note: null means we had an expansion. fail (l) << "value with null attribute"; @@ -3166,10 +3267,10 @@ namespace build2 null = true; // Fall through. } - else if (const value_type* t = map_type (k)) + else if (const value_type* t = map_type (n)) { if (type != nullptr && t != type) - fail (l) << "multiple value types: " << k << ", " << type->name; + fail (l) << "multiple value types: " << n << ", " << type->name; type = t; // Fall through. @@ -3177,7 +3278,7 @@ namespace build2 else { diag_record dr (fail (l)); - dr << "unknown value attribute " << k; + dr << "unknown value attribute " << n; if (!v.null) { @@ -3189,7 +3290,7 @@ namespace build2 if (!v.null) { diag_record dr (fail (l)); - dr << "unexpected value for attribute " << k << ": "; + dr << "unexpected value for attribute " << n << ": "; print (dr, v); } } @@ -3627,7 +3728,7 @@ namespace build2 // Process attributes if any. // - if (!at.first) + if (attributes_top ().empty ()) { attributes_pop (); return v; @@ -3693,7 +3794,7 @@ namespace build2 bool has (tt == type::lsbrace); if (!pre_parse_) - attributes_.push_back (attributes {has, l, {}}); + attributes_.push_back (attributes (l)); if (!has) return make_pair (false, l); @@ -3701,7 +3802,7 @@ namespace build2 mode (lexer_mode::attributes); next (t, tt); - if ((has = (tt != type::rsbrace))) + if (tt != type::rsbrace) { do { @@ -3745,7 +3846,7 @@ namespace build2 } if (!pre_parse_) - attributes_.back ().ats.emplace_back (move (n), move (v)); + attributes_.back ().push_back (attribute {move (n), move (v)}); if (tt == type::comma) next (t, tt); diff --git a/libbuild2/parser.hxx b/libbuild2/parser.hxx index 6d78ce1..b21336d 100644 --- a/libbuild2/parser.hxx +++ b/libbuild2/parser.hxx @@ -72,8 +72,15 @@ namespace build2 // Note that these are not touched by reset(). // public: + // export directive result. + // names export_value; + // config directive result. + // + vector> config_report; // Config value and format. + bool config_report_new = false; // One of values is new. + // Recursive descent parser. // protected: @@ -231,24 +238,28 @@ namespace build2 // In this example we only apply the value attributes after evaluating // the context, which has its own attributes. // - struct attributes + struct attribute + { + string name; + build2::value value; + }; + + struct attributes: small_vector { - bool has; // Has attributes flag. - location loc; // Start location. - small_vector, 1> ats; // Attributes. + location loc; // Start location. - explicit operator bool () const {return has;} + explicit + attributes (location l): loc (move (l)) {} }; - // Push a new entry into the attributes_ stack. If the next token is '[' - // parse the attribute sequence until ']' storing the result in the new - // stack entry and setting the 'has' flag (unless the attribute list is - // empty). Then get the next token and, if standalone is false, verify - // it is not newline/eos (i.e., there is something after it). Return the - // indication of whether there are any attributes and their location. + // Push a new entry into the attributes_ stack. If the next token is `[` + // then parse the attribute sequence until ']' storing the result in the + // new stack entry. Then get the next token and, if standalone is false, + // verify it is not newline/eos (i.e., there is something after it). + // Return the indication of whether we have seen `[` (even if it's the + // `[]` empty list) and its location. // - // Note that during pre-parsing nothing is pushed into the stack and - // the returned attributes object indicates there are no attributes. + // Note that during pre-parsing nothing is pushed into the stack. // pair attributes_push (token&, token_type&, bool standalone = false); diff --git a/libbuild2/scope.hxx b/libbuild2/scope.hxx index fdc660d..cb20f79 100644 --- a/libbuild2/scope.hxx +++ b/libbuild2/scope.hxx @@ -496,6 +496,12 @@ namespace build2 const project_name& project (const scope& root); + // Return the name of the first innermost named project in the strong + // amalgamation chain or empty if all are unnamed. + // + const project_name& + named_project (const scope& root); + // Temporary scope. The idea is to be able to create a temporary scope in // order not to change the variables in the current scope. Such a scope is // not entered in to the scope map. As a result it can only be used as a diff --git a/libbuild2/scope.ixx b/libbuild2/scope.ixx index ec36bf9..f13aa70 100644 --- a/libbuild2/scope.ixx +++ b/libbuild2/scope.ixx @@ -94,4 +94,22 @@ namespace build2 auto l (rs[rs.ctx.var_project]); return l ? cast (l) : empty_project_name; } + + inline const project_name& + named_project (const scope& rs) + { + for (auto r (&rs), a (rs.strong_scope ()); + ; + r = r->parent_scope ()->root_scope ()) + { + const project_name& n (project (*r)); + if (!n.empty ()) + return n; + + if (r == a) + break; + } + + return empty_project_name; + } } diff --git a/tests/directive/config.testscript b/tests/directive/config.testscript index d10f45d..f77a098 100644 --- a/tests/directive/config.testscript +++ b/tests/directive/config.testscript @@ -162,3 +162,67 @@ test.arguments = error: invalid bool value 'junk' in variable config.test.fancy EOE } + +: report +: +{ + .include ../common.testscript + + +cat <+build/bootstrap.build + using config + EOI + + +cat <=build/root.build + config [bool] config.test.a + config [bool] config.test.b ?= false + config [bool, config.report=false] config.test.c ?= true + 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 [bool] config.test.n ?= [null] + + e = "'$config.test.e'" + f = ($config.test.b || $config.test.c) + + EOI + + # This must be a single, serial test since we are sharing config.build. + # + : test + : + cat <=buildfile; + ./: + EOI + + # Unconfigured. + # + $* noop -v 2>>~/EOO/; + /config test@.+/ + b false + d + 1 + 2 + 3 + e 'abc' + f true + n [null] + EOO + + # Configured. + # + $* configure config.test.a=true config.test.e=xyz config.test.n=true; + $* noop -v; + $* noop -V 2>>~/EOO/; + /config test@.+/ + a true + b false + d + 1 + 2 + 3 + e 'xyz' + f true + n true + EOO + $* disfigure +} -- cgit v1.1