aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorBoris Kolpackov <boris@codesynthesis.com>2024-03-01 13:31:19 +0200
committerBoris Kolpackov <boris@codesynthesis.com>2024-03-01 13:31:19 +0200
commitce558a5aebc649e52da9598f248bd15fd08a9ba2 (patch)
tree43d7de32f58cf4f273eea56214a7921da96a3e20
parent82ff2e7df2243b679aadbc6cc120a2b5f7ee73b3 (diff)
Use original variable name in config report
-rw-r--r--doc/manual.cli26
-rw-r--r--libbuild2/file.cxx55
-rw-r--r--libbuild2/parser.cxx74
-rw-r--r--libbuild2/parser.hxx13
-rw-r--r--tests/directive/config.testscript4
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 <libbuild2/file.hxx>
#include <cerrno>
+#include <cstring> // strlen()
#include <iomanip> // left, setw()
#include <sstream>
@@ -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<lookup, string>& 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<lookup, string>& 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<int> (pad)) << n << ' ' << ns;
+ dr << left << setw (static_cast<int> (pad)) << pn << ' ' << ns;
}
else
- dr << left << setw (static_cast<int> (pad)) << n << " [null]";
+ dr << left << setw (static_cast<int> (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<string> (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 () ? "<project>" : 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<bool>::value_type)
- fail (loc) << var << " variable must be of type bool";
+ if (var->type != &value_traits<bool>::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<lookup, string>& 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<pair<lookup, string>> 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<value> values;
+ bool new_value; // One of values is new.
};
small_vector<config_report, 1> 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