From b07c40f8a457bbb8f7f2d4d142e5e5e974465e25 Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Fri, 3 Jun 2022 12:21:26 +0200 Subject: Initial up_negotiate_configuration() implementation --- bpkg/package-configuration.cxx | 128 ++++++++ bpkg/package-configuration.hxx | 119 ++++++++ bpkg/package-skeleton.cxx | 660 +++++++++++++++++++++++++++++++++-------- bpkg/package-skeleton.hxx | 61 ++-- bpkg/package.hxx | 6 + bpkg/pkg-build.cxx | 72 ++++- 6 files changed, 899 insertions(+), 147 deletions(-) create mode 100644 bpkg/package-configuration.cxx create mode 100644 bpkg/package-configuration.hxx diff --git a/bpkg/package-configuration.cxx b/bpkg/package-configuration.cxx new file mode 100644 index 0000000..fb742e8 --- /dev/null +++ b/bpkg/package-configuration.cxx @@ -0,0 +1,128 @@ +// file : bpkg/package-configuration.cxx -*- C++ -*- +// license : MIT; see accompanying LICENSE file + +#include + +#include + +namespace bpkg +{ + using build2::config::variable_origin; + + bool + up_negotiate_configuration ( + package_configurations& cfgs, + package_skeleton& dept, + pair pos, + const small_vector, 1>& depcs) + { + dependent_config_variable_values old_cfgs; + + // Step 1: save a snapshot of the old configuration while unsetting values + // that have this dependent as the originator and reloading the defaults. + // + // While at it, also collect the configurations to pass to dependent's + // evaluate_*() calls. + // + package_skeleton::dependency_configurations depc_cfgs; + depc_cfgs.reserve (depcs.size ()); + + for (package_skeleton& depc: depcs) + { + package_configuration& cfg (cfgs[depc.key]); + + for (config_variable_value& v: cfg) + { + if (v.origin == variable_origin::buildfile) + { + if (*v.dependent == dept.key) + { + old_cfgs.push_back ( + dependent_config_variable_value { + v.name, move (v.value), move (*v.dependent)}); + + v.origin = variable_origin::undefined; + v.dependent = nullopt; + } + else + old_cfgs.push_back ( + dependent_config_variable_value {v.name, v.value, *v.dependent}); + } + } + + 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, + *da.prefer, *da.accept, pos.first))) + { + fail << "unable to negotiate acceptable configuration"; // @@ TODO + } + + // Check if anything changed by comparing to entries in old_cfgs. + // + { + optional n (0); // Number of unchanged. + + for (package_skeleton& depc: depcs) + { + package_configuration& cfg (cfgs[depc.key]); + + for (config_variable_value& v: cfg) + { + if (v.origin == variable_origin::buildfile) + { + auto i (find_if (old_cfgs.begin (), old_cfgs.end (), + [&v] (const dependent_config_variable_value& o) + { + return v.name == o.name; + })); + + if (i == old_cfgs.end () || + i->value != v.value || + i->dependent != *v.dependent) + { + n = nullopt; + break; + } + + ++*n; + } + } + + if (!n) + break; + } + + // If we haven't seen any changed and we've seen the same number, then + // nothing has changed. + // + if (n && *n == old_cfgs.size ()) + return false; + } + + + // @@ TODO: look for cycles in change history. + // @@ TODO: save in change history. + // + /* + dependent_config_variable_values new_cfgs; // @@ TODO. + + old_cfgs.sort (); + new_cfgs.sort (); + */ + + return true; + } +} diff --git a/bpkg/package-configuration.hxx b/bpkg/package-configuration.hxx new file mode 100644 index 0000000..55a43a5 --- /dev/null +++ b/bpkg/package-configuration.hxx @@ -0,0 +1,119 @@ +// file : bpkg/package-configuration.hxx -*- C++ -*- +// license : MIT; see accompanying LICENSE file + +#ifndef BPKG_PACKAGE_CONFIGURATION_HXX +#define BPKG_PACKAGE_CONFIGURATION_HXX + +#include + +#include // build2::names +#include // build2::config::variable_origin + +#include +#include + +#include + +using namespace std; + +namespace bpkg +{ + class package_skeleton; + + struct config_variable_value + { + string name; + + // The variable_origin values have the following meaning: + // + // default -- default value from the config directive + // buildfile -- dependent configuration (config_source::dependent) + // override -- user configuration (config_source::user) + // undefined -- none of the above + // + build2::config::variable_origin origin; + + // If origin is not undefined, then this is the reversed variable value + // with absent signifying NULL. + // + optional value; + + // Variable type name with absent signifying untyped. + // + optional type; + + // If origin is buildfile, then this is the "originating dependent" which + // first set this variable to this value. + // + optional dependent; + + // Value version (used internally by package_skeleton). + // + size_t version; + }; + + class package_configuration: public vector + { + public: + config_variable_value* + find (const string& name) + { + auto i (find_if (begin (), end (), + [&name] (const config_variable_value& v) + { + return v.name == name; + })); + return i != end () ? &*i : nullptr; + } + + const config_variable_value* + find (const string& name) const + { + auto i (find_if (begin (), end (), + [&name] (const config_variable_value& v) + { + return v.name == name; + })); + return i != end () ? &*i : nullptr; + } + }; + + + // @@ Maybe redo as small_vector? + // + using package_configurations = map; + + + // A subset of config_variable_value for variable values set by the + // dependents (origin is buildfile). Used to track change history. + // + struct dependent_config_variable_value + { + string name; + optional value; + package_key dependent; + }; + + class dependent_config_variable_values: + public small_vector + { + public: + /* + @@ + void + sort (); + */ + }; + + // Up-negotiate the configuration for the specified dependencies of the + // specified dependent. Return true if the configuration has changed. + // + bool + up_negotiate_configuration ( + package_configurations&, + package_skeleton& dependent, + pair position, + const small_vector, 1>& dependencies); +} + +#endif // BPKG_PACKAGE_CONFIGURATION_HXX diff --git a/bpkg/package-skeleton.cxx b/bpkg/package-skeleton.cxx index c81700e..0d07981 100644 --- a/bpkg/package-skeleton.cxx +++ b/bpkg/package-skeleton.cxx @@ -54,8 +54,26 @@ namespace bpkg package_skeleton:: package_skeleton (package_skeleton&& v) + : key (move (v.key)), + available (v.available), + co_ (v.co_), + db_ (v.db_), + config_vars_ (move (v.config_vars_)), + disfigure_ (v.disfigure_), + config_srcs_ (v.config_srcs_), + src_root_ (move (v.src_root_)), + out_root_ (move (v.out_root_)), + created_ (v.created_), + verified_ (v.verified_), + ctx_ (move (v.ctx_)), + rs_ (v.rs_), + cmd_vars_ (move (v.cmd_vars_)), + cmd_vars_cache_ (v.cmd_vars_cache_), + dependent_vars_ (move (v.dependent_vars_)), + reflect_vars_ (move (v.reflect_vars_)), + reflect_frag_ (move (v.reflect_frag_)) { - *this = move (v); + v.db_ = nullptr; } package_skeleton& package_skeleton:: @@ -63,9 +81,10 @@ namespace bpkg { if (this != &v) { + key = move (v.key); + available = v.available; co_ = v.co_; db_ = v.db_; - available_ = v.available_; config_vars_ = move (v.config_vars_); disfigure_ = v.disfigure_; config_srcs_ = v.config_srcs_; @@ -82,7 +101,6 @@ namespace bpkg reflect_frag_ = move (v.reflect_frag_); v.db_ = nullptr; - v.available_ = nullptr; } return *this; @@ -90,9 +108,10 @@ namespace bpkg package_skeleton:: package_skeleton (const package_skeleton& v) - : co_ (v.co_), + : key (v.key), + available (v.available), + co_ (v.co_), db_ (v.db_), - available_ (v.available_), config_vars_ (v.config_vars_), disfigure_ (v.disfigure_), config_srcs_ (v.config_srcs_), @@ -119,16 +138,17 @@ namespace bpkg const vector* css, optional src_root, optional out_root) - : co_ (&co), + : key (db, ap.id.name), + available (ap), + co_ (&co), db_ (&db), - available_ (&ap), config_vars_ (move (cvs)), disfigure_ (df), config_srcs_ (df ? nullptr : css) { // Should not be created for stubs. // - assert (available_->bootstrap_build); + assert (ap.bootstrap_build); // We are only interested in old user configuration variables. // @@ -153,10 +173,135 @@ namespace bpkg assert (!out_root); } + // Serialize a variable assignment for a buildfile fragment. + // + static void + serialize_buildfile (string& r, + const string& var, const build2::value& val, + build2::names& storage) + { + using namespace build2; + + r += var; + r += " = "; + + if (val.null) + r += "[null]"; + else + { + storage.clear (); + names_view nv (reverse (val, storage)); + + if (!nv.empty ()) + { + ostringstream os; + to_stream (os, nv, quote_mode::normal, '@'); + r += os.str (); + } + } + + r += '\n'; + } + + // Serialize a variable assignment for a command line override. + // + static string + serialize_cmdline (const string& var, const build2::value& val, + build2::names& storage) + { + using namespace build2; + + string r (var + '='); + + if (val.null) + r += "[null]"; + else + { + storage.clear (); + names_view nv (reverse (val, storage)); + + if (!nv.empty ()) + { + // Note: we need to use command-line (effective) quoting. + // + ostringstream os; + to_stream (os, nv, quote_mode::effective, '@'); + r += os.str (); + } + } + + return r; + } + + static string + serialize_cmdline (const string& var, const optional& val) + { + using namespace build2; + + string r (var + '='); + + if (!val) + r += "[null]"; + else + { + if (!val->empty ()) + { + // Note: we need to use command-line (effective) quoting. + // + ostringstream os; + to_stream (os, *val, quote_mode::effective, '@'); + r += os.str (); + } + } + + return r; + } + + // Reverse value to names. + // + static optional + reverse_value (const build2::value& val) + { + using namespace build2; + + if (val.null) + return nullopt; + + names storage; + names_view nv (reverse (val, storage)); + + return (nv.data () == storage.data () + ? move (storage) + : names (nv.begin (), nv.end ())); + } + + // Return the dependent (origin==buildfile) configuration variables as + // command line overrides. + // + static strings + dependent_cmd_vars (const package_configuration& cfg) + { + using build2::config::variable_origin; + + strings r; + + for (const config_variable_value& v: cfg) + { + if (v.origin == variable_origin::buildfile) + r.push_back (serialize_cmdline (v.name, v.value)); + } + + return r; + } + void package_skeleton:: - load_defaults (const strings& dependent_vars) + reload_defaults (package_configuration& cfg) { - assert (ctx_ == nullptr); // Should only be called before evaluate_*(). + // Should only be called before dependent_config()/evaluate_*(). + // + assert (dependent_vars_.empty () && + reflect_vars_.empty () && + ctx_ == nullptr); if (config_srcs_ != nullptr) load_old_config (); @@ -165,34 +310,59 @@ namespace bpkg { using namespace build2; + // This is what needs to happen to the variables of different origins in + // the passed configuration: + // + // default -- reloaded + // buildfile/dependent -- made command line override + // override/user -- should match what's in config_vars_ + // undefined -- reloaded + // + // Note also that on the first call we will have no configuration. And + // so to keep things simple, we merge variable of the buildfile origin + // into cmd_vars and then rebuild things from scratch. Note, however, + // that below we need to sort our these merged overrides into user and + // dependent, so we keep the old configuration for reference. + // + // Note also that dependent values do not clash with user overrides by + // construction (in evaluate_{prefer_accept,require}()): we do not add + // as dependent variables that have the override origin. + // + package_configuration old (move (cfg)); + scope& rs ( - *bootstrap (*this, merge_cmd_vars (dependent_vars))->second.front ()); + *bootstrap ( + *this, merge_cmd_vars (dependent_cmd_vars (cfg)))->second.front ()); // Load project's root.build. // load_root (rs); // Note that a configuration variable may not have a default value so we - // cannot just iterate over all the config.** values set on root - // scope. Our options seem to be either iterating over the variable pool - // or forcing the config module with config.config.module=true and then - // using its saved variables map. Since the amout of stuff we load is - // quite limited, there shouldn't be too many variables in the pool. So - // let's go with the simpler approach for now. + // cannot just iterate over all the config.** values set on the + // root scope. Our options seem to be either iterating over the variable + // pool or forcing the config module with config.config.module=true and + // then using its saved variables map. Since the amout of stuff we load + // is quite limited, there shouldn't be too many variables in the pool. + // So let's go with the simpler approach for now. // // Though the saved variables map approach would have been more accurate // since that's the variables that were introduced with the config // directive. Potentially the user could just have a buildfile // config.** variable but it feels like that should be harmless // (we will return it but nobody will presumably use that information). + // Also, if/when we start tracking the configuration variable + // dependencies (i.e., which default value depend on which config + // variable), then the saved variables map seem like the natural place + // to keep this information. // string p ("config." + name ().variable ()); size_t n (p.size ()); for (const variable& var: rs.ctx.var_pool) { - if (var.name.compare (0, n, p) != 0 || (var.name[n + 1] != '.' && - var.name[n + 1] != '\0')) + if (var.name.compare (0, n, p) != 0 || (var.name[n] != '.' && + var.name[n] != '\0')) continue; using config::variable_origin; @@ -201,24 +371,42 @@ namespace bpkg switch (ol.first) { - case variable_origin::override_: break; // Value in config_vars. - case variable_origin::undefined: break; // No default value. case variable_origin::default_: + case variable_origin::override_: + case variable_origin::undefined: { - // @@ TODO: save in some form? + config_variable_value v {var.name, ol.first, {}, {}, {}, 0}; + + // Override could mean user override from config_vars_ or the + // dependent override that we have merged above. // - // How will we enter them (along with value.extra=1) in the - // dependent's context. Probably programmatically. Perhaps we - // should just save them as untyped names? Then enter and - // typify. Seems reasonable. Not clear how/if we can convey - // overriden. Maybe we can allow the prefer/require to override - // them but then ignore their values? + if (v.origin == variable_origin::override_) + { + if (config_variable_value* ov = old.find (v.name)) + { + assert (ov->origin == variable_origin::buildfile); + + v.origin = variable_origin::buildfile; + v.dependent = move (ov->dependent); + } + } + + // Save value. // + if (v.origin != variable_origin::undefined) + v.value = reverse_value (*ol.second); + + // Save type. + // + if (var.type != nullptr) + v.type = var.type->name; + + cfg.push_back (move (v)); break; } case variable_origin::buildfile: { - // How can this happen if we disfigured all of them? + // Feel like this shouldn't happen since we have disfigured them. // assert (false); break; @@ -236,9 +424,13 @@ namespace bpkg } pair package_skeleton:: - verify_sensible (const strings& dependent_vars) + verify_sensible (const package_configuration& cfg) { - assert (ctx_ == nullptr); // Should only be called before evaluate_*(). + // Should only be called before dependent_config()/evaluate_*(). + // + assert (dependent_vars_.empty () && + reflect_vars_.empty () && + ctx_ == nullptr); if (config_srcs_ != nullptr) load_old_config (); @@ -269,7 +461,8 @@ namespace bpkg } scope& rs ( - *bootstrap (*this, merge_cmd_vars (dependent_vars))->second.front ()); + *bootstrap ( + *this, merge_cmd_vars (dependent_cmd_vars (cfg)))->second.front ()); // Load project's root.build while redirecting the diagnostics stream. // @@ -299,11 +492,11 @@ namespace bpkg } void package_skeleton:: - dependent_config (strings&& vars) + dependent_config (const package_configuration& cfg) { assert (dependent_vars_.empty ()); // Must be called at most once. - dependent_vars_ = move (vars); + dependent_vars_ = dependent_cmd_vars (cfg); } // Print the location of a depends value in the specified manifest file. @@ -405,66 +598,6 @@ namespace bpkg } } - // Serialize a variable assignment for a buildfile fragment. - // - static void - serialize_buildfile (string& r, - const string& var, const build2::value& val, - build2::names& storage) - { - using namespace build2; - - r += var; - r += " = "; - - if (val.null) - r += "[null]"; - else - { - storage.clear (); - names_view nv (reverse (val, storage)); - - if (!nv.empty ()) - { - ostringstream os; - to_stream (os, nv, quote_mode::normal, '@'); - r += os.str (); - } - } - - r += '\n'; - } - - // Serialize a variable assignment for a command line override. - // - static string - serialize_cmdline (const string& var, const build2::value& val, - build2::names& storage) - { - using namespace build2; - - string r (var + '='); - - if (val.null) - r += "[null]"; - else - { - storage.clear (); - names_view nv (reverse (val, storage)); - - if (!nv.empty ()) - { - // Note: we need to use command-line (effective) quoting. - // - ostringstream os; - to_stream (os, nv, quote_mode::effective, '@'); - r += os.str (); - } - } - - return r; - } - void package_skeleton:: evaluate_reflect (const string& refl, size_t depends_index) { @@ -476,7 +609,9 @@ namespace bpkg // confusion. // // @@ They could also clash with dependent configuration. Probably should - // be handled in the same way (it's just another type of "user"). + // be handled in the same way (it's just another type of "user"). Yes, + // since dependent_vars_ are entered as cmd line overrides, this is + // how they are treated (but may need to adjust the diagnostics). // // It seems like the most straightforward way to achieve the desired // semantics with the mechanisms that we have (in other words, without @@ -497,9 +632,9 @@ namespace bpkg // // We may also have configuration values from the previous reflect clause // which we want to "factor in" before evaluating the next enable or - // reflect clauses (so that they can use the previously reflect values or - // values that are derived from them in root.build). It seems like we have - // two options here: either enter them as true overrides similar to + // reflect clauses (so that they can use the previously reflected values + // or values that are derived from them in root.build). It seems like we + // have two options here: either enter them as true overrides similar to // config_vars_ or just evaluate them similar to loading config.build // (which, BTW, we might have, in case of an external package). The big // problem with the former approach is that it will then prevent any @@ -512,7 +647,7 @@ namespace bpkg // // BTW, a plan B would be to restrict reflect to just config vars in which // case we could merge them with true overrides. Though how exactly would - // we do this merging is unclear. + // we do this merging is unclear. Hm, but they are config vars... // try { @@ -554,7 +689,7 @@ namespace bpkg // Note: a lot of this code is inspired by the config module. // - // Collect all the config..* variables on the first pass and + // Collect all the set config..* variables on the first pass and // filter out unchanged on the second. // auto& vp (rs.var_pool ()); @@ -566,6 +701,8 @@ namespace bpkg size_t ver; }; + // @@ Maybe redo as small_vector? + // map vars; auto process = [&rs, &ns, &vars] (bool collect) @@ -779,6 +916,195 @@ namespace bpkg } } + bool package_skeleton:: + evaluate_prefer_accept (const dependency_configurations& cfgs, + const string& prefer, + const string& accept, + size_t depends_index) + { + try + { + using namespace build2; + using config::variable_origin; + using build2::fail; + using build2::endf; + + // This is what needs to happen to the variables of different origins in + // the passed dependency configurations: + // + // default -- set as default (value.extra=1) + // buildfile/dependent -- set as buildfile + // override/user -- set as override (so cannot be overriden) + // undefined -- ignored + // + // Additionally, for all origins we need to typify the variables. + // + // All of this is done by load(). + // + scope& rs (load (cfgs)); + + // Evaluate the prefer clause. + // + { + istringstream is (prefer); + is.exceptions (istringstream::failbit | istringstream::badbit); + + path_name in (""); + uint64_t il (1); + + auto df = build2::make_diag_frame ( + [&prefer, &rs, depends_index] (const diag_record& dr) + { + dr << info << "prefer fragment:\n" + << prefer; + + // 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); + } + + // Evaluate the accept clause. + // + bool r; + { + istringstream is ('(' + accept + ')'); + is.exceptions (istringstream::failbit | istringstream::badbit); + + path_name in (""); + uint64_t il (1); + + auto df = build2::make_diag_frame ( + [&accept, &rs, depends_index] (const diag_record& dr) + { + dr << info << "accept condition: (" << accept << ")"; + + // 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); + value v (p.parse_eval (l, rs, rs, parser::pattern_mode::expand)); + + try + { + // Should evaluate to 'true' or 'false'. + // + r = build2::convert (move (v)); + } + catch (const invalid_argument& e) + { + fail (build2::location (in, il)) << e << endf; + } + } + + // If acceptable, update the configuration with the new values, if any. + // + // @@ 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) + { + for (config_variable_value& v: cfg) + { + pair ol (config::origin (rs, v.name)); + + // An override cannot become non-override. And a non-override + // cannot become an override. + // + assert ((v.origin == variable_origin::override_) == + (ol.first == variable_origin::override_)); + + switch (ol.first) + { + case variable_origin::buildfile: + { + // Possible transitions: + // + // default/undefine -> buildfile -- override dependency default + // buildfile -> buildfile -- override other dependent + // + optional val (reverse_value (*ol.second)); + + 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 == val) + break; + } + else + v.origin = variable_origin::buildfile; + + v.value = move (val); + v.dependent = key; // We are the originating dependent. + break; + } + case variable_origin::default_: + case variable_origin::undefined: + { + // An undefined can only come from undefined. Likewise, + // default can only come from default. + // + assert (ol.first == v.origin); + break; + } + case variable_origin::override_: + { + // @@ We may still need to see if there is original set + // by this dependent and if so, reflect the overridden + // value. + break; + } + } + } + } + } + + // Drop the build system state since it needs reloading (for one, we + // could have dependency configuration, such as defaults and other + // dependent values, that further clauses should not see). + // + ctx_ = nullptr; + + return r; + } + catch (const build2::failed&) + { + throw failed (); // Assume the diagnostics has already been issued. + } + } + + bool package_skeleton:: + evaluate_require (const dependency_configurations&, + const string&, 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. + + return false; // @@ TODO + } + pair> package_skeleton:: collect_config () && { @@ -911,13 +1237,14 @@ namespace bpkg ctx_ = nullptr; // Free. db_ = nullptr; - available_ = nullptr; return make_pair (move (vars), move (srcs)); } const strings& package_skeleton:: - merge_cmd_vars (const strings& dependent_vars, bool cache) + merge_cmd_vars (const strings& dependent_vars, + const strings& dependency_vars, + bool cache) { // Merge variable overrides (note that the order is important). See also a // custom/optimized version in load_old_config(). @@ -926,11 +1253,13 @@ namespace bpkg { const strings& vs1 (build2_cmd_vars); const strings& vs2 (config_vars_); - const strings& vs3 (dependent_vars); // Should not override. + const strings& vs3 (dependent_vars); // Should not override. + const strings& vs4 (dependency_vars); // Should not override. // Try to reuse both vector and string buffers. // - cmd_vars_.resize (1 + vs1.size () + vs2.size () + vs3.size ()); + cmd_vars_.resize ( + 1 + vs1.size () + vs2.size () + vs3.size () + vs4.size ()); size_t i (0); { @@ -957,6 +1286,7 @@ namespace bpkg for (const string& v: vs1) cmd_vars_[i++] = v; for (const string& v: vs2) cmd_vars_[i++] = v; for (const string& v: vs3) cmd_vars_[i++] = v; + for (const string& v: vs4) cmd_vars_[i++] = v; cmd_vars_cache_ = cache; } @@ -1071,10 +1401,17 @@ namespace bpkg } build2::scope& package_skeleton:: - load () + load (const dependency_configurations& cfgs) { if (ctx_ != nullptr) - return *rs_; + { + // We have to reload if there is any dependency configuration. + // + if (cfgs.empty ()) + return *rs_; + + ctx_ = nullptr; + } if (config_srcs_ != nullptr) load_old_config (); @@ -1082,12 +1419,35 @@ namespace bpkg try { using namespace build2; + using build2::config::variable_origin; + + // If we have any dependency configurations, then here we need to add + // dependency configuration variables with the override origin to the + // command line overrides (see evaluate_prefer_accept() for details). + // + strings dependency_vars; + for (package_configuration& cfg: cfgs) + { + for (config_variable_value& v: cfg) + { + if (v.origin == variable_origin::override_) + { + dependency_vars.push_back (serialize_cmdline (v.name, v.value)); + v.version = 0; // No value, only override. + } + } + } - // We can reuse already merged cmd_vars if this has already been done - // (they don't change during evaluate_*() calls). + // If there aren't any, then we can reuse already merged cmd_vars (they + // don't change during evaluate_*() calls except for the dependency + // overrides case). // - auto rsi ( - bootstrap (*this, merge_cmd_vars (dependent_vars_, true /* cache */))); + const strings& cmd_vars ( + merge_cmd_vars (dependent_vars_, + dependency_vars, + dependency_vars.empty () /* cache */)); + + auto rsi (bootstrap (*this, cmd_vars)); scope& rs (*rsi->second.front ()); // Load project's root.build as well as potentially accumulated reflect @@ -1101,20 +1461,78 @@ namespace bpkg // configuration and then load it with config.config.load and this // approach should work for that case too. // + // This is also where we set dependency configuration variables with the + // default and buildfile origins and typify all dependency variables + // (see evaluate_prefer_accept() for details). + // function pre; - if (!reflect_frag_.empty ()) + struct data { - pre = [this, &rs] (parser& p) + scope& rs; + const dependency_configurations& cfgs; + } d {rs, cfgs}; + + if (!reflect_frag_.empty () || !cfgs.empty ()) + { + pre = [this, &d] (parser& p) { - istringstream is (reflect_frag_); - is.exceptions (istringstream::failbit | istringstream::badbit); + scope& rs (d.rs); - // Note that the fragment is just a bunch of variable assignments - // and thus unlikely to cause any errors. - // - path_name in (""); - p.parse_buildfile (is, in, &rs, rs); + if (!reflect_frag_.empty ()) + { + istringstream is (reflect_frag_); + is.exceptions (istringstream::failbit | istringstream::badbit); + + // Note that the fragment is just a bunch of variable assignments + // and thus unlikely to cause any errors. + // + path_name in (""); + p.parse_buildfile (is, in, &rs, rs); + } + + for (package_configuration& cfg: d.cfgs) + { + for (config_variable_value& v: cfg) + { + const value_type* vt (nullptr); + if (v.type) + { + vt = parser::find_value_type (&rs, *v.type); + assert (vt != nullptr); + } + + const variable& var (rs.var_pool ().insert (v.name, vt)); + + switch (v.origin) + { + case variable_origin::default_: + case variable_origin::buildfile: + { + auto& val ( + static_cast ( + 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; + break; + } + case variable_origin::undefined: + { + v.version = 0; + break; + } + case variable_origin::override_: break; // Already handled. + } + } + } }; } @@ -1138,7 +1556,7 @@ namespace bpkg static build2::scope_map::iterator bootstrap (package_skeleton& skl, const strings& cmd_vars) { - assert (skl.ctx_ == nullptr); + assert (skl.db_ != nullptr && skl.ctx_ == nullptr); // The overall plan is as follows: // @@ -1158,7 +1576,7 @@ namespace bpkg // if (!skl.created_) { - const available_package& ap (*skl.available_); + const available_package& ap (skl.available); // Note that we create the skeleton directories in the skeletons/ // subdirectory of the configuration temporary directory to make sure diff --git a/bpkg/package-skeleton.hxx b/bpkg/package-skeleton.hxx index 1b7e019..1c4ae32 100644 --- a/bpkg/package-skeleton.hxx +++ b/bpkg/package-skeleton.hxx @@ -4,12 +4,13 @@ #ifndef BPKG_PACKAGE_SKELETON_HXX #define BPKG_PACKAGE_SKELETON_HXX -#include // build2::context +#include #include #include #include +#include #include namespace bpkg @@ -62,13 +63,17 @@ namespace bpkg optional src_root, optional out_root); + + package_key key; + reference_wrapper available; + const package_name& - name () const {return available_->id.name;} + name () const {return key.name;} // @@ TMP: get rid (use key.name). // The following functions should be called in the following sequence // (* -- zero or more, ? -- zero or one): // - // * load_defaults() + // * reload_defaults() // * verify_sensible() // ? dependent_config() // * evaluate_enable() | evaluate_reflect() @@ -78,31 +83,26 @@ namespace bpkg // sequence rather than starting from scratch. // public: - // Load the default values and type information for configuration - // variables of the package given a "tentative" dependent configuration. - // - // @@ TODO: if it will be more convenient to pass it as something other - // than strings, then this can be accomodated. + // Reload the default values and type information for configuration + // variables using the values with the buildfile origin as a "tentative" + // dependent configuration. // void - load_defaults (const strings& dependent_vars); + reload_defaults (package_configuration&); - // Verify the specified "tentative" dependents configuration is sensible, - // that is, acceptable to the package. If it is not, then the second half - // of the result contains the diagnostics. - // - // @@ TODO: if it will be more convenient to pass it as something other - // than strings, then this can be accomodated. + // Verify the specified "tentative" dependent configuration is sensible, + // that is, acceptable to the dependency itself. If it is not, then the + // second half of the result contains the diagnostics. // pair - verify_sensible (const strings& dependent_vars); + verify_sensible (const package_configuration&); // Incorporate the "final" dependent configuration into subsequent // evaluations. Dependent configuration variables are expected not to // clash with user. // void - dependent_config (strings&&); + dependent_config (const package_configuration&); // For the following evaluate_*() functions assume that the clause belongs // to the specified (by index) depends value (used to print its location @@ -119,6 +119,23 @@ namespace bpkg void evaluate_reflect (const string&, size_t depends_index); + // Evaluate the prefer/accept or require clauses on the specified + // dependency configurations (serves as both input and output). + // + // Return true is acceptable and false otherwise. If acceptable, the + // passed configuration is updated with new values, if any. + // + using dependency_configurations = + small_vector, 1>; + + bool + evaluate_prefer_accept (const dependency_configurations&, + const string&, const string&, size_t depends_index); + + bool + evaluate_require (const dependency_configurations&, + const string&, size_t depends_index); + // Return the accumulated configuration variables (first) and project // configuration variable sources (second). Note that the arrays are not // necessarily parallel (config_vars may contain non-project variables). @@ -161,8 +178,11 @@ 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. + // build2::scope& - load (); + load (const dependency_configurations& = {}); // Merge command line variable overrides into one list (normally to be // passed to bootstrap()). @@ -171,7 +191,9 @@ namespace bpkg // calls. // const strings& - merge_cmd_vars (const strings& dependent_vars, bool cache = false); + merge_cmd_vars (const strings& dependent_vars, + const strings& dependency_vars = {}, + bool cache = false); // Implementation details (public for bootstrap()). // @@ -180,7 +202,6 @@ namespace bpkg // const common_options* co_; database* db_; - const available_package* available_; strings config_vars_; bool disfigure_; diff --git a/bpkg/package.hxx b/bpkg/package.hxx index 6ae2109..dc5031c 100644 --- a/bpkg/package.hxx +++ b/bpkg/package.hxx @@ -1590,6 +1590,12 @@ namespace bpkg } bool + operator!= (const package_key& v) const + { + return !(*this == v); + } + + bool operator< (const package_key&) const; // Return the package string representation in the form: diff --git a/bpkg/pkg-build.cxx b/bpkg/pkg-build.cxx index 43a8084..4194e3e 100644 --- a/bpkg/pkg-build.cxx +++ b/bpkg/pkg-build.cxx @@ -37,6 +37,7 @@ #include #include #include +#include using namespace std; using namespace butl; @@ -1332,6 +1333,15 @@ namespace bpkg dependents_map dependents; packages dependencies; + // Dependency configuration. + // + // Note that this container may not yet contain some entries that are + // already in the dependencies member above. And it may already contain + // entries that are not yet in dependencies due to the retry_configuration + // logic. + // + package_configurations dependency_configurations; + // Shadow dependencies and clusters. // // See the collect lambda in collect_build_prerequisites() for details. @@ -6011,6 +6021,10 @@ namespace bpkg // Negotiate the configuration. // + // The overall plan is as follows: continue refining the configuration + // until there are no more changes by giving each dependent a chance + // to make further adjustments. + // for (auto b (pcfg->dependents.begin ()), i (b), e (pcfg->dependents.end ()); i != e; ) @@ -6023,11 +6037,15 @@ namespace bpkg // recursively collecting it). But we could be re-resolving the same // dependency multiple times. // + package_skeleton* dept; { build_package* b (entered_build (i->first)); assert (b != nullptr && b->skeleton); + dept = &*b->skeleton; } + pair pos; + small_vector, 1> depcs; { // A non-negotiated cluster must only have one depends position // for each dependent. @@ -6037,14 +6055,29 @@ namespace bpkg const postponed_configuration::dependency& ds ( i->second.dependencies.front ()); + pos = ds.position; + + depcs.reserve (ds.size ()); for (const package_key& pk: ds) { build_package* b (entered_build (pk)); - assert (b != nullptr && !b->skeleton); + assert (b != nullptr); + + depcs.push_back (b->skeleton + ? *b->skeleton + : b->init_skeleton (o /* options */)); } } - // up_negotiate (...); + if (up_negotiate_configuration ( + pcfg->dependency_configurations, *dept, pos, depcs)) + { + if (i != b) + { + i = b; // Restart from the beginning. + continue; + } + } ++i; } @@ -6086,8 +6119,33 @@ namespace bpkg // if (!b->recursive_collection) { - build_package_refs dep_chain; + // Verify and set the dependent configuration for this dependency. + // + { + assert (b->skeleton); // Should have been init'ed above. + + const package_configuration& cfg ( + pcfg->dependency_configurations[p]); + + pair pr (b->skeleton->verify_sensible (cfg)); + + if (!pr.first) + { + // @@ TODO: improve (print dependencies, dependents, config). + // + // Note that the diagnostics from the dependent will most + // likely be in the "error ..." form (potentially with + // additional info lines) and by printing it with a two-space + // indentation we make it "fit" into our diag record. + // + fail << "unable to negotiate sensible configuration\n" + << " " << pr.second; + } + b->skeleton->dependent_config (cfg); + } + + build_package_refs dep_chain; collect_build_prerequisites (o, *b, fdb, @@ -6434,7 +6492,8 @@ namespace bpkg // dependent/dependencies (i.e., we add them to the cluster but // they never get re-visited). @@ While seems harmless, maybe we // need to prune the part of the configuration that was added - // by them? + // by them? Maybe we should have the confirmed flag which is + // set when the originating dependent confirms the value?! // pc->add_shadow (move (e.dependent), e.position); } @@ -6461,9 +6520,10 @@ namespace bpkg assert (!pc->negotiated); - // Drop any accumulated shadow dependents (which could be - // carried over from retry_configuration logic). + // Drop any accumulated configuration and shadow dependents + // (which could be carried over from retry_configuration logic). // + pc->dependency_configurations.clear (); pc->shadow_dependents.clear (); l5 ([&]{trace << "cfg-negotiation of " << *pc << " failed due " -- cgit v1.1