aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorBoris Kolpackov <boris@codesynthesis.com>2022-06-06 11:40:50 +0200
committerBoris Kolpackov <boris@codesynthesis.com>2022-06-09 12:32:36 +0200
commitf0f5af955fe03fa120b69c39f4a23ff3a177769b (patch)
tree29b614333ec7c32d13290a992a30fe07e948f464
parentb07c40f8a457bbb8f7f2d4d142e5e5e974465e25 (diff)
Implementation of evaluate_require() plus other tweaks
-rw-r--r--bpkg/package-configuration.cxx34
-rw-r--r--bpkg/package-configuration.hxx25
-rw-r--r--bpkg/package-skeleton.cxx340
-rw-r--r--bpkg/package-skeleton.hxx9
-rw-r--r--bpkg/utility.hxx4
5 files changed, 358 insertions, 54 deletions
diff --git a/bpkg/package-configuration.cxx b/bpkg/package-configuration.cxx
index fb742e8..e74be64 100644
--- a/bpkg/package-configuration.cxx
+++ b/bpkg/package-configuration.cxx
@@ -16,7 +16,12 @@ namespace bpkg
pair<size_t, size_t> pos,
const small_vector<reference_wrapper<package_skeleton>, 1>& depcs)
{
- dependent_config_variable_values old_cfgs;
+ pos.first--; pos.second--; // Convert to 0-base.
+
+ const dependency_alternative& da (
+ dept.available.get ().dependencies[pos.first][pos.second]);
+
+ assert (da.require || da.prefer);
// Step 1: save a snapshot of the old configuration while unsetting values
// that have this dependent as the originator and reloading the defaults.
@@ -24,6 +29,22 @@ namespace bpkg
// While at it, also collect the configurations to pass to dependent's
// evaluate_*() calls.
//
+ // Our assumptions regarding require:
+ //
+ // - Can only set bool configuration variables and only to true.
+ //
+ // - Should not have any conditions on the state of other configuration
+ // variables, including their origin (but can have other conditions,
+ // for example on the target platform).
+ //
+ // This means that we don't need to set the default values, but will need
+ // the type information as well as overrides. So what we will do is only
+ // call reload_defaults() for the first time to load types/override. Note
+ // that this assumes the set of configuration variables cannot change
+ // based on the values of other configuration variables (we have a note
+ // in the manual instructing the user not to do this).
+ //
+ dependent_config_variable_values old_cfgs;
package_skeleton::dependency_configurations depc_cfgs;
depc_cfgs.reserve (depcs.size ());
@@ -41,7 +62,10 @@ namespace bpkg
dependent_config_variable_value {
v.name, move (v.value), move (*v.dependent)});
+ // Note that we will not reload it to default in case of require.
+ //
v.origin = variable_origin::undefined;
+ v.value = nullopt;
v.dependent = nullopt;
}
else
@@ -50,18 +74,14 @@ namespace bpkg
}
}
- depc.reload_defaults (cfg);
+ if (da.prefer || cfg.empty ())
+ depc.reload_defaults (cfg);
depc_cfgs.push_back (cfg);
}
// Step 2: execute the prefer/accept or requires clauses.
//
- pos.first--; pos.second--; // Convert to 0-base.
-
- const dependency_alternative& da (
- dept.available.get ().dependencies[pos.first][pos.second]);
-
if (!(da.require
? dept.evaluate_require (depc_cfgs, *da.require, pos.first)
: dept.evaluate_prefer_accept (depc_cfgs,
diff --git a/bpkg/package-configuration.hxx b/bpkg/package-configuration.hxx
index 55a43a5..ba7240e 100644
--- a/bpkg/package-configuration.hxx
+++ b/bpkg/package-configuration.hxx
@@ -55,6 +55,11 @@ namespace bpkg
class package_configuration: public vector<config_variable_value>
{
public:
+ package_key package;
+
+ explicit
+ package_configuration (package_key p): package (move (p)) {}
+
config_variable_value*
find (const string& name)
{
@@ -78,10 +83,24 @@ namespace bpkg
}
};
+ class package_configurations: public small_vector<package_configuration, 1>
+ {
+ public:
+ package_configuration&
+ operator[] (const package_key& p)
+ {
+ auto i (find_if (begin (), end (),
+ [&p] (const package_configuration& pc)
+ {
+ return pc.package == p;
+ }));
+ if (i != end ())
+ return *i;
- // @@ Maybe redo as small_vector?
- //
- using package_configurations = map<package_key, package_configuration>;
+ push_back (package_configuration (p));
+ return back ();
+ }
+ };
// A subset of config_variable_value for variable values set by the
diff --git a/bpkg/package-skeleton.cxx b/bpkg/package-skeleton.cxx
index 0d07981..24f155c 100644
--- a/bpkg/package-skeleton.cxx
+++ b/bpkg/package-skeleton.cxx
@@ -328,7 +328,7 @@ namespace bpkg
// construction (in evaluate_{prefer_accept,require}()): we do not add
// as dependent variables that have the override origin.
//
- package_configuration old (move (cfg));
+ package_configuration old (move (cfg)); cfg.package = move (old.package);
scope& rs (
*bootstrap (
@@ -356,6 +356,11 @@ namespace bpkg
// variable), then the saved variables map seem like the natural place
// to keep this information.
//
+ // @@ One potentially-bogus config variable could be config.*.develop.
+ // Would have been nice not to drag it around if not used by the
+ // package. And, could be helpful to warn that configuration variable
+ // does not exist.
+ //
string p ("config." + name ().variable ());
size_t n (p.size ());
@@ -507,7 +512,7 @@ namespace bpkg
// the temp directory is not cleaned in case of an error. Maybe one day.
//
static void
- depends_location (const diag_record& dr,
+ depends_location (const build2::diag_record& dr,
const path& mf,
size_t depends_index)
{
@@ -527,10 +532,10 @@ namespace bpkg
{
if (nv.name == "depends" && i++ == depends_index)
{
- dr << info (location (p.name (),
- nv.value_line,
- nv.value_column))
- << "depends value defined here";
+ dr << build2::info (build2::location (mf,
+ nv.value_line,
+ nv.value_column))
+ << "in depends value defined here";
break;
}
}
@@ -547,6 +552,7 @@ namespace bpkg
{
using namespace build2;
using build2::fail;
+ using build2::info;
using build2::endf;
scope& rs (load ());
@@ -564,7 +570,7 @@ namespace bpkg
uint64_t il (1);
auto df = build2::make_diag_frame (
- [&cond, &rs, depends_index] (const diag_record& dr)
+ [&cond, &rs, depends_index] (const build2::diag_record& dr)
{
dr << info << "enable condition: (" << cond << ")";
@@ -655,6 +661,7 @@ namespace bpkg
//
using namespace build2;
using build2::fail;
+ using build2::info;
using build2::endf;
scope& rs (load ());
@@ -666,16 +673,16 @@ namespace bpkg
uint64_t il (1);
auto df = build2::make_diag_frame (
- [&refl, &rs, depends_index] (const diag_record& dr)
+ [&refl, &rs, depends_index] (const build2::diag_record& dr)
{
// Probably safe to assume a one-line fragment contains a variable
// assignment.
//
if (refl.find ('\n') == string::npos)
- dr << info << "reflect variable: " << refl;
+ dr << info << "reflect variable: " << trim (string (refl));
else
- dr << info << "reflect fragment:\n"
- << refl;
+ dr << info << "reflect clause:\n"
+ << trim_right (string (refl));
// For external packages we have the manifest so print the location
// of the depends value in questions.
@@ -693,7 +700,7 @@ namespace bpkg
// filter out unchanged on the second.
//
auto& vp (rs.var_pool ());
- const variable& ns (vp.insert ("config." + name ().variable ()));
+ string ns ("config." + name ().variable ());
struct value_data
{
@@ -823,6 +830,9 @@ namespace bpkg
// config.hello.backend = foo # reflect
// config.hello.backend += bar # user
//
+
+ // @@ Can't we redo it via origin() call like in other places?
+ //
pair<lookup, size_t> org {lookup {val, var, rs.vars}, 1 /* depth */};
pair<lookup, size_t> ovr;
@@ -927,6 +937,7 @@ namespace bpkg
using namespace build2;
using config::variable_origin;
using build2::fail;
+ using build2::info;
using build2::endf;
// This is what needs to happen to the variables of different origins in
@@ -953,10 +964,10 @@ namespace bpkg
uint64_t il (1);
auto df = build2::make_diag_frame (
- [&prefer, &rs, depends_index] (const diag_record& dr)
+ [&prefer, &rs, depends_index] (const build2::diag_record& dr)
{
- dr << info << "prefer fragment:\n"
- << prefer;
+ dr << info << "prefer clause:\n"
+ << trim_right (string (prefer));
// For external packages we have the manifest so print the
// location of the depends value in questions.
@@ -970,6 +981,34 @@ namespace bpkg
lexer l (is, in, il /* start line */);
parser p (rs.ctx);
p.parse_buildfile (l, &rs, rs);
+
+ // Check if the dependent set any stray configuration variables.
+ //
+ for (package_configuration& cfg: cfgs)
+ {
+ string ns ("config." + cfg.package.name.variable ());
+
+ for (auto p (rs.vars.lookup_namespace (ns));
+ p.first != p.second;
+ ++p.first)
+ {
+ const variable& var (p.first->first);
+
+ // This can be one of the overrides (__override, __prefix, etc),
+ // which we skip.
+ //
+ if (var.override ())
+ continue;
+
+ if (cfg.find (var.name) == nullptr)
+ {
+ fail << "package " << cfg.package.name << " has no "
+ << "configuration variable " << var.name <<
+ info << var.name << " set in require clause of dependent "
+ << key.string ();
+ }
+ }
+ }
}
// Evaluate the accept clause.
@@ -983,7 +1022,7 @@ namespace bpkg
uint64_t il (1);
auto df = build2::make_diag_frame (
- [&accept, &rs, depends_index] (const diag_record& dr)
+ [&accept, &rs, depends_index] (const build2::diag_record& dr)
{
dr << info << "accept condition: (" << accept << ")";
@@ -1022,11 +1061,28 @@ namespace bpkg
{
for (package_configuration& cfg: cfgs)
{
- for (config_variable_value& v: cfg)
+ string ns ("config." + cfg.package.name.variable ());
+
+ for (auto p (rs.vars.lookup_namespace (ns));
+ p.first != p.second;
+ ++p.first)
{
- pair<variable_origin, lookup> ol (config::origin (rs, v.name));
+ const variable& var (p.first->first);
- // An override cannot become non-override. And a non-override
+ if (var.override ())
+ continue;
+
+ const variable_map::value_data& val (p.first->second);
+
+ pair<variable_origin, lookup> ol (
+ config::origin (rs,
+ var,
+ pair<lookup, size_t> {
+ lookup {val, var, rs.vars}, 1 /* depth */}));
+
+ config_variable_value& v (*cfg.find (var.name));
+
+ // An override cannot become a non-override. And a non-override
// cannot become an override.
//
assert ((v.origin == variable_origin::override_) ==
@@ -1041,7 +1097,7 @@ namespace bpkg
// default/undefine -> buildfile -- override dependency default
// buildfile -> buildfile -- override other dependent
//
- optional<names> val (reverse_value (*ol.second));
+ optional<names> ns (reverse_value (val));
if (v.origin == variable_origin::buildfile)
{
@@ -1049,13 +1105,13 @@ namespace bpkg
// (even if the value was technically "overwritten" by this
// dependent).
//
- if (v.value == val)
+ if (v.value == ns)
break;
}
else
v.origin = variable_origin::buildfile;
- v.value = move (val);
+ v.value = move (ns);
v.dependent = key; // We are the originating dependent.
break;
}
@@ -1073,6 +1129,10 @@ namespace bpkg
// @@ We may still need to see if there is original set
// by this dependent and if so, reflect the overridden
// value.
+ //
+ // Maybe we don't need the version: the presence of the
+ // value in rs is enough.
+ //
break;
}
}
@@ -1095,14 +1155,205 @@ namespace bpkg
}
bool package_skeleton::
- evaluate_require (const dependency_configurations&,
- const string&, size_t /*depends_index*/)
+ evaluate_require (const dependency_configurations& cfgs,
+ const string& require, size_t depends_index)
{
- // How will we square the implied accept logic with user overrides?
- // Feels like the only way is to not enter them as overrides and then
- // see of any of them override manually? Nasty.
+ try
+ {
+ using namespace build2;
+ using config::variable_origin;
+ using build2::fail;
+ using build2::info;
+ using build2::endf;
+
+ // A require clause can only set bool configuration variables and only
+ // to true and may not have any conditions on other configuration
+ // variables (including their origin). As a result, we don't need to set
+ // the default (or other dependent) values, but will need the type
+ // information as well as overrides (see up_negotiate_configuration()
+ // for details).
+ //
+ scope& rs (load (cfgs, false /* defaults */));
+
+ // Evaluate the require clause.
+ //
+ istringstream is (require);
+ is.exceptions (istringstream::failbit | istringstream::badbit);
+
+ path_name in ("<depends-require-clause>");
+ uint64_t il (1);
+
+ // Note: keep the diag frame beyond parse_buildfile().
+ //
+ auto df = build2::make_diag_frame (
+ [&require, &rs, depends_index] (const build2::diag_record& dr)
+ {
+ dr << info << "require clause:\n"
+ << trim_right (string (require));
+
+ // For external packages we have the manifest so print the
+ // location of the depends value in questions.
+ //
+ if (!rs.out_eq_src ())
+ depends_location (dr,
+ rs.src_path () / manifest_file,
+ depends_index);
+ });
+
+ lexer l (is, in, il /* start line */);
+ parser p (rs.ctx);
+ p.parse_buildfile (l, &rs, rs);
+
+ // First determine if acceptable.
+ //
+ // While at it, also check for stray variables and enforce all the
+ // require restrictions (bool, set to true, etc).
+ //
+ bool r (true);
+ for (package_configuration& cfg: cfgs)
+ {
+ string ns ("config." + cfg.package.name.variable ());
+
+ // Note that because we didn't set any default (or other dependent)
+ // values, all the values we see are set by this dependent.
+ //
+ for (auto p (rs.vars.lookup_namespace (ns));
+ p.first != p.second;
+ ++p.first)
+ {
+ const variable& var (p.first->first);
+
+ // This can be one of the overrides (__override, __prefix, etc),
+ // which we skip.
+ //
+ if (var.override ())
+ continue;
+
+ const config_variable_value* v (cfg.find (var.name));
+
+ if (v == nullptr)
+ {
+ fail << "package " << cfg.package.name << " has no configuration "
+ << "variable " << var.name <<
+ info << var.name << " set in require clause of dependent "
+ << key.string ();
+ }
+
+ if (!v->type || *v->type != "bool")
+ {
+ fail << "configuration variable " << var.name << " is not of "
+ << "bool type" <<
+ info << var.name << " set in require clause of dependent "
+ << key.string ();
+ }
+
+ const value& val (p.first->second);
+
+ if (!cast_false<bool> (val))
+ {
+ fail << "configuration variable " << var.name << " is not set "
+ << "to true" <<
+ info << var.name << " set in require clause of dependent "
+ << key.string ();
+ }
+
+ // The only situation where the result would not be acceptable is if
+ // one of the values were overridden to false.
+ //
+ pair<variable_origin, lookup> ol (
+ config::origin (rs,
+ var,
+ pair<lookup, size_t> {
+ lookup {val, var, rs.vars}, 1 /* depth */}));
+
+ // An override cannot become a non-override. And a non-override
+ // cannot become an override.
+ //
+ assert ((v->origin == variable_origin::override_) ==
+ (ol.first == variable_origin::override_));
+
+ if (ol.first == variable_origin::override_)
+ {
+ if (!cast_false<bool> (*ol.second))
+ r = false;
+ }
+ }
+ }
+
+ // If acceptable, update the configuration with the new values, if any.
+ //
+ // Note that we cannot easily combine this loop with the above because
+ // we should not modify configurations if the result is not acceptable.
+ //
+ // @@ TODO: we also need to save the subset of values that were "set"
+ // (for some definition of "set") by this dependent to be reflected
+ // to further clauses.
+ //
+ if (r)
+ {
+ for (package_configuration& cfg: cfgs)
+ {
+ string ns ("config." + cfg.package.name.variable ());
+
+ for (auto p (rs.vars.lookup_namespace (ns));
+ p.first != p.second;
+ ++p.first)
+ {
+ const variable& var (p.first->first);
+
+ if (var.override ())
+ continue;
+
+ config_variable_value& v (*cfg.find (var.name));
- return false; // @@ TODO
+ if (v.origin != variable_origin::override_)
+ {
+ // Possible transitions:
+ //
+ // default/undefine -> buildfile -- override dependency default
+ // buildfile -> buildfile -- override other dependent
+ //
+ optional<names> ns (names {build2::name ("true")});
+
+ // @@ TODO: reflect (before continue)
+
+ if (v.origin == variable_origin::buildfile)
+ {
+ // If unchanged, then we keep the old originating dependent
+ // (even if the value was technically "overwritten" by this
+ // dependent).
+ //
+ if (v.value == ns)
+ continue;
+ }
+ else
+ v.origin = variable_origin::buildfile;
+
+ v.value = move (ns);
+ v.dependent = key; // We are the originating dependent.
+ }
+ else
+ {
+ // @@ TODO: reflect (true). Could probably handle with the
+ // same code as !=override case.
+ }
+ }
+ }
+ }
+
+ // Drop the build system state since it needs reloading (while it may
+ // seem safe for us to keep the state since we didn't set any defaults,
+ // we may have overrides that the clause did not set, so let's drop it
+ // for good measure and also to keep things simple).
+ //
+ ctx_ = nullptr;
+
+ return r;
+ }
+ catch (const build2::failed&)
+ {
+ throw failed (); // Assume the diagnostics has already been issued.
+ }
}
pair<strings, vector<config_variable>> package_skeleton::
@@ -1401,7 +1652,7 @@ namespace bpkg
}
build2::scope& package_skeleton::
- load (const dependency_configurations& cfgs)
+ load (const dependency_configurations& cfgs, bool defaults)
{
if (ctx_ != nullptr)
{
@@ -1471,7 +1722,8 @@ namespace bpkg
{
scope& rs;
const dependency_configurations& cfgs;
- } d {rs, cfgs};
+ bool defaults;
+ } d {rs, cfgs, defaults};
if (!reflect_frag_.empty () || !cfgs.empty ())
{
@@ -1509,19 +1761,25 @@ namespace bpkg
case variable_origin::default_:
case variable_origin::buildfile:
{
- auto& val (
- static_cast<variable_map::value_data&> (
- rs.assign (var)));
-
- if (v.value)
- val.assign (names (*v.value), &var);
+ if (d.defaults)
+ {
+ auto& val (
+ static_cast<variable_map::value_data&> (
+ rs.assign (var)));
+
+ if (v.value)
+ val.assign (names (*v.value), &var);
+ else
+ val = nullptr;
+
+ if (v.origin == variable_origin::default_)
+ val.extra = 1;
+
+ v.version = val.version;
+ }
else
- val = nullptr;
-
- if (v.origin == variable_origin::default_)
- val.extra = 1;
+ v.version = 0;
- v.version = val.version;
break;
}
case variable_origin::undefined:
diff --git a/bpkg/package-skeleton.hxx b/bpkg/package-skeleton.hxx
index 1c4ae32..e949784 100644
--- a/bpkg/package-skeleton.hxx
+++ b/bpkg/package-skeleton.hxx
@@ -178,11 +178,14 @@ namespace bpkg
//
// Call this function before evaluating every clause.
//
- // If dependency configurations are specified, then set their values and
- // save the resulting versions in config_variable_value::version.
+ // If dependency configurations are specified, then typify the variables
+ // and set their values saving the resulting value versions in
+ // config_variable_value::version. If defaults is false, then only typify
+ // the variables and set overrides without setting the default/buildfile
+ // values.
//
build2::scope&
- load (const dependency_configurations& = {});
+ load (const dependency_configurations& = {}, bool defaults = true);
// Merge command line variable overrides into one list (normally to be
// passed to bootstrap()).
diff --git a/bpkg/utility.hxx b/bpkg/utility.hxx
index 2dcf46d..597800c 100644
--- a/bpkg/utility.hxx
+++ b/bpkg/utility.hxx
@@ -48,6 +48,10 @@ namespace bpkg
using butl::digit;
using butl::xdigit;
+ using butl::trim;
+ using butl::trim_left;
+ using butl::trim_right;
+
using butl::make_guard;
using butl::make_exception_guard;